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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
