diff mbox series

[v2] ceph: fix variable dereferenced before check in ceph_umount_begin()

Message ID 20250602184956.58865-1-slava@dubeyko.com
State New
Headers show
Series [v2] ceph: fix variable dereferenced before check in ceph_umount_begin() | expand

Commit Message

Viacheslav Dubeyko June 2, 2025, 6:49 p.m. UTC 
From: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>

smatch warnings:
fs/ceph/super.c:1042 ceph_umount_begin() warn: variable dereferenced before check 'fsc' (see line 1041)

vim +/fsc +1042 fs/ceph/super.c

void ceph_umount_begin(struct super_block *sb)
{
	struct ceph_fs_client *fsc = ceph_sb_to_fs_client(sb);

	doutc(fsc->client, "starting forced umount\n");
              ^^^^^^^^^^^
Dereferenced

	if (!fsc)
            ^^^^
Checked too late.

		return;
	fsc->mount_state = CEPH_MOUNT_SHUTDOWN;
	__ceph_umount_begin(fsc);
}

The VFS guarantees that the superblock is still
alive when it calls into ceph via ->umount_begin().
Finally, we don't need to check the fsc and
it should be valid. This patch simply removes
the fsc check.

Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_r_202503280852.YDB3pxUY-2Dlkp-40intel.com_&d=DwIBAg&c=BSDicqBQBDjDI9RkVyTcHQ&r=q5bIm4AXMzc8NJu1_RGmnQ2fMWKq4Y4RAkElvUgSs00&m=Ud7uNdqBY_Z7LJ_oI4fwdhvxOYt_5Q58tpkMQgDWhV3199_TCnINFU28Esc0BaAH&s=QOKWZ9HKLyd6XCxW-AUoKiFFg9roId6LOM01202zAk0&e=
Signed-off-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
---
 fs/ceph/super.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Alex Markuze June 3, 2025, 10:25 a.m. UTC | #1 
Reviewed by: Alex Markuze <amarkuze@redhat.com>

On Mon, Jun 2, 2025 at 9:50 PM Viacheslav Dubeyko <slava@dubeyko.com> wrote:
>
> From: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
>
> smatch warnings:
> fs/ceph/super.c:1042 ceph_umount_begin() warn: variable dereferenced before check 'fsc' (see line 1041)
>
> vim +/fsc +1042 fs/ceph/super.c
>
> void ceph_umount_begin(struct super_block *sb)
> {
>         struct ceph_fs_client *fsc = ceph_sb_to_fs_client(sb);
>
>         doutc(fsc->client, "starting forced umount\n");
>               ^^^^^^^^^^^
> Dereferenced
>
>         if (!fsc)
>             ^^^^
> Checked too late.
>
>                 return;
>         fsc->mount_state = CEPH_MOUNT_SHUTDOWN;
>         __ceph_umount_begin(fsc);
> }
>
> The VFS guarantees that the superblock is still
> alive when it calls into ceph via ->umount_begin().
> Finally, we don't need to check the fsc and
> it should be valid. This patch simply removes
> the fsc check.
>
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_r_202503280852.YDB3pxUY-2Dlkp-40intel.com_&d=DwIBAg&c=BSDicqBQBDjDI9RkVyTcHQ&r=q5bIm4AXMzc8NJu1_RGmnQ2fMWKq4Y4RAkElvUgSs00&m=Ud7uNdqBY_Z7LJ_oI4fwdhvxOYt_5Q58tpkMQgDWhV3199_TCnINFU28Esc0BaAH&s=QOKWZ9HKLyd6XCxW-AUoKiFFg9roId6LOM01202zAk0&e=
> Signed-off-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
> ---
>  fs/ceph/super.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/fs/ceph/super.c b/fs/ceph/super.c
> index f3951253e393..68a6d434093f 100644
> --- a/fs/ceph/super.c
> +++ b/fs/ceph/super.c
> @@ -1033,8 +1033,7 @@ void ceph_umount_begin(struct super_block *sb)
>         struct ceph_fs_client *fsc = ceph_sb_to_fs_client(sb);
>
>         doutc(fsc->client, "starting forced umount\n");
> -       if (!fsc)
> -               return;
> +
>         fsc->mount_state = CEPH_MOUNT_SHUTDOWN;
>         __ceph_umount_begin(fsc);
>  }
> --
> 2.49.0
>
diff mbox series

Patch

diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index f3951253e393..68a6d434093f 100644
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -1033,8 +1033,7 @@  void ceph_umount_begin(struct super_block *sb)
 	struct ceph_fs_client *fsc = ceph_sb_to_fs_client(sb);
 
 	doutc(fsc->client, "starting forced umount\n");
-	if (!fsc)
-		return;
+
 	fsc->mount_state = CEPH_MOUNT_SHUTDOWN;
 	__ceph_umount_begin(fsc);
 }