| Message ID | 20240123002814.1396804-47-keescook@chromium.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | overflow: Refactor open-coded arithmetic wrap-around | expand |
On Mon, Jan 22 2024 at 7:27P -0500, Kees Cook <keescook@chromium.org> wrote: > In an effort to separate intentional arithmetic wrap-around from > unexpected wrap-around, we need to refactor places that depend on this > kind of math. One of the most common code patterns of this is: > > VAR + value < VAR > > Notably, this is considered "undefined behavior" for signed and pointer > types, which the kernel works around by using the -fno-strict-overflow > option in the build[1] (which used to just be -fwrapv). Regardless, we > want to get the kernel source to the position where we can meaningfully > instrument arithmetic wrap-around conditions and catch them when they > are unexpected, regardless of whether they are signed[2], unsigned[3], > or pointer[4] types. > > Refactor open-coded wrap-around addition test to use add_would_overflow(). > This paves the way to enabling the wrap-around sanitizers in the future. > > Link: https://git.kernel.org/linus/68df3755e383e6fecf2354a67b08f92f18536594 [1] > Link: https://github.com/KSPP/linux/issues/26 [2] > Link: https://github.com/KSPP/linux/issues/27 [3] > Link: https://github.com/KSPP/linux/issues/344 [4] > Cc: Alasdair Kergon <agk@redhat.com> > Cc: Mike Snitzer <snitzer@kernel.org> > Cc: Mikulas Patocka <mpatocka@redhat.com> > Cc: dm-devel@lists.linux.dev > Signed-off-by: Kees Cook <keescook@chromium.org> Please change subject to: "dm: Refactor intentional wrap-around test in a few targets" Reviewed-by: Mike Snitzer <snitzer@kernel.org>
diff --git a/drivers/md/dm-switch.c b/drivers/md/dm-switch.c index dfd9fb52a6f3..9053d7e65603 100644 --- a/drivers/md/dm-switch.c +++ b/drivers/md/dm-switch.c @@ -410,7 +410,7 @@ static int process_set_region_mappings(struct switch_ctx *sctx, cycle_length - 1, region_index); return -EINVAL; } - if (unlikely(region_index + num_write < region_index) || + if (unlikely(add_would_overflow(region_index, num_write)) || unlikely(region_index + num_write >= sctx->nr_regions)) { DMWARN("invalid set_region_mappings region number: %lu + %lu >= %lu", region_index, num_write, sctx->nr_regions); diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c index 14e58ae70521..f2676c8c83c0 100644 --- a/drivers/md/dm-verity-target.c +++ b/drivers/md/dm-verity-target.c @@ -1392,7 +1392,7 @@ static int verity_ctr(struct dm_target *ti, unsigned int argc, char **argv) v->hash_level_block[i] = hash_position; s = (v->data_blocks + ((sector_t)1 << ((i + 1) * v->hash_per_block_bits)) - 1) >> ((i + 1) * v->hash_per_block_bits); - if (hash_position + s < hash_position) { + if (add_would_overflow(hash_position, s)) { ti->error = "Hash device offset overflow"; r = -E2BIG; goto bad; diff --git a/drivers/md/dm-writecache.c b/drivers/md/dm-writecache.c index 074cb785eafc..45e54edd24aa 100644 --- a/drivers/md/dm-writecache.c +++ b/drivers/md/dm-writecache.c @@ -2631,7 +2631,7 @@ static int writecache_ctr(struct dm_target *ti, unsigned int argc, char **argv) offset = (offset + wc->block_size - 1) & ~(size_t)(wc->block_size - 1); data_size = wc->n_blocks * (size_t)wc->block_size; if (!offset || (data_size / wc->block_size != wc->n_blocks) || - (offset + data_size < offset)) + (add_would_overflow(offset, data_size))) goto overflow; if (offset + data_size > wc->memory_map_size) { ti->error = "Memory area is too small";
In an effort to separate intentional arithmetic wrap-around from unexpected wrap-around, we need to refactor places that depend on this kind of math. One of the most common code patterns of this is: VAR + value < VAR Notably, this is considered "undefined behavior" for signed and pointer types, which the kernel works around by using the -fno-strict-overflow option in the build[1] (which used to just be -fwrapv). Regardless, we want to get the kernel source to the position where we can meaningfully instrument arithmetic wrap-around conditions and catch them when they are unexpected, regardless of whether they are signed[2], unsigned[3], or pointer[4] types. Refactor open-coded wrap-around addition test to use add_would_overflow(). This paves the way to enabling the wrap-around sanitizers in the future. Link: https://git.kernel.org/linus/68df3755e383e6fecf2354a67b08f92f18536594 [1] Link: https://github.com/KSPP/linux/issues/26 [2] Link: https://github.com/KSPP/linux/issues/27 [3] Link: https://github.com/KSPP/linux/issues/344 [4] Cc: Alasdair Kergon <agk@redhat.com> Cc: Mike Snitzer <snitzer@kernel.org> Cc: Mikulas Patocka <mpatocka@redhat.com> Cc: dm-devel@lists.linux.dev Signed-off-by: Kees Cook <keescook@chromium.org> --- drivers/md/dm-switch.c | 2 +- drivers/md/dm-verity-target.c | 2 +- drivers/md/dm-writecache.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)