md: dm-ioctl: drop always-false condition

[PATCH] md: dm-ioctl: drop always-false condition

Posted by Sergey Shtylyov 2 years, 7 months ago

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@omp.ru>

---
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
 		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;
 	}

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

Posted by Mikulas Patocka 2 years, 7 months ago


On Tue, 17 Jan 2023, Sergey Shtylyov 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@omp.ru>

Reviewed-by: Mikulas Patocka <mpatocka@redhat.com>

> ---
> 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
>  		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;
>  	}
> 
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://listman.redhat.com/mailman/listinfo/dm-devel
>

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

Posted by Sergey Shtylyov 2 years, 7 months ago

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@omp.ru>
> 
> Reviewed-by: Mikulas Patocka <mpatocka@redhat.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

Re: md: dm-ioctl: drop always-false condition

Posted by Mike Snitzer 2 years, 7 months ago

On Mon, Feb 06 2023 at  3:58P -0500,
Sergey Shtylyov <s.shtylyov@omp.ru> wrote:

> 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@omp.ru>
> > 
> > Reviewed-by: Mikulas Patocka <mpatocka@redhat.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...

Not a big deal, I've already staged it so please don't resend.

See:
https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-6.3&id=151d812251202aa0dce1fdeabd64794292d40b75