[v1] ALSA: asihpi: avoid write overflow check warning

[PATCH] ALSA: asihpi: avoid write overflow check warning

Posted by Arnd Bergmann 2 weeks, 5 days ago

From: Arnd Bergmann <arnd@arndb.de>

clang-22 rightfully warns that the memcpy() in adapter_prepare() copies
between different structures, crossing the boundary of nested
structures inside it:

In file included from sound/pci/asihpi/hpimsgx.c:13:
In file included from include/linux/string.h:386:
include/linux/fortify-string.h:569:4: error: call to '__write_overflow_field' declared with 'warning' attribute: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror,-Wattribute-warning]
  569 |                         __write_overflow_field(p_size_field, size);

The two structures seem to refer to the same layout, despite the
separate definitions, so the code is in fact correct.

Avoid the warning by copying the two inner structures separately.
I see the same pattern happens in other functions in the same file,
so there is a chance that this may come back in the future, but
this instance is the only one that I saw in practice, hitting it
multiple times per day in randconfig build.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 sound/pci/asihpi/hpimsgx.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/sound/pci/asihpi/hpimsgx.c b/sound/pci/asihpi/hpimsgx.c
index b68e6bfbbfba..432a5b787a2a 100644
--- a/sound/pci/asihpi/hpimsgx.c
+++ b/sound/pci/asihpi/hpimsgx.c
@@ -581,8 +581,10 @@ static u16 adapter_prepare(u16 adapter)
 		HPI_ADAPTER_OPEN);
 	hm.adapter_index = adapter;
 	hw_entry_point(&hm, &hr);
-	memcpy(&rESP_HPI_ADAPTER_OPEN[adapter], &hr,
-		sizeof(rESP_HPI_ADAPTER_OPEN[0]));
+	memcpy(&rESP_HPI_ADAPTER_OPEN[adapter].h, &hr,
+		sizeof(rESP_HPI_ADAPTER_OPEN[adapter].h));
+	memcpy(&rESP_HPI_ADAPTER_OPEN[adapter].a, &hr.u.s,
+		sizeof(rESP_HPI_ADAPTER_OPEN[adapter].a));
 	if (hr.error)
 		return hr.error;
 
-- 
2.39.5

Re: [PATCH] ALSA: asihpi: avoid write overflow check warning

Posted by Takashi Iwai 2 weeks, 5 days ago

On Wed, 18 Mar 2026 11:50:08 +0100,
Arnd Bergmann wrote:
> 
> From: Arnd Bergmann <arnd@arndb.de>
> 
> clang-22 rightfully warns that the memcpy() in adapter_prepare() copies
> between different structures, crossing the boundary of nested
> structures inside it:
> 
> In file included from sound/pci/asihpi/hpimsgx.c:13:
> In file included from include/linux/string.h:386:
> include/linux/fortify-string.h:569:4: error: call to '__write_overflow_field' declared with 'warning' attribute: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror,-Wattribute-warning]
>   569 |                         __write_overflow_field(p_size_field, size);
> 
> The two structures seem to refer to the same layout, despite the
> separate definitions, so the code is in fact correct.
> 
> Avoid the warning by copying the two inner structures separately.
> I see the same pattern happens in other functions in the same file,
> so there is a chance that this may come back in the future, but
> this instance is the only one that I saw in practice, hitting it
> multiple times per day in randconfig build.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  sound/pci/asihpi/hpimsgx.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/pci/asihpi/hpimsgx.c b/sound/pci/asihpi/hpimsgx.c
> index b68e6bfbbfba..432a5b787a2a 100644
> --- a/sound/pci/asihpi/hpimsgx.c
> +++ b/sound/pci/asihpi/hpimsgx.c
> @@ -581,8 +581,10 @@ static u16 adapter_prepare(u16 adapter)
>  		HPI_ADAPTER_OPEN);
>  	hm.adapter_index = adapter;
>  	hw_entry_point(&hm, &hr);
> -	memcpy(&rESP_HPI_ADAPTER_OPEN[adapter], &hr,
> -		sizeof(rESP_HPI_ADAPTER_OPEN[0]));
> +	memcpy(&rESP_HPI_ADAPTER_OPEN[adapter].h, &hr,
> +		sizeof(rESP_HPI_ADAPTER_OPEN[adapter].h));
> +	memcpy(&rESP_HPI_ADAPTER_OPEN[adapter].a, &hr.u.s,
> +		sizeof(rESP_HPI_ADAPTER_OPEN[adapter].a));

I think the last one is still a bit confusing as if doing an overflow
copy again: hr.u.s is struct hpi_subsys_res which is smaller than
rESP_HPI_ADAPTER_OPEN[adapter].a which is struct hpi_adapter_res.

If any, the last one should be

	memcpy(&rESP_HPI_ADAPTER_OPEN[adapter].a, &hr.u.ax.info,
		sizeof(rESP_HPI_ADAPTER_OPEN[adapter].a));

Or it could be even

	rESP_HPI_ADAPTER_OPEN[adapter].a = hr.u.ax.info;

(I'm not sure which one can be a better result, though.)


thanks,

Takashi

Re: [PATCH] ALSA: asihpi: avoid write overflow check warning

Posted by Arnd Bergmann 2 weeks, 5 days ago

On Wed, Mar 18, 2026, at 12:54, Takashi Iwai wrote:
> On Wed, 18 Mar 2026 11:50:08 +0100,

>> +	memcpy(&rESP_HPI_ADAPTER_OPEN[adapter].h, &hr,
>> +		sizeof(rESP_HPI_ADAPTER_OPEN[adapter].h));
>> +	memcpy(&rESP_HPI_ADAPTER_OPEN[adapter].a, &hr.u.s,
>> +		sizeof(rESP_HPI_ADAPTER_OPEN[adapter].a));
>
> I think the last one is still a bit confusing as if doing an overflow
> copy again: hr.u.s is struct hpi_subsys_res which is smaller than
> rESP_HPI_ADAPTER_OPEN[adapter].a which is struct hpi_adapter_res.

Right, I got confused by the various similarly named types
again.

> If any, the last one should be
>
> 	memcpy(&rESP_HPI_ADAPTER_OPEN[adapter].a, &hr.u.ax.info,
> 		sizeof(rESP_HPI_ADAPTER_OPEN[adapter].a));
>
> Or it could be even
>
> 	rESP_HPI_ADAPTER_OPEN[adapter].a = hr.u.ax.info;
>
> (I'm not sure which one can be a better result, though.)

Code generation wise, I would expect them to be the same,
as gcc is good at transforming the memcpy() calls into
direct copies and vice versa.

I'll send a v2 with the first one after it passes build tests,
unless someone has another preference.

      Arnd