Linux Sound subsystem development
 help / color / mirror / Atom feed
* [RFC PATCH] ASoC: Intel: avs: Fix reading 1 or more bytes from a region of size 0
@ 2025-09-02  8:08 Brahmajit Das
  2025-09-02  8:15 ` Takashi Iwai
  0 siblings, 1 reply; 5+ messages in thread
From: Brahmajit Das @ 2025-09-02  8:08 UTC (permalink / raw)
  To: linux-sound, linux-next
  Cc: cezary.rojewski, liam.r.girdwood, peter.ujfalusi, yung-chuan.liao,
	broonie, listout

Building the next tree with GCC 16, results in the following error:

sound/soc/intel/avs/path.c:137:38: error: ‘strcmp’ reading 1 or more bytes from a region of size 0 [-Werror=stringop-overread]
  137 |         return id->id == id2->id && !strcmp(id->tplg_name, id2->tplg_name);
      |                                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from sound/soc/intel/avs/path.h:14,
                 from sound/soc/intel/avs/path.c:15:
sound/soc/intel/avs/topology.h: In function ‘avs_condpaths_walk’:
sound/soc/intel/avs/topology.h:150:13: note: at offset 4 into source object ‘id’ of size 4
  150 |         u32 id;
      |             ^~
sound/soc/intel/avs/topology.h:150:13: note: at offset 4 into source object ‘id’ of size 4

I'm not quite sure if this is a GCC bug or a problem with the source
code.
As an workaround, instead of using strcmp, strncmp helps. But would
really appriciate comments from developers as I'm sure there might be a
better way to fix this.

Introduced by commit 595b7f155b926 ("ASoC: Intel: avs: Conditional-path
support")

Signed-off-by: Brahmajit Das <listout@listout.xyz>
---
 sound/soc/intel/avs/path.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sound/soc/intel/avs/path.c b/sound/soc/intel/avs/path.c
index 7aa20fcf1a33..8c3df2002b58 100644
--- a/sound/soc/intel/avs/path.c
+++ b/sound/soc/intel/avs/path.c
@@ -134,7 +134,8 @@ static struct avs_tplg_path *avs_condpath_find_variant(struct avs_dev *adev,
 static bool avs_tplg_path_template_id_equal(struct avs_tplg_path_template_id *id,
 					    struct avs_tplg_path_template_id *id2)
 {
-	return id->id == id2->id && !strcmp(id->tplg_name, id2->tplg_name);
+	return id->id == id2->id &&
+	       !strncmp(id->tplg_name, id2->tplg_name, strlen(id->tplg_name));
 }
 
 static struct avs_path *avs_condpath_find_match(struct avs_dev *adev,
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [RFC PATCH] ASoC: Intel: avs: Fix reading 1 or more bytes from a region of size 0
  2025-09-02  8:08 [RFC PATCH] ASoC: Intel: avs: Fix reading 1 or more bytes from a region of size 0 Brahmajit Das
@ 2025-09-02  8:15 ` Takashi Iwai
  2025-09-02 10:13   ` [RFC PATCH v2] " Brahmajit Das
  2025-09-04  8:40   ` [RFC PATCH] " Brahmajit Das
  0 siblings, 2 replies; 5+ messages in thread
From: Takashi Iwai @ 2025-09-02  8:15 UTC (permalink / raw)
  To: Brahmajit Das
  Cc: linux-sound, linux-next, cezary.rojewski, liam.r.girdwood,
	peter.ujfalusi, yung-chuan.liao, broonie

