Messages in this thread | | | Date | Tue, 27 Sep 2005 20:52:06 +0400 | From | Sergey Vlasov <> | Subject | Re: [linux-usb-devel] Re: [Security] [vendor-sec] [BUG/PATCH/RFC] Oops while completing async USB via usbdevio |
| |
On Tue, Sep 27, 2005 at 09:09:28AM -0700, Linus Torvalds wrote: > On Tue, 27 Sep 2005, Sergey Vlasov wrote: > > > > And then a process calls USBDEVFS_SUBMITURB and immediately exits; its > > pid gets reused by a completely different process (maybe even > > root-owned), then the urb completes, and kill_proc_info() sends the > > signal to the unsuspecting process. > > Ehh.. pid's don't get re-used until they wrap.
#define PID_MAX_DEFAULT (CONFIG_BASE_SMALL ? 0x1000 : 0x8000)
which is not that big (and on 32-bit systems PID_MAX_LIMIT is also 32K).
> Your _current_ code has that problem, though - "struct task_struct" _does_ > get re-used.
The initial patch added get_task_struct()/put_task_struct() calls to fix this - are they forbidden too?
> Don't assume that the fixes are as bad. > > Anyway, Christoph is certainly correct that what you _should_ be using is > the SIGIO infrastructure, even if you don't actually use the fcntl() to > register it.
It at least has sigio_perm(), which prevents exploiting it to send signals to tasks you don't have access to.
> > Hmm, then probably send_sig_info() should check for non-NULL > > p->sighand after taking tasklist_lock? Otherwise all uses of > > send_sig_info() for non-current tasks are unsafe. > > I don't think so. > > Your oops is because you're using a STALE POINTER. > > If you look it up by pid, it won't be stale, now will it? > > Hint: the point where sighand is released is also the point where the > process is unhashed.
When using kill_proc_info() - yes, because it takes tasklist_lock during its operation. However, lookup by pid is not safe against pid wrapping (at least without adding more checks like sigio_perm(), which ensure that you can zap only your own process with that signal).
(Why they did not make a kind of "file descriptor" for processes...) [unhandled content-type:application/pgp-signature] | |