* Re: Fw: [PATCH 1/1] file capabilities: simplify signal check
2008-02-24 6:50 ` Fw: [PATCH 1/1] file capabilities: simplify signal check Eric W. Biederman
@ 2008-02-24 18:09 ` Oleg Nesterov
2008-02-24 21:08 ` Harald Welte
2008-02-27 4:18 ` serge
2008-02-27 4:33 ` serge
2 siblings, 1 reply; 8+ messages in thread
From: Oleg Nesterov @ 2008-02-24 18:09 UTC (permalink / raw)
To: Eric W. Biederman, Harald Welte
Cc: Andrew Morton, Pavel Emelyanov, linux-kernel
On 02/23, Eric W. Biederman wrote:
>
> Andrew Morton <akpm@linux-foundation.org> writes:
>
> > um, is that code namespace-clean?
>
> Choke, gag.
>
> There are uid namespace issues but since no one has finished the
> uid namespace that I am aware of that is minor.
>
> However the code does not appear clean/maintainable. The normal linux
> signal sending policy has already been enforce before we get to this
> point.
>
> So unless I am totally mistaken the code should read:
>
> int cap_task_kill(struct task_struct *p, struct siginfo *info,
> int sig, u32 secid)
> {
> if (info != SEND_SIG_NOINFO && (is_si_special(info) || SI_FROMKERNEL(info)))
> return 0;
>
> if (!cap_issubset(p->cap_permitted, current->cap_permitted))
> return -EPERM;
>
> return 0;
> }
>
> Although doing it that way violates:
> /*
> * Running a setuid root program raises your capabilities.
> * Killing your own setuid root processes was previously
> * allowed.
> * We must preserve legacy signal behavior in this case.
> */
>
>
> Which says to me the code should really read:
> int cap_task_kill(struct task_struct *p, struct siginfo *info,
> int sig, u32 secid)
> {
> return 0;
> }
>
> The entire point of defining cap_task_kill under
> CONFIG_SECURITY_FILE_CAPABLITIES appears to be deny killing processes
> with more caps. Killing processes that we could ordinarily kill
> which have more caps appears to be precisely the case we have decided
> to allow. So the patched version of cap_task_kill appears to be an
> expensive way of doing nothing, just waiting for someone to complain
> about the last couple of cases it denies until it is truly a noop.
(Can't comment, I never understood this security magic, but Eric's
explanation looks very reasonable to me).
I just have an almost off-topic (sorry ;) question. Do we really need
kill_pid_info_as_uid() ? Harald Welte cc'ed.
>From "[PATCH] Fix signal sending in usbdevio on async URB completion"
commit 46113830a18847cff8da73005e57bc49c2f95a56
> If a process issues an URB from userspace and (starts to) terminate
> before the URB comes back, we run into the issue described above. This
> is because the urb saves a pointer to "current" when it is posted to the
> device, but there's no guarantee that this pointer is still valid
> afterwards.
>
> In fact, there are three separate issues:
>
> 1) the pointer to "current" can become invalid, since the task could be
> completely gone when the URB completion comes back from the device.
>
> 2) Even if the saved task pointer is still pointing to a valid task_struct,
> task_struct->sighand could have gone meanwhile.
>
> 3) Even if the process is perfectly fine, permissions may have changed,
> and we can no longer send it a signal.
The problems 1) and 2) are solved by converting to a struct pid. Is 3) a real
problem? The task which does ioctl(USBDEVFS_SUBMITURB) explicitly asks to send
the signal to it, should we deny the signal even if it changes its credentials
in some way?
Just curious. Thanks,
Oleg.
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: Fw: [PATCH 1/1] file capabilities: simplify signal check
2008-02-24 18:09 ` Oleg Nesterov
@ 2008-02-24 21:08 ` Harald Welte
2008-02-25 18:23 ` Oleg Nesterov
0 siblings, 1 reply; 8+ messages in thread
From: Harald Welte @ 2008-02-24 21:08 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Eric W. Biederman, Andrew Morton, Pavel Emelyanov, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2664 bytes --]
On Sun, Feb 24, 2008 at 09:09:31PM +0300, Oleg Nesterov wrote:
> I just have an almost off-topic (sorry ;) question. Do we really need
> kill_pid_info_as_uid() ? Harald Welte cc'ed.
>
> From "[PATCH] Fix signal sending in usbdevio on async URB completion"
> commit 46113830a18847cff8da73005e57bc49c2f95a56
>
> > If a process issues an URB from userspace and (starts to) terminate
> > before the URB comes back, we run into the issue described above. This
> > is because the urb saves a pointer to "current" when it is posted to the
> > device, but there's no guarantee that this pointer is still valid
> > afterwards.
> >
> > In fact, there are three separate issues:
> >
> > 1) the pointer to "current" can become invalid, since the task could be
> > completely gone when the URB completion comes back from the device.
> >
> > 2) Even if the saved task pointer is still pointing to a valid task_struct,
> > task_struct->sighand could have gone meanwhile.
> >
> > 3) Even if the process is perfectly fine, permissions may have changed,
> > and we can no longer send it a signal.
>
> The problems 1) and 2) are solved by converting to a struct pid. Is 3) a real
> problem? The task which does ioctl(USBDEVFS_SUBMITURB) explicitly asks to send
> the signal to it, should we deny the signal even if it changes its credentials
> in some way?
At the time I discovered the abovementioned problem, '1' and '2' were
real practical issues that I was seeing on live systems, triggerable
from userspace with no problems. '3' was more of a theoretical issue
that was discovered while reading the code and spending some thought on
it.
I personally am too remote to whatever you're currently doing to the
code ('using struct pid') in order to give any comment. The overall
process of 'saving the current pointer and re-using it at some later
point while the original task might be gone or modified' must work.
Whether or not we should deny the signal even if the process changes its
own credentials in some way sounds like a much more esoteric question to
me. I think it's fair to say that the resulting behavior is
"unspecified but shouldn't cause the process and/or kernel to misbehave"
At least I'm not aware of any usbdevio logic that would require some
specific behaviour here.
--
- Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: Fw: [PATCH 1/1] file capabilities: simplify signal check
2008-02-24 21:08 ` Harald Welte
@ 2008-02-25 18:23 ` Oleg Nesterov
0 siblings, 0 replies; 8+ messages in thread
From: Oleg Nesterov @ 2008-02-25 18:23 UTC (permalink / raw)
To: Harald Welte
Cc: Eric W. Biederman, Andrew Morton, Pavel Emelyanov, linux-kernel
On 02/24, Harald Welte wrote:
>
> On Sun, Feb 24, 2008 at 09:09:31PM +0300, Oleg Nesterov wrote:
> > I just have an almost off-topic (sorry ;) question. Do we really need
> > kill_pid_info_as_uid() ? Harald Welte cc'ed.
> >
> > From "[PATCH] Fix signal sending in usbdevio on async URB completion"
> > commit 46113830a18847cff8da73005e57bc49c2f95a56
> >
> > > If a process issues an URB from userspace and (starts to) terminate
> > > before the URB comes back, we run into the issue described above. This
> > > is because the urb saves a pointer to "current" when it is posted to the
> > > device, but there's no guarantee that this pointer is still valid
> > > afterwards.
> > >
> > > In fact, there are three separate issues:
> > >
> > > 1) the pointer to "current" can become invalid, since the task could be
> > > completely gone when the URB completion comes back from the device.
> > >
> > > 2) Even if the saved task pointer is still pointing to a valid task_struct,
> > > task_struct->sighand could have gone meanwhile.
> > >
> > > 3) Even if the process is perfectly fine, permissions may have changed,
> > > and we can no longer send it a signal.
> >
> > The problems 1) and 2) are solved by converting to a struct pid. Is 3) a real
> > problem? The task which does ioctl(USBDEVFS_SUBMITURB) explicitly asks to send
> > the signal to it, should we deny the signal even if it changes its credentials
> > in some way?
>
> At the time I discovered the abovementioned problem, '1' and '2' were
> real practical issues that I was seeing on live systems, triggerable
> from userspace with no problems. '3' was more of a theoretical issue
> that was discovered while reading the code and spending some thought on
> it.
Yes, yes, I see, the patch was fine.
> Whether or not we should deny the signal even if the process changes its
> own credentials in some way sounds like a much more esoteric question to
> me. I think it's fair to say that the resulting behavior is
> "unspecified but shouldn't cause the process and/or kernel to misbehave"
>
> At least I'm not aware of any usbdevio logic that would require some
> specific behaviour here.
OK, thanks.
So, the only reason why we can't kill kill_pid_info_as_uid() and just use
kill_pid_info() is that USB uses .si_code = SI_ASYNCIO. The latter means
that SI_FROMUSER(info) == T.
I assume that it is not an option to change USB to use .si_code > 0, yes?
Oleg.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Fw: [PATCH 1/1] file capabilities: simplify signal check
2008-02-24 6:50 ` Fw: [PATCH 1/1] file capabilities: simplify signal check Eric W. Biederman
2008-02-24 18:09 ` Oleg Nesterov
@ 2008-02-27 4:18 ` serge
2008-02-27 4:33 ` serge
2 siblings, 0 replies; 8+ messages in thread
From: serge @ 2008-02-27 4:18 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Andrew Morton, Pavel Emelyanov, Oleg Nesterov, linux-kernel,
Andrew Morgan
Quoting Eric W. Biederman (ebiederm@xmission.com):
> Andrew Morton <akpm@linux-foundation.org> writes:
>
> > um, is that code namespace-clean?
>
> Choke, gag.
>
> There are uid namespace issues but since no one has finished the
> uid namespace that I am aware of that is minor.
>
> However the code does not appear clean/maintainable. The normal linux
> signal sending policy has already been enforce before we get to this
> point.
>
> So unless I am totally mistaken the code should read:
>
> int cap_task_kill(struct task_struct *p, struct siginfo *info,
> int sig, u32 secid)
> {
> if (info != SEND_SIG_NOINFO && (is_si_special(info) || SI_FROMKERNEL(info)))
> return 0;
>
> if (!cap_issubset(p->cap_permitted, current->cap_permitted))
> return -EPERM;
>
> return 0;
> }
>
> Although doing it that way violates:
> /*
> * Running a setuid root program raises your capabilities.
> * Killing your own setuid root processes was previously
> * allowed.
> * We must preserve legacy signal behavior in this case.
> */
>
>
> Which says to me the code should really read:
> int cap_task_kill(struct task_struct *p, struct siginfo *info,
> int sig, u32 secid)
> {
> return 0;
> }
>
> The entire point of defining cap_task_kill under
> CONFIG_SECURITY_FILE_CAPABLITIES appears to be deny killing processes
> with more caps.
... and owned by the same uid, since the case of owned by a different
uid is handled earlier.
> Killing processes that we could ordinarily kill
> which have more caps appears to be precisely the case we have decided
> to allow.
Yes, although it might be a good idea to be stricter when
issecure(SECURE_NOROOT), which will become meaningful when Andrew
Morgan's per-process securebits patch gets more use.
> So the patched version of cap_task_kill appears to be an
> expensive way of doing nothing, just waiting for someone to complain
> about the last couple of cases it denies until it is truly a noop.
Yes, the only difference right now is that some of the euid/uid/suid
combos aren't allowed for in cap_task_kill(). If we're not going to
be stricter with SECURE_NOROOT, then I plan to try to remove
cap_task_kill() and just apologize for the huge mess it caused.
-serge
> > Thanks.
> >
> > Begin forwarded message:
> >
> > Date: Wed, 20 Feb 2008 10:15:50 -0600
> > From: "Serge E. Hallyn" <serue@us.ibm.com>
> > To: lkml <linux-kernel@vger.kernel.org>
> > Subject: [PATCH 1/1] file capabilities: simplify signal check
> >
> >
> >>From bd076c7245d02be0cc01b7c09bd7170ec5946492 Mon Sep 17 00:00:00 2001
> > From: Serge E. Hallyn <serue@us.ibm.com>
> > Date: Sun, 17 Feb 2008 20:28:07 -0500
> > Subject: [PATCH 1/1] file capabilities: simplify signal check
> >
> > Simplify the uid equivalence check in cap_task_kill(). Anyone
> > can kill a process owned by the same uid.
> >
> > Without this patch wireshark is reported to fail.
> >
> > Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
> > Signed-off-by: Andrew G. Morgan <morgan@kernel.org>
> > ---
> > security/commoncap.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/security/commoncap.c b/security/commoncap.c
> > index 5aba826..bb0c095 100644
> > --- a/security/commoncap.c
> > +++ b/security/commoncap.c
> > @@ -552,7 +552,7 @@ int cap_task_kill(struct task_struct *p, struct siginfo
> > *info,
> > * allowed.
> > * We must preserve legacy signal behavior in this case.
> > */
> > - if (p->euid == 0 && p->uid == current->uid)
> > + if (p->uid == current->uid)
> > return 0;
> >
> > /* sigcont is permitted within same session */
> > --
> > 1.5.1.1.GIT
>
> So it looks to me like we should just give up trying to deny a few
> cases now.
>
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 5aba826..c1d1fd7 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -540,41 +540,6 @@ int cap_task_setnice (struct task_struct *p, int nice)
> return cap_safe_nice(p);
> }
>
> -int cap_task_kill(struct task_struct *p, struct siginfo *info,
> - int sig, u32 secid)
> -{
> - if (info != SEND_SIG_NOINFO && (is_si_special(info) || SI_FROMKERNEL(info)))
> - return 0;
> -
> - /*
> - * Running a setuid root program raises your capabilities.
> - * Killing your own setuid root processes was previously
> - * allowed.
> - * We must preserve legacy signal behavior in this case.
> - */
> - if (p->euid == 0 && p->uid == current->uid)
> - return 0;
> -
> - /* sigcont is permitted within same session */
> - if (sig == SIGCONT && (task_session_nr(current) == task_session_nr(p)))
> - return 0;
> -
> - if (secid)
> - /*
> - * Signal sent as a particular user.
> - * Capabilities are ignored. May be wrong, but it's the
> - * only thing we can do at the moment.
> - * Used only by usb drivers?
> - */
> - return 0;
> - if (cap_issubset(p->cap_permitted, current->cap_permitted))
> - return 0;
> - if (capable(CAP_KILL))
> - return 0;
> -
> - return -EPERM;
> -}
> -
> /*
> * called from kernel/sys.c for prctl(PR_CABSET_DROP)
> * done without task_capability_lock() because it introduces
> @@ -605,13 +570,13 @@ int cap_task_setnice (struct task_struct *p, int nice)
> {
> return 0;
> }
> +#endif
> +
> int cap_task_kill(struct task_struct *p, struct siginfo *info,
> int sig, u32 secid)
> {
> return 0;
> }
> -#endif
> -
> void cap_task_reparent_to_init (struct task_struct *p)
> {
> cap_set_init_eff(p->cap_effective);
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: Fw: [PATCH 1/1] file capabilities: simplify signal check
2008-02-24 6:50 ` Fw: [PATCH 1/1] file capabilities: simplify signal check Eric W. Biederman
2008-02-24 18:09 ` Oleg Nesterov
2008-02-27 4:18 ` serge
@ 2008-02-27 4:33 ` serge
2008-02-28 20:25 ` Eric W. Biederman
2 siblings, 1 reply; 8+ messages in thread
From: serge @ 2008-02-27 4:33 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Andrew Morton, Pavel Emelyanov, Oleg Nesterov, linux-kernel,
Andrew Morgan
Quoting Eric W. Biederman (ebiederm@xmission.com):
> Andrew Morton <akpm@linux-foundation.org> writes:
>
> > um, is that code namespace-clean?
>
> Choke, gag.
Oh, sorry, I got lost in the set of patches in the message. To be
clear, my little 4-patch uid-ns-signal patchset can simply be updated
to make the cap_task_kill() uid check into if (task_user_equiv(current, p)
But Eric if you simply drop cap_task_kill() (don't make it return 0,
just drop the function and go back to not setting task_kill in the
capability_security_ops) I'll ack that. Else I'll write the patch
thursday. At this point the only thing that will be denied by
cap_task_kill() but not by check_kill_permission() is funky euid cases.
That's wrong. (cc'ing amorgan in the event I'm forgetting something
useful the fn is doing)
-serge
> There are uid namespace issues but since no one has finished the
> uid namespace that I am aware of that is minor.
>
> However the code does not appear clean/maintainable. The normal linux
> signal sending policy has already been enforce before we get to this
> point.
>
> So unless I am totally mistaken the code should read:
>
> int cap_task_kill(struct task_struct *p, struct siginfo *info,
> int sig, u32 secid)
> {
> if (info != SEND_SIG_NOINFO && (is_si_special(info) || SI_FROMKERNEL(info)))
> return 0;
>
> if (!cap_issubset(p->cap_permitted, current->cap_permitted))
> return -EPERM;
>
> return 0;
> }
>
> Although doing it that way violates:
> /*
> * Running a setuid root program raises your capabilities.
> * Killing your own setuid root processes was previously
> * allowed.
> * We must preserve legacy signal behavior in this case.
> */
>
>
> Which says to me the code should really read:
> int cap_task_kill(struct task_struct *p, struct siginfo *info,
> int sig, u32 secid)
> {
> return 0;
> }
>
> The entire point of defining cap_task_kill under
> CONFIG_SECURITY_FILE_CAPABLITIES appears to be deny killing processes
> with more caps. Killing processes that we could ordinarily kill
> which have more caps appears to be precisely the case we have decided
> to allow. So the patched version of cap_task_kill appears to be an
> expensive way of doing nothing, just waiting for someone to complain
> about the last couple of cases it denies until it is truly a noop.
>
>
>
> > Thanks.
> >
> > Begin forwarded message:
> >
> > Date: Wed, 20 Feb 2008 10:15:50 -0600
> > From: "Serge E. Hallyn" <serue@us.ibm.com>
> > To: lkml <linux-kernel@vger.kernel.org>
> > Subject: [PATCH 1/1] file capabilities: simplify signal check
> >
> >
> >>From bd076c7245d02be0cc01b7c09bd7170ec5946492 Mon Sep 17 00:00:00 2001
> > From: Serge E. Hallyn <serue@us.ibm.com>
> > Date: Sun, 17 Feb 2008 20:28:07 -0500
> > Subject: [PATCH 1/1] file capabilities: simplify signal check
> >
> > Simplify the uid equivalence check in cap_task_kill(). Anyone
> > can kill a process owned by the same uid.
> >
> > Without this patch wireshark is reported to fail.
> >
> > Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
> > Signed-off-by: Andrew G. Morgan <morgan@kernel.org>
> > ---
> > security/commoncap.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/security/commoncap.c b/security/commoncap.c
> > index 5aba826..bb0c095 100644
> > --- a/security/commoncap.c
> > +++ b/security/commoncap.c
> > @@ -552,7 +552,7 @@ int cap_task_kill(struct task_struct *p, struct siginfo
> > *info,
> > * allowed.
> > * We must preserve legacy signal behavior in this case.
> > */
> > - if (p->euid == 0 && p->uid == current->uid)
> > + if (p->uid == current->uid)
> > return 0;
> >
> > /* sigcont is permitted within same session */
> > --
> > 1.5.1.1.GIT
>
> So it looks to me like we should just give up trying to deny a few
> cases now.
>
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 5aba826..c1d1fd7 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -540,41 +540,6 @@ int cap_task_setnice (struct task_struct *p, int nice)
> return cap_safe_nice(p);
> }
>
> -int cap_task_kill(struct task_struct *p, struct siginfo *info,
> - int sig, u32 secid)
> -{
> - if (info != SEND_SIG_NOINFO && (is_si_special(info) || SI_FROMKERNEL(info)))
> - return 0;
> -
> - /*
> - * Running a setuid root program raises your capabilities.
> - * Killing your own setuid root processes was previously
> - * allowed.
> - * We must preserve legacy signal behavior in this case.
> - */
> - if (p->euid == 0 && p->uid == current->uid)
> - return 0;
> -
> - /* sigcont is permitted within same session */
> - if (sig == SIGCONT && (task_session_nr(current) == task_session_nr(p)))
> - return 0;
> -
> - if (secid)
> - /*
> - * Signal sent as a particular user.
> - * Capabilities are ignored. May be wrong, but it's the
> - * only thing we can do at the moment.
> - * Used only by usb drivers?
> - */
> - return 0;
> - if (cap_issubset(p->cap_permitted, current->cap_permitted))
> - return 0;
> - if (capable(CAP_KILL))
> - return 0;
> -
> - return -EPERM;
> -}
> -
> /*
> * called from kernel/sys.c for prctl(PR_CABSET_DROP)
> * done without task_capability_lock() because it introduces
> @@ -605,13 +570,13 @@ int cap_task_setnice (struct task_struct *p, int nice)
> {
> return 0;
> }
> +#endif
> +
> int cap_task_kill(struct task_struct *p, struct siginfo *info,
> int sig, u32 secid)
> {
> return 0;
> }
> -#endif
> -
> void cap_task_reparent_to_init (struct task_struct *p)
> {
> cap_set_init_eff(p->cap_effective);
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: Fw: [PATCH 1/1] file capabilities: simplify signal check
2008-02-27 4:33 ` serge
@ 2008-02-28 20:25 ` Eric W. Biederman
2008-02-28 21:35 ` serge
0 siblings, 1 reply; 8+ messages in thread
From: Eric W. Biederman @ 2008-02-28 20:25 UTC (permalink / raw)
To: serge
Cc: Andrew Morton, Pavel Emelyanov, Oleg Nesterov, linux-kernel,
Andrew Morgan
serge@hallyn.com writes:
> Quoting Eric W. Biederman (ebiederm@xmission.com):
>> Andrew Morton <akpm@linux-foundation.org> writes:
>>
>> > um, is that code namespace-clean?
>>
>> Choke, gag.
>
> Oh, sorry, I got lost in the set of patches in the message. To be
> clear, my little 4-patch uid-ns-signal patchset can simply be updated
> to make the cap_task_kill() uid check into if (task_user_equiv(current, p)
>
> But Eric if you simply drop cap_task_kill() (don't make it return 0,
> just drop the function and go back to not setting task_kill in the
> capability_security_ops) I'll ack that. Else I'll write the patch
> thursday. At this point the only thing that will be denied by
> cap_task_kill() but not by check_kill_permission() is funky euid cases.
> That's wrong. (cc'ing amorgan in the event I'm forgetting something
> useful the fn is doing)
Go ahead. I'm fighting a cold and am fairly overloaded at the moment.
Eric
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Fw: [PATCH 1/1] file capabilities: simplify signal check
2008-02-28 20:25 ` Eric W. Biederman
@ 2008-02-28 21:35 ` serge
0 siblings, 0 replies; 8+ messages in thread
From: serge @ 2008-02-28 21:35 UTC (permalink / raw)
To: Eric W. Biederman
Cc: serge, Andrew Morton, Pavel Emelyanov, Oleg Nesterov,
linux-kernel, Andrew Morgan
Quoting Eric W. Biederman (ebiederm@xmission.com):
> serge@hallyn.com writes:
>
> > Quoting Eric W. Biederman (ebiederm@xmission.com):
> >> Andrew Morton <akpm@linux-foundation.org> writes:
> >>
> >> > um, is that code namespace-clean?
> >>
> >> Choke, gag.
> >
> > Oh, sorry, I got lost in the set of patches in the message. To be
> > clear, my little 4-patch uid-ns-signal patchset can simply be updated
> > to make the cap_task_kill() uid check into if (task_user_equiv(current, p)
> >
> > But Eric if you simply drop cap_task_kill() (don't make it return 0,
> > just drop the function and go back to not setting task_kill in the
> > capability_security_ops) I'll ack that. Else I'll write the patch
> > thursday. At this point the only thing that will be denied by
> > cap_task_kill() but not by check_kill_permission() is funky euid cases.
> > That's wrong. (cc'ing amorgan in the event I'm forgetting something
> > useful the fn is doing)
>
> Go ahead. I'm fighting a cold and am fairly overloaded at the moment.
>
> Eric
Thanks - patch sent a little while ago. The description explains why
I believe cap_task_kill() became worthless (not just 'it's inconvenient' :)
-serge
^ permalink raw reply [flat|nested] 8+ messages in thread