Date: Sun, 6 Nov 2011 20:10:41 +0200 From: Kostik Belousov <kostikbel@gmail.com> To: Mikolaj Golub <trociny@freebsd.org> Cc: freebsd-hackers@freebsd.org, Robert Watson <rwatson@freebsd.org> Subject: Re: "ps -e" without procfs(5) Message-ID: <20111106181041.GH50300@deviant.kiev.zoral.com.ua> In-Reply-To: <8662iyjof9.fsf@kopusha.home.net> References: <20111025082451.GO50300@deviant.kiev.zoral.com.ua> <86aa8k2im0.fsf@kopusha.home.net> <20111031094948.GB50300@deviant.kiev.zoral.com.ua> <86vcr21agm.fsf@kopusha.home.net> <20111105135801.GT50300@deviant.kiev.zoral.com.ua> <86ehxmpogp.fsf@kopusha.home.net> <20111105154443.GB50300@deviant.kiev.zoral.com.ua> <86ehxmjsza.fsf@kopusha.home.net> <20111105194553.GK50300@deviant.kiev.zoral.com.ua> <8662iyjof9.fsf@kopusha.home.net>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --] On Sat, Nov 05, 2011 at 10:37:46PM +0200, Mikolaj Golub wrote: > > On Sat, 5 Nov 2011 21:45:53 +0200 Kostik Belousov wrote: > > KB> On Sat, Nov 05, 2011 at 08:59:21PM +0200, Mikolaj Golub wrote: > >> > >> On Sat, 5 Nov 2011 17:44:43 +0200 Kostik Belousov wrote: > >> > >> >> KB> I think that the aux vector must be naturally aligned. You can return > >> >> KB> ENOEXEC early if vptr is not aligned. > >> >> > >> >> Not sure I see what you mean. vptr for auxv is calculated just couple lines > >> >> above, and I check the result here, in the part common for all vector types. > >> KB> You do not check for the alignment. Am I wrong ? > >> > >> I see now. If natural alignment means "addr % sizeof(aux) == 0" then the aux > >> vectors are not naturally aligned. After adding this check: > >> > >> if (vptr % sizeof(aux) != 0) > >> return (ENOEXEC); > KB> No, the natural alignment of the structure is the alignment of the most > KB> demanding member. So it is 4 bytes on 32bit, and 8 bytes on 64. > > >> > >> I started to observe many ENOEXEC errors. Adding printf showed that the > >> vectors are half size aligned. > >> > >> On i386: > >> > >> get_proc_vector(pid = getty[3442], type = 2): vptr (2143284876) % sizeof(aux) (8) = 4) > >> > >> On amd64: > >> > >> get_proc_vector(pid = getty[2425], type = 2): vptr (140737488346568) % sizeof(aux) (16) = 8) > >> > >> Looking at exec_copyout_strings() from kern_exec.c, how destp is calculated, I > >> think they are sizeof(char *) aligned. > >> > >> Do you think it is worth adding the check for sizeof(char *) alignment? > >> > >> if (vptr % (sizeof(char *) != 0) > >> return (ENOEXEC); > KB> I suggest to use #if __ELF_WORD_SIZE == 32 or 64. > > Thanks. The updated patch: > > http://people.freebsd.org/~trociny/env.sys.3.patch Oops, I missed this in the previous review. You cannot use fubyte in proc_read_mem(). fubyte reads a byte from the address space of the current process. The fix is easy, use proc_rwmem for 1 byte. I do not think that fall back to single byte read is warranted for proc_read_mem calls e.g. for ps_strings. Add a flag to indicate whether the proc_read_mem should fall back to byte read ? I would prefer using sizeof(uint64_t) and sizeof(uint32_t) instead of 8 and 4 constants in the align checks. Might be, add PROC_ASSERT_HELD() to get_ps_string() ? procfs patch looks good. [-- Attachment #2 --] -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.18 (FreeBSD) iEYEARECAAYFAk62zaEACgkQC3+MBN1Mb4icCACeJcDhyPfaGTk7frkifRXO5mUE nYYAoMf82kNB6RVH1dmMK+DRmmYTcehs =wxV9 -----END PGP SIGNATURE-----
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20111106181041.GH50300>