Re: [PATCH 2/2] ceph: fix a NULL pointer dereference in ceph_handle_caps()

[Jeff Layton <jlayton@kernel.org>]

On Wed, 2022-03-02 at 16:54 +0800, xiubli@redhat.com wrote:
> From: Xiubo Li <xiubli@redhat.com>
> 
> The ceph_find_inode() may will fail and return NULL.
> 
> Signed-off-by: Xiubo Li <xiubli@redhat.com>
> ---
>  fs/ceph/caps.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
> index 0b36020207fd..0762b55fdbcb 100644
> --- a/fs/ceph/caps.c
> +++ b/fs/ceph/caps.c
> @@ -4303,7 +4303,6 @@ void ceph_handle_caps(struct ceph_mds_session *session,
>  
>  	/* lookup ino */
>  	inode = ceph_find_inode(mdsc->fsc->sb, vino);
> -	ci = ceph_inode(inode);
>  	dout(" op %s ino %llx.%llx inode %p\n", ceph_cap_op_name(op), vino.ino,
>  	     vino.snap, inode);
>  
> @@ -4333,6 +4332,7 @@ void ceph_handle_caps(struct ceph_mds_session *session,
>  		}
>  		goto flush_cap_releases;
>  	}
> +	ci = ceph_inode(inode);
>  
>  	/* these will work even if we don't have a cap yet */
>  	switch (op) {

I don't think this is an actual bug. We're just assigning "ci" here and
that doesn't involve a dereference of inode. If "inode" is NULL, then ci
will be close to NULL, but it doesn't get used in that case.

Assigning this lower in the function is fine though, and it discourages
anyone trying to use ci when they shouldn't, so you can add my ack, but
maybe fix the patch description since there is no dereference here.

Acked-by: Jeff Layton <jlayton@kernel.org>