[Top] [All Lists]

Re: [PATCH v3 2/4] xfs: initialize inode security on tmpfile creation

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>, Brian Foster <bfoster@xxxxxxxxxx>
Subject: Re: [PATCH v3 2/4] xfs: initialize inode security on tmpfile creation
From: Stephen Smalley <sds@xxxxxxxxxxxxx>
Date: Tue, 15 Apr 2014 16:04:32 -0400
Cc: xfs@xxxxxxxxxxx, linux-security-module@xxxxxxxxxxxxxxx, linux-fsdevel@xxxxxxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20140415175033.GB26404@xxxxxxxxxxxxx>
Organization: National Security Agency
References: <1397578706-5385-1-git-send-email-bfoster@xxxxxxxxxx> <1397578706-5385-3-git-send-email-bfoster@xxxxxxxxxx> <20140415175033.GB26404@xxxxxxxxxxxxx>
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0
On 04/15/2014 01:50 PM, Christoph Hellwig wrote:
> On Tue, Apr 15, 2014 at 12:18:24PM -0400, Brian Foster wrote:
>> +    error = xfs_init_security(inode, dir, &dentry->d_name);
>> +    if (unlikely(error)) {
>> +            iput(inode);
>> +            return -error;
>> +    }
>> +
>>      d_tmpfile(dentry, inode);
> I'd really love to hear from the LSM people who they plan to deal with
> O_TMPFILE inodes.    But given that this seems to fix a real life bug
> let's go with it for now.

Is there a reason that xfs_init_security() isn't called from the inode
allocation function (e.g. xfs_ialloc), as in ext4 (__ext4_new_inode
calls ext4_init_security and also calls ext4_init_acl)?  That would have
ensured that tmpfile inodes would have been labeled without requiring a
separate change and more generally ensures complete coverage for all inodes.

For SELinux, we need the tmpfile inodes to be labeled at creation time,
not just if linked into the namespace, since they may be shared via
local socket IPC or inherited across a label-changing exec and since we
revalidate access on transfer or use.

Labeling based on the provided directory could be a bit random, although
it will work out with current policy if the provided directory
corresponds to existing tmpfile locations (e.g. /tmp, /var/tmp) and
therefore already has a label associated with temporary files.
Otherwise we might want some indication that it is a tmpfile passed into
security_inode_init_security() so that we can always select a stable
label irrespective of the directory.

<Prev in Thread] Current Thread [Next in Thread>