Reviewed-by: Alex Markuze amarkuze@xxxxxxxxxx
On Thu, Aug 28, 2025 at 9:45 PM Viacheslav Dubeyko <slava@xxxxxxxxxxx> wrote:
>
> From: Viacheslav Dubeyko <Slava.Dubeyko@xxxxxxx>
>
> The Coverity Scan service has detected a potential dereference of
> an explicit NULL value in ceph_fill_trace() [1].
>
> The variable in is declared in the beggining of
> ceph_fill_trace() [2]:
>
> struct inode *in = NULL;
>
> However, the initialization of the variable is happening under
> condition [3]:
>
> if (rinfo->head->is_target) {
> <skipped>
> in = req->r_target_inode;
> <skipped>
> }
>
> Potentially, if rinfo->head->is_target == FALSE, then
> in variable continues to be NULL and later the dereference of
> NULL value could happen in ceph_fill_trace() logic [4,5]:
>
> else if ((req->r_op == CEPH_MDS_OP_LOOKUPSNAP ||
> req->r_op == CEPH_MDS_OP_MKSNAP) &&
> test_bit(CEPH_MDS_R_PARENT_LOCKED, &req->r_req_flags) &&
> !test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags)) {
> <skipped>
> ihold(in);
> err = splice_dentry(&req->r_dentry, in);
> if (err < 0)
> goto done;
> }
>
> This patch adds the checking of in variable for NULL value
> and it returns -EINVAL error code if it has NULL value.
>
> v2
> Alex Markuze suggested to add unlikely macro
> in the checking condition.
>
> [1] https://scan5.scan.coverity.com/#/project-view/64304/10063?selectedIssue=1141197
> [2] https://elixir.bootlin.com/linux/v6.17-rc3/source/fs/ceph/inode.c#L1522
> [3] https://elixir.bootlin.com/linux/v6.17-rc3/source/fs/ceph/inode.c#L1629
> [4] https://elixir.bootlin.com/linux/v6.17-rc3/source/fs/ceph/inode.c#L1745
> [5] https://elixir.bootlin.com/linux/v6.17-rc3/source/fs/ceph/inode.c#L1777
>
> Signed-off-by: Viacheslav Dubeyko <Slava.Dubeyko@xxxxxxx>
> cc: Alex Markuze <amarkuze@xxxxxxxxxx>
> cc: Ilya Dryomov <idryomov@xxxxxxxxx>
> cc: Ceph Development <ceph-devel@xxxxxxxxxxxxxxx>
> ---
> fs/ceph/inode.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> index fc543075b827..8ef6b3e561cf 100644
> --- a/fs/ceph/inode.c
> +++ b/fs/ceph/inode.c
> @@ -1739,6 +1739,11 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
> goto done;
> }
>
> + if (unlikely(!in)) {
> + err = -EINVAL;
> + goto done;
> + }
> +
> /* attach proper inode */
> if (d_really_is_negative(dn)) {
> ceph_dir_clear_ordered(dir);
> @@ -1774,6 +1779,12 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
> doutc(cl, " linking snapped dir %p to dn %p\n", in,
> req->r_dentry);
> ceph_dir_clear_ordered(dir);
> +
> + if (unlikely(!in)) {
> + err = -EINVAL;
> + goto done;
> + }
> +
> ihold(in);
> err = splice_dentry(&req->r_dentry, in);
> if (err < 0)
> --
> 2.51.0
>
