On 2024/09/22 10:59, Zhu Yanjun wrote:
> The member variable size in struct nullb_device is the type
> unsigned long, and the module parameter g_gb is the type int.
> In 32 bit architecture, unsigned long has 32 bit. This
> introduces overflow risks.
>
> Use the type u64 in struct nullb_device and configfs. This
> can avoid overflow risks.
>
> Fixes: 2984c8684f96 ("nullb: factor disk parameters")
> Signed-off-by: Zhu Yanjun <yanjun.zhu@xxxxxxxxx>
> ---
> drivers/block/null_blk/main.c | 23 +++++++++++++++++++++--
> drivers/block/null_blk/null_blk.h | 2 +-
> 2 files changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
> index 2f0431e42c49..88c6d6277d09 100644
> --- a/drivers/block/null_blk/main.c
> +++ b/drivers/block/null_blk/main.c
> @@ -289,6 +289,11 @@ static inline ssize_t nullb_device_ulong_attr_show(unsigned long val,
> return snprintf(page, PAGE_SIZE, "%lu\n", val);
> }
>
> +static inline ssize_t nullb_device_u64_attr_show(u64 val, char *page)
> +{
> + return snprintf(page, PAGE_SIZE, "%llu\n", val);
> +}
> +
> static inline ssize_t nullb_device_bool_attr_show(bool val, char *page)
> {
> return snprintf(page, PAGE_SIZE, "%u\n", val);
> @@ -322,6 +327,20 @@ static ssize_t nullb_device_ulong_attr_store(unsigned long *val,
> return count;
> }
>
> +static ssize_t nullb_device_u64_attr_store(u64 *val, const char *page,
> + size_t count)
> +{
> + int result;
> + u64 tmp;
> +
> + result = kstrtou64(page, 0, &tmp);
> + if (result < 0)
> + return result;
> +
> + *val = tmp;
> + return count;
> +}
> +
> static ssize_t nullb_device_bool_attr_store(bool *val, const char *page,
> size_t count)
> {
> @@ -438,7 +457,7 @@ static int nullb_apply_poll_queues(struct nullb_device *dev,
> return ret;
> }
>
> -NULLB_DEVICE_ATTR(size, ulong, NULL);
> +NULLB_DEVICE_ATTR(size, u64, NULL);
> NULLB_DEVICE_ATTR(completion_nsec, ulong, NULL);
> NULLB_DEVICE_ATTR(submit_queues, uint, nullb_apply_submit_queues);
> NULLB_DEVICE_ATTR(poll_queues, uint, nullb_apply_poll_queues);
> @@ -762,7 +781,7 @@ static struct nullb_device *null_alloc_dev(void)
> return NULL;
> }
>
> - dev->size = g_gb * 1024;
> + dev->size = (u64)g_gb * 1024;
As already commented on your previous version that was casting to an unsigned
long, this is *not* avoiding an overflow. It is only changing the overflow value
to a bigger one. So as suggested before, if you really want to fix this, fix it
properly using check_mul_overflow().
--
Damien Le Moal
Western Digital Research
