DM RAID: fix a couple integer overflows
Subsystems (3)
PATCH 1/1 DM RAID: fix a couple integer overflows
My static checker complains that if "num_raid_params" is UINT_MAX then the "if (num_raid_params + 1 > argc) {" check doesn't work as intended.
The other change is that I moved the "if (argc != (num_raid_devs * 2))" condition forward a few lines so it was before the call to context_alloc(). If we had an integer overflow inside that function then it would lead to an immediate crash.
---
Static checker stuff. Not tested.
drivers/md/dm-raid.c +7 -9
@@ -1189,7 +1189,7 @@ static int raid_ctr(struct dm_target *ti, unsigned argc, char **argv) argv++;/* Skip over RAID params for now and find out # of devices */ -if (num_raid_params +1 > argc) { +if (num_raid_params >= argc) { ti->error ="Arguments do not agree with counts given" ;return -EINVAL ; }
@@ -1200,6 +1200,12 @@ static int raid_ctr(struct dm_target *ti, unsigned argc, char **argv)return -EINVAL ; } + argc -= num_raid_params +1 ;/* +1: we already have num_raid_devs */ +if (argc != (num_raid_devs *2 )) { + ti->error ="Supplied RAID devices does not match the count given" ; +return -EINVAL ; + } + rs =context_alloc (ti, rt, (unsigned )num_raid_devs);if (IS_ERR (rs))return PTR_ERR (rs);
@@ -1208,16 +1214,8 @@ static int raid_ctr(struct dm_target *ti, unsigned argc, char **argv)if (ret) goto bad; - ret = -EINVAL ; - - argc -= num_raid_params +1 ;/* +1: we already have num_raid_devs */ argv += num_raid_params +1 ; -if (argc != (num_raid_devs *2 )) { - ti->error ="Supplied RAID devices does not match the count given" ; - goto bad; - } - ret =dev_parms (rs, argv);if (ret) goto bad;
These array overflows are still there in linux-next.
regards,
dan carpenter
Quoted 55 lines · Dan Carpenter
My static checker complains that if "
num_raid_params" isUINT_MAXthen the "if (num_raid_params+ 1 > argc) {" check doesn't work as intended.The other change is that I moved the "if (argc != (
num_raid_devs* 2))" condition forward a few lines so it was before the call tocontext_alloc(). If we had an integer overflow inside that function then it would lead to an immediate crash.Signed-off-by Dan Carpenter <[email protected]>---
Static checker stuff. Not tested.drivers/md/dm-raid.c +7
-9@@ -1189,7 +1189,7 @@ static int raid_ctr(struct dm_target *ti, unsigned argc, char **argv) argv++;/* Skip over RAID params for now and find out # of devices */ -if (num_raid_params +1 > argc) { +if (num_raid_params >= argc) { ti->error ="Arguments do not agree with counts given" ;return -EINVAL ; }@@ -1200,6 +1200,12 @@ static int raid_ctr(struct dm_target *ti, unsigned argc, char **argv)return -EINVAL ; } + argc -= num_raid_params +1 ;/* +1: we already have num_raid_devs */ +if (argc != (num_raid_devs *2 )) { + ti->error ="Supplied RAID devices does not match the count given" ; +return -EINVAL ; + } + rs =context_alloc (ti, rt, (unsigned )num_raid_devs);if (IS_ERR (rs))return PTR_ERR (rs);@@ -1208,16 +1214,8 @@ static int raid_ctr(struct dm_target *ti, unsigned argc, char **argv)if (ret) goto bad; - ret = -EINVAL ; - - argc -= num_raid_params +1 ;/* +1: we already have num_raid_devs */ argv += num_raid_params +1 ; -if (argc != (num_raid_devs *2 )) { - ti->error ="Supplied RAID devices does not match the count given" ; - goto bad; - } - ret =dev_parms (rs, argv);if (ret) goto bad;