Re: yama_ptrace_access_check(): possible recursive locking detected
On Wed, Aug 15, 2012 at 11:44 AM, Alan Cox wrote: >> It sounds like get_task_comm shouldn't have locking at all then? It >> should just do a length-limited copy and make sure there is a trailing >> 0-byte? > > It has locking so that it has a consistent state and more importantly it > has an accessor function > > Directly accessing it is asking for bugs in future. If you hold the > needed lock then just add an > > __get_task_comm() > > method that asserts the lock is held. That way the rest of the behaviour > remains properly encapsulated for when someone changes it. But what's been discussed is that no lock is needed if the caller doesn't care about the accuracy of the contents, which is the situation I'm in. Looking at other readers of ->comm, they just either use it directly or copy it directly. Only when accuracy matters does the kernel use get_task_comm. -Kees -- Kees Cook Chrome OS Security -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: yama_ptrace_access_check(): possible recursive locking detected
> It sounds like get_task_comm shouldn't have locking at all then? It > should just do a length-limited copy and make sure there is a trailing > 0-byte? It has locking so that it has a consistent state and more importantly it has an accessor function Directly accessing it is asking for bugs in future. If you hold the needed lock then just add an __get_task_comm() method that asserts the lock is held. That way the rest of the behaviour remains properly encapsulated for when someone changes it. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: yama_ptrace_access_check(): possible recursive locking detected
On Wed, Aug 15, 2012 at 11:17 AM, Oleg Nesterov wrote: > On 08/15, Kees Cook wrote: >> >> It sounds like get_task_comm shouldn't have locking at all then? It >> should just do a length-limited copy > > Without task_lock() get_task_comm() can copy incomplete new name. > > Honestly, I do not know any user which "strictly" needs the correct > name. may be proc. Right, which is my point -- if the race to read against set_task_comm() isn't useful to anything, why lock in get_task_comm at all? > >> and make sure there is a trailing >> 0-byte? > > get_task_comm()->strncpy() should always see (and copy) 0-byte. > comm[TASK_COMM_LEN - 1] == '\0' and this byte is never changed. > > set_task_comm()->strlcpy() can write to this byte, but it can > only write 0 again. Right, and set_task_comm even does a memset() of 0 over the whole area before the strlcpy too. Regardless, it sounds like just using ->comm directly is fine; I'll send a patch. -Kees -- Kees Cook Chrome OS Security -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: yama_ptrace_access_check(): possible recursive locking detected
On 08/15, Kees Cook wrote: > > It sounds like get_task_comm shouldn't have locking at all then? It > should just do a length-limited copy Without task_lock() get_task_comm() can copy incomplete new name. Honestly, I do not know any user which "strictly" needs the correct name. may be proc. > and make sure there is a trailing > 0-byte? get_task_comm()->strncpy() should always see (and copy) 0-byte. comm[TASK_COMM_LEN - 1] == '\0' and this byte is never changed. set_task_comm()->strlcpy() can write to this byte, but it can only write 0 again. Or I am totally confused ;) Oleg. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: yama_ptrace_access_check(): possible recursive locking detected
On Wed, Aug 15, 2012 at 10:56 AM, Oleg Nesterov wrote: > On 08/15, Peter Zijlstra wrote: >> >> On Wed, 2012-08-15 at 15:01 +0200, Oleg Nesterov wrote: >> > BTW, set_task_comm()->wmb() and memset() should die. There are >> > not needed afaics, and the comment is misleading. >> >> As long as we guarantee there's always a terminating '\0', > > Yes, but we already have this guarantee? > > Unless of course some buggy code does something wrong with task->comm[], > but nobody should do this. > > IOW, task->comm[TASK_COMM_LEN - 1] is always 0, no? > >> now strlcpy() >> doesn't pad the result, > > afaics set_task_comm()->strlcpy() doesn't change the last byte too. > >> however if we initialize the ->comm to all 0s in >> fork() > > fork() is special, yes. ->comm is copied by dup_task_struct() and > the new task_struct can have everything in ->comm. But nobody can > see the new task yet, and nobody can play with its ->comm. > > Or I misunderstood? > >> That barrier is indeed completely pointless as there's no pairing >> barrier anywhere. > > Yes, agreed. It sounds like get_task_comm shouldn't have locking at all then? It should just do a length-limited copy and make sure there is a trailing 0-byte? -Kees -- Kees Cook Chrome OS Security -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: yama_ptrace_access_check(): possible recursive locking detected
On 08/15, Peter Zijlstra wrote: > > On Wed, 2012-08-15 at 15:01 +0200, Oleg Nesterov wrote: > > BTW, set_task_comm()->wmb() and memset() should die. There are > > not needed afaics, and the comment is misleading. > > As long as we guarantee there's always a terminating '\0', Yes, but we already have this guarantee? Unless of course some buggy code does something wrong with task->comm[], but nobody should do this. IOW, task->comm[TASK_COMM_LEN - 1] is always 0, no? > now strlcpy() > doesn't pad the result, afaics set_task_comm()->strlcpy() doesn't change the last byte too. > however if we initialize the ->comm to all 0s in > fork() fork() is special, yes. ->comm is copied by dup_task_struct() and the new task_struct can have everything in ->comm. But nobody can see the new task yet, and nobody can play with its ->comm. Or I misunderstood? > That barrier is indeed completely pointless as there's no pairing > barrier anywhere. Yes, agreed. Oleg. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: yama_ptrace_access_check(): possible recursive locking detected
On Wed, 2012-08-15 at 15:01 +0200, Oleg Nesterov wrote: > BTW, set_task_comm()->wmb() and memset() should die. There are > not needed afaics, and the comment is misleading. As long as we guarantee there's always a terminating '\0', now strlcpy() doesn't pad the result, however if we initialize the ->comm to all 0s in fork() or thereabouts, we should get this guarantee from the strlcpy() since that will never replace the last byte with anything but 0. That barrier is indeed completely pointless as there's no pairing barrier anywhere. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: yama_ptrace_access_check(): possible recursive locking detected
On 08/14, Kees Cook wrote:
>
> Okay, I've now managed to reproduce this locally. I added a bunch of
> debugging, and I think I understand what's going on. This warning is,
> actually, a false positive.
Sure. I mean that yes, this warning doesn't mean we already hit deadlock.
> get used recursively (the task_struct->alloc_lock), but they are
> separate instantiations ("task" is never "current").
Yes. But suppose that we have 2 tasks T1 and T2,
- T1 does ptrace(PTRACE_ATTACH, T2);
- T2 does ptrace(PTRACE_ATTACH, T1);
at the same time. This can lead to the "real" deadlock, no?
> So Oleg's suggestion of removing the locking around the reading of
> ->comm is wrong since it really does need the lock.
Nothing bad can happen without the lock. Yes, printk() can print
some string "in between" if we race with set_task_comm() but this
is all.
BTW, set_task_comm()->wmb() and memset() should die. There are
not needed afaics, and the comment is misleading.
Oleg.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: yama_ptrace_access_check(): possible recursive locking detected
On Tue, 2012-08-14 at 22:56 -0700, Kees Cook wrote: > So Oleg's suggestion of removing the locking around the reading of > ->comm is wrong since it really does need the lock. There's tons of code reading comm without locking.. you're saying that all is broken? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: yama_ptrace_access_check(): possible recursive locking detected
On Tue, 2012-08-14 at 22:56 -0700, Kees Cook wrote: So Oleg's suggestion of removing the locking around the reading of -comm is wrong since it really does need the lock. There's tons of code reading comm without locking.. you're saying that all is broken? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: yama_ptrace_access_check(): possible recursive locking detected
On 08/14, Kees Cook wrote: Okay, I've now managed to reproduce this locally. I added a bunch of debugging, and I think I understand what's going on. This warning is, actually, a false positive. Sure. I mean that yes, this warning doesn't mean we already hit deadlock. get used recursively (the task_struct-alloc_lock), but they are separate instantiations (task is never current). Yes. But suppose that we have 2 tasks T1 and T2, - T1 does ptrace(PTRACE_ATTACH, T2); - T2 does ptrace(PTRACE_ATTACH, T1); at the same time. This can lead to the real deadlock, no? So Oleg's suggestion of removing the locking around the reading of -comm is wrong since it really does need the lock. Nothing bad can happen without the lock. Yes, printk() can print some string in between if we race with set_task_comm() but this is all. BTW, set_task_comm()-wmb() and memset() should die. There are not needed afaics, and the comment is misleading. Oleg. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: yama_ptrace_access_check(): possible recursive locking detected
On Wed, 2012-08-15 at 15:01 +0200, Oleg Nesterov wrote: BTW, set_task_comm()-wmb() and memset() should die. There are not needed afaics, and the comment is misleading. As long as we guarantee there's always a terminating '\0', now strlcpy() doesn't pad the result, however if we initialize the -comm to all 0s in fork() or thereabouts, we should get this guarantee from the strlcpy() since that will never replace the last byte with anything but 0. That barrier is indeed completely pointless as there's no pairing barrier anywhere. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: yama_ptrace_access_check(): possible recursive locking detected
On 08/15, Peter Zijlstra wrote: On Wed, 2012-08-15 at 15:01 +0200, Oleg Nesterov wrote: BTW, set_task_comm()-wmb() and memset() should die. There are not needed afaics, and the comment is misleading. As long as we guarantee there's always a terminating '\0', Yes, but we already have this guarantee? Unless of course some buggy code does something wrong with task-comm[], but nobody should do this. IOW, task-comm[TASK_COMM_LEN - 1] is always 0, no? now strlcpy() doesn't pad the result, afaics set_task_comm()-strlcpy() doesn't change the last byte too. however if we initialize the -comm to all 0s in fork() fork() is special, yes. -comm is copied by dup_task_struct() and the new task_struct can have everything in -comm. But nobody can see the new task yet, and nobody can play with its -comm. Or I misunderstood? That barrier is indeed completely pointless as there's no pairing barrier anywhere. Yes, agreed. Oleg. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: yama_ptrace_access_check(): possible recursive locking detected
On Wed, Aug 15, 2012 at 10:56 AM, Oleg Nesterov o...@redhat.com wrote: On 08/15, Peter Zijlstra wrote: On Wed, 2012-08-15 at 15:01 +0200, Oleg Nesterov wrote: BTW, set_task_comm()-wmb() and memset() should die. There are not needed afaics, and the comment is misleading. As long as we guarantee there's always a terminating '\0', Yes, but we already have this guarantee? Unless of course some buggy code does something wrong with task-comm[], but nobody should do this. IOW, task-comm[TASK_COMM_LEN - 1] is always 0, no? now strlcpy() doesn't pad the result, afaics set_task_comm()-strlcpy() doesn't change the last byte too. however if we initialize the -comm to all 0s in fork() fork() is special, yes. -comm is copied by dup_task_struct() and the new task_struct can have everything in -comm. But nobody can see the new task yet, and nobody can play with its -comm. Or I misunderstood? That barrier is indeed completely pointless as there's no pairing barrier anywhere. Yes, agreed. It sounds like get_task_comm shouldn't have locking at all then? It should just do a length-limited copy and make sure there is a trailing 0-byte? -Kees -- Kees Cook Chrome OS Security -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: yama_ptrace_access_check(): possible recursive locking detected
On 08/15, Kees Cook wrote: It sounds like get_task_comm shouldn't have locking at all then? It should just do a length-limited copy Without task_lock() get_task_comm() can copy incomplete new name. Honestly, I do not know any user which strictly needs the correct name. may be proc. and make sure there is a trailing 0-byte? get_task_comm()-strncpy() should always see (and copy) 0-byte. comm[TASK_COMM_LEN - 1] == '\0' and this byte is never changed. set_task_comm()-strlcpy() can write to this byte, but it can only write 0 again. Or I am totally confused ;) Oleg. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: yama_ptrace_access_check(): possible recursive locking detected
On Wed, Aug 15, 2012 at 11:17 AM, Oleg Nesterov o...@redhat.com wrote: On 08/15, Kees Cook wrote: It sounds like get_task_comm shouldn't have locking at all then? It should just do a length-limited copy Without task_lock() get_task_comm() can copy incomplete new name. Honestly, I do not know any user which strictly needs the correct name. may be proc. Right, which is my point -- if the race to read against set_task_comm() isn't useful to anything, why lock in get_task_comm at all? and make sure there is a trailing 0-byte? get_task_comm()-strncpy() should always see (and copy) 0-byte. comm[TASK_COMM_LEN - 1] == '\0' and this byte is never changed. set_task_comm()-strlcpy() can write to this byte, but it can only write 0 again. Right, and set_task_comm even does a memset() of 0 over the whole area before the strlcpy too. Regardless, it sounds like just using -comm directly is fine; I'll send a patch. -Kees -- Kees Cook Chrome OS Security -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: yama_ptrace_access_check(): possible recursive locking detected
It sounds like get_task_comm shouldn't have locking at all then? It should just do a length-limited copy and make sure there is a trailing 0-byte? It has locking so that it has a consistent state and more importantly it has an accessor function Directly accessing it is asking for bugs in future. If you hold the needed lock then just add an __get_task_comm() method that asserts the lock is held. That way the rest of the behaviour remains properly encapsulated for when someone changes it. Alan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: yama_ptrace_access_check(): possible recursive locking detected
On Wed, Aug 15, 2012 at 11:44 AM, Alan Cox a...@lxorguk.ukuu.org.uk wrote: It sounds like get_task_comm shouldn't have locking at all then? It should just do a length-limited copy and make sure there is a trailing 0-byte? It has locking so that it has a consistent state and more importantly it has an accessor function Directly accessing it is asking for bugs in future. If you hold the needed lock then just add an __get_task_comm() method that asserts the lock is held. That way the rest of the behaviour remains properly encapsulated for when someone changes it. But what's been discussed is that no lock is needed if the caller doesn't care about the accuracy of the contents, which is the situation I'm in. Looking at other readers of -comm, they just either use it directly or copy it directly. Only when accuracy matters does the kernel use get_task_comm. -Kees -- Kees Cook Chrome OS Security -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: yama_ptrace_access_check(): possible recursive locking detected
On Tue, Aug 14, 2012 at 8:01 PM, Fengguang Wu wrote:
> On Tue, Aug 14, 2012 at 02:16:52PM -0700, Kees Cook wrote:
>> On Thu, Aug 9, 2012 at 6:52 PM, Fengguang Wu wrote:
>> > On Thu, Aug 09, 2012 at 06:39:34PM -0700, Kees Cook wrote:
>> >> Hi,
>> >>
>> >> So, after taking a closer look at this, I cannot understand how it's
>> >> possible. Yama's task_lock call is against "current", not "child",
>> >> which is what ptrace_may_access() is locking. And the same code makes
>> >> sure that current != child. Yama would never get called if current ==
>> >> child.
>> >>
>> >> How did you reproduce this situation?
>> >
>> > This warning can be triggered with Dave Jones' trinity tool:
>> >
>> > git://git.codemonkey.org.uk/trinity
>> >
>> > That's a very dangerous tool, please only run it as normal user in a
>> > backed up and chrooted test box. I personally run it inside an initrd.
>> > If you are interested in reproducing this, I can send you the ready
>> > made initrd in private email.
>>
>> Well, even with your initrd, I can't reproduce this. You're running
>> this against a stock kernel? I can't see how the path you've shown can
>
> Yes, it happens on 3.6-rc1.
>
>> possible happen. It could only happen if "task" was "current", but
>> there is an explicit test for that in ptrace_may_access(). Based on
>> the traceback, this is from reading /proc/$pid/stack (or
>> /proc/$pid/task/$tid/stack), rather than a direct ptrace() call, but
>> the code path for task != current still stands.
>>
>> I've tried both normal and "trinity -c read" and I haven't seen the
>> trace you found. :(
>>
>> If you can isolate the case further, I'm happy to fix it, but
>> currently, I don't see a path where this can deadlock.
>
> Even if it's proved to be a false warning, it's still very worthwhile
> to apply Oleg's fix to quiet the warning. Such warnings will mislead
> my bisect script. The sooner it's fixed, the better. And I like Oleg's
> fix because it makes things more simple and a little bit faster.
>
> btw, I see some different warnings when digging through the boot logs:
>
> (x86_64-randconfig-b050)
> [ 128.725667]
> [ 128.728649] =
> [ 128.733989] [ INFO: possible recursive locking detected ]
> [ 128.733989] 3.6.0-rc1 #1 Not tainted
> [ 128.733989] -
> [ 128.733989] trinity-child0/523 is trying to acquire lock:
> [ 128.733989] (&(>alloc_lock)->rlock){+.+...}, at: []
> get_task_comm+0x20/0x47
> [ 128.733989]
> [ 128.733989] but task is already holding lock:
> [ 128.733989] (&(>alloc_lock)->rlock){+.+...}, at: []
> sys_ptrace+0x158/0x313
> [ 128.733989]
> [ 128.733989] other info that might help us debug this:
> [ 128.733989] Possible unsafe locking scenario:
> [ 128.733989]
> [ 128.733989]CPU0
> [ 128.733989]
> [ 128.733989] lock(&(>alloc_lock)->rlock);
> [ 128.733989] lock(&(>alloc_lock)->rlock);
> [ 128.733989]
> [ 128.733989] *** DEADLOCK ***
> [ 128.733989]
> [ 128.733989] May be due to missing lock nesting notation
> [ 128.733989]
> [ 128.733989] 2 locks held by trinity-child0/523:
> [ 128.733989] #0: (>cred_guard_mutex){+.+.+.}, at:
> [] sys_ptrace+0x13d/0x313
> [ 128.733989] #1: (&(>alloc_lock)->rlock){+.+...}, at:
> [] sys_ptrace+0x158/0x313
> [ 128.733989]
> [ 128.733989] stack backtrace:
> [ 128.733989] Pid: 523, comm: trinity-child0 Not tainted 3.6.0-rc1 #1
> [ 128.733989] Call Trace:
> [ 128.733989] [] __lock_acquire+0xbe0/0xcfb
> [ 128.733989] [] ? mark_lock+0x2d/0x212
> [ 128.733989] [] ? mark_lock+0x2d/0x212
> [ 128.733989] [] lock_acquire+0x82/0x9d
> [ 128.733989] [] ? get_task_comm+0x20/0x47
> [ 128.733989] [] _raw_spin_lock+0x3b/0x4a
> [ 128.733989] [] ? get_task_comm+0x20/0x47
> [ 128.733989] [] get_task_comm+0x20/0x47
> [ 128.733989] [] yama_ptrace_access_check+0x16a/0x1c7
> [ 128.733989] [] ? lock_release+0x12b/0x157
> [ 128.733989] [] security_ptrace_access_check+0xe/0x10
> [ 128.733989] [] __ptrace_may_access+0x109/0x11b
> [ 128.733989] [] sys_ptrace+0x165/0x313
> [ 128.733989] [] system_call_fastpath+0x16/0x1b
> [ 128.823670] ptrace of pid 522 was attempted by: trinity-child0 (pid 523)
Okay, I've now managed to reproduce this locally. I added a bunch of
debugging, and I think I understand what's going on. This warning is,
actually, a false positive. It is correct in that the _class_ of locks
get used recursively (the task_struct->alloc_lock), but they are
separate instantiations ("task" is never "current").
So Oleg's suggestion of removing the locking around the reading of
->comm is wrong since it really does need the lock. I've read the bit
on declaring nested locking, but it doesn't seem to apply here. I have
no idea what the correct solution for this is since the code already
verifies that the same task_struct instance will never be the locked
twice. How can I teach the lockdep checker about this?
-Kees
--
Kees Cook
Re: yama_ptrace_access_check(): possible recursive locking detected
On Tue, Aug 14, 2012 at 02:16:52PM -0700, Kees Cook wrote:
> On Thu, Aug 9, 2012 at 6:52 PM, Fengguang Wu wrote:
> > On Thu, Aug 09, 2012 at 06:39:34PM -0700, Kees Cook wrote:
> >> Hi,
> >>
> >> So, after taking a closer look at this, I cannot understand how it's
> >> possible. Yama's task_lock call is against "current", not "child",
> >> which is what ptrace_may_access() is locking. And the same code makes
> >> sure that current != child. Yama would never get called if current ==
> >> child.
> >>
> >> How did you reproduce this situation?
> >
> > This warning can be triggered with Dave Jones' trinity tool:
> >
> > git://git.codemonkey.org.uk/trinity
> >
> > That's a very dangerous tool, please only run it as normal user in a
> > backed up and chrooted test box. I personally run it inside an initrd.
> > If you are interested in reproducing this, I can send you the ready
> > made initrd in private email.
>
> Well, even with your initrd, I can't reproduce this. You're running
> this against a stock kernel? I can't see how the path you've shown can
Yes, it happens on 3.6-rc1.
> possible happen. It could only happen if "task" was "current", but
> there is an explicit test for that in ptrace_may_access(). Based on
> the traceback, this is from reading /proc/$pid/stack (or
> /proc/$pid/task/$tid/stack), rather than a direct ptrace() call, but
> the code path for task != current still stands.
>
> I've tried both normal and "trinity -c read" and I haven't seen the
> trace you found. :(
>
> If you can isolate the case further, I'm happy to fix it, but
> currently, I don't see a path where this can deadlock.
Even if it's proved to be a false warning, it's still very worthwhile
to apply Oleg's fix to quiet the warning. Such warnings will mislead
my bisect script. The sooner it's fixed, the better. And I like Oleg's
fix because it makes things more simple and a little bit faster.
btw, I see some different warnings when digging through the boot logs:
(x86_64-randconfig-b050)
[ 128.725667]
[ 128.728649] =
[ 128.733989] [ INFO: possible recursive locking detected ]
[ 128.733989] 3.6.0-rc1 #1 Not tainted
[ 128.733989] -
[ 128.733989] trinity-child0/523 is trying to acquire lock:
[ 128.733989] (&(>alloc_lock)->rlock){+.+...}, at: []
get_task_comm+0x20/0x47
[ 128.733989]
[ 128.733989] but task is already holding lock:
[ 128.733989] (&(>alloc_lock)->rlock){+.+...}, at: []
sys_ptrace+0x158/0x313
[ 128.733989]
[ 128.733989] other info that might help us debug this:
[ 128.733989] Possible unsafe locking scenario:
[ 128.733989]
[ 128.733989]CPU0
[ 128.733989]
[ 128.733989] lock(&(>alloc_lock)->rlock);
[ 128.733989] lock(&(>alloc_lock)->rlock);
[ 128.733989]
[ 128.733989] *** DEADLOCK ***
[ 128.733989]
[ 128.733989] May be due to missing lock nesting notation
[ 128.733989]
[ 128.733989] 2 locks held by trinity-child0/523:
[ 128.733989] #0: (>cred_guard_mutex){+.+.+.}, at: []
sys_ptrace+0x13d/0x313
[ 128.733989] #1: (&(>alloc_lock)->rlock){+.+...}, at:
[] sys_ptrace+0x158/0x313
[ 128.733989]
[ 128.733989] stack backtrace:
[ 128.733989] Pid: 523, comm: trinity-child0 Not tainted 3.6.0-rc1 #1
[ 128.733989] Call Trace:
[ 128.733989] [] __lock_acquire+0xbe0/0xcfb
[ 128.733989] [] ? mark_lock+0x2d/0x212
[ 128.733989] [] ? mark_lock+0x2d/0x212
[ 128.733989] [] lock_acquire+0x82/0x9d
[ 128.733989] [] ? get_task_comm+0x20/0x47
[ 128.733989] [] _raw_spin_lock+0x3b/0x4a
[ 128.733989] [] ? get_task_comm+0x20/0x47
[ 128.733989] [] get_task_comm+0x20/0x47
[ 128.733989] [] yama_ptrace_access_check+0x16a/0x1c7
[ 128.733989] [] ? lock_release+0x12b/0x157
[ 128.733989] [] security_ptrace_access_check+0xe/0x10
[ 128.733989] [] __ptrace_may_access+0x109/0x11b
[ 128.733989] [] sys_ptrace+0x165/0x313
[ 128.733989] [] system_call_fastpath+0x16/0x1b
[ 128.823670] ptrace of pid 522 was attempted by: trinity-child0 (pid 523)
(x86_64-randconfig-k056)
[ 87.057392]
[ 87.058009] =
[ 87.058009] [ INFO: possible recursive locking detected ]
[ 87.058009] 3.6.0-rc1-00011-gf8cdda8 #2 Not tainted
[ 87.058009] -
[ 87.058009] trinity-child0/328 is trying to acquire lock:
[ 87.058009] (&(>alloc_lock)->rlock){+.+...}, at: []
spin_lock+0x9/0xb
[ 87.058009]
[ 87.058009] but task is already holding lock:
[ 87.058009] (&(>alloc_lock)->rlock){+.+...}, at: []
ptrace_attach+0xa4/0x208
[ 87.058009]
[ 87.058009] other info that might help us debug this:
[ 87.058009] Possible unsafe locking scenario:
[ 87.058009]
[ 87.058009]CPU0
[ 87.058009]
[ 87.058009] lock(&(>alloc_lock)->rlock);
[ 87.058009] lock(&(>alloc_lock)->rlock);
[ 87.058009]
[ 87.058009] *** DEADLOCK ***
[ 87.058009]
[ 87.058009] May be due to missing lock nesting
Re: yama_ptrace_access_check(): possible recursive locking detected
On Thu, Aug 9, 2012 at 6:52 PM, Fengguang Wu wrote:
> On Thu, Aug 09, 2012 at 06:39:34PM -0700, Kees Cook wrote:
>> Hi,
>>
>> So, after taking a closer look at this, I cannot understand how it's
>> possible. Yama's task_lock call is against "current", not "child",
>> which is what ptrace_may_access() is locking. And the same code makes
>> sure that current != child. Yama would never get called if current ==
>> child.
>>
>> How did you reproduce this situation?
>
> This warning can be triggered with Dave Jones' trinity tool:
>
> git://git.codemonkey.org.uk/trinity
>
> That's a very dangerous tool, please only run it as normal user in a
> backed up and chrooted test box. I personally run it inside an initrd.
> If you are interested in reproducing this, I can send you the ready
> made initrd in private email.
Well, even with your initrd, I can't reproduce this. You're running
this against a stock kernel? I can't see how the path you've shown can
possible happen. It could only happen if "task" was "current", but
there is an explicit test for that in ptrace_may_access(). Based on
the traceback, this is from reading /proc/$pid/stack (or
/proc/$pid/task/$tid/stack), rather than a direct ptrace() call, but
the code path for task != current still stands.
I've tried both normal and "trinity -c read" and I haven't seen the
trace you found. :(
If you can isolate the case further, I'm happy to fix it, but
currently, I don't see a path where this can deadlock.
Thanks,
-Kees
>> On Thu, Jul 26, 2012 at 6:47 AM, Fengguang Wu wrote:
>> > Here is a recursive lock possibility:
>> >
>> > ptrace_may_access()
>> > =>task_lock(task);
>> > yama_ptrace_access_check()
>> > get_task_comm()
>> > => task_lock(task);
>> >
>> > [ 60.230444] =
>> > [ 60.232078] [ INFO: possible recursive locking detected ]
>> > [ 60.232078] 3.5.0+ #281 Not tainted
>> > [ 60.232078] -
>> > [ 60.232078] trinity-child0/17019 is trying to acquire lock:
>> > [ 60.232078] (&(>alloc_lock)->rlock){+.+...}, at: []
>> > get_task_comm+0x4a/0xf0
>> > [ 60.232078]
>> > [ 60.232078] but task is already holding lock:
>> > [ 60.232078] (&(>alloc_lock)->rlock){+.+...}, at: []
>> > ptrace_may_access+0x4a/0xf0
>> > [ 60.232078]
>> > [ 60.232078] other info that might help us debug this:
>> > [ 60.232078] Possible unsafe locking scenario:
>> > [ 60.232078]
>> > [ 60.232078]CPU0
>> > [ 60.232078]
>> > [ 60.232078] lock(&(>alloc_lock)->rlock);
>> > [ 60.232078] lock(&(>alloc_lock)->rlock);
>> > [ 60.232078]
>> > [ 60.232078] *** DEADLOCK ***
>> > [ 60.232078]
>> > [ 60.232078] May be due to missing lock nesting notation
>> > [ 60.232078]
>> > [ 60.232078] 3 locks held by trinity-child0/17019:
>> > [ 60.232078] #0: (>lock){+.+.+.}, at: []
>> > seq_read+0x33/0x6b0
>> > [ 60.232078] #1: (>cred_guard_mutex){+.+.+.}, at: []
>> > lock_trace+0x2e/0xb0
>> > [ 60.232078] #2: (&(>alloc_lock)->rlock){+.+...}, at: []
>> > ptrace_may_access+0x4a/0xf0
>> > [ 60.232078]
>> > [ 60.232078] stack backtrace:
>> > [ 60.232078] Pid: 17019, comm: trinity-child0 Not tainted 3.5.0+ #281
>> > [ 60.232078] Call Trace:
>> > [ 60.232078] [] __lock_acquire+0x1498/0x14f0
>> > [ 60.232078] [] ? trace_hardirqs_off+0x27/0x40
>> > [ 60.232078] [] lock_acquire+0xd0/0x110
>> > [ 60.232078] [] ? get_task_comm+0x4a/0xf0
>> > [ 60.232078] [] _raw_spin_lock+0x60/0x110
>> > [ 60.232078] [] ? get_task_comm+0x4a/0xf0
>> > [ 60.232078] [] get_task_comm+0x4a/0xf0
>> > [ 60.232078] [] yama_ptrace_access_check+0x468/0x4a0
>> > [ 60.232078] [] ? yama_ptrace_access_check+0x15f/0x4a0
>> > [ 60.232078] [] security_ptrace_access_check+0x1a/0x30
>> > [ 60.232078] [] __ptrace_may_access+0x189/0x310
>> > [ 60.232078] [] ? __ptrace_may_access+0x2c/0x310
>> > [ 60.232078] [] ptrace_may_access+0x7d/0xf0
>> > [ 60.232078] [] lock_trace+0x6a/0xb0
>> > [ 60.232078] [] proc_pid_stack+0x76/0x170
>> > [ 60.232078] [] proc_single_show+0x74/0x100
>> > [ 60.232078] [] seq_read+0x163/0x6b0
>> > [ 60.232078] [] ? do_setitimer+0x220/0x330
>> > [ 60.232078] [] ? seq_lseek+0x1f0/0x1f0
>> > [ 60.232078] [] vfs_read+0xca/0x280
>> > [ 60.232078] [] ? seq_lseek+0x1f0/0x1f0
>> > [ 60.232078] [] sys_read+0x66/0xe0
>> > [ 60.232078] [] syscall_call+0x7/0xb
>> > [ 60.232078] [] ? __schedule+0x2a0/0xc80
--
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: yama_ptrace_access_check(): possible recursive locking detected
On Thu, Aug 9, 2012 at 6:52 PM, Fengguang Wu fengguang...@intel.com wrote:
On Thu, Aug 09, 2012 at 06:39:34PM -0700, Kees Cook wrote:
Hi,
So, after taking a closer look at this, I cannot understand how it's
possible. Yama's task_lock call is against current, not child,
which is what ptrace_may_access() is locking. And the same code makes
sure that current != child. Yama would never get called if current ==
child.
How did you reproduce this situation?
This warning can be triggered with Dave Jones' trinity tool:
git://git.codemonkey.org.uk/trinity
That's a very dangerous tool, please only run it as normal user in a
backed up and chrooted test box. I personally run it inside an initrd.
If you are interested in reproducing this, I can send you the ready
made initrd in private email.
Well, even with your initrd, I can't reproduce this. You're running
this against a stock kernel? I can't see how the path you've shown can
possible happen. It could only happen if task was current, but
there is an explicit test for that in ptrace_may_access(). Based on
the traceback, this is from reading /proc/$pid/stack (or
/proc/$pid/task/$tid/stack), rather than a direct ptrace() call, but
the code path for task != current still stands.
I've tried both normal and trinity -c read and I haven't seen the
trace you found. :(
If you can isolate the case further, I'm happy to fix it, but
currently, I don't see a path where this can deadlock.
Thanks,
-Kees
On Thu, Jul 26, 2012 at 6:47 AM, Fengguang Wu fengguang...@intel.com wrote:
Here is a recursive lock possibility:
ptrace_may_access()
=task_lock(task);
yama_ptrace_access_check()
get_task_comm()
= task_lock(task);
[ 60.230444] =
[ 60.232078] [ INFO: possible recursive locking detected ]
[ 60.232078] 3.5.0+ #281 Not tainted
[ 60.232078] -
[ 60.232078] trinity-child0/17019 is trying to acquire lock:
[ 60.232078] ((p-alloc_lock)-rlock){+.+...}, at: [c1176ffa]
get_task_comm+0x4a/0xf0
[ 60.232078]
[ 60.232078] but task is already holding lock:
[ 60.232078] ((p-alloc_lock)-rlock){+.+...}, at: [c10653fa]
ptrace_may_access+0x4a/0xf0
[ 60.232078]
[ 60.232078] other info that might help us debug this:
[ 60.232078] Possible unsafe locking scenario:
[ 60.232078]
[ 60.232078]CPU0
[ 60.232078]
[ 60.232078] lock((p-alloc_lock)-rlock);
[ 60.232078] lock((p-alloc_lock)-rlock);
[ 60.232078]
[ 60.232078] *** DEADLOCK ***
[ 60.232078]
[ 60.232078] May be due to missing lock nesting notation
[ 60.232078]
[ 60.232078] 3 locks held by trinity-child0/17019:
[ 60.232078] #0: (p-lock){+.+.+.}, at: [c11a9683]
seq_read+0x33/0x6b0
[ 60.232078] #1: (sig-cred_guard_mutex){+.+.+.}, at: [c11ff8ae]
lock_trace+0x2e/0xb0
[ 60.232078] #2: ((p-alloc_lock)-rlock){+.+...}, at: [c10653fa]
ptrace_may_access+0x4a/0xf0
[ 60.232078]
[ 60.232078] stack backtrace:
[ 60.232078] Pid: 17019, comm: trinity-child0 Not tainted 3.5.0+ #281
[ 60.232078] Call Trace:
[ 60.232078] [c10c6238] __lock_acquire+0x1498/0x14f0
[ 60.232078] [c10be7e7] ? trace_hardirqs_off+0x27/0x40
[ 60.232078] [c10c6360] lock_acquire+0xd0/0x110
[ 60.232078] [c1176ffa] ? get_task_comm+0x4a/0xf0
[ 60.232078] [c1422290] _raw_spin_lock+0x60/0x110
[ 60.232078] [c1176ffa] ? get_task_comm+0x4a/0xf0
[ 60.232078] [c1176ffa] get_task_comm+0x4a/0xf0
[ 60.232078] [c1246798] yama_ptrace_access_check+0x468/0x4a0
[ 60.232078] [c124648f] ? yama_ptrace_access_check+0x15f/0x4a0
[ 60.232078] [c124209a] security_ptrace_access_check+0x1a/0x30
[ 60.232078] [c1065229] __ptrace_may_access+0x189/0x310
[ 60.232078] [c10650cc] ? __ptrace_may_access+0x2c/0x310
[ 60.232078] [c106542d] ptrace_may_access+0x7d/0xf0
[ 60.232078] [c11ff8ea] lock_trace+0x6a/0xb0
[ 60.232078] [c11ffa46] proc_pid_stack+0x76/0x170
[ 60.232078] [c1201064] proc_single_show+0x74/0x100
[ 60.232078] [c11a97b3] seq_read+0x163/0x6b0
[ 60.232078] [c105bf70] ? do_setitimer+0x220/0x330
[ 60.232078] [c11a9650] ? seq_lseek+0x1f0/0x1f0
[ 60.232078] [c116b55a] vfs_read+0xca/0x280
[ 60.232078] [c11a9650] ? seq_lseek+0x1f0/0x1f0
[ 60.232078] [c116b776] sys_read+0x66/0xe0
[ 60.232078] [c1423d9d] syscall_call+0x7/0xb
[ 60.232078] [c142] ? __schedule+0x2a0/0xc80
--
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: yama_ptrace_access_check(): possible recursive locking detected
On Tue, Aug 14, 2012 at 02:16:52PM -0700, Kees Cook wrote:
On Thu, Aug 9, 2012 at 6:52 PM, Fengguang Wu fengguang...@intel.com wrote:
On Thu, Aug 09, 2012 at 06:39:34PM -0700, Kees Cook wrote:
Hi,
So, after taking a closer look at this, I cannot understand how it's
possible. Yama's task_lock call is against current, not child,
which is what ptrace_may_access() is locking. And the same code makes
sure that current != child. Yama would never get called if current ==
child.
How did you reproduce this situation?
This warning can be triggered with Dave Jones' trinity tool:
git://git.codemonkey.org.uk/trinity
That's a very dangerous tool, please only run it as normal user in a
backed up and chrooted test box. I personally run it inside an initrd.
If you are interested in reproducing this, I can send you the ready
made initrd in private email.
Well, even with your initrd, I can't reproduce this. You're running
this against a stock kernel? I can't see how the path you've shown can
Yes, it happens on 3.6-rc1.
possible happen. It could only happen if task was current, but
there is an explicit test for that in ptrace_may_access(). Based on
the traceback, this is from reading /proc/$pid/stack (or
/proc/$pid/task/$tid/stack), rather than a direct ptrace() call, but
the code path for task != current still stands.
I've tried both normal and trinity -c read and I haven't seen the
trace you found. :(
If you can isolate the case further, I'm happy to fix it, but
currently, I don't see a path where this can deadlock.
Even if it's proved to be a false warning, it's still very worthwhile
to apply Oleg's fix to quiet the warning. Such warnings will mislead
my bisect script. The sooner it's fixed, the better. And I like Oleg's
fix because it makes things more simple and a little bit faster.
btw, I see some different warnings when digging through the boot logs:
(x86_64-randconfig-b050)
[ 128.725667]
[ 128.728649] =
[ 128.733989] [ INFO: possible recursive locking detected ]
[ 128.733989] 3.6.0-rc1 #1 Not tainted
[ 128.733989] -
[ 128.733989] trinity-child0/523 is trying to acquire lock:
[ 128.733989] ((p-alloc_lock)-rlock){+.+...}, at: [810e0481]
get_task_comm+0x20/0x47
[ 128.733989]
[ 128.733989] but task is already holding lock:
[ 128.733989] ((p-alloc_lock)-rlock){+.+...}, at: [810572ab]
sys_ptrace+0x158/0x313
[ 128.733989]
[ 128.733989] other info that might help us debug this:
[ 128.733989] Possible unsafe locking scenario:
[ 128.733989]
[ 128.733989]CPU0
[ 128.733989]
[ 128.733989] lock((p-alloc_lock)-rlock);
[ 128.733989] lock((p-alloc_lock)-rlock);
[ 128.733989]
[ 128.733989] *** DEADLOCK ***
[ 128.733989]
[ 128.733989] May be due to missing lock nesting notation
[ 128.733989]
[ 128.733989] 2 locks held by trinity-child0/523:
[ 128.733989] #0: (sig-cred_guard_mutex){+.+.+.}, at: [81057290]
sys_ptrace+0x13d/0x313
[ 128.733989] #1: ((p-alloc_lock)-rlock){+.+...}, at:
[810572ab] sys_ptrace+0x158/0x313
[ 128.733989]
[ 128.733989] stack backtrace:
[ 128.733989] Pid: 523, comm: trinity-child0 Not tainted 3.6.0-rc1 #1
[ 128.733989] Call Trace:
[ 128.733989] [81085649] __lock_acquire+0xbe0/0xcfb
[ 128.733989] [81084884] ? mark_lock+0x2d/0x212
[ 128.733989] [81084884] ? mark_lock+0x2d/0x212
[ 128.733989] [8108639d] lock_acquire+0x82/0x9d
[ 128.733989] [810e0481] ? get_task_comm+0x20/0x47
[ 128.733989] [81a35ddf] _raw_spin_lock+0x3b/0x4a
[ 128.733989] [810e0481] ? get_task_comm+0x20/0x47
[ 128.733989] [810e0481] get_task_comm+0x20/0x47
[ 128.733989] [81392c01] yama_ptrace_access_check+0x16a/0x1c7
[ 128.733989] [810864e3] ? lock_release+0x12b/0x157
[ 128.733989] [81390bfb] security_ptrace_access_check+0xe/0x10
[ 128.733989] [81056e2b] __ptrace_may_access+0x109/0x11b
[ 128.733989] [810572b8] sys_ptrace+0x165/0x313
[ 128.733989] [81a37079] system_call_fastpath+0x16/0x1b
[ 128.823670] ptrace of pid 522 was attempted by: trinity-child0 (pid 523)
(x86_64-randconfig-k056)
[ 87.057392]
[ 87.058009] =
[ 87.058009] [ INFO: possible recursive locking detected ]
[ 87.058009] 3.6.0-rc1-00011-gf8cdda8 #2 Not tainted
[ 87.058009] -
[ 87.058009] trinity-child0/328 is trying to acquire lock:
[ 87.058009] ((p-alloc_lock)-rlock){+.+...}, at: [81104632]
spin_lock+0x9/0xb
[ 87.058009]
[ 87.058009] but task is already holding lock:
[ 87.058009] ((p-alloc_lock)-rlock){+.+...}, at: [8106cb40]
ptrace_attach+0xa4/0x208
[ 87.058009]
[ 87.058009] other info that might help us debug this:
[ 87.058009] Possible unsafe locking scenario:
[
Re: yama_ptrace_access_check(): possible recursive locking detected
On Tue, Aug 14, 2012 at 8:01 PM, Fengguang Wu fengguang...@intel.com wrote:
On Tue, Aug 14, 2012 at 02:16:52PM -0700, Kees Cook wrote:
On Thu, Aug 9, 2012 at 6:52 PM, Fengguang Wu fengguang...@intel.com wrote:
On Thu, Aug 09, 2012 at 06:39:34PM -0700, Kees Cook wrote:
Hi,
So, after taking a closer look at this, I cannot understand how it's
possible. Yama's task_lock call is against current, not child,
which is what ptrace_may_access() is locking. And the same code makes
sure that current != child. Yama would never get called if current ==
child.
How did you reproduce this situation?
This warning can be triggered with Dave Jones' trinity tool:
git://git.codemonkey.org.uk/trinity
That's a very dangerous tool, please only run it as normal user in a
backed up and chrooted test box. I personally run it inside an initrd.
If you are interested in reproducing this, I can send you the ready
made initrd in private email.
Well, even with your initrd, I can't reproduce this. You're running
this against a stock kernel? I can't see how the path you've shown can
Yes, it happens on 3.6-rc1.
possible happen. It could only happen if task was current, but
there is an explicit test for that in ptrace_may_access(). Based on
the traceback, this is from reading /proc/$pid/stack (or
/proc/$pid/task/$tid/stack), rather than a direct ptrace() call, but
the code path for task != current still stands.
I've tried both normal and trinity -c read and I haven't seen the
trace you found. :(
If you can isolate the case further, I'm happy to fix it, but
currently, I don't see a path where this can deadlock.
Even if it's proved to be a false warning, it's still very worthwhile
to apply Oleg's fix to quiet the warning. Such warnings will mislead
my bisect script. The sooner it's fixed, the better. And I like Oleg's
fix because it makes things more simple and a little bit faster.
btw, I see some different warnings when digging through the boot logs:
(x86_64-randconfig-b050)
[ 128.725667]
[ 128.728649] =
[ 128.733989] [ INFO: possible recursive locking detected ]
[ 128.733989] 3.6.0-rc1 #1 Not tainted
[ 128.733989] -
[ 128.733989] trinity-child0/523 is trying to acquire lock:
[ 128.733989] ((p-alloc_lock)-rlock){+.+...}, at: [810e0481]
get_task_comm+0x20/0x47
[ 128.733989]
[ 128.733989] but task is already holding lock:
[ 128.733989] ((p-alloc_lock)-rlock){+.+...}, at: [810572ab]
sys_ptrace+0x158/0x313
[ 128.733989]
[ 128.733989] other info that might help us debug this:
[ 128.733989] Possible unsafe locking scenario:
[ 128.733989]
[ 128.733989]CPU0
[ 128.733989]
[ 128.733989] lock((p-alloc_lock)-rlock);
[ 128.733989] lock((p-alloc_lock)-rlock);
[ 128.733989]
[ 128.733989] *** DEADLOCK ***
[ 128.733989]
[ 128.733989] May be due to missing lock nesting notation
[ 128.733989]
[ 128.733989] 2 locks held by trinity-child0/523:
[ 128.733989] #0: (sig-cred_guard_mutex){+.+.+.}, at:
[81057290] sys_ptrace+0x13d/0x313
[ 128.733989] #1: ((p-alloc_lock)-rlock){+.+...}, at:
[810572ab] sys_ptrace+0x158/0x313
[ 128.733989]
[ 128.733989] stack backtrace:
[ 128.733989] Pid: 523, comm: trinity-child0 Not tainted 3.6.0-rc1 #1
[ 128.733989] Call Trace:
[ 128.733989] [81085649] __lock_acquire+0xbe0/0xcfb
[ 128.733989] [81084884] ? mark_lock+0x2d/0x212
[ 128.733989] [81084884] ? mark_lock+0x2d/0x212
[ 128.733989] [8108639d] lock_acquire+0x82/0x9d
[ 128.733989] [810e0481] ? get_task_comm+0x20/0x47
[ 128.733989] [81a35ddf] _raw_spin_lock+0x3b/0x4a
[ 128.733989] [810e0481] ? get_task_comm+0x20/0x47
[ 128.733989] [810e0481] get_task_comm+0x20/0x47
[ 128.733989] [81392c01] yama_ptrace_access_check+0x16a/0x1c7
[ 128.733989] [810864e3] ? lock_release+0x12b/0x157
[ 128.733989] [81390bfb] security_ptrace_access_check+0xe/0x10
[ 128.733989] [81056e2b] __ptrace_may_access+0x109/0x11b
[ 128.733989] [810572b8] sys_ptrace+0x165/0x313
[ 128.733989] [81a37079] system_call_fastpath+0x16/0x1b
[ 128.823670] ptrace of pid 522 was attempted by: trinity-child0 (pid 523)
Okay, I've now managed to reproduce this locally. I added a bunch of
debugging, and I think I understand what's going on. This warning is,
actually, a false positive. It is correct in that the _class_ of locks
get used recursively (the task_struct-alloc_lock), but they are
separate instantiations (task is never current).
So Oleg's suggestion of removing the locking around the reading of
-comm is wrong since it really does need the lock. I've read the bit
on declaring nested locking, but it doesn't seem to apply here. I have
no idea what the correct solution for this is since the code
Re: yama_ptrace_access_check(): possible recursive locking detected
On Thu, Aug 09, 2012 at 06:39:34PM -0700, Kees Cook wrote:
> Hi,
>
> So, after taking a closer look at this, I cannot understand how it's
> possible. Yama's task_lock call is against "current", not "child",
> which is what ptrace_may_access() is locking. And the same code makes
> sure that current != child. Yama would never get called if current ==
> child.
>
> How did you reproduce this situation?
This warning can be triggered with Dave Jones' trinity tool:
git://git.codemonkey.org.uk/trinity
That's a very dangerous tool, please only run it as normal user in a
backed up and chrooted test box. I personally run it inside an initrd.
If you are interested in reproducing this, I can send you the ready
made initrd in private email.
Thanks,
Fengguang
> On Thu, Jul 26, 2012 at 6:47 AM, Fengguang Wu wrote:
> > Hi Kees,
> >
> > Here is a recursive lock possibility:
> >
> > ptrace_may_access()
> > =>task_lock(task);
> > yama_ptrace_access_check()
> > get_task_comm()
> > => task_lock(task);
> >
> > [ 60.230444] =
> > [ 60.232078] [ INFO: possible recursive locking detected ]
> > [ 60.232078] 3.5.0+ #281 Not tainted
> > [ 60.232078] -
> > [ 60.232078] trinity-child0/17019 is trying to acquire lock:
> > [ 60.232078] (&(>alloc_lock)->rlock){+.+...}, at: []
> > get_task_comm+0x4a/0xf0
> > [ 60.232078]
> > [ 60.232078] but task is already holding lock:
> > [ 60.232078] (&(>alloc_lock)->rlock){+.+...}, at: []
> > ptrace_may_access+0x4a/0xf0
> > [ 60.232078]
> > [ 60.232078] other info that might help us debug this:
> > [ 60.232078] Possible unsafe locking scenario:
> > [ 60.232078]
> > [ 60.232078]CPU0
> > [ 60.232078]
> > [ 60.232078] lock(&(>alloc_lock)->rlock);
> > [ 60.232078] lock(&(>alloc_lock)->rlock);
> > [ 60.232078]
> > [ 60.232078] *** DEADLOCK ***
> > [ 60.232078]
> > [ 60.232078] May be due to missing lock nesting notation
> > [ 60.232078]
> > [ 60.232078] 3 locks held by trinity-child0/17019:
> > [ 60.232078] #0: (>lock){+.+.+.}, at: []
> > seq_read+0x33/0x6b0
> > [ 60.232078] #1: (>cred_guard_mutex){+.+.+.}, at: []
> > lock_trace+0x2e/0xb0
> > [ 60.232078] #2: (&(>alloc_lock)->rlock){+.+...}, at: []
> > ptrace_may_access+0x4a/0xf0
> > [ 60.232078]
> > [ 60.232078] stack backtrace:
> > [ 60.232078] Pid: 17019, comm: trinity-child0 Not tainted 3.5.0+ #281
> > [ 60.232078] Call Trace:
> > [ 60.232078] [] __lock_acquire+0x1498/0x14f0
> > [ 60.232078] [] ? trace_hardirqs_off+0x27/0x40
> > [ 60.232078] [] lock_acquire+0xd0/0x110
> > [ 60.232078] [] ? get_task_comm+0x4a/0xf0
> > [ 60.232078] [] _raw_spin_lock+0x60/0x110
> > [ 60.232078] [] ? get_task_comm+0x4a/0xf0
> > [ 60.232078] [] get_task_comm+0x4a/0xf0
> > [ 60.232078] [] yama_ptrace_access_check+0x468/0x4a0
> > [ 60.232078] [] ? yama_ptrace_access_check+0x15f/0x4a0
> > [ 60.232078] [] security_ptrace_access_check+0x1a/0x30
> > [ 60.232078] [] __ptrace_may_access+0x189/0x310
> > [ 60.232078] [] ? __ptrace_may_access+0x2c/0x310
> > [ 60.232078] [] ptrace_may_access+0x7d/0xf0
> > [ 60.232078] [] lock_trace+0x6a/0xb0
> > [ 60.232078] [] proc_pid_stack+0x76/0x170
> > [ 60.232078] [] proc_single_show+0x74/0x100
> > [ 60.232078] [] seq_read+0x163/0x6b0
> > [ 60.232078] [] ? do_setitimer+0x220/0x330
> > [ 60.232078] [] ? seq_lseek+0x1f0/0x1f0
> > [ 60.232078] [] vfs_read+0xca/0x280
> > [ 60.232078] [] ? seq_lseek+0x1f0/0x1f0
> > [ 60.232078] [] sys_read+0x66/0xe0
> > [ 60.232078] [] syscall_call+0x7/0xb
> > [ 60.232078] [] ? __schedule+0x2a0/0xc80
> >
> > Thanks,
> > Fengguang
>
>
>
> --
> Kees Cook
> Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: yama_ptrace_access_check(): possible recursive locking detected
Hi,
So, after taking a closer look at this, I cannot understand how it's
possible. Yama's task_lock call is against "current", not "child",
which is what ptrace_may_access() is locking. And the same code makes
sure that current != child. Yama would never get called if current ==
child.
How did you reproduce this situation?
Thanks,
-Kees
On Thu, Jul 26, 2012 at 6:47 AM, Fengguang Wu wrote:
> Hi Kees,
>
> Here is a recursive lock possibility:
>
> ptrace_may_access()
> =>task_lock(task);
> yama_ptrace_access_check()
> get_task_comm()
> => task_lock(task);
>
> [ 60.230444] =
> [ 60.232078] [ INFO: possible recursive locking detected ]
> [ 60.232078] 3.5.0+ #281 Not tainted
> [ 60.232078] -
> [ 60.232078] trinity-child0/17019 is trying to acquire lock:
> [ 60.232078] (&(>alloc_lock)->rlock){+.+...}, at: []
> get_task_comm+0x4a/0xf0
> [ 60.232078]
> [ 60.232078] but task is already holding lock:
> [ 60.232078] (&(>alloc_lock)->rlock){+.+...}, at: []
> ptrace_may_access+0x4a/0xf0
> [ 60.232078]
> [ 60.232078] other info that might help us debug this:
> [ 60.232078] Possible unsafe locking scenario:
> [ 60.232078]
> [ 60.232078]CPU0
> [ 60.232078]
> [ 60.232078] lock(&(>alloc_lock)->rlock);
> [ 60.232078] lock(&(>alloc_lock)->rlock);
> [ 60.232078]
> [ 60.232078] *** DEADLOCK ***
> [ 60.232078]
> [ 60.232078] May be due to missing lock nesting notation
> [ 60.232078]
> [ 60.232078] 3 locks held by trinity-child0/17019:
> [ 60.232078] #0: (>lock){+.+.+.}, at: [] seq_read+0x33/0x6b0
> [ 60.232078] #1: (>cred_guard_mutex){+.+.+.}, at: []
> lock_trace+0x2e/0xb0
> [ 60.232078] #2: (&(>alloc_lock)->rlock){+.+...}, at: []
> ptrace_may_access+0x4a/0xf0
> [ 60.232078]
> [ 60.232078] stack backtrace:
> [ 60.232078] Pid: 17019, comm: trinity-child0 Not tainted 3.5.0+ #281
> [ 60.232078] Call Trace:
> [ 60.232078] [] __lock_acquire+0x1498/0x14f0
> [ 60.232078] [] ? trace_hardirqs_off+0x27/0x40
> [ 60.232078] [] lock_acquire+0xd0/0x110
> [ 60.232078] [] ? get_task_comm+0x4a/0xf0
> [ 60.232078] [] _raw_spin_lock+0x60/0x110
> [ 60.232078] [] ? get_task_comm+0x4a/0xf0
> [ 60.232078] [] get_task_comm+0x4a/0xf0
> [ 60.232078] [] yama_ptrace_access_check+0x468/0x4a0
> [ 60.232078] [] ? yama_ptrace_access_check+0x15f/0x4a0
> [ 60.232078] [] security_ptrace_access_check+0x1a/0x30
> [ 60.232078] [] __ptrace_may_access+0x189/0x310
> [ 60.232078] [] ? __ptrace_may_access+0x2c/0x310
> [ 60.232078] [] ptrace_may_access+0x7d/0xf0
> [ 60.232078] [] lock_trace+0x6a/0xb0
> [ 60.232078] [] proc_pid_stack+0x76/0x170
> [ 60.232078] [] proc_single_show+0x74/0x100
> [ 60.232078] [] seq_read+0x163/0x6b0
> [ 60.232078] [] ? do_setitimer+0x220/0x330
> [ 60.232078] [] ? seq_lseek+0x1f0/0x1f0
> [ 60.232078] [] vfs_read+0xca/0x280
> [ 60.232078] [] ? seq_lseek+0x1f0/0x1f0
> [ 60.232078] [] sys_read+0x66/0xe0
> [ 60.232078] [] syscall_call+0x7/0xb
> [ 60.232078] [] ? __schedule+0x2a0/0xc80
>
> Thanks,
> Fengguang
--
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: yama_ptrace_access_check(): possible recursive locking detected
Hi,
So, after taking a closer look at this, I cannot understand how it's
possible. Yama's task_lock call is against current, not child,
which is what ptrace_may_access() is locking. And the same code makes
sure that current != child. Yama would never get called if current ==
child.
How did you reproduce this situation?
Thanks,
-Kees
On Thu, Jul 26, 2012 at 6:47 AM, Fengguang Wu fengguang...@intel.com wrote:
Hi Kees,
Here is a recursive lock possibility:
ptrace_may_access()
=task_lock(task);
yama_ptrace_access_check()
get_task_comm()
= task_lock(task);
[ 60.230444] =
[ 60.232078] [ INFO: possible recursive locking detected ]
[ 60.232078] 3.5.0+ #281 Not tainted
[ 60.232078] -
[ 60.232078] trinity-child0/17019 is trying to acquire lock:
[ 60.232078] ((p-alloc_lock)-rlock){+.+...}, at: [c1176ffa]
get_task_comm+0x4a/0xf0
[ 60.232078]
[ 60.232078] but task is already holding lock:
[ 60.232078] ((p-alloc_lock)-rlock){+.+...}, at: [c10653fa]
ptrace_may_access+0x4a/0xf0
[ 60.232078]
[ 60.232078] other info that might help us debug this:
[ 60.232078] Possible unsafe locking scenario:
[ 60.232078]
[ 60.232078]CPU0
[ 60.232078]
[ 60.232078] lock((p-alloc_lock)-rlock);
[ 60.232078] lock((p-alloc_lock)-rlock);
[ 60.232078]
[ 60.232078] *** DEADLOCK ***
[ 60.232078]
[ 60.232078] May be due to missing lock nesting notation
[ 60.232078]
[ 60.232078] 3 locks held by trinity-child0/17019:
[ 60.232078] #0: (p-lock){+.+.+.}, at: [c11a9683] seq_read+0x33/0x6b0
[ 60.232078] #1: (sig-cred_guard_mutex){+.+.+.}, at: [c11ff8ae]
lock_trace+0x2e/0xb0
[ 60.232078] #2: ((p-alloc_lock)-rlock){+.+...}, at: [c10653fa]
ptrace_may_access+0x4a/0xf0
[ 60.232078]
[ 60.232078] stack backtrace:
[ 60.232078] Pid: 17019, comm: trinity-child0 Not tainted 3.5.0+ #281
[ 60.232078] Call Trace:
[ 60.232078] [c10c6238] __lock_acquire+0x1498/0x14f0
[ 60.232078] [c10be7e7] ? trace_hardirqs_off+0x27/0x40
[ 60.232078] [c10c6360] lock_acquire+0xd0/0x110
[ 60.232078] [c1176ffa] ? get_task_comm+0x4a/0xf0
[ 60.232078] [c1422290] _raw_spin_lock+0x60/0x110
[ 60.232078] [c1176ffa] ? get_task_comm+0x4a/0xf0
[ 60.232078] [c1176ffa] get_task_comm+0x4a/0xf0
[ 60.232078] [c1246798] yama_ptrace_access_check+0x468/0x4a0
[ 60.232078] [c124648f] ? yama_ptrace_access_check+0x15f/0x4a0
[ 60.232078] [c124209a] security_ptrace_access_check+0x1a/0x30
[ 60.232078] [c1065229] __ptrace_may_access+0x189/0x310
[ 60.232078] [c10650cc] ? __ptrace_may_access+0x2c/0x310
[ 60.232078] [c106542d] ptrace_may_access+0x7d/0xf0
[ 60.232078] [c11ff8ea] lock_trace+0x6a/0xb0
[ 60.232078] [c11ffa46] proc_pid_stack+0x76/0x170
[ 60.232078] [c1201064] proc_single_show+0x74/0x100
[ 60.232078] [c11a97b3] seq_read+0x163/0x6b0
[ 60.232078] [c105bf70] ? do_setitimer+0x220/0x330
[ 60.232078] [c11a9650] ? seq_lseek+0x1f0/0x1f0
[ 60.232078] [c116b55a] vfs_read+0xca/0x280
[ 60.232078] [c11a9650] ? seq_lseek+0x1f0/0x1f0
[ 60.232078] [c116b776] sys_read+0x66/0xe0
[ 60.232078] [c1423d9d] syscall_call+0x7/0xb
[ 60.232078] [c142] ? __schedule+0x2a0/0xc80
Thanks,
Fengguang
--
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: yama_ptrace_access_check(): possible recursive locking detected
On Thu, Aug 09, 2012 at 06:39:34PM -0700, Kees Cook wrote:
Hi,
So, after taking a closer look at this, I cannot understand how it's
possible. Yama's task_lock call is against current, not child,
which is what ptrace_may_access() is locking. And the same code makes
sure that current != child. Yama would never get called if current ==
child.
How did you reproduce this situation?
This warning can be triggered with Dave Jones' trinity tool:
git://git.codemonkey.org.uk/trinity
That's a very dangerous tool, please only run it as normal user in a
backed up and chrooted test box. I personally run it inside an initrd.
If you are interested in reproducing this, I can send you the ready
made initrd in private email.
Thanks,
Fengguang
On Thu, Jul 26, 2012 at 6:47 AM, Fengguang Wu fengguang...@intel.com wrote:
Hi Kees,
Here is a recursive lock possibility:
ptrace_may_access()
=task_lock(task);
yama_ptrace_access_check()
get_task_comm()
= task_lock(task);
[ 60.230444] =
[ 60.232078] [ INFO: possible recursive locking detected ]
[ 60.232078] 3.5.0+ #281 Not tainted
[ 60.232078] -
[ 60.232078] trinity-child0/17019 is trying to acquire lock:
[ 60.232078] ((p-alloc_lock)-rlock){+.+...}, at: [c1176ffa]
get_task_comm+0x4a/0xf0
[ 60.232078]
[ 60.232078] but task is already holding lock:
[ 60.232078] ((p-alloc_lock)-rlock){+.+...}, at: [c10653fa]
ptrace_may_access+0x4a/0xf0
[ 60.232078]
[ 60.232078] other info that might help us debug this:
[ 60.232078] Possible unsafe locking scenario:
[ 60.232078]
[ 60.232078]CPU0
[ 60.232078]
[ 60.232078] lock((p-alloc_lock)-rlock);
[ 60.232078] lock((p-alloc_lock)-rlock);
[ 60.232078]
[ 60.232078] *** DEADLOCK ***
[ 60.232078]
[ 60.232078] May be due to missing lock nesting notation
[ 60.232078]
[ 60.232078] 3 locks held by trinity-child0/17019:
[ 60.232078] #0: (p-lock){+.+.+.}, at: [c11a9683]
seq_read+0x33/0x6b0
[ 60.232078] #1: (sig-cred_guard_mutex){+.+.+.}, at: [c11ff8ae]
lock_trace+0x2e/0xb0
[ 60.232078] #2: ((p-alloc_lock)-rlock){+.+...}, at: [c10653fa]
ptrace_may_access+0x4a/0xf0
[ 60.232078]
[ 60.232078] stack backtrace:
[ 60.232078] Pid: 17019, comm: trinity-child0 Not tainted 3.5.0+ #281
[ 60.232078] Call Trace:
[ 60.232078] [c10c6238] __lock_acquire+0x1498/0x14f0
[ 60.232078] [c10be7e7] ? trace_hardirqs_off+0x27/0x40
[ 60.232078] [c10c6360] lock_acquire+0xd0/0x110
[ 60.232078] [c1176ffa] ? get_task_comm+0x4a/0xf0
[ 60.232078] [c1422290] _raw_spin_lock+0x60/0x110
[ 60.232078] [c1176ffa] ? get_task_comm+0x4a/0xf0
[ 60.232078] [c1176ffa] get_task_comm+0x4a/0xf0
[ 60.232078] [c1246798] yama_ptrace_access_check+0x468/0x4a0
[ 60.232078] [c124648f] ? yama_ptrace_access_check+0x15f/0x4a0
[ 60.232078] [c124209a] security_ptrace_access_check+0x1a/0x30
[ 60.232078] [c1065229] __ptrace_may_access+0x189/0x310
[ 60.232078] [c10650cc] ? __ptrace_may_access+0x2c/0x310
[ 60.232078] [c106542d] ptrace_may_access+0x7d/0xf0
[ 60.232078] [c11ff8ea] lock_trace+0x6a/0xb0
[ 60.232078] [c11ffa46] proc_pid_stack+0x76/0x170
[ 60.232078] [c1201064] proc_single_show+0x74/0x100
[ 60.232078] [c11a97b3] seq_read+0x163/0x6b0
[ 60.232078] [c105bf70] ? do_setitimer+0x220/0x330
[ 60.232078] [c11a9650] ? seq_lseek+0x1f0/0x1f0
[ 60.232078] [c116b55a] vfs_read+0xca/0x280
[ 60.232078] [c11a9650] ? seq_lseek+0x1f0/0x1f0
[ 60.232078] [c116b776] sys_read+0x66/0xe0
[ 60.232078] [c1423d9d] syscall_call+0x7/0xb
[ 60.232078] [c142] ? __schedule+0x2a0/0xc80
Thanks,
Fengguang
--
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: yama_ptrace_access_check(): possible recursive locking detected
On Thu, Jul 26, 2012 at 8:41 AM, Oleg Nesterov wrote:
> On 07/26, Fengguang Wu wrote:
>>
>> Here is a recursive lock possibility:
>>
>> ptrace_may_access()
>> =>task_lock(task);
>> yama_ptrace_access_check()
>> get_task_comm()
>> => task_lock(task);
>
> I think yama_ptrace_access_check() can simply use ->comm
>
> Oleg.
>
> --- x/security/yama/yama_lsm.c
> +++ x/security/yama/yama_lsm.c
> @@ -279,12 +279,9 @@ static int yama_ptrace_access_check(stru
> }
>
> if (rc) {
> - char name[sizeof(current->comm)];
> printk_ratelimited(KERN_NOTICE
> "ptrace of pid %d was attempted by: %s (pid %d)\n",
> - child->pid,
> - get_task_comm(name, current),
> - current->pid);
> + child->pid, current->comm, current->pid);
> }
>
> return rc;
>
Great catch, thanks! I've sent Oleg's suggestion (with an added
comment) separately.
-Kees
--
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: yama_ptrace_access_check(): possible recursive locking detected
On Thu, Jul 26, 2012 at 8:41 AM, Oleg Nesterov o...@redhat.com wrote:
On 07/26, Fengguang Wu wrote:
Here is a recursive lock possibility:
ptrace_may_access()
=task_lock(task);
yama_ptrace_access_check()
get_task_comm()
= task_lock(task);
I think yama_ptrace_access_check() can simply use -comm
Oleg.
--- x/security/yama/yama_lsm.c
+++ x/security/yama/yama_lsm.c
@@ -279,12 +279,9 @@ static int yama_ptrace_access_check(stru
}
if (rc) {
- char name[sizeof(current-comm)];
printk_ratelimited(KERN_NOTICE
ptrace of pid %d was attempted by: %s (pid %d)\n,
- child-pid,
- get_task_comm(name, current),
- current-pid);
+ child-pid, current-comm, current-pid);
}
return rc;
Great catch, thanks! I've sent Oleg's suggestion (with an added
comment) separately.
-Kees
--
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: yama_ptrace_access_check(): possible recursive locking detected
On 07/26, Fengguang Wu wrote:
>
> Here is a recursive lock possibility:
>
> ptrace_may_access()
> =>task_lock(task);
> yama_ptrace_access_check()
> get_task_comm()
> => task_lock(task);
I think yama_ptrace_access_check() can simply use ->comm
Oleg.
--- x/security/yama/yama_lsm.c
+++ x/security/yama/yama_lsm.c
@@ -279,12 +279,9 @@ static int yama_ptrace_access_check(stru
}
if (rc) {
- char name[sizeof(current->comm)];
printk_ratelimited(KERN_NOTICE
"ptrace of pid %d was attempted by: %s (pid %d)\n",
- child->pid,
- get_task_comm(name, current),
- current->pid);
+ child->pid, current->comm, current->pid);
}
return rc;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: yama_ptrace_access_check(): possible recursive locking detected
On 07/26, Fengguang Wu wrote:
Here is a recursive lock possibility:
ptrace_may_access()
=task_lock(task);
yama_ptrace_access_check()
get_task_comm()
= task_lock(task);
I think yama_ptrace_access_check() can simply use -comm
Oleg.
--- x/security/yama/yama_lsm.c
+++ x/security/yama/yama_lsm.c
@@ -279,12 +279,9 @@ static int yama_ptrace_access_check(stru
}
if (rc) {
- char name[sizeof(current-comm)];
printk_ratelimited(KERN_NOTICE
ptrace of pid %d was attempted by: %s (pid %d)\n,
- child-pid,
- get_task_comm(name, current),
- current-pid);
+ child-pid, current-comm, current-pid);
}
return rc;
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
