nacl: return error if pointers are not aligned

mayank-ramnani commented 7 months ago

Description

Fixes https://github.com/Lind-Project/lind_project/issues/327

Returning an error if pointer is not 8-byte aligned.

Type of change

[x] Bug fix (non-breaking change which fixes an issue)
[x] Breaking change (fix or feature that would cause existing functionality to not work as expected)

How Has This Been Tested?

Running test: hello.c TEST PASSED
Running test: cpuid.c TEST PASSED
Running test: dup2.c TEST PASSED
Running test: dupwrite.c TEST PASSED
Running test: forkdup.c TEST PASSED
Running test: write.c TEST PASSED
Running test: forknodup.c TEST PASSED
Running test: pipepong.c TEST PASSED
Running test: exit.c TEST PASSED
Running test: readbytes.c TEST FAILED Running test: writeloop.c TEST FAILED Running test: writepartial.c TEST PASSED Running test: noforkfiles.c TEST PASSED Running test: pread_pwrite.c TEST PASSED Running test: makepipe.c TEST PASSED Running test: mprotect.c TEST PASSED Running test: doubleclose.c TEST PASSED Running test: filetest.c TEST PASSED Running test: filetest1000.c TEST PASSED Running test: mmaptest.c TEST PASSED Running test: fstat.c TEST PASSED Running test: stat.c TEST PASSED Running test: forkexecv.cTEST FAILED Running test: socket.c TEST PASSED Running test: getuid.c TEST PASSED Running test: forkexecuid.c TEST PASSED Running test: ioctl.c TEST PASSED Running test: setsid.c TEST PASSED Running test: dnstest.cTEST FAILED Running test: cloexec.cTEST FAILED Running test: chmod.c TEST PASSED Running test: rename.c TEST PASSED Running test: fchmod.c TEST PASSED Running test: truncate.c TEST PASSED Running test: mutex.c TEST PASSED Running test: socketpair.c TEST PASSED Running test: flock.c TEST PASSED Running test: getppid.c TEST PASSED
Running test: creat_access.c TEST PASSED Running test: mkdir_rmdir.c TEST PASSED Running test: poll.c TEST PASSED Running test: sem_forks.c TEST PASSED Running test: fsync.cTEST FAILED Running test: fdatasync.cTEST FAILED Running test: sync_file_range.c TEST PASSED Running test: getpid.c TEST PASSED Running test: read.c TEST PASSED Running test: dup.c TEST PASSED Running test: fork.cTEST FAILED Running test: fork2malloc.c TEST PASSED Running test: forkandopen.c TEST PASSED Running test: forkfiles.c TEST PASSED Running test: forkmalloc.c TEST PASSED Running test: pipe.cTEST FAILED Running test: pipeinput.cTEST FAILED Running test: pipeinput2.cTEST FAILED Running test: pipewrite.c TEST PASSED Running test: pipeonestring.cTEST FAILED Running test: chdir_getcwd.c TEST PASSED Running test: fchdir.c TEST PASSED Running test: segfault.cTEST FAILED Running test: sysconf.c TEST PASSED Running test: template.c TEST PASSED Running test: gethostname.c TEST PASSED Running test: serverclient.c STUCK

Checklist:

[x] My code follows the style guidelines of this project
[x] I have commented my code, particularly in hard-to-understand areas
[x] My changes generate no new warnings
[ ] I have added tests that prove my fix is effective or that my feature works
[ ] Any dependent changes have been added to a pull request and/or merged in other modules (lind-project, lind-glibc, safeposix-rust)

mayank-ramnani commented 7 months ago

@yzhang71 @rennergade A lot of test cases fail after this change. I tried figuring out the reason they are failing, seems like once we return the kNaClBadAddress error after address alignment check, the program exits. Thus there might be changes required in other places to make sure addresses are always aligned.

Here is an example for readbytes.c failing

syscalls with ./readbytes-gcc

openat(AT_FDCWD, "test.txt", O_RDWR|O_CREAT, 0777) = 3 
write(3, "This is a test of the wonderful "..., 51) = 51
close(3)                                = 0 
openat(AT_FDCWD, "test.txt", O_RDWR)    = 3 
read(3, "T", 1)                         = 1 
write(1, "T", 1)                        = 1 
read(3, "h", 1)                         = 1 
write(1, "h", 1)                        = 1 
read(3, "i", 1)                         = 1 
write(1, "i", 1)                        = 1 
read(3, "s", 1)                         = 1 
write(1, "s", 1)                        = 1
... and so on

syscalls with /readbytes-lind.nexe

