[PATCH] ASoC: cs-amp-lib: Annotate struct cirrus_amp_efi_data with __counted_by()

[PATCH] ASoC: cs-amp-lib: Annotate struct cirrus_amp_efi_data with __counted_by()

[Thorsten Blum]

Add the __counted_by() compiler attribute to the flexible array member
'data' to improve access bounds-checking via CONFIG_UBSAN_BOUNDS and
CONFIG_FORTIFY_SOURCE.

No functional changes intended.

Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 include/sound/cs-amp-lib.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/sound/cs-amp-lib.h b/include/sound/cs-amp-lib.h
index f481148735e1..5459c221badf 100644
--- a/include/sound/cs-amp-lib.h
+++ b/include/sound/cs-amp-lib.h
@@ -23,7 +23,7 @@ struct cirrus_amp_cal_data {
 struct cirrus_amp_efi_data {
 	u32 size;
 	u32 count;
-	struct cirrus_amp_cal_data data[];
+	struct cirrus_amp_cal_data data[] __counted_by(count);
 } __packed;
 
 /**
-- 
2.49.0

Re: [PATCH] ASoC: cs-amp-lib: Annotate struct cirrus_amp_efi_data with __counted_by()

[Richard Fitzgerald]

On 15/04/2025 10:03 am, Thorsten Blum wrote:
> Add the __counted_by() compiler attribute to the flexible array member
> 'data' to improve access bounds-checking via CONFIG_UBSAN_BOUNDS and
> CONFIG_FORTIFY_SOURCE.
> 
> No functional changes intended.
> 
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
>   include/sound/cs-amp-lib.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/sound/cs-amp-lib.h b/include/sound/cs-amp-lib.h
> index f481148735e1..5459c221badf 100644
> --- a/include/sound/cs-amp-lib.h
> +++ b/include/sound/cs-amp-lib.h
> @@ -23,7 +23,7 @@ struct cirrus_amp_cal_data {
>   struct cirrus_amp_efi_data {
>   	u32 size;
>   	u32 count;
> -	struct cirrus_amp_cal_data data[];
> +	struct cirrus_amp_cal_data data[] __counted_by(count);
>   } __packed;
>   
>   /**

I wrote a patch to do exactly the same, but you got your patch out
first! It is identical to my patch so:

Reviewed-by: Richard Fitzgerald <rf@opensource.cirrus.com>

(Note that I did some testing with Clang-19 and the __counted_by() seems
to only be checked in certain functions, like memcpy() and memset(). The
get_random_bytes() used in the KUnit test can overrun the array without
triggering any warning.)

Re: [PATCH] ASoC: cs-amp-lib: Annotate struct cirrus_amp_efi_data with __counted_by()

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 553 bytes --]

On Tue, Apr 15, 2025 at 11:03:55AM +0200, Thorsten Blum wrote:
> Add the __counted_by() compiler attribute to the flexible array member
> 'data' to improve access bounds-checking via CONFIG_UBSAN_BOUNDS and
> CONFIG_FORTIFY_SOURCE.

As documented in submitting-patches.rst please send patches to the 
maintainers for the code you would like to change.  The normal kernel
workflow is that people apply patches from their inboxes, if they aren't
copied they are likely to not see the patch at all and it is much more
difficult to apply patches.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] ASoC: cs-amp-lib: Annotate struct cirrus_amp_efi_data with __counted_by()

[Thorsten Blum]

On 15. Apr 2025, at 12:56, Mark Brown wrote:
> On Tue, Apr 15, 2025 at 11:03:55AM +0200, Thorsten Blum wrote:
>> Add the __counted_by() compiler attribute to the flexible array member
>> 'data' to improve access bounds-checking via CONFIG_UBSAN_BOUNDS and
>> CONFIG_FORTIFY_SOURCE.
> 
> As documented in submitting-patches.rst please send patches to the 
> maintainers for the code you would like to change.  The normal kernel
> workflow is that people apply patches from their inboxes, if they aren't
> copied they are likely to not see the patch at all and it is much more
> difficult to apply patches.

I just use whatever scripts/get_maintainer.pl outputs. Maybe the
MAINTAINERS file should be updated?

Best,
Thorsten

Re: [PATCH] ASoC: cs-amp-lib: Annotate struct cirrus_amp_efi_data with __counted_by()

[Richard Fitzgerald]

On 15/04/2025 12:51 pm, Thorsten Blum wrote:
> On 15. Apr 2025, at 12:56, Mark Brown wrote:
>> On Tue, Apr 15, 2025 at 11:03:55AM +0200, Thorsten Blum wrote:
>>> Add the __counted_by() compiler attribute to the flexible array member
>>> 'data' to improve access bounds-checking via CONFIG_UBSAN_BOUNDS and
>>> CONFIG_FORTIFY_SOURCE.
>>
>> As documented in submitting-patches.rst please send patches to the
>> maintainers for the code you would like to change.  The normal kernel
>> workflow is that people apply patches from their inboxes, if they aren't
>> copied they are likely to not see the patch at all and it is much more
>> difficult to apply patches.
> 
> I just use whatever scripts/get_maintainer.pl outputs. Maybe the
> MAINTAINERS file should be updated?
> 
> Best,
> Thorsten
> 

MAINTAINERS only lists Takashi and Jaroslav as maintainers of
include/sound/*.

A separate section lists Mark as maintainer of a specific set of files
within include/sound/

I guess any files we put in include/sound that are for an ASoC driver
have to also be added to Mark's ASoC section of MAINTAINERS?

Re: [PATCH] ASoC: cs-amp-lib: Annotate struct cirrus_amp_efi_data with __counted_by()

[Takashi Iwai]

On Tue, 15 Apr 2025 14:03:18 +0200,
Richard Fitzgerald wrote:
> 
> On 15/04/2025 12:51 pm, Thorsten Blum wrote:
> > On 15. Apr 2025, at 12:56, Mark Brown wrote:
> >> On Tue, Apr 15, 2025 at 11:03:55AM +0200, Thorsten Blum wrote:
> >>> Add the __counted_by() compiler attribute to the flexible array member
> >>> 'data' to improve access bounds-checking via CONFIG_UBSAN_BOUNDS and
> >>> CONFIG_FORTIFY_SOURCE.
> >> 
> >> As documented in submitting-patches.rst please send patches to the
> >> maintainers for the code you would like to change.  The normal kernel
> >> workflow is that people apply patches from their inboxes, if they aren't
> >> copied they are likely to not see the patch at all and it is much more
> >> difficult to apply patches.
> > 
> > I just use whatever scripts/get_maintainer.pl outputs. Maybe the
> > MAINTAINERS file should be updated?
> > 
> > Best,
> > Thorsten
> > 
> 
> MAINTAINERS only lists Takashi and Jaroslav as maintainers of
> include/sound/*.
> 
> A separate section lists Mark as maintainer of a specific set of files
> within include/sound/
> 
> I guess any files we put in include/sound that are for an ASoC driver
> have to also be added to Mark's ASoC section of MAINTAINERS?

Yes, it makes sense.


Takashi

Re: [PATCH] ASoC: cs-amp-lib: Annotate struct cirrus_amp_efi_data with __counted_by()

[Mark Brown]

On Tue, 15 Apr 2025 11:03:55 +0200, Thorsten Blum wrote:
> Add the __counted_by() compiler attribute to the flexible array member
> 'data' to improve access bounds-checking via CONFIG_UBSAN_BOUNDS and
> CONFIG_FORTIFY_SOURCE.
> 
> No functional changes intended.
> 
> 
> [...]

Applied to

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

Thanks!

[1/1] ASoC: cs-amp-lib: Annotate struct cirrus_amp_efi_data with __counted_by()
      commit: fcdf212fd9b36c299d90229e9546c077db2215ce

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