Re: [PATCH] nfs: Don't assume we have a security structure

[Eric Paris <eparis@redhat.com>]

On Tue, 2014-03-11 at 18:00 -0400, Trond Myklebust wrote:
> On Mar 11, 2014, at 17:27, Trond Myklebust <trond.myklebust@primarydata.com> wrote:
> 
> > 
> > On Mar 11, 2014, at 17:11, Anna Schumaker <Anna.Schumaker@netapp.com> wrote:
> > 
> >> If the i_security field isn't set then security_dentry_init_security()
> >> won't initialize some of the values used by the security label.  This
> >> causes my client to hit a BUG_ON() while encoding a label of size
> >> -2128927414.
> >> 
> >> I hit this bug while testing on a client without SELinux installed.
> >> 
> >> Signed-off-by: Anna Schumaker <anna.schumaker@netapp.com>
> >> ---
> >> fs/nfs/nfs4proc.c | 3 +++
> >> 1 file changed, 3 insertions(+)
> >> 
> >> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> >> index b8cd560..994ccc2 100644
> >> --- a/fs/nfs/nfs4proc.c
> >> +++ b/fs/nfs/nfs4proc.c
> >> @@ -105,6 +105,9 @@ nfs4_label_init_security(struct inode *dir, struct dentry *dentry,
> >> 	if (nfs_server_capable(dir, NFS_CAP_SECURITY_LABEL) == 0)
> >> 		return NULL;
> >> 
> >> +	if (!dir->i_security)
> >> +		return NULL;
> >> +
> >> 	err = security_dentry_init_security(dentry, sattr->ia_mode,
> >> 				&dentry->d_name, (void **)&label->label, &label->len);
> >> 	if (err == 0)
> > 
> > Hi Anna,
> > 
> > This looks like a check that needs to be done by selinux_dentry_init_security() itself. The dir->i_security field is not something that NFS knows about.
> > David, what needs to happen there when dentry->d_parent->i_security (a.k.a. dsec) is NULL?
> > 
> 
> Oh, wait. I missed the bit about ‘without SELinux installed’. So the problem here is that you have a NFS client that does not have SELinux set up, but running against a server that is advertising NFSv4.2 with labeled NFS. Is that correct?
> 
> It looks to me as if cap_dentry_init_security() will indeed trigger this behaviour since it returns ‘0’ without doing anything to the label. As far as I can see, the right thing to do there is to return -EOPNOTSUPP, no?

I feel like Jeff Layton was looking at the same thing, and came to the
same conclusion...

Jeff?

-Eric