On 3/31/23 07:52, Chaitanya Kulkarni wrote:
> On 3/30/23 15:45, Damien Le Moal wrote:
>> On 3/31/23 06:31, Chaitanya Kulkarni wrote:
>>> Right now we don't check for valid module parameter value for
>>> block size, that allows user to set negative values.
>>>
>>> Add a callback to error out when block size value is set < 1 before
>>> module is loaded.
>>>
>>> Signed-off-by: Chaitanya Kulkarni <kch@xxxxxxxxxx>
>>> ---
>>> drivers/block/null_blk/main.c | 17 ++++++++++++++++-
>>> 1 file changed, 16 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
>>> index f55d88ebd7e6..d8d79c66a7aa 100644
>>> --- a/drivers/block/null_blk/main.c
>>> +++ b/drivers/block/null_blk/main.c
>>> @@ -190,8 +190,23 @@ static int g_gb = 250;
>>> device_param_cb(gb, &null_gb_param_ops, &g_gb, 0444);
>>> MODULE_PARM_DESC(gb, "Size in GB");
>>>
>>> +static int null_set_bs(const char *s, const struct kernel_param *kp)
>>> +{
>>> + int ret;
>>> +
>>> + ret = null_param_store_int(s, kp->arg, 512, INT_MAX);
>>> + if (ret)
>>> + pr_err("valid range for bs value [512 ... %d]\n", INT_MAX);
>> This is is only checking the range. block sizes must be power-of-2 as well but
>> that is not checked. And for the range, block size up to INT_MAX ? That is not
>> very reasonable.
>>
>>
>
> I'll add ^2 check to next version.
> any suggestions on what is a reasonable size we should limit to ?
4K if you want to limit this to testing only things that actually exist. 64K
sound like a reasonable limit to test things that could eventually (maybe)
appear. But allowing > 4K block size may actually trigger a lot of bugs in other
code (FS). So I wonder if it is really wise to allow that.
--
Damien Le Moal
Western Digital Research
