* [PATCH] greybus: audio_codec: fix three missing initializers for data
@ 2022-03-27 6:01 Xiaomeng Tong
2022-03-27 19:24 ` Mark Greer
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Xiaomeng Tong @ 2022-03-27 6:01 UTC (permalink / raw)
To: vaibhav.sr
Cc: mgreer, johan, elder, gregkh, greybus-dev, linux-staging,
linux-kernel, Xiaomeng Tong, stable
These three bugs are here:
struct gbaudio_data_connection *data;
If the list '&codec->module_list' is empty then the 'data' will
keep unchanged. However, the 'data' is not initialized and filled
with trash value. As a result, if the value is not NULL, the check
'if (!data) {' will always be false and never exit expectly.
To fix these bug, just initialize 'data' with NULL.
Cc: stable@vger.kernel.org
Fixes: 6dd67645f22cf ("greybus: audio: Use single codec driver registration")
Signed-off-by: Xiaomeng Tong <xiam0nd.tong@gmail.com>
---
drivers/staging/greybus/audio_codec.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/greybus/audio_codec.c b/drivers/staging/greybus/audio_codec.c
index b589cf6b1d03..939e05af4dcf 100644
--- a/drivers/staging/greybus/audio_codec.c
+++ b/drivers/staging/greybus/audio_codec.c
@@ -397,7 +397,7 @@ static int gbcodec_hw_params(struct snd_pcm_substream *substream,
u8 sig_bits, channels;
u32 format, rate;
struct gbaudio_module_info *module;
- struct gbaudio_data_connection *data;
+ struct gbaudio_data_connection *data = NULL;
struct gb_bundle *bundle;
struct gbaudio_codec_info *codec = dev_get_drvdata(dai->dev);
struct gbaudio_stream_params *params;
@@ -498,7 +498,7 @@ static int gbcodec_prepare(struct snd_pcm_substream *substream,
{
int ret;
struct gbaudio_module_info *module;
- struct gbaudio_data_connection *data;
+ struct gbaudio_data_connection *data = NULL;
struct gb_bundle *bundle;
struct gbaudio_codec_info *codec = dev_get_drvdata(dai->dev);
struct gbaudio_stream_params *params;
@@ -562,7 +562,7 @@ static int gbcodec_prepare(struct snd_pcm_substream *substream,
static int gbcodec_mute_stream(struct snd_soc_dai *dai, int mute, int stream)
{
int ret;
- struct gbaudio_data_connection *data;
+ struct gbaudio_data_connection *data = NULL;
struct gbaudio_module_info *module;
struct gb_bundle *bundle;
struct gbaudio_codec_info *codec = dev_get_drvdata(dai->dev);
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] greybus: audio_codec: fix three missing initializers for data
2022-03-27 6:01 [PATCH] greybus: audio_codec: fix three missing initializers for data Xiaomeng Tong
@ 2022-03-27 19:24 ` Mark Greer
2022-03-28 8:14 ` Vaibhav Agarwal
2022-03-28 14:19 ` Dan Carpenter
2 siblings, 0 replies; 6+ messages in thread
From: Mark Greer @ 2022-03-27 19:24 UTC (permalink / raw)
To: Xiaomeng Tong
Cc: vaibhav.sr, mgreer, johan, elder, gregkh, greybus-dev,
linux-staging, linux-kernel, stable
On Sun, Mar 27, 2022 at 02:01:20PM +0800, Xiaomeng Tong wrote:
> These three bugs are here:
> struct gbaudio_data_connection *data;
>
> If the list '&codec->module_list' is empty then the 'data' will
> keep unchanged. However, the 'data' is not initialized and filled
> with trash value. As a result, if the value is not NULL, the check
> 'if (!data) {' will always be false and never exit expectly.
>
> To fix these bug, just initialize 'data' with NULL.
>
> Cc: stable@vger.kernel.org
> Fixes: 6dd67645f22cf ("greybus: audio: Use single codec driver registration")
> Signed-off-by: Xiaomeng Tong <xiam0nd.tong@gmail.com>
> ---
> drivers/staging/greybus/audio_codec.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/greybus/audio_codec.c b/drivers/staging/greybus/audio_codec.c
> index b589cf6b1d03..939e05af4dcf 100644
> --- a/drivers/staging/greybus/audio_codec.c
> +++ b/drivers/staging/greybus/audio_codec.c
> @@ -397,7 +397,7 @@ static int gbcodec_hw_params(struct snd_pcm_substream *substream,
> u8 sig_bits, channels;
> u32 format, rate;
> struct gbaudio_module_info *module;
> - struct gbaudio_data_connection *data;
> + struct gbaudio_data_connection *data = NULL;
> struct gb_bundle *bundle;
> struct gbaudio_codec_info *codec = dev_get_drvdata(dai->dev);
> struct gbaudio_stream_params *params;
> @@ -498,7 +498,7 @@ static int gbcodec_prepare(struct snd_pcm_substream *substream,
> {
> int ret;
> struct gbaudio_module_info *module;
> - struct gbaudio_data_connection *data;
> + struct gbaudio_data_connection *data = NULL;
> struct gb_bundle *bundle;
> struct gbaudio_codec_info *codec = dev_get_drvdata(dai->dev);
> struct gbaudio_stream_params *params;
> @@ -562,7 +562,7 @@ static int gbcodec_prepare(struct snd_pcm_substream *substream,
> static int gbcodec_mute_stream(struct snd_soc_dai *dai, int mute, int stream)
> {
> int ret;
> - struct gbaudio_data_connection *data;
> + struct gbaudio_data_connection *data = NULL;
> struct gbaudio_module_info *module;
> struct gb_bundle *bundle;
> struct gbaudio_codec_info *codec = dev_get_drvdata(dai->dev);
> --
> 2.17.1
Those changes appear to fix real bugs. Thanks Xiaomeng.
Reviewed-by: Mark Greer <mgreer@animalcreek.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] greybus: audio_codec: fix three missing initializers for data
2022-03-27 6:01 [PATCH] greybus: audio_codec: fix three missing initializers for data Xiaomeng Tong
2022-03-27 19:24 ` Mark Greer
@ 2022-03-28 8:14 ` Vaibhav Agarwal
2022-03-28 14:19 ` Dan Carpenter
2 siblings, 0 replies; 6+ messages in thread
From: Vaibhav Agarwal @ 2022-03-28 8:14 UTC (permalink / raw)
To: Xiaomeng Tong
Cc: Mark Greer, Johan Hovold, Alex Elder, Greg Kroah-Hartman,
moderated list:GREYBUS SUBSYSTEM, linux-staging, open list,
stable
On Sun, Mar 27, 2022 at 11:31 AM Xiaomeng Tong <xiam0nd.tong@gmail.com> wrote:
>
> These three bugs are here:
> struct gbaudio_data_connection *data;
>
> If the list '&codec->module_list' is empty then the 'data' will
> keep unchanged. However, the 'data' is not initialized and filled
> with trash value. As a result, if the value is not NULL, the check
> 'if (!data) {' will always be false and never exit expectly.
>
> To fix these bug, just initialize 'data' with NULL.
>
> Cc: stable@vger.kernel.org
> Fixes: 6dd67645f22cf ("greybus: audio: Use single codec driver registration")
> Signed-off-by: Xiaomeng Tong <xiam0nd.tong@gmail.com>
> ---
> drivers/staging/greybus/audio_codec.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/greybus/audio_codec.c b/drivers/staging/greybus/audio_codec.c
> index b589cf6b1d03..939e05af4dcf 100644
> --- a/drivers/staging/greybus/audio_codec.c
> +++ b/drivers/staging/greybus/audio_codec.c
> @@ -397,7 +397,7 @@ static int gbcodec_hw_params(struct snd_pcm_substream *substream,
> u8 sig_bits, channels;
> u32 format, rate;
> struct gbaudio_module_info *module;
> - struct gbaudio_data_connection *data;
> + struct gbaudio_data_connection *data = NULL;
> struct gb_bundle *bundle;
> struct gbaudio_codec_info *codec = dev_get_drvdata(dai->dev);
> struct gbaudio_stream_params *params;
> @@ -498,7 +498,7 @@ static int gbcodec_prepare(struct snd_pcm_substream *substream,
> {
> int ret;
> struct gbaudio_module_info *module;
> - struct gbaudio_data_connection *data;
> + struct gbaudio_data_connection *data = NULL;
> struct gb_bundle *bundle;
> struct gbaudio_codec_info *codec = dev_get_drvdata(dai->dev);
> struct gbaudio_stream_params *params;
> @@ -562,7 +562,7 @@ static int gbcodec_prepare(struct snd_pcm_substream *substream,
> static int gbcodec_mute_stream(struct snd_soc_dai *dai, int mute, int stream)
> {
> int ret;
> - struct gbaudio_data_connection *data;
> + struct gbaudio_data_connection *data = NULL;
> struct gbaudio_module_info *module;
> struct gb_bundle *bundle;
> struct gbaudio_codec_info *codec = dev_get_drvdata(dai->dev);
> --
> 2.17.1
>
Thanks Xiaomeng for sharing the fix.
Reviewed by: Vaibhav Agarwal <vaibhav.sr@gmail.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] greybus: audio_codec: fix three missing initializers for data
2022-03-27 6:01 [PATCH] greybus: audio_codec: fix three missing initializers for data Xiaomeng Tong
2022-03-27 19:24 ` Mark Greer
2022-03-28 8:14 ` Vaibhav Agarwal
@ 2022-03-28 14:19 ` Dan Carpenter
2022-03-28 16:08 ` Mark Greer
2022-03-29 2:03 ` Xiaomeng Tong
2 siblings, 2 replies; 6+ messages in thread
From: Dan Carpenter @ 2022-03-28 14:19 UTC (permalink / raw)
To: Xiaomeng Tong
Cc: vaibhav.sr, mgreer, johan, elder, gregkh, greybus-dev,
linux-staging, linux-kernel, stable
On Sun, Mar 27, 2022 at 02:01:20PM +0800, Xiaomeng Tong wrote:
> These three bugs are here:
> struct gbaudio_data_connection *data;
>
> If the list '&codec->module_list' is empty then the 'data' will
> keep unchanged.
All three of these functions check for if the codec->module_list is
empty at the start of the function so these are not real bugs.
Smatch is supposed to be able to figure this out, but apparently that
code is broken so Smatch still prints a warning. :(
Apparently GCC does not print a warning for this. Even when I delete
the check for list_empty() then GCC does not print a warning. GCC often
assumes that we enter loops one time. I haven't looked at that, but I
have noticed it in reviewing Smatch vs GCC warnings.
Generally we do not apply static checker work arounds.
I do not have a problem with this particular work around, but it needs
an updated commit message which says it is just to silence static
checker warnings and not to fix bugs. Remove the Fixes tag. Don't CC
stable.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] greybus: audio_codec: fix three missing initializers for data
2022-03-28 14:19 ` Dan Carpenter
@ 2022-03-28 16:08 ` Mark Greer
2022-03-29 2:03 ` Xiaomeng Tong
1 sibling, 0 replies; 6+ messages in thread
From: Mark Greer @ 2022-03-28 16:08 UTC (permalink / raw)
To: Dan Carpenter
Cc: Xiaomeng Tong, vaibhav.sr, mgreer, johan, elder, gregkh,
greybus-dev, linux-staging, linux-kernel, stable
On Mon, Mar 28, 2022 at 05:19:45PM +0300, Dan Carpenter wrote:
> On Sun, Mar 27, 2022 at 02:01:20PM +0800, Xiaomeng Tong wrote:
> > These three bugs are here:
> > struct gbaudio_data_connection *data;
> >
> > If the list '&codec->module_list' is empty then the 'data' will
> > keep unchanged.
>
> All three of these functions check for if the codec->module_list is
> empty at the start of the function so these are not real bugs.
Umm, yep, oops. Thanks Dan.
Mark
--
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] greybus: audio_codec: fix three missing initializers for data
2022-03-28 14:19 ` Dan Carpenter
2022-03-28 16:08 ` Mark Greer
@ 2022-03-29 2:03 ` Xiaomeng Tong
1 sibling, 0 replies; 6+ messages in thread
From: Xiaomeng Tong @ 2022-03-29 2:03 UTC (permalink / raw)
To: dan.carpenter
Cc: elder, gregkh, greybus-dev, johan, linux-kernel, linux-staging,
mgreer, stable, vaibhav.sr, xiam0nd.tong
On Mon, 28 Mar 2022 17:19:45 +0300, Dan Carpenter wrote:
> On Sun, Mar 27, 2022 at 02:01:20PM +0800, Xiaomeng Tong wrote:
> > These three bugs are here:
> > struct gbaudio_data_connection *data;
> >
> > If the list '&codec->module_list' is empty then the 'data' will
> > keep unchanged.
>
> All three of these functions check for if the codec->module_list is
> empty at the start of the function so these are not real bugs.
>
> Smatch is supposed to be able to figure this out, but apparently that
> code is broken so Smatch still prints a warning. :(
>
> Apparently GCC does not print a warning for this. Even when I delete
> the check for list_empty() then GCC does not print a warning. GCC often
> assumes that we enter loops one time. I haven't looked at that, but I
> have noticed it in reviewing Smatch vs GCC warnings.
>
> Generally we do not apply static checker work arounds.
>
> I do not have a problem with this particular work around, but it needs
> an updated commit message which says it is just to silence static
> checker warnings and not to fix bugs. Remove the Fixes tag. Don't CC
> stable.
Yes, you are right. I have resend a PATCH with updated commit message as
you suggested, and cc you. Thank you.
--
Xiaomeng Tong
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-03-29 2:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-27 6:01 [PATCH] greybus: audio_codec: fix three missing initializers for data Xiaomeng Tong
2022-03-27 19:24 ` Mark Greer
2022-03-28 8:14 ` Vaibhav Agarwal
2022-03-28 14:19 ` Dan Carpenter
2022-03-28 16:08 ` Mark Greer
2022-03-29 2:03 ` Xiaomeng Tong
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.