Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230516141619.2160800-1-chengzhihao1@huawei.com> <20230516141619.2160800-3-chengzhihao1@huawei.com>
In-Reply-To: <20230516141619.2160800-3-chengzhihao1@huawei.com>
From:   Amir Goldstein <amir73il@gmail.com>
Date:   Tue, 16 May 2023 21:56:05 +0300
Message-ID: <CAOQ4uxhHZiCSYKC3Kh8r_jK0pD_Bubeturc0k78-GyV83=czWg@mail.gmail.com>
Subject: Re: [PATCH v3 2/3] ovl: ovl_permission: Fix null pointer dereference
 at realinode in rcu-walk mode
To:     Zhihao Cheng <chengzhihao1@huawei.com>
Cc:     miklos@szeredi.hu, brauner@kernel.org,
        linux-unionfs@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Tue, May 16, 2023 at 5:19=E2=80=AFPM Zhihao Cheng <chengzhihao1@huawei.c=
om> wrote:
>
> Following process:
>           P1                     P2
>  path_lookupat
>   link_path_walk
>    inode_permission
>     ovl_permission
>       ovl_i_path_real(inode, &realpath)
>         path->dentry =3D ovl_i_dentry_upper(inode)
>                           drop_cache
>                            __dentry_kill(ovl_dentry)
>                             iput(ovl_inode)
>                              ovl_destroy_inode(ovl_inode)
>                               dput(oi->__upperdentry)
>                                dentry_kill(upperdentry)
>                                 dentry_unlink_inode
>                                  upperdentry->d_inode =3D NULL
>       realinode =3D d_inode(realpath.dentry) // return NULL
>       inode_permission(realinode)
>        inode->i_sb  // NULL pointer dereference
> , will trigger an null pointer dereference at realinode:
>   [  335.664979] BUG: kernel NULL pointer dereference,
>                  address: 0000000000000002
>   [  335.668032] CPU: 0 PID: 2592 Comm: ls Not tainted 6.3.0
>   [  335.669956] RIP: 0010:inode_permission+0x33/0x2c0
>   [  335.678939] Call Trace:
>   [  335.679165]  <TASK>
>   [  335.679371]  ovl_permission+0xde/0x320
>   [  335.679723]  inode_permission+0x15e/0x2c0
>   [  335.680090]  link_path_walk+0x115/0x550
>   [  335.680771]  path_lookupat.isra.0+0xb2/0x200
>   [  335.681170]  filename_lookup+0xda/0x240
>   [  335.681922]  vfs_statx+0xa6/0x1f0
>   [  335.682233]  vfs_fstatat+0x7b/0xb0
>
> Fetch a reproducer in [Link].
>
> Use the helper ovl_i_path_realinode() to get realinode and then do
> non-nullptr checking.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=3D217405
> Fixes: 4b7791b2e958 ("ovl: handle idmappings in ovl_permission()")
> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
> Suggested-by: Christian Brauner <brauner@kernel.org>
> Suggested-by: Amir Goldstein <amir73il@gmail.com>

Reviewed-by: Amir Goldstein <amir73il@gmail.com>

> ---
>  fs/overlayfs/inode.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
> index 541cf3717fc2..ca56b1328a2c 100644
> --- a/fs/overlayfs/inode.c
> +++ b/fs/overlayfs/inode.c
> @@ -288,8 +288,8 @@ int ovl_permission(struct mnt_idmap *idmap,
>         int err;
>
>         /* Careful in RCU walk mode */
> -       ovl_i_path_real(inode, &realpath);
> -       if (!realpath.dentry) {
> +       realinode =3D ovl_i_path_real(inode, &realpath);
> +       if (!realinode) {
>                 WARN_ON(!(mask & MAY_NOT_BLOCK));
>                 return -ECHILD;
>         }
> @@ -302,7 +302,6 @@ int ovl_permission(struct mnt_idmap *idmap,
>         if (err)
>                 return err;
>
> -       realinode =3D d_inode(realpath.dentry);
>         old_cred =3D ovl_override_creds(inode->i_sb);
>         if (!upperinode &&
>             !special_file(realinode->i_mode) && mask & MAY_WRITE) {
> --
> 2.39.2
>