[pid 366278] statx(AT_FDCWD, "./linddata.1961", AT_STATX_SYNC_AS_STAT, STATX_ALL, {stx_mask=STATX_ALL|STATX_MNT_ID, stx_attributes=0, stx_mode=S_IFREG|0644, stx_size=51, ...}) = 0 
[pid 366278] openat(AT_FDCWD, "linddata.1961", O_RDWR|O_CLOEXEC) = 8
[pid 366278] getcwd("/home/lind/lind_project/lind/lindenv/fs", 1024) = 40 
[pid 366278] readlink("/home/lind/lind_project/lind/lindenv/fs/linddata.1961", 0x7f3ba68b7040, 1023) = -1 EINVAL (Invalid argument)
[pid 366278] statx(8, "", AT_STATX_SYNC_AS_STAT|AT_EMPTY_PATH, STATX_ALL, {stx_mask=STATX_ALL|STATX_MNT_ID, stx_attributes=0, stx_mode=S_IFREG|0644, stx_size=51, ...}) = 0 
[pid 366278] lseek(8, 0, SEEK_SET)      = 0  
[pid 366278] write(8, "This is a test of the wonderful "..., 51) = 51 
[pid 366278] close(8)                   = 0  
[pid 366278] statx(AT_FDCWD, "./linddata.1961", AT_STATX_SYNC_AS_STAT, STATX_ALL, {stx_mask=STATX_ALL|STATX_MNT_ID, stx_attributes=0, stx_mode=S_IFREG|0644, stx_size=51, ...}) = 0 
[pid 366278] openat(AT_FDCWD, "linddata.1961", O_RDWR|O_CLOEXEC) = 8
[pid 366278] getcwd("/home/lind/lind_project/lind/lindenv/fs", 1024) = 40 
[pid 366278] readlink("/home/lind/lind_project/lind/lindenv/fs/linddata.1961", 0x7f3ba68b7040, 1023) = -1 EINVAL (Invalid argument)
[pid 366278] statx(8, "", AT_STATX_SYNC_AS_STAT|AT_EMPTY_PATH, STATX_ALL, {stx_mask=STATX_ALL|STATX_MNT_ID, stx_attributes=0, stx_mode=S_IFREG|0644, stx_size=51, ...}) = 0 
[pid 366278] close(8)                   = 0

Backtrace at the point where uaddr was not aligned:

Thread 3 "sel_ldr" hit Breakpoint 3,  (nap=0x555555710eb0, uaddr=4294901351, count=1, prot=2) at /home/lind/lind_project/src/native_client/src/trusted/service_runtime/sel_ldr-inl.h:178
178         return kNaClBadAddress;
3: (uaddr % 8) = 7 
@ [#3:NaClUserToSysAddrRangeProt()] bt
#0  NaClUserToSysAddrRangeProt (nap=0x555555710eb0, uaddr=4294901351, count=1, prot=2) at /home/lind/lind_project/src/native_client/src/trusted/service_runtime/sel_ldr-inl.h:178
#1  0x00005555555771c7 in NaClSysRead (natp=0x5555557c7c30, d=3, buf=0xfffefe67, count=1) at src/trusted/service_runtime/nacl_syscall_common.c:736
#2  0x0000555555581899 in NaClSysReadDecoder (natp=0x5555557c7c30) at scons-out/dbg-linux-x86-64/gen/native_client/src/trusted/service_runtime/nacl_syscall_handlers.c:620
#3  0x000055555559e3fc in NaClSyscallCSegHook (ntcp=0x5555557c7c30) at src/trusted/service_runtime/nacl_syscall_hook.c:147
#4  0x0000555555592776 in NaClSyscallSegRegsSaved () at src/trusted/service_runtime/arch/x86_64/nacl_syscall_64.S:211
#5  0x00007ffff7619d40 in ?? ()
#6  0x00005555555926ee in NaClStartThreadInApp (natp=0x206c75667265646e, new_prog_ctr=8031924080354795622) at src/trusted/service_runtime/arch/x86_64/nacl_switch_to_app_64.c:86
Backtrace stopped: previous frame inner to this frame (corrupt stack?)

Right after this, program exits, thus leaving the testcase failed. Let me know if you need me to dig in further to figure out why addresses are misaligned in the first place.

rennergade commented 7 months ago

So I think I made a mistake when defining the problem. This is good work though into investigating what was wrong.

I believe the error actually comes from casting non-aligned addresses, not using them in general. For example, you should be able to use a buffer pointer in read/write thats not 8 byte aligned. But we shouldn't be able to cast a struct that isn't 8-byte aligned.

Let's discuss this further but it may be better to handle this with a different approach in RustPOSIX in the types.rs file.

Lind-Project / native_client