linux-kernel - Re: [PATCH v2] drivers/block/loop: Replace deprecated function in option parsing code

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <eb0b0981-aba3-93dc-5ae5-d36f1f728024@stud.uni-hannover.de>
Date:   Sat, 29 Jun 2019 21:46:03 +0200
From:   Florian Knauf <florian.knauf@...d.uni-hannover.de>
To:     Chaitanya Kulkarni <Chaitanya.Kulkarni@....com>
Cc:     "linux-kernel@...cs.fau.de" <linux-kernel@...cs.fau.de>,
        Jens Axboe <axboe@...nel.dk>,
        "linux-block@...r.kernel.org" <linux-block@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Christian Ewert <christian.ewert@...d.uni-hannover.de>
Subject: Re: [PATCH v2] drivers/block/loop: Replace deprecated function in
 option parsing code

I have now, on the latest staging master (test log attached, everything 
green), and also learned a lesson about looking more thoroughly for 
automated test cases. That's a mea culpa, I suppose. :P

Before this I'd only found the Linux Test Project, which (if I'm not 
mistaken) contains tests that use loopback devices but no tests that 
specifically test the loopback driver itself. Given the small scope of 
the change, we then considered it sufficient to test manually that the 
loop device still worked and that the max_loop parameter was handled 
correctly. Of course, the blktests way is better.

Thanks for taking the time to answer and review.

Am 25.06.19 um 21:24 schrieb Chaitanya Kulkarni:
> I believe you have tested this patch with loop testcases present in the
> :- https://github.com/osandov/blktests/tree/master/tests/loop.
> 
> With that, looks good.
> 
> Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@....com>.
> 
> On 06/25/2019 10:55 AM, Florian Knauf wrote:
>> This patch removes the deprecated simple_strtol function from the option
>> parsing logic in the loopback device driver. Instead kstrtoint is used to
>> parse int max_loop, to ensure that input values it cannot represent are
>> ignored.
>>
>> Signed-off-by: Florian Knauf <florian.knauf@...d.uni-hannover.de>
>> Signed-off-by: Christian Ewert <christian.ewert@...d.uni-hannover.de>
>> ---
>> Thank you for your feedback.
>>
>> There's no specific reason to use kstrtol, other than the fact that we
>> weren't yet aware that kstrtoint exists. (We're new at this, I'm afraid.)
>>
>> We've amended the patch to make use of kstrtoint, which is of course much
>> more straightforward.
>>
>> drivers/block/loop.c | 2 +-
>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
>> index 102d79575895..adfaf4ad37d1 100644
>> --- a/drivers/block/loop.c
>> +++ b/drivers/block/loop.c
>> @@ -2289,7 +2289,7 @@ module_exit(loop_exit);
>>    #ifndef MODULE
>>    static int __init max_loop_setup(char *str)
>>    {
>> -	max_loop = simple_strtol(str, NULL, 0);
>> +	kstrtoint(str, 0, &max_loop);
>>    	return 1;
>>    }
>>
>>
> 

View attachment "check.log" of type "text/x-log" (1371 bytes)