[PATCH] fs: reiserfs: fix NULL pointer dereference in

[PATCH] fs: reiserfs: fix NULL pointer dereference in

[tcs.kernel]

From: Haimin Zhang <tcs_kernel@tencent.com> 

If root_inode->i_op is reiserfs_special_inode_operations
reiserfs_special_inode_operations doesn't implement the lookup callback
function,resulting an NULL pointer when the reiserfs_lookup_privroot() 
function was called.

Call Trace:
 __lookup_slow+0x267/0x490 build/../fs/namei.c:1646
 lookup_one_len+0x163/0x190 build/../fs/namei.c:2663
 reiserfs_lookup_privroot+0x92/0x290 build/../fs/reiserfs/xattr.c:980
 reiserfs_fill_super+0x1f2a/0x2d80 build/../fs/reiserfs/super.c:2176
 mount_bdev+0x33d/0x410 build/../fs/super.c:1368
 legacy_get_tree+0x103/0x210 build/../fs/fs_context.c:610
 vfs_get_tree+0x86/0x2f0 build/../fs/super.c:1498
 do_new_mount build/../fs/namespace.c:2905 [inline]
 path_mount+0x688/0x1d10 build/../fs/namespace.c:3235
 do_mount+0xf1/0x110 build/../fs/namespace.c:3248
 __do_sys_mount build/../fs/namespace.c:3456 [inline]
 __se_sys_mount build/../fs/namespace.c:3433 [inline]
 __x64_sys_mount+0x1d5/0x220 build/../fs/namespace.c:3433
 do_syscall_x64 build/../arch/x86/entry/common.c:50 [inline]
 do_syscall_64+0x34/0xb0 build/../arch/x86/entry/common.c:80
 entry_SYSCALL_64_after_hwframe+0x44/0xae

Reported-by: syzbot+11c49c...@syzkaller.appspotmail.com
Signed-off-by: Haimin Zhang <tcs_kernel@tencent.com>
---
 fs/reiserfs/namei.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fs/reiserfs/namei.c b/fs/reiserfs/namei.c
index 3d7a35d..947b51b 100644
--- a/fs/reiserfs/namei.c
+++ b/fs/reiserfs/namei.c
@@ -350,6 +350,12 @@ static int reiserfs_find_entry(struct inode *dir, const char *name, int namelen,
 	}			/* while (1) */
 }
 
+static struct dentry *reiserfs_noop_lookup(struct inode *dir, struct dentry *dentry,
+				      unsigned int flags)
+{
+	return ERR_PTR(-ENOENT);
+}
+
 static struct dentry *reiserfs_lookup(struct inode *dir, struct dentry *dentry,
 				      unsigned int flags)
 {
@@ -1680,6 +1686,7 @@ static int reiserfs_rename(struct user_namespace *mnt_userns,
  * special file operations.. just xattr/acl stuff
  */
 const struct inode_operations reiserfs_special_inode_operations = {
+	.lookup = reiserfs_noop_lookup,
 	.setattr = reiserfs_setattr,
 	.listxattr = reiserfs_listxattr,
 	.permission = reiserfs_permission,
-- 
1.8.3.1

Re: [PATCH] fs: reiserfs: fix NULL pointer dereference in

[Christian Brauner]

On Wed, Aug 18, 2021 at 04:01:07PM +0800, tcs.kernel@gmail.com wrote:
> From: Haimin Zhang <tcs_kernel@tencent.com> 
> 
> If root_inode->i_op is reiserfs_special_inode_operations
> reiserfs_special_inode_operations doesn't implement the lookup callback
> function,resulting an NULL pointer when the reiserfs_lookup_privroot() 
> function was called.
> 
> Call Trace:
>  __lookup_slow+0x267/0x490 build/../fs/namei.c:1646
>  lookup_one_len+0x163/0x190 build/../fs/namei.c:2663
>  reiserfs_lookup_privroot+0x92/0x290 build/../fs/reiserfs/xattr.c:980
>  reiserfs_fill_super+0x1f2a/0x2d80 build/../fs/reiserfs/super.c:2176
>  mount_bdev+0x33d/0x410 build/../fs/super.c:1368
>  legacy_get_tree+0x103/0x210 build/../fs/fs_context.c:610
>  vfs_get_tree+0x86/0x2f0 build/../fs/super.c:1498
>  do_new_mount build/../fs/namespace.c:2905 [inline]
>  path_mount+0x688/0x1d10 build/../fs/namespace.c:3235
>  do_mount+0xf1/0x110 build/../fs/namespace.c:3248
>  __do_sys_mount build/../fs/namespace.c:3456 [inline]
>  __se_sys_mount build/../fs/namespace.c:3433 [inline]
>  __x64_sys_mount+0x1d5/0x220 build/../fs/namespace.c:3433
>  do_syscall_x64 build/../arch/x86/entry/common.c:50 [inline]
>  do_syscall_64+0x34/0xb0 build/../arch/x86/entry/common.c:80
>  entry_SYSCALL_64_after_hwframe+0x44/0xae
> 

Hey Haimin,

This is rather suspicious. I'd really like to see the syzbot reproducer
for this. Which brings us to:

> Reported-by: syzbot+11c49c...@syzkaller.appspotmail.com

what's the full syzbot link? This is hacked off.

> Signed-off-by: Haimin Zhang <tcs_kernel@tencent.com>
> ---
>  fs/reiserfs/namei.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/fs/reiserfs/namei.c b/fs/reiserfs/namei.c
> index 3d7a35d..947b51b 100644
> --- a/fs/reiserfs/namei.c
> +++ b/fs/reiserfs/namei.c
> @@ -350,6 +350,12 @@ static int reiserfs_find_entry(struct inode *dir, const char *name, int namelen,
>  	}			/* while (1) */
>  }
>  
> +static struct dentry *reiserfs_noop_lookup(struct inode *dir, struct dentry *dentry,
> +				      unsigned int flags)
> +{
> +	return ERR_PTR(-ENOENT);
> +}
> +
>  static struct dentry *reiserfs_lookup(struct inode *dir, struct dentry *dentry,
>  				      unsigned int flags)
>  {
> @@ -1680,6 +1686,7 @@ static int reiserfs_rename(struct user_namespace *mnt_userns,
>   * special file operations.. just xattr/acl stuff
>   */
>  const struct inode_operations reiserfs_special_inode_operations = {
> +	.lookup = reiserfs_noop_lookup,
>  	.setattr = reiserfs_setattr,
>  	.listxattr = reiserfs_listxattr,
>  	.permission = reiserfs_permission,

In any case, this seems like the wrong fix tapering over the real issue.
The issue here seems to me some sort of corruption going on. The root
inode should not be a special file. So reiserfs should do it like xfs,
ext4, btrfs etc. and verify one way or the other that the root inode is
a directory and return an error when it isn't. Sm like this (completely
untested and produced with marginal knowledge about reiserfs):

diff --git a/fs/reiserfs/super.c b/fs/reiserfs/super.c
index 3ffafc73acf0..be4c301f5c94 100644
--- a/fs/reiserfs/super.c
+++ b/fs/reiserfs/super.c
@@ -2062,7 +2062,7 @@ static int reiserfs_fill_super(struct super_block *s, void *data, int silent)
        root_inode =
            iget5_locked(s, REISERFS_ROOT_OBJECTID, reiserfs_find_actor,
                         reiserfs_init_locked_inode, (void *)&args);
-       if (!root_inode) {
+       if (!root_inode || !S_ISDIR(root_inode)) {
                SWARN(silent, s, "jmacd-10", "get root inode failed");
                goto error_unlocked;
        }