[bug report] bcachefs: Add an ioctl for resizing journal on a device

[bug report] bcachefs: Add an ioctl for resizing journal on a device

[Dan Carpenter]

Hello Kent Overstreet,

The patch f9e4304e0562: "bcachefs: Add an ioctl for resizing journal
on a device" from Nov 16, 2020 (linux-next), leads to the following
(unpublished) Smatch static checker warning:

	fs/bcachefs/chardev.c:624 bch2_ioctl_disk_resize_journal()
	warn: truncating user data 'arg.nbuckets' '0-u64max'

fs/bcachefs/chardev.c
    607 static long bch2_ioctl_disk_resize_journal(struct bch_fs *c,
    608                                    struct bch_ioctl_disk_resize_journal arg)
    609 {
    610         struct bch_dev *ca;
    611         int ret;
    612 
    613         if (!capable(CAP_SYS_ADMIN))
    614                 return -EPERM;
    615 
    616         if ((arg.flags & ~BCH_BY_INDEX) ||
    617             arg.pad)
    618                 return -EINVAL;
    619 
    620         ca = bch2_device_lookup(c, arg.dev, arg.flags);
    621         if (IS_ERR(ca))
    622                 return PTR_ERR(ca);
    623 
--> 624         ret = bch2_set_nr_journal_buckets(c, ca, arg.nbuckets);

This is harmless.  arg.nbuckets is a u64 and bch2_set_nr_journal_buckets()
takes a u32.  However this u32 vs u64 inconsistency with ->nbuckets is
not just here but also in other places.  (struct journal_device)->nr is
a u32 for example.  It would be better if it were consistent everywhere.

    625 
    626         percpu_ref_put(&ca->ref);
    627         return ret;
    628 }

regards,
dan carpenter

Re: [bug report] bcachefs: Add an ioctl for resizing journal on a device

[Kent Overstreet]

On Thu, Sep 14, 2023 at 06:03:08PM +0300, Dan Carpenter wrote:
> Hello Kent Overstreet,
> 
> The patch f9e4304e0562: "bcachefs: Add an ioctl for resizing journal
> on a device" from Nov 16, 2020 (linux-next), leads to the following
> (unpublished) Smatch static checker warning:
> 
> 	fs/bcachefs/chardev.c:624 bch2_ioctl_disk_resize_journal()
> 	warn: truncating user data 'arg.nbuckets' '0-u64max'
> 
> fs/bcachefs/chardev.c
>     607 static long bch2_ioctl_disk_resize_journal(struct bch_fs *c,
>     608                                    struct bch_ioctl_disk_resize_journal arg)
>     609 {
>     610         struct bch_dev *ca;
>     611         int ret;
>     612 
>     613         if (!capable(CAP_SYS_ADMIN))
>     614                 return -EPERM;
>     615 
>     616         if ((arg.flags & ~BCH_BY_INDEX) ||
>     617             arg.pad)
>     618                 return -EINVAL;
>     619 
>     620         ca = bch2_device_lookup(c, arg.dev, arg.flags);
>     621         if (IS_ERR(ca))
>     622                 return PTR_ERR(ca);
>     623 
> --> 624         ret = bch2_set_nr_journal_buckets(c, ca, arg.nbuckets);
> 
> This is harmless.  arg.nbuckets is a u64 and bch2_set_nr_journal_buckets()
> takes a u32.  However this u32 vs u64 inconsistency with ->nbuckets is
> not just here but also in other places.  (struct journal_device)->nr is
> a u32 for example.  It would be better if it were consistent everywhere.

applying this fix

From 1c0cfb0abe14594347fa0639fe728f068be92deb Mon Sep 17 00:00:00 2001
From: Kent Overstreet <kent.overstreet@linux.dev>
Date: Tue, 19 Sep 2023 22:26:18 -0400
Subject: [PATCH] bcachefs: bch2_ioctl_disk_resize_journal(): check for integer
 truncation

Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>

diff --git a/fs/bcachefs/chardev.c b/fs/bcachefs/chardev.c
index 51d671267741..e8b6733e7f71 100644
--- a/fs/bcachefs/chardev.c
+++ b/fs/bcachefs/chardev.c
@@ -627,6 +627,9 @@ static long bch2_ioctl_disk_resize_journal(struct bch_fs *c,
 	    arg.pad)
 		return -EINVAL;
 
+	if (arg.nbuckets > U32_MAX)
+		return -EINVAL;
+
 	ca = bch2_device_lookup(c, arg.dev, arg.flags);
 	if (IS_ERR(ca))
 		return PTR_ERR(ca);