| Message ID | 20241216123712.297722-1-dmantipov@yandex.ru (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | nvdimm: add extra LBA check for map read and write operations | expand |
Dmitry Antipov wrote: > In 'btt_map_read()' and '__btt_map_write()', add an extra check > whether requested LBA may be represented as valid offset against > an offset of the map area of the given arena. Compile tested only. Does this fix a real problem? Ira > > Found by Linux Verification Center (linuxtesting.org) with SVACE. > > Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru> > --- > drivers/nvdimm/btt.c | 18 +++++++++++++----- > 1 file changed, 13 insertions(+), 5 deletions(-) > > diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c > index 423dcd190906..2bd03143c8c3 100644 > --- a/drivers/nvdimm/btt.c > +++ b/drivers/nvdimm/btt.c > @@ -96,12 +96,17 @@ static int btt_info_read(struct arena_info *arena, struct btt_sb *super) > static int __btt_map_write(struct arena_info *arena, u32 lba, __le32 mapping, > unsigned long flags) > { > - u64 ns_off = arena->mapoff + (lba * MAP_ENT_SIZE); > + u32 lba_off; > + u64 ns_off; > > - if (unlikely(lba >= arena->external_nlba)) > + if (unlikely(lba >= arena->external_nlba || > + check_mul_overflow(lba, MAP_ENT_SIZE, &lba_off))) > dev_err_ratelimited(to_dev(arena), > "%s: lba %#x out of range (max: %#x)\n", > __func__, lba, arena->external_nlba); > + > + ns_off = arena->mapoff + lba_off; > + > return arena_write_bytes(arena, ns_off, &mapping, MAP_ENT_SIZE, flags); > } > > @@ -154,14 +159,17 @@ static int btt_map_read(struct arena_info *arena, u32 lba, u32 *mapping, > { > int ret; > __le32 in; > - u32 raw_mapping, postmap, ze, z_flag, e_flag; > - u64 ns_off = arena->mapoff + (lba * MAP_ENT_SIZE); > + u64 ns_off; > + u32 raw_mapping, postmap, ze, z_flag, e_flag, lba_off; > > - if (unlikely(lba >= arena->external_nlba)) > + if (unlikely(lba >= arena->external_nlba || > + check_mul_overflow(lba, MAP_ENT_SIZE, &lba_off))) > dev_err_ratelimited(to_dev(arena), > "%s: lba %#x out of range (max: %#x)\n", > __func__, lba, arena->external_nlba); > > + ns_off = arena->mapoff + lba_off; > + > ret = arena_read_bytes(arena, ns_off, &in, MAP_ENT_SIZE, rwb_flags); > if (ret) > return ret; > -- > 2.47.1 >
On Wed, 2024-12-18 at 16:29 -0600, Ira Weiny wrote:
> Does this fix a real problem?
None as I'm aware of. But growing NVDIMM sizes may be a problem for u32
range. IMHO maintainers are better to be prepared for such a scenario.
Dmitry
diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c index 423dcd190906..2bd03143c8c3 100644 --- a/drivers/nvdimm/btt.c +++ b/drivers/nvdimm/btt.c @@ -96,12 +96,17 @@ static int btt_info_read(struct arena_info *arena, struct btt_sb *super) static int __btt_map_write(struct arena_info *arena, u32 lba, __le32 mapping, unsigned long flags) { - u64 ns_off = arena->mapoff + (lba * MAP_ENT_SIZE); + u32 lba_off; + u64 ns_off; - if (unlikely(lba >= arena->external_nlba)) + if (unlikely(lba >= arena->external_nlba || + check_mul_overflow(lba, MAP_ENT_SIZE, &lba_off))) dev_err_ratelimited(to_dev(arena), "%s: lba %#x out of range (max: %#x)\n", __func__, lba, arena->external_nlba); + + ns_off = arena->mapoff + lba_off; + return arena_write_bytes(arena, ns_off, &mapping, MAP_ENT_SIZE, flags); } @@ -154,14 +159,17 @@ static int btt_map_read(struct arena_info *arena, u32 lba, u32 *mapping, { int ret; __le32 in; - u32 raw_mapping, postmap, ze, z_flag, e_flag; - u64 ns_off = arena->mapoff + (lba * MAP_ENT_SIZE); + u64 ns_off; + u32 raw_mapping, postmap, ze, z_flag, e_flag, lba_off; - if (unlikely(lba >= arena->external_nlba)) + if (unlikely(lba >= arena->external_nlba || + check_mul_overflow(lba, MAP_ENT_SIZE, &lba_off))) dev_err_ratelimited(to_dev(arena), "%s: lba %#x out of range (max: %#x)\n", __func__, lba, arena->external_nlba); + ns_off = arena->mapoff + lba_off; + ret = arena_read_bytes(arena, ns_off, &in, MAP_ENT_SIZE, rwb_flags); if (ret) return ret;
In 'btt_map_read()' and '__btt_map_write()', add an extra check whether requested LBA may be represented as valid offset against an offset of the map area of the given arena. Compile tested only. Found by Linux Verification Center (linuxtesting.org) with SVACE. Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru> --- drivers/nvdimm/btt.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)