On 03/07, Bryan Donlan wrote: > > On Sun, Mar 7, 2010 at 14:28, Oleg Nesterov <[email protected]> wrote: > > On 03/06, Linus Torvalds wrote: > >> > >> On Fri, 5 Mar 2010, [email protected] wrote: > >> > > >> > This patch makes it possible for userspace to implement setproctitle() > >> > cleanly. It adds a new PR_SET_PROCTITLE_AREA option for prctl(), which > >> > updates task's mm_struct->arg_start and arg_end to the given area. > >> > >> This looks overly complicated. Why do you change the whole locking rules, > >> instead of protecting _only_ the "arg_start/arg_end" case? > >> > >> The thing is, there's no reason to hold the mmap_sem over the whole thing, > >> and I don't think this is important enough to be a valid reason for > >> exposing a whole new "locked" access variant, when a simple "protect just > >> the arg_start/end" would handle it. > > > > It was me who suggested to re-use mm->mmap_sem instead of adding the new > > lock, but looking at this patch again I do not understand the reason this > > lock should be held throughout in proc_pid_cmdline(). If there is no such > > a reason, then the new access_process_vm_locked (cough, also suggested by > > me ;) is not needed. > > Consider: > > Process A is reading /proc/pidB/cmdline. As it enters > proc_pid_cmdline, the compiler stashes mm->arg_end and mm->arg_start > in a register or something, and then the process is preempted. > Process B now changes the location of its cmdline buffer. After > changing it, it free()s the buffer. Then it malloc()s a buffer for > sensitive data. Unfortunately, this happens to be at the same address.
Indeed, I see, thanks. Oleg. -- To unsubscribe from this list: send the line "unsubscribe linux-api" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
