> 2015-10-28 8:03 GMT+09:00 Mikulas Patocka: > On Wed, 28 Oct 2015, Tomohiro Kusumi wrote: > > > It's always (sctx->nr_regions == nr_regions) considering > > the assignment right before this conditional, and this > > function is only used by .ctr. > > The variable sctx->nr_regions has type unsigned long and the variable > nr_regions has type sector_t. > > Thus the variables may be different when overflow happens. > > It could be changed to "if (nr_regions >= ULONG_MAX)". Sorry about that. Changed the conditional to "if (nr_regions >= ULONG_MAX)", and brought the assignment of nr_regions after sector_div() and the sanity check which looks more sane. Signed-off-by: Tomohiro Kusumi <kusumi.tomohiro@xxxxxxxxx> Reviewed-by: Mikulas Patocka <mpatocka@xxxxxxxxxx> --- drivers/md/dm-switch.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/md/dm-switch.c b/drivers/md/dm-switch.c index 50fca46..e1b6722 100644 --- a/drivers/md/dm-switch.c +++ b/drivers/md/dm-switch.c @@ -99,11 +99,11 @@ static int alloc_region_table(struct dm_target *ti, unsigned nr_paths) if (sector_div(nr_regions, sctx->region_size)) nr_regions++; - sctx->nr_regions = nr_regions; - if (sctx->nr_regions != nr_regions || sctx->nr_regions >= ULONG_MAX) { + if (nr_regions >= ULONG_MAX) { ti->error = "Region table too large"; return -EINVAL; } + sctx->nr_regions = nr_regions; nr_slots = nr_regions; if (sector_div(nr_slots, sctx->region_entries_per_slot)) -- 1.7.1 -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel