linux-kernel - [PATCH 2/5] staging: erofs: fix fast symlink w/o xattr when fs xattr is on

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190114114026.11728-3-gaoxiang25@huawei.com>
Date:   Mon, 14 Jan 2019 19:40:23 +0800
From:   Gao Xiang <gaoxiang25@...wei.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Chao Yu <yuchao0@...wei.com>, <devel@...verdev.osuosl.org>
CC:     LKML <linux-kernel@...r.kernel.org>,
        <linux-erofs@...ts.ozlabs.org>, "Chao Yu" <chao@...nel.org>,
        Miao Xie <miaoxie@...wei.com>, <weidu.du@...wei.com>,
        Fang Wei <fangwei1@...wei.com>,
        Gao Xiang <gaoxiang25@...wei.com>, <stable@...r.kernel.org>
Subject: [PATCH 2/5] staging: erofs: fix fast symlink w/o xattr when fs xattr is on

Currently, this will hit a BUG_ON for these symlinks as follows:

- kernel message
------------[ cut here ]------------
kernel BUG at drivers/staging/erofs/xattr.c:59!
SMP PTI
CPU: 1 PID: 1170 Comm: getllxattr Not tainted 4.20.0-rc6+ #92
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-2.fc27 04/01/2014
RIP: 0010:init_inode_xattrs+0x22b/0x270
Code: 48 0f 45 ea f0 ff 4d 34 74 0d 41 83 4c 24 e0 01 31 c0 e9 00 fe ff ff 48 89 ef e8 e0 31 9e ff eb e9 89 e8 e9 ef fd ff ff 0f 0$
 <0f> 0b 48 89 ef e8 fb f6 9c ff 48 8b 45 08 a8 01 75 24 f0 ff 4d 34
RSP: 0018:ffffa03ac026bdf8 EFLAGS: 00010246
------------[ cut here ]------------
...
Call Trace:
 erofs_listxattr+0x30/0x2c0
 ? selinux_inode_listxattr+0x5a/0x80
 ? kmem_cache_alloc+0x33/0x170
 ? security_inode_listxattr+0x27/0x40
 listxattr+0xaf/0xc0
 path_listxattr+0x5a/0xa0
 do_syscall_64+0x43/0xf0
 entry_SYSCALL_64_after_hwframe+0x44/0xa9
...
---[ end trace 3c24b49408dc0c72 ]---

Fix it by checking ->xattr_isize in init_inode_xattrs(),
and it also fixes improper return value -ENOTSUPP
(it should be -ENODATA if xattr is enabled) for those inodes.

Fixes: b17500a0fdba ("staging: erofs: introduce xattr & acl support")
Cc: <stable@...r.kernel.org> # 4.19+
Reported-by: Li Guifu <bluce.liguifu@...wei.com>
Tested-by: Li Guifu <bluce.liguifu@...wei.com>
Reviewed-by: Chao Yu <yuchao0@...wei.com>
Signed-off-by: Gao Xiang <gaoxiang25@...wei.com>
---
 drivers/staging/erofs/inode.c |  8 ++++----
 drivers/staging/erofs/xattr.c | 25 ++++++++++++++++++++-----
 2 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/erofs/inode.c b/drivers/staging/erofs/inode.c
index d7fbf5f4600f..f99954dbfdb5 100644
--- a/drivers/staging/erofs/inode.c
+++ b/drivers/staging/erofs/inode.c
@@ -185,16 +185,16 @@ static int fill_inode(struct inode *inode, int isdir)
 		/* setup the new inode */
 		if (S_ISREG(inode->i_mode)) {
 #ifdef CONFIG_EROFS_FS_XATTR
-			if (vi->xattr_isize)
-				inode->i_op = &erofs_generic_xattr_iops;
+			inode->i_op = &erofs_generic_xattr_iops;
 #endif
 			inode->i_fop = &generic_ro_fops;
 		} else if (S_ISDIR(inode->i_mode)) {
 			inode->i_op =
 #ifdef CONFIG_EROFS_FS_XATTR
-				vi->xattr_isize ? &erofs_dir_xattr_iops :
-#endif
+				&erofs_dir_xattr_iops;
+#else
 				&erofs_dir_iops;
+#endif
 			inode->i_fop = &erofs_dir_fops;
 		} else if (S_ISLNK(inode->i_mode)) {
 			/* by default, page_get_link is used for symlink */
diff --git a/drivers/staging/erofs/xattr.c b/drivers/staging/erofs/xattr.c
index 80dca6a4adbe..e30de2476fd0 100644
--- a/drivers/staging/erofs/xattr.c
+++ b/drivers/staging/erofs/xattr.c
@@ -56,7 +56,26 @@ static int init_inode_xattrs(struct inode *inode)
 		return 0;
 
 	vi = EROFS_V(inode);
-	BUG_ON(!vi->xattr_isize);
+
+	/*
+	 * bypass all xattr operations if ->xattr_isize is not greater than
+	 * sizeof(struct erofs_xattr_ibody_header), in detail:
+	 * 1) it is not enough to contain erofs_xattr_ibody_header then
+	 *    ->xattr_isize should be 0 (it means no xattr);
+	 * 2) it is just to contain erofs_xattr_ibody_header, which is on-disk
+	 *    undefined right now (maybe use later with some new sb feature).
+	 */
+	if (vi->xattr_isize == sizeof(struct erofs_xattr_ibody_header)) {
+		errln("xattr_isize %d of nid %llu is not supported yet",
+		      vi->xattr_isize, vi->nid);
+		return -ENOTSUPP;
+	} else if (vi->xattr_isize < sizeof(struct erofs_xattr_ibody_header)) {
+		if (unlikely(vi->xattr_isize)) {
+			DBG_BUGON(1);
+			return -EIO;	/* xattr ondisk layout error */
+		}
+		return -ENOATTR;
+	}
 
 	sb = inode->i_sb;
 	sbi = EROFS_SB(sb);
@@ -422,7 +441,6 @@ static int erofs_xattr_generic_get(const struct xattr_handler *handler,
 		struct dentry *unused, struct inode *inode,
 		const char *name, void *buffer, size_t size)
 {
-	struct erofs_vnode *const vi = EROFS_V(inode);
 	struct erofs_sb_info *const sbi = EROFS_I_SB(inode);
 
 	switch (handler->flags) {
@@ -440,9 +458,6 @@ static int erofs_xattr_generic_get(const struct xattr_handler *handler,
 		return -EINVAL;
 	}
 
-	if (!vi->xattr_isize)
-		return -ENOATTR;
-
 	return erofs_getxattr(inode, handler->flags, name, buffer, size);
 }
 
-- 
2.14.4