linux-kernel - Re: [dm-devel] [PATCH] md: dm-ioctl: drop always-false condition

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: <b8269463-9f7e-4c28-2bfe-44663f3764c1@omp.ru>
Date:   Mon, 6 Feb 2023 23:58:54 +0300
From:   Sergey Shtylyov <s.shtylyov@....ru>
To:     Mikulas Patocka <mpatocka@...hat.com>
CC:     Alasdair Kergon <agk@...hat.com>,
        Mike Snitzer <snitzer@...nel.org>, <dm-devel@...hat.com>,
        <lvc-patches@...uxtesting.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [dm-devel] [PATCH] md: dm-ioctl: drop always-false condition

On 1/18/23 2:59 PM, Mikulas Patocka wrote:

[...]

>> The expression 'indata[3] > ULONG_MAX' always evaluates to false since
>> indata[] is declared as an array of *unsigned long* elements and #define
>> ULONG_MAX represents the max value of that exact type...
>>
>> Note that gcc seems to be able to detect the dead code here and eliminate
>> this check anyway...
>>
>> Found by Linux Verification Center (linuxtesting.org) with the SVACE static
>> analysis tool.
>>
>> Signed-off-by: Sergey Shtylyov <s.shtylyov@....ru>
> 
> Reviewed-by: Mikulas Patocka <mpatocka@...hat.com>

   Thank you!

>> ---
>> This patch is atop of the 'for-next' branch of the device-mapper repo...
>>
>>  drivers/md/dm-ioctl.c |    3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> Index: linux-dm/drivers/md/dm-ioctl.c
>> ===================================================================
>> --- linux-dm.orig/drivers/md/dm-ioctl.c
>> +++ linux-dm/drivers/md/dm-ioctl.c
>> @@ -1073,8 +1073,7 @@ static int dev_set_geometry(struct file

   Returning to this patch, I think I should've added the name of the function
in question in both the subject and and commit msg... I'll recast...

>>  		goto out;
>>  	}
>>  
>> -	if (indata[0] > 65535 || indata[1] > 255 ||
>> -	    indata[2] > 255 || indata[3] > ULONG_MAX) {
>> +	if (indata[0] > 65535 || indata[1] > 255 || indata[2] > 255) {
>>  		DMERR("Geometry exceeds range limits.");
>>  		goto out;
>>  	}
>>

MBR, Sergey