Re: [PATCH] firmware: cs_dsp: test_control_parse: null-terminate test strings

[Richard Fitzgerald <rf@opensource.cirrus.com>]

On 11/02/2025 3:00 pm, Thomas Weißschuh wrote:
> The char pointers in 'struct cs_dsp_mock_coeff_def' are expected to
> point to C strings. They need to be terminated by a null byte.
> However the code does not allocate that trailing null byte and only
> works if by chance the allocation is followed by such a null byte.
> 
> Refactor the repeated string allocation logic into a new helper which
> makes sure the terminating null is always present.
> It also makes the code more readable.
> 
> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> Fixes: 83baecd92e7c ("firmware: cs_dsp: Add KUnit testing of control parsing")
> Cc: stable@vger.kernel.org
> ---
>   .../cirrus/test/cs_dsp_test_control_parse.c        | 51 ++++++++--------------
>   1 file changed, 19 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/firmware/cirrus/test/cs_dsp_test_control_parse.c b/drivers/firmware/cirrus/test/cs_dsp_test_control_parse.c
> index cb90964740ea351113dac274f0366de7cedfd3d1..942ba1af5e7c1e47e8a2fbe548a7993b94f96515 100644
> --- a/drivers/firmware/cirrus/test/cs_dsp_test_control_parse.c
> +++ b/drivers/firmware/cirrus/test/cs_dsp_test_control_parse.c
> @@ -73,6 +73,18 @@ static const struct cs_dsp_mock_coeff_def mock_coeff_template = {
>   	.length_bytes = 4,
>   };
>   
> +static char *cs_dsp_ctl_alloc_test_string(struct kunit *test, char c, size_t len)
> +{
> +	char *str;
> +
> +	str = kunit_kmalloc(test, len + 1, GFP_KERNEL);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, str);
> +	memset(str, c, len);
> +	str[len] = '\0';
> +
> +	return str;
> +}
> +
>   /* Algorithm info block without controls should load */
>   static void cs_dsp_ctl_parse_no_coeffs(struct kunit *test)
>   {
> @@ -160,12 +172,8 @@ static void cs_dsp_ctl_parse_max_v1_name(struct kunit *test)
>   	struct cs_dsp_mock_coeff_def def = mock_coeff_template;
>   	struct cs_dsp_coeff_ctl *ctl;
>   	struct firmware *wmfw;
> -	char *name;
>   
> -	name = kunit_kzalloc(test, 256, GFP_KERNEL);

This allocates 256 bytes of zero-filled memory...

> -	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, name);
> -	memset(name, 'A', 255);

... and this fills the first 255 bytes, leaving the last byte still as
zero. So the string is zero-terminated. I don't see a problem here.

Just fix the other allocs to be kzalloc with the correct length?