Re: [PATCH] exec: atomic MAY_EXEC check and SUID/SGID handling

[Chris Wright <chrisw@osdl.org>]

* Andrey Savochkin (saw@saw.sw.com.sg) wrote:
> There is a time window between permission(MAY_EXEC) check in
> open_exec() and S_ISUID check plus bprm->e_uid setting in prepare_binprm().
> And S_ISUID is checked and bprm->e_uid is copied from the inode without
> any serialization with attribute updates.
> 
> That means that some executable may have permissions
> -rwxr-xr-x    root     disk     /bin/file
> at the moment of MAY_EXEC check and
> -rwsr-x---    root     disk     /bin/file
> at the moment of S_ISUID check, providing lucky users starting /bin/file at
> the moment of permission change with a setuid-root program.
> 
> It's arguable whether it's a big security issue, but certainly such behavior
> is not what administrators may expect.

If you can find a way for a user to exploit this it's an issue.  Looks
like it's not, and doesn't warrant such a big change as your patch.
The fact that you introduce a new field and then almost always supply it
with NULL is a clue that it's not the right direction IMO.  Something
simple (as you mentioned) that grabs i_sem and rechecks during suid
setup in binprm_prepare is sufficient.  Worth it?  Guess I'm not
convinced.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net