LKML Archive on
help / color / mirror / Atom feed
From: Harald Welte <>
To: Oleg Nesterov <>
Cc: "Eric W. Biederman" <>,
	Andrew Morton <>,
	Pavel Emelyanov <>,
Subject: Re: Fw: [PATCH 1/1] file capabilities: simplify signal check
Date: Sun, 24 Feb 2008 22:08:42 +0100	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

[-- 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

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 <> 
"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 --]

  reply	other threads:[~2008-02-24 21:43 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <>
2008-02-24  6:50 ` Eric W. Biederman
2008-02-24 18:09   ` Oleg Nesterov
2008-02-24 21:08     ` Harald Welte [this message]
2008-02-25 18:23       ` Oleg Nesterov
2008-02-27  4:18   ` serge
2008-02-27  4:33   ` serge
2008-02-28 20:25     ` Eric W. Biederman
2008-02-28 21:35       ` serge

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \
    --subject='Re: Fw: [PATCH 1/1] file capabilities: simplify signal check' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).