Added PR_SET_PROCTITLE_AREA option for prctl()
in [Kernel]
|
Prev: pciehp_driver.c: PCIe hot plug for Intel iommu - call IOMMU API in hot remove
Next: 2.6.31 regression sis5513 PIO Mode 0 hang
From: Ingo Molnar on 7 Dec 2009 23:50 * KOSAKI Motohiro <kosaki.motohiro(a)jp.fujitsu.com> wrote: > + /* > + * If argv and environ aren't continuous (i.e. the process used > + * prctl(PR_SET_PROCTITLE_AREA)), we don't care environ override. s/dont't care environ override/don't care about the evironment override/ > + case PR_SET_PROCTITLE_AREA: { > + struct mm_struct *mm = current->mm; > + unsigned long addr = arg2; > + unsigned long len = arg3; > + unsigned long end = arg2 + arg3; would be cleaner to write the latter as 'addr + len'. > + if (len > PAGE_SIZE) > + return -EINVAL; > + > + if (addr >= end) > + return -EINVAL; > + > + /* > + * If the process pass broken pointer, EFAULT is might better > + * than ps output zero-length proctitle. Plus if > + * the process pass kernel address (or something-else), > + * We have to block it. Oherwise, strange exploit > + * chance is there. > + */ > + if (!access_ok(VERIFY_READ, addr, len)) > + return -EFAULT; the addr >= end check looks (partly) duplicative of the access_ok() check. > + > + down_write(&mm->mmap_sem); > + mm->arg_start = addr; > + mm->arg_end = end; > + up_write(&mm->mmap_sem); well we might as well name 'addr' as 'start' and have a match then here too. The feature looks useful, but the choice of a prctl as an API is strange - it limits us to the current task only - while the ability to set arguments for another task looks a more generic (and potentially more useful) solution. Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: KOSAKI Motohiro on 8 Dec 2009 00:20 > > * KOSAKI Motohiro <kosaki.motohiro(a)jp.fujitsu.com> wrote: > > > + /* > > + * If argv and environ aren't continuous (i.e. the process used > > + * prctl(PR_SET_PROCTITLE_AREA)), we don't care environ override. > > s/dont't care environ override/don't care about the evironment override/ Thanks. I'll fix. > > > + case PR_SET_PROCTITLE_AREA: { > > + struct mm_struct *mm = current->mm; > > + unsigned long addr = arg2; > > + unsigned long len = arg3; > > + unsigned long end = arg2 + arg3; > > would be cleaner to write the latter as 'addr + len'. Will fix. > > > + if (len > PAGE_SIZE) > > + return -EINVAL; > > + > > + if (addr >= end) > > + return -EINVAL; > > + > > + /* > > + * If the process pass broken pointer, EFAULT is might better > > + * than ps output zero-length proctitle. Plus if > > + * the process pass kernel address (or something-else), > > + * We have to block it. Oherwise, strange exploit > > + * chance is there. > > + */ > > + if (!access_ok(VERIFY_READ, addr, len)) > > + return -EFAULT; > > the addr >= end check looks (partly) duplicative of the access_ok() > check. ok. > > + > > + down_write(&mm->mmap_sem); > > + mm->arg_start = addr; > > + mm->arg_end = end; > > + up_write(&mm->mmap_sem); > > well we might as well name 'addr' as 'start' and have a match then here > too. I'll do. > The feature looks useful, but the choice of a prctl as an API is strange > - it limits us to the current task only - while the ability to set > arguments for another task looks a more generic (and potentially more > useful) solution. No. It's impossible. /proc/{pid}/cmdline read user process's memory. iow, this prctl() don't receive string, it receive virtual address itself. I don't want any task allow to change another task's memory except ptrace. Plus, glibc don't need such capability. setproctitle() can only cahnge own task's title. Of cource, we can change argv is held in kernel structure (e.g. task_struct or mm_struct). but it increase kernel memory footprint and I'm not sure its worth. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Ingo Molnar on 8 Dec 2009 00:40 * KOSAKI Motohiro <kosaki.motohiro(a)jp.fujitsu.com> wrote: > > The feature looks useful, but the choice of a prctl as an API is strange > > - it limits us to the current task only - while the ability to set > > arguments for another task looks a more generic (and potentially more > > useful) solution. > > No. It's impossible. > /proc/{pid}/cmdline read user process's memory. iow, this prctl() don't > receive string, it receive virtual address itself. [...] it's not 'impossible' at all, you yourself mention ptrace: > [...] I don't want any task allow to change another task's memory > except ptrace. And i did not mean to allow 'any' task to be allowed to do this - security checks apply, obviously. Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: KOSAKI Motohiro on 8 Dec 2009 01:00 > > * KOSAKI Motohiro <kosaki.motohiro(a)jp.fujitsu.com> wrote: > > > > The feature looks useful, but the choice of a prctl as an API is strange > > > - it limits us to the current task only - while the ability to set > > > arguments for another task looks a more generic (and potentially more > > > useful) solution. > > > > No. It's impossible. > > /proc/{pid}/cmdline read user process's memory. iow, this prctl() don't > > receive string, it receive virtual address itself. [...] > > it's not 'impossible' at all, you yourself mention ptrace: Ah yes, 'impossible' was wrong word. but it doesn't works intentionally. 1. setproctitle() unaware application continue to see argv[0] directly. it makes some inconsistent behavior. 2. proc title (i.e. string) injection need to map new page as process title area. implicit mapping increasing makes new trouble - mihgt cause to exceed max_map_count awhile after. - might cause leak proc title area (who know when it should be freed?) I think reasonable way is 1. send signal (or use another inter process communication way) to target process 2. target process change own proc title themself. Plus, I haven't seen the use-case of changin another task. iow I doubt it's worth to change lots code. > > [...] I don't want any task allow to change another task's memory > > except ptrace. > > And i did not mean to allow 'any' task to be allowed to do this - > security checks apply, obviously. Agreed. My 'any' didn't intent bypass security check ;-) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Bryan Donlan on 8 Dec 2009 01:30
On Tue, Dec 8, 2009 at 12:38 AM, Ingo Molnar <mingo(a)elte.hu> wrote: > > * KOSAKI Motohiro <kosaki.motohiro(a)jp.fujitsu.com> wrote: > >> > The feature looks useful, but the choice of a prctl as an API is strange >> > - it limits us to the current task only - while the ability to set >> > arguments for another task looks a more generic (and potentially more >> > useful) solution. >> >> No. It's impossible. >> /proc/{pid}/cmdline read user process's memory. iow, this prctl() don't >> receive string, it receive virtual address itself. [...] > > it's not 'impossible' at all, you yourself mention ptrace: If another process is going to use ptrace to inject the cmdline string into the victim's address space, it can also temporarily hijack a thread to run prctl() on its behalf... -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |