On 2021/12/18 1:25, Jens Axboe wrote:
> On 12/17/21 4:38 AM, Tetsuo Handa wrote:
>> Use of "unsigned short" for loop_validate_block_size() is wrong [1], and
>> commit af3c570fb0df422b ("loop: Use blk_validate_block_size() to validate
>> block size") changed to use "unsigned int".
>>
>> However, since lo_simple_ioctl(LOOP_SET_BLOCK_SIZE) passes "unsigned long
>> arg" to loop_set_block_size(), blk_validate_block_size() can't validate
>> the upper 32bits on 64bits environment. A block size like 0x100000200
>> should be rejected.
>
> Wouldn't it make more sense to validate that part on the loop side? A
> block size > 32-bit doesn't make any sense.
>
I think doing below is embarrassing, for there is blk_validate_block_size() which is
meant for validating the arg. Although use of "unsigned long" for blk_validate_block_size()
might cause small bloating on 64 bits kernels, I think 64 bits kernels would not care.
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index c3a36cfaa855..98871d7b601d 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1474,6 +1474,10 @@ static int loop_set_block_size(struct loop_device *lo, unsigned long arg)
err = blk_validate_block_size(arg);
if (err)
return err;
+#if BITS_PER_LONG == 64
+ if (arg > UINT_MAX)
+ return -EINVAL;
+#endif
if (lo->lo_queue->limits.logical_block_size == arg)
return 0;
And reviving loop_validate_block_size() in order to use "unsigned long" does not make sense
for 32bits kernels.
