On Wed, 12 Mar 2014 05:21:13 -0400
Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx> wrote:
>
> On Mar 11, 2014, at 21:06, Jeffrey Layton <jlayton@xxxxxxxxxx> wrote:
>
> > On Tue, 11 Mar 2014 20:38:18 -0400
> > Eric Paris <eparis@xxxxxxxxxx> wrote:
> >
> >> On Tue, 2014-03-11 at 18:00 -0400, Trond Myklebust wrote:
> >>> On Mar 11, 2014, at 17:27, Trond Myklebust
> >>> <trond.myklebust@xxxxxxxxxxxxxxx> wrote:
> >>>
> >>>>
> >>>> On Mar 11, 2014, at 17:11, Anna Schumaker
> >>>> <Anna.Schumaker@xxxxxxxxxx> 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@xxxxxxxxxx>
> >>>>> ---
> >>>>> 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?
> >>
> >
> > I posted a patch for this last week and James has merged it:
> >
> > [PATCH] security: have cap_dentry_init_security return error
> >
> > I didn't note it in the patch description but it fixes 4.2 when SELinux
> > is compiled in but disabled.
>
> Thanks! Then I expect no further action is needed on our part, and that the fix will come through the security tree?
>
FWIW, this is the bug that was causing the NFS server to spew messages
like this to the ring buffer when Anna was testing against my server at
Connectathon:
[ 243.479524] SELinux: Context \xffffffdf is not valid (left unmapped).
We may want to do a follow-on patch to clean up the struct nfs4_label
initializers since they're not really needed. But that should probably
wait until James merges this up to Linus.
--
Jeff Layton <jlayton@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
