On Fri, Mar 03, 2023 at 10:09:50AM -0600, Roger Heflin wrote:
> On Fri, Mar 3, 2023 at 8:31 AM Dan Carpenter <error27@xxxxxxxxx> wrote:
> >
> > [ Ancient code, but you're still at the same email address... -dan ]
> >
> > Hello NeilBrown,
> >
> > The patch ba1b41b6b4e3: "md: range check slot number when manually
> > adding a spare." from Jan 14, 2011, leads to the following Smatch
> > static checker warning:
> >
> > drivers/md/md.c:3170 slot_store() warn: no lower bound on 'slot'
> > drivers/md/md.c:3172 slot_store() warn: no lower bound on 'slot'
> > drivers/md/md.c:3190 slot_store() warn: no lower bound on 'slot'
> >
> > drivers/md/md.c
> > 3117 static ssize_t
> > 3118 slot_store(struct md_rdev *rdev, const char *buf, size_t len)
> > 3119 {
> > 3120 int slot;
> > 3121 int err;
> > 3122
> > 3123 if (test_bit(Journal, &rdev->flags))
> > 3124 return -EBUSY;
> > 3125 if (strncmp(buf, "none", 4)==0)
> > 3126 slot = -1;
> > 3127 else {
> > 3128 err = kstrtouint(buf, 10, (unsigned int *)&slot);
> >
> > slot comes from the user.
> >
> > 3129 if (err < 0)
> > 3130 return err;
> > 3131 }
>
> kstrtouint is string to unsigned int, it has this code:
>
> if (tmp != (unsigned int)tmp)
> return -ERANGE;
>
> And so will not return a negative number without an error, so 0 is the
> lower limit as enforced by the function.
Some of what you have written is correct, but your main conclusion is
wrong. The kstrtouint() gives you unsigned ints. If you take a very
large unsigned int and cast it to an int then you get a negative value
so the underflow issue is real.
Btw, in more modern code we would write:
err = kstrtoint(buf, 10, &slot);
if (err)
return err;
It still has the underflow issue... I have been wanting to say that and
resisted saying it because it was obvious to everyone. However I am
only human and can only resist the temptation to point out the obvious
for so long.
regards,
dan carpenter
