Message ID | 20221027163654.414017-2-teo.coupriediaz@arm.com |
---|---|
State | RFC |
Headers | show |
Series | Change return type of tst_syscall | expand |
Hi! > Some syscalls directly return pointers, like brk or mmap. int is currently > used for the return value in tst_syscall but is not large enough > to guarantee that such a returned value will fit. > Instead, use intptr_t which is guaranted to be castable to (void *) > without loss of data. Sounds reasonable, glibc syscall returns long but I guess that's because there was no intptr_t when that was introduced. > Signed-off-by: Teo Couprie Diaz <teo.coupriediaz@arm.com> > --- > include/lapi/syscalls/regen.sh | 2 +- > runtest/check_tst_syscall | 190 +++++++++++++++++++++++++++++++++ > 2 files changed, 191 insertions(+), 1 deletion(-) > create mode 100644 runtest/check_tst_syscall > > diff --git a/include/lapi/syscalls/regen.sh b/include/lapi/syscalls/regen.sh > index 3bf38fd03..97027e2f3 100755 > --- a/include/lapi/syscalls/regen.sh > +++ b/include/lapi/syscalls/regen.sh > @@ -48,7 +48,7 @@ cat << EOF > "${output_pid}" > #endif > > #define tst_syscall(NR, ...) ({ \\ > - int tst_ret; \\ > + intptr_t tst_ret; \\ > if (NR == __LTP__NR_INVALID_SYSCALL) { \\ > errno = ENOSYS; \\ > tst_ret = -1; \\ > diff --git a/runtest/check_tst_syscall b/runtest/check_tst_syscall > new file mode 100644 > index 000000000..7a6003593 > --- /dev/null > +++ b/runtest/check_tst_syscall I do not think that we shoud add this file, at least not in this commit and without any good description.
Hello, On 31/10/2022 13:32, Cyril Hrubis wrote: > Hi! >> Some syscalls directly return pointers, like brk or mmap. int is currently >> used for the return value in tst_syscall but is not large enough >> to guarantee that such a returned value will fit. >> Instead, use intptr_t which is guaranted to be castable to (void *) >> without loss of data. > Sounds reasonable, glibc syscall returns long but I guess that's because > there was no intptr_t when that was introduced. > >> Signed-off-by: Teo Couprie Diaz <teo.coupriediaz@arm.com> >> --- >> include/lapi/syscalls/regen.sh | 2 +- >> runtest/check_tst_syscall | 190 +++++++++++++++++++++++++++++++++ >> 2 files changed, 191 insertions(+), 1 deletion(-) >> create mode 100644 runtest/check_tst_syscall >> >> diff --git a/include/lapi/syscalls/regen.sh b/include/lapi/syscalls/regen.sh >> index 3bf38fd03..97027e2f3 100755 >> --- a/include/lapi/syscalls/regen.sh >> +++ b/include/lapi/syscalls/regen.sh >> @@ -48,7 +48,7 @@ cat << EOF > "${output_pid}" >> #endif >> >> #define tst_syscall(NR, ...) ({ \\ >> - int tst_ret; \\ >> + intptr_t tst_ret; \\ >> if (NR == __LTP__NR_INVALID_SYSCALL) { \\ >> errno = ENOSYS; \\ >> tst_ret = -1; \\ >> diff --git a/runtest/check_tst_syscall b/runtest/check_tst_syscall >> new file mode 100644 >> index 000000000..7a6003593 >> --- /dev/null >> +++ b/runtest/check_tst_syscall > I do not think that we shoud add this file, at least not in this commit > and without any good description. I agree, I wouldn't want to merge it. As mentioned in the cover, I wanted to share the list of tests I have tested the patch with, both for people to test themselves if they want to and as a way to ask if there was anything I missed while testing with that. I didn't really know how to share this, so I added it as part of the commit for the RFC. Maybe it would fit better as part of the cover letter ? Or after the commit description with the shortlog ? It might be better removed altogether and people can test with a larger scope anyway ! Thanks for having a look, Best regards Téo Couprie Diaz
Hi! > I agree, I wouldn't want to merge it. > As mentioned in the cover, I wanted to share the list of tests I have > tested the patch with, > both for people to test themselves if they want to and as a way to ask > if there was anything I missed > while testing with that. > I didn't really know how to share this, so I added it as part of the > commit for the RFC. Maybe it would > fit better as part of the cover letter ? Or after the commit description > with the shortlog ? > It might be better removed altogether and people can test with a larger > scope anyway ! Maybe just a separate patch with [DO NOT MERGE] in the tittle?
Hi, > Hi! >> I agree, I wouldn't want to merge it. >> As mentioned in the cover, I wanted to share the list of tests I have >> tested the patch with, >> both for people to test themselves if they want to and as a way to ask >> if there was anything I missed >> while testing with that. >> I didn't really know how to share this, so I added it as part of the >> commit for the RFC. Maybe it would >> fit better as part of the cover letter ? Or after the commit description >> with the shortlog ? >> It might be better removed altogether and people can test with a larger >> scope anyway ! > Maybe just a separate patch with [DO NOT MERGE] in the tittle? That does make sense, I will split it as such when sending the non-RFC version. Thanks for the guidance ! Best regards, Téo Couprie Diaz
diff --git a/include/lapi/syscalls/regen.sh b/include/lapi/syscalls/regen.sh index 3bf38fd03..97027e2f3 100755 --- a/include/lapi/syscalls/regen.sh +++ b/include/lapi/syscalls/regen.sh @@ -48,7 +48,7 @@ cat << EOF > "${output_pid}" #endif #define tst_syscall(NR, ...) ({ \\ - int tst_ret; \\ + intptr_t tst_ret; \\ if (NR == __LTP__NR_INVALID_SYSCALL) { \\ errno = ENOSYS; \\ tst_ret = -1; \\ diff --git a/runtest/check_tst_syscall b/runtest/check_tst_syscall new file mode 100644 index 000000000..7a6003593 --- /dev/null +++ b/runtest/check_tst_syscall @@ -0,0 +1,190 @@ +cve-2015-3290 cve-2015-3290 +cve-2016-10044 cve-2016-10044 +cve-2016-7117 cve-2016-7117 +clock_settime03 clock_settime03 +accept4_01 accept4_01 +stime_var stime_var +rt_sigprocmask02 rt_sigprocmask02 +rt_sigprocmask01 rt_sigprocmask01 +pwritev pwritev +perf_event_open01 perf_event_open01 +perf_event_open perf_event_open +capset03 capset03 +capset04 capset04 +capset01 capset01 +capset02 capset02 +readahead02 readahead02 +readahead01 readahead01 +io_submit02 io_submit02 +io_submit02 io_submit02 +io_submit03 io_submit03 +io_submit03 io_submit03 +clock_adjtime clock_adjtime +getrusage02 getrusage02 +faccessat01 faccessat01 +timer_delete01 timer_delete01 +timer_delete02 timer_delete02 +rt_sigsuspend01 rt_sigsuspend01 +renameat2 renameat2 +ppoll01 ppoll01 +connect01 connect01 +semop semop +msgctl04 msgctl04 +semctl03 semctl03 +semctl09 semctl09 +shmctl02 shmctl02 +shmctl05 shmctl05 +migrate_pages01 migrate_pages01 +migrate_pages03 migrate_pages03 +migrate_pages02 migrate_pages02 +tkill02 tkill02 +tkill02 tkill02 +tkill01 tkill01 +tkill01 tkill01 +capget01 capget01 +capget02 capget02 +pkey pkey +clone09 clone09 +clone08 clone08 +gettimeofday01 gettimeofday01 +gettimeofday02 gettimeofday02 +signalfd01 signalfd01 +futimesat01 futimesat01 +vhangup02 vhangup02 +vhangup01 vhangup01 +sched_setaffinity01 sched_setaffinity01 +setdomainname setdomainname +fchownat fchownat +timer_settime01 timer_settime01 +timer_settime02 timer_settime02 +readdir21 readdir21 +mlock202 mlock202 +mlock201 mlock201 +mlock203 mlock203 +timerfd03 timerfd03 +timerfd02 timerfd02 +compat_tst_16 compat_tst_16 +compat_16 compat_16 +sigpending02 sigpending02 +signalfd4_02 signalfd4_02 +signalfd4_01 signalfd4_01 +inotify_init1_01 inotify_init1_01 +inotify_init1_02 inotify_init1_02 +inotify_init1_01 inotify_init1_01 +inotify_init1_02 inotify_init1_02 +memfd_create_common memfd_create_common +getcpu01 getcpu01 +tgkill tgkill +sysctl03 sysctl03 +sysctl01 sysctl01 +sysctl04 sysctl04 +get_robust_list01 get_robust_list01 +sendmmsg_var sendmmsg_var +copy_file_range copy_file_range +rt_sigqueueinfo rt_sigqueueinfo +rt_sigqueueinfo01 rt_sigqueueinfo01 +setns02 setns02 +setns01 setns01 +mprotect01 mprotect01 +delete_module02 delete_module02 +delete_module03 delete_module03 +delete_module01 delete_module01 +sched_get_priority_max02 sched_get_priority_max02 +sched_get_priority_max01 sched_get_priority_max01 +sched_get_priority_max02 sched_get_priority_max02 +mknodat mknodat +mknodat02 mknodat02 +utimensat01 utimensat01 +syslog12 syslog12 +syslog11 syslog11 +inotify inotify +preadv preadv +sync_file_range01 sync_file_range01 +eventfd01 eventfd01 +sched_get_priority_min02 sched_get_priority_min02 +sched_get_priority_min01 sched_get_priority_min01 +sched_get_priority_min02 sched_get_priority_min02 +fstatat01 fstatat01 +adjtimex02 adjtimex02 +sgetmask01 sgetmask01 +setitimer02 setitimer02 +timer_getoverrun01 timer_getoverrun01 +io_cancel01 io_cancel01 +io_cancel01 io_cancel01 +exit_group01 exit_group01 +timer_create02 timer_create02 +io_getevents01 io_getevents01 +fork05 fork05 +fcntl_common fcntl_common +fcntl31 fcntl31 +timer_gettime01 timer_gettime01 +membarrier01 membarrier01 +set_thread_area01 set_thread_area01 +swapon01 swapon01 +swapon02 swapon02 +swapon03 swapon03 +ssetmask01 ssetmask01 +eventfd2_02 eventfd2_02 +eventfd2_01 eventfd2_01 +eventfd2_03 eventfd2_03 +request_key05 request_key05 +epoll_create epoll_create +openat openat +remap_file_pages02 remap_file_pages02 +io_setup02 io_setup02 +io_setup02 io_setup02 +rt_sigtimedwait01 rt_sigtimedwait01 +linkat02 linkat02 +linkat01 linkat01 +cacheflush01 cacheflush01 +getdents getdents +epoll_create1_01 epoll_create1_01 +epoll_create1_02 epoll_create1_02 +fchmodat01 fchmodat01 +userfaultfd01 userfaultfd01 +symlinkat01 symlinkat01 +socketcall02 socketcall02 +socketcall03 socketcall03 +socketcall01 socketcall01 +sysfs03 sysfs03 +sysfs04 sysfs04 +sysfs05 sysfs05 +sysfs02 sysfs02 +sysfs01 sysfs01 +select_var select_var +newuname01 newuname01 +process_vm_readv02 process_vm_readv02 +process_vm_writev02 process_vm_writev02 +process_vm_readv03 process_vm_readv03 +process_vm_writev02 process_vm_writev02 +process_vm_readv01 process_vm01 -r +process_vm_writev01 process_vm01 -w +io_destroy02 io_destroy02 +getrandom04 getrandom04 +getrandom03 getrandom03 +getrandom01 getrandom01 +getrandom02 getrandom02 +swapoff02 swapoff02 +swapoff01 swapoff01 +futex2test futex2test +ioprio ioprio +getitimer02 getitimer02 +set_tid_address01 set_tid_address01 +getrlimit03 getrlimit03 +ustat01 ustat01 +ustat02 ustat02 +prctl04 prctl04 +prctl05 prctl05 +libclone libclone +userns04 userns04 +userns04 userns04 +pidns30 pidns30 +pidns31 pidns31 +pidns13 pidns13 +mqns_04 mqns_04 +mqns_02 mqns_02 +mqns_01 mqns_01 +mqns_03 mqns_03 +kmsg01 kmsg01 +pt_test pt_test +libcpuset libcpuset
Some syscalls directly return pointers, like brk or mmap. int is currently used for the return value in tst_syscall but is not large enough to guarantee that such a returned value will fit. Instead, use intptr_t which is guaranted to be castable to (void *) without loss of data. Signed-off-by: Teo Couprie Diaz <teo.coupriediaz@arm.com> --- include/lapi/syscalls/regen.sh | 2 +- runtest/check_tst_syscall | 190 +++++++++++++++++++++++++++++++++ 2 files changed, 191 insertions(+), 1 deletion(-) create mode 100644 runtest/check_tst_syscall