| Message ID | c8c9df45-1d59-3195-7631-51b3a58148ee@omp.ru (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Mike Snitzer |
| Headers | show |
| Series | md: dm-ioctl: drop always-false condition | expand |
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 > -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
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 -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
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 -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
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; }
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(-) -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel