Re: [TOMOYO #13 (mmotm 2008-11-19-02-19) 01/11] Introducesecurity_path_clear() hook.

From: Stephen Smalley
Date: Tue Dec 02 2008 - 08:52:36 EST

On Tue, 2008-12-02 at 19:39 +0900, Tetsuo Handa wrote:
> Hello.
> Stephen Smalley wrote:
> > On Thu, 2008-11-20 at 20:25 +0900, Tetsuo Handa wrote:
> > > plain text document attachment (introduce-security_path_clear.patch)
> > > To perform DAC performed in vfs_foo() before MAC, we let security_path_foo()
> > > save a result into our own hash table and return 0, and let security_inode_foo()
> > > return the saved result. Since security_inode_foo() is not always called after
> > > security_path_foo(), we need security_path_clear() to clear the hash table.
> >
> > This seems very fragile and unmaintainable to me. The fact that you
> > even need a security_path_clear() hook suggests that something is wrong
> > with the other security_path* hooks. I'd suggest that you explicitly
> > pass the result of the security_path* hooks down to the security_inode*
> > hooks instead. What do others think?
> You are recommending us to pass variables required for security_inode_*() via
> stack memory rather than private hash, aren't you?

To be precise, I was recommending passing the return value of
security_path* down to security_inode* explicitly rather than doing it
implicitly as you presently do. Thereby making the actual control flow
and relationship between the security_path* and security_inode* hooks
evident. However, I guess that is moot given your statements below.

> I think there are two problems.
> One is that the variable passed via stack memory won't be used by SELinux and
> SMACK and "CONFIG_SECURITY=n kernels", which will be a waste of stack memory.

I'm more concerned with the hook interface being understandable and

> The other one is that TOMOYO will need another variable for telling how the
> security_inode_*() are called. Passing the variable via stack memory requires
> modification of all vfs_*() calls, but TOMOYO doesn't check requests issued
> by (e.g.) stackable filesystems.

I'm not clear on why that requires a separate argument; if the caller is
passing in the access decision result as an input, then certain callers
(e.g. stackable filesystems) can always pass 0 (success).

> By the way, this security_path_clear() is intended to be able to return DAC's
> error code in priority to MAC's error code, but there are two problems for
> One is that pathnames which will be later denied by DAC are appended by
> TOMOYO's learning mode (i.e. garbage entries appears in the learned policy).
> The other is that warning messages on pathnames which will be later denied by
> DAC are generated by TOMOYO's enforcing mode.
> Thus, it will be preferable for TOMOYO to "do MAC checks after DAC checks"
> rather than to "return DAC's error in priority to MAC's error while doing MAC
> checks before DAC checks".

It sounds like the existing security_path* hooks are not adequate for
your needs then, and that patch should not in fact be merged. Yes?

> To do so, "security_path_*() should be replaced by security_path_set(vfsmount)"
> and "let security_inode_*() do MAC checks using the result of
> security_path_set()" and "let security_path_clear() clear the result of
> security_path_set() in case security_inode_*() was not called".
> So, I think storing the pathname of "struct vfsmount" in the form of "char *"
> into private hash at security_path_set() and clearing the private hash at
> security_path_clear() should be most preferable.

Then I guess you need to redo your patches along those lines and
re-submit them. Likely starting with just a patch adding the
security_path_set/clear hooks, posted to lsm and fsdevel.

Stephen Smalley
National Security Agency

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at