security/selinux/hooks.c:flush_unauthorized_files()

security/selinux/hooks.c:flush_unauthorized_files()

[Christoph Hellwig]

Folks, what is this code doing in a security module?

The check for unauthorized files should really move into
flush_old_files, removing the horrible devnull hack at the same time.

The tty loop isn't in the right place either, and it seems we might
be missing a call to disassociate_tty if the current process is the
session leader.  I'd suggest moving this code into fs/exec.c aswell,
aksing the security module for the actual permissions through an LSM
hook.

Re: security/selinux/hooks.c:flush_unauthorized_files()

[Stephen Smalley]

On Mon, 2005-09-26 at 14:55 +0200, Christoph Hellwig wrote:
> Folks, what is this code doing in a security module?
> 
> The check for unauthorized files should really move into
> flush_old_files, removing the horrible devnull hack at the same time.

Not objecting to moving it into core code, but a couple of concerns
about the details:
- Doing it in flush_old_files will require propagating the brpm down so
that we can extract the new task credentials from it for use in the
checks (current checks can use the current task credentials since it
occurs after the new credentials are applied, which depends on the prior
checks for shared state to avoid the obvious potential leak),
- Associating the descriptors closed by policy denials with devnull is
important to avoid application misbehavior; that logic was specifically
added to address real breakage in applications.  We really don't want to
remove that logic.  It currently uses a devnull node from selinuxfs at
Viro's suggestion, so it is currently SELinux-dependent.

> The tty loop isn't in the right place either, and it seems we might
> be missing a call to disassociate_tty if the current process is the
> session leader.  I'd suggest moving this code into fs/exec.c aswell,
> aksing the security module for the actual permissions through an LSM
> hook.

What would you suggest as a better place for rechecking access to the
controlling tty?  As above, we could move it to flush_old_files, but
would need to extract the new task credentials from the bprm for the
check.

-- 
Stephen Smalley
National Security Agency