| Message ID | 20221114082048.3477027-1-d-tatianin@yandex-team.ru |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] sound/pci/hda/patch_realtek: don't call alc_shutup_pins without a spec | expand |
On Mon, 14 Nov 2022 09:20:48 +0100, Daniil Tatianin wrote: > > alc_shutup_pins always expects the spec to be present, so make sure > it is before we call it. > > Found by Linux Verification Center (linuxtesting.org) with the SVACE > static analysis tool. > > Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru> In which path can it be without spec assigned? That's the internal callback that is set only by the codec driver where the allocation of codec->spec is mandatory. thanks, Takashi > --- > sound/pci/hda/patch_realtek.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c > index 60e3bc124836..2cf4b64971d7 100644 > --- a/sound/pci/hda/patch_realtek.c > +++ b/sound/pci/hda/patch_realtek.c > @@ -939,10 +939,12 @@ static inline void alc_shutup(struct hda_codec *codec) > { > struct alc_spec *spec = codec->spec; > > + if (!spec) > + return; > if (!snd_hda_get_bool_hint(codec, "shutup")) > return; /* disabled explicitly by hints */ > > - if (spec && spec->shutup) > + if (spec->shutup) > spec->shutup(codec); > else > alc_shutup_pins(codec); > -- > 2.25.1 >
On 11/14/22 11:26 AM, Takashi Iwai wrote: > On Mon, 14 Nov 2022 09:20:48 +0100, > Daniil Tatianin wrote: >> >> alc_shutup_pins always expects the spec to be present, so make sure >> it is before we call it. >> >> Found by Linux Verification Center (linuxtesting.org) with the SVACE >> static analysis tool. >> >> Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru> > > In which path can it be without spec assigned? > That's the internal callback that is set only by the codec driver > where the allocation of codec->spec is mandatory. Would you then say that the "if (spec && ...)" that was there before was redundant? Should we remove it perhaps to avoid further confusion about it being optional? The else branch with alc_shutup_pins would crash if it was null anyway. Thanks > > thanks, > > Takashi > > >> --- >> sound/pci/hda/patch_realtek.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c >> index 60e3bc124836..2cf4b64971d7 100644 >> --- a/sound/pci/hda/patch_realtek.c >> +++ b/sound/pci/hda/patch_realtek.c >> @@ -939,10 +939,12 @@ static inline void alc_shutup(struct hda_codec *codec) >> { >> struct alc_spec *spec = codec->spec; >> >> + if (!spec) >> + return; >> if (!snd_hda_get_bool_hint(codec, "shutup")) >> return; /* disabled explicitly by hints */ >> >> - if (spec && spec->shutup) >> + if (spec->shutup) >> spec->shutup(codec); >> else >> alc_shutup_pins(codec); >> -- >> 2.25.1 >>
On Mon, 14 Nov 2022 09:35:10 +0100, Daniil Tatianin wrote: > > > > On 11/14/22 11:26 AM, Takashi Iwai wrote: > > On Mon, 14 Nov 2022 09:20:48 +0100, > > Daniil Tatianin wrote: > >> > >> alc_shutup_pins always expects the spec to be present, so make sure > >> it is before we call it. > >> > >> Found by Linux Verification Center (linuxtesting.org) with the SVACE > >> static analysis tool. > >> > >> Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru> > > > > In which path can it be without spec assigned? > > That's the internal callback that is set only by the codec driver > > where the allocation of codec->spec is mandatory. > > Would you then say that the "if (spec && ...)" that was there before > was redundant? Yes. Takashi
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index 60e3bc124836..2cf4b64971d7 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -939,10 +939,12 @@ static inline void alc_shutup(struct hda_codec *codec) { struct alc_spec *spec = codec->spec; + if (!spec) + return; if (!snd_hda_get_bool_hint(codec, "shutup")) return; /* disabled explicitly by hints */ - if (spec && spec->shutup) + if (spec->shutup) spec->shutup(codec); else alc_shutup_pins(codec);
alc_shutup_pins always expects the spec to be present, so make sure it is before we call it. Found by Linux Verification Center (linuxtesting.org) with the SVACE static analysis tool. Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru> --- sound/pci/hda/patch_realtek.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)