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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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?

_________________________________
Trond Myklebust
Linux NFS client maintainer, PrimaryData
trond.myklebust@xxxxxxxxxxxxxxx

--
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




[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux