Messages in this thread | | | Date | Tue, 26 Feb 2008 01:18:32 +0300 | From | Oleg Nesterov <> | Subject | Re: [PATCH 2/3] kill_pid_info_as_uid: don't use security_task_kill() |
| |
On 02/25, Oleg Nesterov wrote: > > On 02/25, Stephen Smalley wrote: > > > > On Mon, 2008-02-25 at 20:42 +0300, Oleg Nesterov wrote: > > > kill_pid_info_as_uid() is solely used by drivers/usb/core/. The original > > > "[PATCH] Fix signal sending in usbdevio on async URB completion" commit > > > 46113830a18847cff8da73005e57bc49c2f95a56 was right, but nowadays we use > > > struct pid and this solves most of the addressed problems. > > > > > > It would be nice to use kill_pid_info() instead, but we can't because USB > > > uses .si_code = SI_ASYNCIO which fools SI_FROMUSER() and thus security checks. > > > > > > I think we should omit the permission checks completely, the task which does > > > ioctl(USBDEVFS_SUBMITURB) explicitly asks to send the signal to it, we should > > > not deny the signal even if the task changes its credentials in any way. > > > > If we are applying checks based on uid/gid to protect suid/sgid > > programs, then we ought to also invoke the LSM hook to allow protection > > of other credential-changing transformations, like SELinux context > > transitions. You either remove all checking or none, please. > > Yes, you are right. I'd like to remove all uid/euid checks.
Actually, I may be wrong, sorry. If the task does setuid exec, we probably should do these checks.
OK, please ignore 2nd and 3rd patches.
Still. The usage of security_task_kill(secid) doesn't look good, imho. We have the similar issues with send_sigio(). In that case we use security_file_send_sigiotask(), not security_task_kill().
Oleg.
| |