| Message ID | 20150824101740.GA13757@mo-online.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 24 Aug 2015 12:17:40 +0200, Markus Osterhoff wrote: > > Dear Takashi, > > maybe I am mistaken or have not made my patch clear, please let me > apologise. > > * Takashi Iwai <tiwai@suse.de> [150824 08:30]: > > On Sun, 23 Aug 2015 20:37:05 +0200, > > Markus Osterhoff wrote: > > > With commits bbbc7e85 and 751e2216, a for-loop was restated using > > > list_for_each_entry(...); originally, a local pcm* was extracted from an array > > > and checked against NULL, if not-NULL then passed along to > > > snd_pcm_add_chmap_ctls(...). > > > > > > For a history of the function in question, have a look at lines 2430ff for > > > git diff v4.0 v4.1 sound/pci/hda/hda_codec.c > > > > > > The two commits that touched this function are > > > Commit bbbc7e8502c9 ("ALSA: hda - Allocate hda_pcm objects dynamically") > > > Commit 751e2216899c ("ALSA: hda: fix possible null dereference") > > > > > > where the latter only fixes the order of two fixes, but not the pcm / pcm->pcm confusion. > > > Thanks for the patch. But this was already fixed by commit > > 751e2216899c in 4.1-rc1. > So I re-checked and diff'ed against the current v4.2-rc8 and found that > the if-statement reads, /sound/pci/hda/hda_codec.c, line 3172: > if (!pcm || pcm->own_chmap || > while I propose > if (!pcm->pcm || pcm->own_chmap || Then your previous patch did exactly opposite :) > With the mentioned 751e2216899c I do get a NULL dereference (for > whatever reason), because pcm->pcm can indeed by NULL inside the loop. Please > reconsider the patch: Actually you're trying to shoot two things in a shot, and it's confusing. What we need is to *add* the NULL check of pcm->pcm. It's basically different from replacing pcm NULL check itself. The pcm NULL check is superfluous, indeed, and can be removed. But it can be done in a different patch, or mention properly in the changelog. Could you revisit your patch with the proper description? thanks, Takashi > > > Takashi > Thanks for your time, Markus > > > Signed-off-by: Markus Osterhoff <linux-kernel@k-raum.org> > --- > sound/pci/hda/hda_codec.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c > index 5de3c5d..47206b9 100644 > --- a/sound/pci/hda/hda_codec.c > +++ b/sound/pci/hda/hda_codec.c > @@ -3172,7 +3172,7 @@ static int add_std_chmaps(struct hda_codec *codec) > struct snd_pcm_chmap *chmap; > const struct snd_pcm_chmap_elem *elem; > > - if (!pcm || pcm->own_chmap || > + if (!pcm->pcm || pcm->own_chmap || > !hinfo->substreams) > continue; > elem = hinfo->chmap ? hinfo->chmap : snd_pcm_std_chmaps; >
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index 5de3c5d..47206b9 100644 --- a/sound/pci/hda/hda_codec.c +++ b/sound/pci/hda/hda_codec.c @@ -3172,7 +3172,7 @@ static int add_std_chmaps(struct hda_codec *codec) struct snd_pcm_chmap *chmap; const struct snd_pcm_chmap_elem *elem; - if (!pcm || pcm->own_chmap || + if (!pcm->pcm || pcm->own_chmap || !hinfo->substreams) continue; elem = hinfo->chmap ? hinfo->chmap : snd_pcm_std_chmaps;