[PATCH] regmap: kunit: Fix array overflow in stride() test

[PATCH] regmap: kunit: Fix array overflow in stride() test

[Richard Fitzgerald]

Force the max_register of the test regmap to be one register longer
than the number of test registers, to prevent an array overflow in
the test loop.

The test defines num_reg_defaults = 6. With 6 registers and
stride == 2 the valid register addresses would be 0, 2, 4, 6, 8, 10.
However the loop checks attempting to access the odd address, so on
the final register it accesses address 11, and it writes entry [11]
of the read/written arrays.

Originally this worked because the max_register of the regmap was
hardcoded to be BLOCK_TEST_SIZE (== 12).

commit 710915743d53 ("regmap: kunit: Run sparse cache tests at non-zero
register addresses")
introduced the ability to start the test address range from any address,
which means adjusting the max_register. If max_register was not forced,
it was calculated either from num_reg_defaults or BLOCK_TEST_SIZE. This
correctly calculated that with num_reg_defaults == 6 and stride == 2 the
final valid address is 10. So the read/written arrays are allocated to
contain entries [0..10]. When stride attempted to access [11] it was
overflowing the array.

Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Fixes: 710915743d53 ("regmap: kunit: Run sparse cache tests at non-zero register addresses")
---
 drivers/base/regmap/regmap-kunit.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/base/regmap/regmap-kunit.c b/drivers/base/regmap/regmap-kunit.c
index 9c5314785fc2..be32cd4e84da 100644
--- a/drivers/base/regmap/regmap-kunit.c
+++ b/drivers/base/regmap/regmap-kunit.c
@@ -609,12 +609,19 @@ static void stride(struct kunit *test)
 	config.reg_stride = 2;
 	config.num_reg_defaults = BLOCK_TEST_SIZE / 2;
 
+	/*
+	 * Allow one extra register so that the read/written arrays
+	 * are sized big enough to include an entry for the odd
+	 * address past the final reg_default register.
+	 */
+	config.max_register = BLOCK_TEST_SIZE;
+
 	map = gen_regmap(test, &config, &data);
 	KUNIT_ASSERT_FALSE(test, IS_ERR(map));
 	if (IS_ERR(map))
 		return;
 
-	/* Only even registers can be accessed, try both read and write */
+	/* Only even addresses can be accessed, try both read and write */
 	for (i = 0; i < BLOCK_TEST_SIZE; i++) {
 		data->read[i] = false;
 		data->written[i] = false;
-- 
2.39.2

Re: [PATCH] regmap: kunit: Fix array overflow in stride() test

[Guenter Roeck]

On Fri, May 17, 2024 at 03:47:03PM +0100, Richard Fitzgerald wrote:
> Force the max_register of the test regmap to be one register longer
> than the number of test registers, to prevent an array overflow in
> the test loop.
> 
> The test defines num_reg_defaults = 6. With 6 registers and
> stride == 2 the valid register addresses would be 0, 2, 4, 6, 8, 10.
> However the loop checks attempting to access the odd address, so on
> the final register it accesses address 11, and it writes entry [11]
> of the read/written arrays.
> 
> Originally this worked because the max_register of the regmap was
> hardcoded to be BLOCK_TEST_SIZE (== 12).
> 
> commit 710915743d53 ("regmap: kunit: Run sparse cache tests at non-zero
> register addresses")
> introduced the ability to start the test address range from any address,
> which means adjusting the max_register. If max_register was not forced,
> it was calculated either from num_reg_defaults or BLOCK_TEST_SIZE. This
> correctly calculated that with num_reg_defaults == 6 and stride == 2 the
> final valid address is 10. So the read/written arrays are allocated to
> contain entries [0..10]. When stride attempted to access [11] it was
> overflowing the array.
> 
> Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
> Fixes: 710915743d53 ("regmap: kunit: Run sparse cache tests at non-zero register addresses")

Tested-by: Guenter Roeck <linux@roeck-us.net>

Re: [PATCH] regmap: kunit: Fix array overflow in stride() test

[Mark Brown]

On Fri, 17 May 2024 15:47:03 +0100, Richard Fitzgerald wrote:
> Force the max_register of the test regmap to be one register longer
> than the number of test registers, to prevent an array overflow in
> the test loop.
> 
> The test defines num_reg_defaults = 6. With 6 registers and
> stride == 2 the valid register addresses would be 0, 2, 4, 6, 8, 10.
> However the loop checks attempting to access the odd address, so on
> the final register it accesses address 11, and it writes entry [11]
> of the read/written arrays.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git for-next

Thanks!

[1/1] regmap: kunit: Fix array overflow in stride() test
      commit: 7ba822189e6060a8a2833b721d430f833bf0db43

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark