2023-01-17 23:47:56

by Sergey Shtylyov

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

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 <[email protected]>

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

2023-01-18 13:13:15

by Mikulas Patocka

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

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 <[email protected]>

Reviewed-by: Mikulas Patocka <[email protected]>

> ---
> 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
> [email protected]
> https://listman.redhat.com/mailman/listinfo/dm-devel
>

2023-02-06 21:00:22

by Sergey Shtylyov

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 <[email protected]>
>
> Reviewed-by: Mikulas Patocka <[email protected]>

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

2023-02-06 21:04:35

by Mike Snitzer

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

On Mon, Feb 06 2023 at 3:58P -0500,
Sergey Shtylyov <[email protected]> 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 <[email protected]>
> >
> > Reviewed-by: Mikulas Patocka <[email protected]>
>
> 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