Re: [linux-usb-devel] Re: [Security] [vendor-sec] [BUG/PATCH/RFC] Oops while completing async USB via usbdevio

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 

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...)

Attachment: pgpzRMg4nbYcK.pgp
Description: PGP signature

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]
  Powered by Linux