[PATCH v3] Add security.* XATTR support for the UBIFS

[Stephen Smalley]

On Wed, 2012-04-11 at 14:28 -0700, subodh.nijsure at gmail.com wrote:
> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
> index ec9f187..422bcec 100644
> --- a/fs/ubifs/dir.c
> +++ b/fs/ubifs/dir.c
> @@ -292,6 +292,18 @@ static int ubifs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
>  
>  	ubifs_release_budget(c, &req);
>  	insert_inode_hash(inode);
> +
> +#ifdef CONFIG_UBIFS_FS_XATTR
> +
> +	err = ubifs_init_security(dir, inode, &dentry->d_name);
> +	if (err) {
> +		ubifs_err("cannot initialize extended attribute, error %d",
> +			  err);
> +		goto out_cancel;
> +	}
> +
> +#endif
> +
>  	d_instantiate(dentry, inode);
>  	return 0;

I don't know this code very well, but simply reusing the out_cancel
error path when ubifs_init_security() fails looks wrong to me.  You'll
end up calling ubifs_release_budget() a second time.  You could move the
ubifs_init_security() call a bit earlier, prior to the
ubifs_release_budget() call, so that it only happens once on the error
path.  I also am unclear on what if anything you need to do about the
prior ubifs_jnl_update() if ubifs_init_security() fails; it seems like
you either need to update the journal again to reflect the inode
deletion or you need to move ubifs_init_security() prior to
ubifs_jnl_update() as well.

I don't know if this would work, but I would be inclined to move the
call to ubifs_init_security() immediately after the ubifs_new_inode()
call, and introduce a new out_security: label just before the
make_bad_inode() call to which you can jump on error.  However, that
might produce a strange state in the journal, with the setxattr recorded
before the inode creation.  Optimally they would both be part of the
same journal transaction, as in ext4, and thus applied atomically or not
at all (i.e. a file is either created with a security xattr or not at
all).

-- 
Stephen Smalley
National Security Agency