|
From: Matthias S. <zz...@ge...> - 2017-04-26 19:40:43
|
Am 24.04.2017 um 22:35 schrieb Ivo Raisr: > 2017-04-24 22:03 GMT+02:00 Matthias Schwarzott <zz...@ge...>: >> Am 24.04.2017 um 17:00 schrieb Ivo Raisr: >>> Any comments or objections to patch v2 for bug 379039? >>> https://bugs.kde.org/show_bug.cgi?id=379039 >>> >>> I. >>> >> Hi! >> >> The code seems to work, but the len variable does not mean length of the >> string, so it could be misleading. >> >> Additionally I am not sure if the function POST(sys_prctl) must also be >> modified. > > Hi Matthias, Hi Ivo > > Thank you for your comments. > You are right, I had to modify POST(sys_prctl) so it takes into account > that ARG2 might not need to be nul-terminated. > Good. But not exactly matching what the kernel does. This is from kernel/sys.c: kernel/sys.c: case PR_SET_NAME: kernel/sys.c- comm[sizeof(me->comm) - 1] = 0; kernel/sys.c- if (strncpy_from_user(comm, (char __user *)arg2, kernel/sys.c- sizeof(me->comm) - 1) < 0) kernel/sys.c- return -EFAULT; kernel/sys.c- set_task_comm(me, comm); kernel/sys.c- proc_comm_connector(me); kernel/sys.c- break; The kernel copies up to 15 characters from userspace. To mimic that behaviour, I had to modify POST(sys_prctl) like this. --- coregrind/m_syswrap/syswrap-linux.c +++ coregrind/m_syswrap/syswrap-linux.c @@ -1535,7 +1535,7 @@ POST(sys_prctl) const HChar* new_name = (const HChar*) ARG2; if (new_name) { // Paranoia ThreadState* tst = VG_(get_ThreadState)(tid); - SizeT new_len = VG_(strnlen)(new_name, VKI_TASK_COMM_LEN); + SizeT new_len = VG_(strnlen)(new_name, VKI_TASK_COMM_LEN - 1); /* Don't bother reusing the memory. This is a rare event. */ tst->thread_name = > >> The test memcheck/tests/threadname.c maybe needs more cases: >> >> * Set threadname to a long string and check that only the first 15 >> characters are printed as threadname for the next error. > > Why? We do not want to do functional testing of libpthread or prctl syscall. The only reason is to see if valgrind can show the correct threadname in error messages and I found the small behaviour difference above. > >> * If possible a test that proves, that POST(sys_prctl) does not access >> memory after byte 16 (but I do not know how to test this). > > That would be appropriate for prctl(get-name) case. Different story. > No, I really meant if POST(sys_prctl) might crash when putting a carefully crafted pointer to the syscall. Maybe 16 bytes before the end of a memory page and having the next page not mapped. Regards Matthias |