| Message ID | 167805578596.8008.14753053536858832084@noble.neil.brown.name (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Song Liu |
| Headers | show |
| Series | md: avoid signed overflow in slot_store() | expand |
On Sun, Mar 5, 2023 at 2:36 PM NeilBrown <neilb@suse.de> wrote: > > > slot_store() uses kstrtouint() to get a slot number, but stores the > result in an "int" variable (by casting a pointer). > This can result in a negative slot number if the unsigned int value is > very large. > > A negative number means that the slot is empty, but setting a negative > slot number this way will not remove the device from the array. I don't > think this is a serious problem, but it could cause confusion and it is > best to fix it. > > Reported-by: Dan Carpenter <error27@gmail.com> > Signed-off-by: NeilBrown <neilb@suse.de> Applied to md-fixes. Thanks! Song > --- > drivers/md/md.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/md/md.c b/drivers/md/md.c > index 02b0240e7c71..d4bfa35fb20a 100644 > --- a/drivers/md/md.c > +++ b/drivers/md/md.c > @@ -3131,6 +3131,9 @@ slot_store(struct md_rdev *rdev, const char *buf, size_t len) > err = kstrtouint(buf, 10, (unsigned int *)&slot); > if (err < 0) > return err; > + if (slot < 0) > + /* overflow */ > + return -ENOSPC; > } > if (rdev->mddev->pers && slot == -1) { > /* Setting 'slot' on an active array requires also > -- > 2.39.2 >
diff --git a/drivers/md/md.c b/drivers/md/md.c index 02b0240e7c71..d4bfa35fb20a 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c @@ -3131,6 +3131,9 @@ slot_store(struct md_rdev *rdev, const char *buf, size_t len) err = kstrtouint(buf, 10, (unsigned int *)&slot); if (err < 0) return err; + if (slot < 0) + /* overflow */ + return -ENOSPC; } if (rdev->mddev->pers && slot == -1) { /* Setting 'slot' on an active array requires also
slot_store() uses kstrtouint() to get a slot number, but stores the result in an "int" variable (by casting a pointer). This can result in a negative slot number if the unsigned int value is very large. A negative number means that the slot is empty, but setting a negative slot number this way will not remove the device from the array. I don't think this is a serious problem, but it could cause confusion and it is best to fix it. Reported-by: Dan Carpenter <error27@gmail.com> Signed-off-by: NeilBrown <neilb@suse.de> --- drivers/md/md.c | 3 +++ 1 file changed, 3 insertions(+)