From 272eda8298dc82eb411ece82bbb2c62911087b24 Mon Sep 17 00:00:00 2001 From: Richard Weinberger Date: Wed, 17 May 2017 00:20:27 +0200 Subject: ubifs: Correctly evict xattr inodes UBIFS handles extended attributes just like files, as consequence of that, they also have inodes. Therefore UBIFS does all the inode machinery also for xattrs. Since new inodes have i_nlink of 1, a file or xattr inode will be evicted if i_nlink goes down to 0 after an unlink. UBIFS assumes this model also for xattrs, which is not correct. One can create a file "foo" with xattr "user.test". By reading "user.test" an inode will be created, and by deleting "user.test" it will get evicted later. The assumption breaks if the file "foo", which hosts the xattrs, will be removed. VFS nor UBIFS does not remove each xattr via ubifs_xattr_remove(), it just removes the host inode from the TNC and all underlying xattr nodes too and the inode will remain in the cache and wastes memory. To solve this problem, remove xattr inodes from the VFS inode cache in ubifs_xattr_remove() to make sure that they get evicted. Fixes: 1e51764a3c2ac05a ("UBIFS: add new flash file system") Cc: Signed-off-by: Richard Weinberger --- fs/ubifs/xattr.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) (limited to 'fs/ubifs/xattr.c') diff --git a/fs/ubifs/xattr.c b/fs/ubifs/xattr.c index 6c9e62c2ef55..98f11257d66c 100644 --- a/fs/ubifs/xattr.c +++ b/fs/ubifs/xattr.c @@ -513,6 +513,28 @@ out_cancel: return err; } +/** + * ubifs_evict_xattr_inode - Evict an xattr inode. + * @c: UBIFS file-system description object + * @xattr_inum: xattr inode number + * + * When an inode that hosts xattrs is being removed we have to make sure + * that cached inodes of the xattrs also get removed from the inode cache + * otherwise we'd waste memory. This function looks up an inode from the + * inode cache and clears the link counter such that iput() will evict + * the inode. + */ +void ubifs_evict_xattr_inode(struct ubifs_info *c, ino_t xattr_inum) +{ + struct inode *inode; + + inode = ilookup(c->vfs_sb, xattr_inum); + if (inode) { + clear_nlink(inode); + iput(inode); + } +} + static int ubifs_xattr_remove(struct inode *host, const char *name) { struct inode *inode; -- cgit From d8db5b1ca9d4c57e49893d0f78e6d5ce81450cc8 Mon Sep 17 00:00:00 2001 From: Xiaolei Li Date: Fri, 23 Jun 2017 10:37:23 +0800 Subject: ubifs: Massage assert in ubifs_xattr_set() wrt. init_xattrs The inode is not locked in init_xattrs when creating a new inode. Without this patch, there will occurs assert when booting or creating a new file, if the kernel config CONFIG_SECURITY_SMACK is enabled. Log likes: UBIFS assert failed in ubifs_xattr_set at 298 (pid 1156) CPU: 1 PID: 1156 Comm: ldconfig Tainted: G S 4.12.0-rc1-207440-g1e70b02 #2 Hardware name: MediaTek MT2712 evaluation board (DT) Call trace: [] dump_backtrace+0x0/0x238 [] show_stack+0x14/0x20 [] dump_stack+0x9c/0xc0 [] ubifs_xattr_set+0x374/0x5e0 [] init_xattrs+0x5c/0xb8 [] security_inode_init_security+0x110/0x190 [] ubifs_init_security+0x30/0x68 [] ubifs_mkdir+0x100/0x200 [] vfs_mkdir+0x11c/0x1b8 [] SyS_mkdirat+0x74/0xd0 [] __sys_trace_return+0x0/0x4 Signed-off-by: Xiaolei Li Signed-off-by: Richard Weinberger --- fs/ubifs/xattr.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) (limited to 'fs/ubifs/xattr.c') diff --git a/fs/ubifs/xattr.c b/fs/ubifs/xattr.c index 98f11257d66c..c13eae819cbc 100644 --- a/fs/ubifs/xattr.c +++ b/fs/ubifs/xattr.c @@ -280,7 +280,7 @@ static struct inode *iget_xattr(struct ubifs_info *c, ino_t inum) } int ubifs_xattr_set(struct inode *host, const char *name, const void *value, - size_t size, int flags) + size_t size, int flags, bool check_lock) { struct inode *inode; struct ubifs_info *c = host->i_sb->s_fs_info; @@ -289,12 +289,7 @@ int ubifs_xattr_set(struct inode *host, const char *name, const void *value, union ubifs_key key; int err; - /* - * Creating an encryption context is done unlocked since we - * operate on a new inode which is not visible to other users - * at this point. - */ - if (strcmp(name, UBIFS_XATTR_NAME_ENCRYPTION_CONTEXT) != 0) + if (check_lock) ubifs_assert(inode_is_locked(host)); if (size > UBIFS_MAX_INO_DATA) @@ -598,8 +593,12 @@ static int init_xattrs(struct inode *inode, const struct xattr *xattr_array, } strcpy(name, XATTR_SECURITY_PREFIX); strcpy(name + XATTR_SECURITY_PREFIX_LEN, xattr->name); + /* + * creating a new inode without holding the inode rwsem, + * no need to check whether inode is locked. + */ err = ubifs_xattr_set(inode, name, xattr->value, - xattr->value_len, 0); + xattr->value_len, 0, false); kfree(name); if (err < 0) break; @@ -646,7 +645,7 @@ static int xattr_set(const struct xattr_handler *handler, name = xattr_full_name(handler, name); if (value) - return ubifs_xattr_set(inode, name, value, size, flags); + return ubifs_xattr_set(inode, name, value, size, flags, true); else return ubifs_xattr_remove(inode, name); } -- cgit