On Tue, 02 Sep 2025 10:08:12 +0200,
Brahmajit Das wrote:
> 
> Building the next tree with GCC 16, results in the following error:
> 
> sound/soc/intel/avs/path.c:137:38: error: ‘strcmp’ reading 1 or more bytes from a region of size 0 [-Werror=stringop-overread]
>   137 |         return id->id == id2->id && !strcmp(id->tplg_name, id2->tplg_name);
>       |                                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> In file included from sound/soc/intel/avs/path.h:14,
>                  from sound/soc/intel/avs/path.c:15:
> sound/soc/intel/avs/topology.h: In function ‘avs_condpaths_walk’:
> sound/soc/intel/avs/topology.h:150:13: note: at offset 4 into source object ‘id’ of size 4
>   150 |         u32 id;
>       |             ^~
> sound/soc/intel/avs/topology.h:150:13: note: at offset 4 into source object ‘id’ of size 4
> 
> I'm not quite sure if this is a GCC bug or a problem with the source
> code.
> As an workaround, instead of using strcmp, strncmp helps. But would
> really appriciate comments from developers as I'm sure there might be a
> better way to fix this.
> 
> Introduced by commit 595b7f155b926 ("ASoC: Intel: avs: Conditional-path
> support")
> 
> Signed-off-by: Brahmajit Das <listout@listout.xyz>
> ---
>  sound/soc/intel/avs/path.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/soc/intel/avs/path.c b/sound/soc/intel/avs/path.c
> index 7aa20fcf1a33..8c3df2002b58 100644
> --- a/sound/soc/intel/avs/path.c
> +++ b/sound/soc/intel/avs/path.c
> @@ -134,7 +134,8 @@ static struct avs_tplg_path *avs_condpath_find_variant(struct avs_dev *adev,
>  static bool avs_tplg_path_template_id_equal(struct avs_tplg_path_template_id *id,
>  					    struct avs_tplg_path_template_id *id2)
>  {
> -	return id->id == id2->id && !strcmp(id->tplg_name, id2->tplg_name);
> +	return id->id == id2->id &&
> +	       !strncmp(id->tplg_name, id2->tplg_name, strlen(id->tplg_name));

Please use sizeof()-1 instead of strlen(), as it's a fixed size array.

Practically seen, it's likely a false-positive from the new compiler.
But it's still safer to add the boundary check in the code itself.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [RFC PATCH v2] ASoC: Intel: avs: Fix reading 1 or more bytes from a region of size 0
  2025-09-02  8:15 ` Takashi Iwai
@ 2025-09-02 10:13   ` Brahmajit Das
  2025-09-04  8:40   ` [RFC PATCH] " Brahmajit Das
  1 sibling, 0 replies; 5+ messages in thread
From: Brahmajit Das @ 2025-09-02 10:13 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: linux-sound, linux-next, cezary.rojewski, liam.r.girdwood,
	peter.ujfalusi, yung-chuan.liao, broonie

Building the next tree with GCC 16, results in the following error:

sound/soc/intel/avs/path.c:137:38: error: ‘strcmp’ reading 1 or more bytes from a region of size 0 [-Werror=stringop-overread]
  137 |         return id->id == id2->id && !strcmp(id->tplg_name, id2->tplg_name);
      |                                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from sound/soc/intel/avs/path.h:14,
                 from sound/soc/intel/avs/path.c:15:
sound/soc/intel/avs/topology.h: In function ‘avs_condpaths_walk’:
sound/soc/intel/avs/topology.h:150:13: note: at offset 4 into source object ‘id’ of size 4
  150 |         u32 id;
      |             ^~
sound/soc/intel/avs/topology.h:150:13: note: at offset 4 into source object ‘id’ of size 4

I'm not quite sure if this is a GCC bug or a problem with the source
code.
As an workaround, instead of using strcmp, strncmp helps. But would
really appriciate comments from developers as I'm sure there might be a
better way to fix this.

Introduced by commit 595b7f155b926 ("ASoC: Intel: avs: Conditional-path
support")

Suggested-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Brahmajit Das <listout@listout.xyz>
---
Changes in v2:
        - using sizeof()-1 instead of strlen() due to tplg_name being
        fixed size array
---
 sound/soc/intel/avs/path.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sound/soc/intel/avs/path.c b/sound/soc/intel/avs/path.c
index 7aa20fcf1a33..367de5225ec4 100644
--- a/sound/soc/intel/avs/path.c
+++ b/sound/soc/intel/avs/path.c
@@ -134,7 +134,8 @@ static struct avs_tplg_path *avs_condpath_find_variant(struct avs_dev *adev,
 static bool avs_tplg_path_template_id_equal(struct avs_tplg_path_template_id *id,
                                            struct avs_tplg_path_template_id *id2)
 {
-       return id->id == id2->id && !strcmp(id->tplg_name, id2->tplg_name);
+       return id->id == id2->id && !strncmp(id->tplg_name, id2->tplg_name,
+                                            sizeof(id->tplg_name) - 1);
 }

 static struct avs_path *avs_condpath_find_match(struct avs_dev *adev,
--
2.51.0

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [RFC PATCH] ASoC: Intel: avs: Fix reading 1 or more bytes from a region of size 0
  2025-09-02  8:15 ` Takashi Iwai
  2025-09-02 10:13   ` [RFC PATCH v2] " Brahmajit Das
@ 2025-09-04  8:40   ` Brahmajit Das
  2025-09-04  9:14     ` Takashi Iwai
  1 sibling, 1 reply; 5+ messages in thread
From: Brahmajit Das @ 2025-09-04  8:40 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: linux-sound, linux-next, cezary.rojewski, liam.r.girdwood,
	peter.ujfalusi, yung-chuan.liao, broonie

On 02.09.2025 10:15, Takashi Iwai wrote:
> On Tue, 02 Sep 2025 10:08:12 +0200,
> Brahmajit Das wrote:
> > 
> > Building the next tree with GCC 16, results in the following error:
> > 
> > sound/soc/intel/avs/path.c:137:38: error: ‘strcmp’ reading 1 or more bytes from a region of size 0 [-Werror=stringop-overread]
> >   137 |         return id->id == id2->id && !strcmp(id->tplg_name, id2->tplg_name);
> >       |                                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
...snip...
> > Signed-off-by: Brahmajit Das <listout@listout.xyz>
> > ---
> >  sound/soc/intel/avs/path.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/sound/soc/intel/avs/path.c b/sound/soc/intel/avs/path.c
> > index 7aa20fcf1a33..8c3df2002b58 100644
> > --- a/sound/soc/intel/avs/path.c
> > +++ b/sound/soc/intel/avs/path.c
> > @@ -134,7 +134,8 @@ static struct avs_tplg_path *avs_condpath_find_variant(struct avs_dev *adev,
> >  static bool avs_tplg_path_template_id_equal(struct avs_tplg_path_template_id *id,
> >  					    struct avs_tplg_path_template_id *id2)
> >  {
> > -	return id->id == id2->id && !strcmp(id->tplg_name, id2->tplg_name);
> > +	return id->id == id2->id &&
> > +	       !strncmp(id->tplg_name, id2->tplg_name, strlen(id->tplg_name));
> 
> Please use sizeof()-1 instead of strlen(), as it's a fixed size array.
> 
> Practically seen, it's likely a false-positive from the new compiler.
> But it's still safer to add the boundary check in the code itself.
> 
> 
> thanks,
> 
> Takashi
So, I was testing again and with sizeof, I'm getting

sound/soc/intel/avs/path.c:137:38: error: ‘strncmp’ specified bound 43 exceeds source size 0 [-Werror=stringop-overread]
  137 |         return id->id == id2->id && !strncmp(id->tplg_name, id2->tplg_name,
      |                                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  138 |                                              sizeof(id->tplg_name) - 1);
      |                                              ~~~~~~~~~~~~~~~~~~~~~~~~~~

So my previous v2 is wrong, please ignore that.
Whereas, with strlen there's no warning :( . I'm quite confused, and
lack the GCC knowledge.

-- 
Regards,
listout

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC PATCH] ASoC: Intel: avs: Fix reading 1 or more bytes from a region of size 0
  2025-09-04  8:40   ` [RFC PATCH] " Brahmajit Das
@ 2025-09-04  9:14     ` Takashi Iwai
  0 siblings, 0 replies; 5+ messages in thread
From: Takashi Iwai @ 2025-09-04  9:14 UTC (permalink / raw)
  To: Brahmajit Das
  Cc: Takashi Iwai, linux-sound, linux-next, cezary.rojewski,
	liam.r.girdwood, peter.ujfalusi, yung-chuan.liao, broonie

On Thu, 04 Sep 2025 10:40:58 +0200,
Brahmajit Das wrote:
> 
> On 02.09.2025 10:15, Takashi Iwai wrote:
> > On Tue, 02 Sep 2025 10:08:12 +0200,
> > Brahmajit Das wrote:
> > > 
> > > Building the next tree with GCC 16, results in the following error:
> > > 
> > > sound/soc/intel/avs/path.c:137:38: error: ‘strcmp’ reading 1 or more bytes from a region of size 0 [-Werror=stringop-overread]
> > >   137 |         return id->id == id2->id && !strcmp(id->tplg_name, id2->tplg_name);
> > >       |                                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ...snip...
> > > Signed-off-by: Brahmajit Das <listout@listout.xyz>
> > > ---
> > >  sound/soc/intel/avs/path.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/sound/soc/intel/avs/path.c b/sound/soc/intel/avs/path.c
> > > index 7aa20fcf1a33..8c3df2002b58 100644
> > > --- a/sound/soc/intel/avs/path.c
> > > +++ b/sound/soc/intel/avs/path.c
> > > @@ -134,7 +134,8 @@ static struct avs_tplg_path *avs_condpath_find_variant(struct avs_dev *adev,
> > >  static bool avs_tplg_path_template_id_equal(struct avs_tplg_path_template_id *id,
> > >  					    struct avs_tplg_path_template_id *id2)
> > >  {
> > > -	return id->id == id2->id && !strcmp(id->tplg_name, id2->tplg_name);
> > > +	return id->id == id2->id &&
> > > +	       !strncmp(id->tplg_name, id2->tplg_name, strlen(id->tplg_name));
> > 
> > Please use sizeof()-1 instead of strlen(), as it's a fixed size array.
> > 
> > Practically seen, it's likely a false-positive from the new compiler.
> > But it's still safer to add the boundary check in the code itself.
> > 
> > 
> > thanks,
> > 
> > Takashi
> So, I was testing again and with sizeof, I'm getting
> 
> sound/soc/intel/avs/path.c:137:38: error: ‘strncmp’ specified bound 43 exceeds source size 0 [-Werror=stringop-overread]
>   137 |         return id->id == id2->id && !strncmp(id->tplg_name, id2->tplg_name,
>       |                                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   138 |                                              sizeof(id->tplg_name) - 1);
>       |                                              ~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> So my previous v2 is wrong, please ignore that.
> Whereas, with strlen there's no warning :( . I'm quite confused, and
> lack the GCC knowledge.

It smells more like a compiler problem.
Care to report to gcc people before scratching too much here?


thanks,

Takashi

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-09-04  9:14 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-02  8:08 [RFC PATCH] ASoC: Intel: avs: Fix reading 1 or more bytes from a region of size 0 Brahmajit Das
2025-09-02  8:15 ` Takashi Iwai
2025-09-02 10:13   ` [RFC PATCH v2] " Brahmajit Das
2025-09-04  8:40   ` [RFC PATCH] " Brahmajit Das
2025-09-04  9:14     ` Takashi Iwai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox