* Should snd_card_free() check for null pointer? @ 2016-02-09 14:30 Jerome Marchand 2016-02-09 21:56 ` Takashi Iwai 0 siblings, 1 reply; 6+ messages in thread From: Jerome Marchand @ 2016-02-09 14:30 UTC (permalink / raw) To: Jaroslav Kysela, Takashi Iwai; +Cc: alsa-devel, linux-kernel [-- Attachment #1: Type: text/plain, Size: 559 bytes --] Hi, Before commit f24640648186b (ALSA: Use standard device refcount for card accounting), snd_card_free() would return -EINVAL on a null pointer. Now it ends up in a null pointer dereference. There is at least one driver that can call snd_card_free() with null argument: saa7134_alsa. It can easily be triggered by just inserting and removing the module (no need to have the hardware). I don't think that is a rule, but it seems that the standard behavior of *_free() functions is to check for null pointer. What do you think? Thanks, Jerome [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Should snd_card_free() check for null pointer? 2016-02-09 14:30 Should snd_card_free() check for null pointer? Jerome Marchand @ 2016-02-09 21:56 ` Takashi Iwai 0 siblings, 0 replies; 6+ messages in thread From: Takashi Iwai @ 2016-02-09 21:56 UTC (permalink / raw) To: Jerome Marchand; +Cc: Jaroslav Kysela, alsa-devel, linux-kernel On Tue, 09 Feb 2016 15:30:16 +0100, Jerome Marchand wrote: > > Hi, > > Before commit f24640648186b (ALSA: Use standard device refcount for card > accounting), snd_card_free() would return -EINVAL on a null pointer. Now > it ends up in a null pointer dereference. There is at least one driver > that can call snd_card_free() with null argument: saa7134_alsa. It can > easily be triggered by just inserting and removing the module (no need > to have the hardware). > I don't think that is a rule, but it seems that the standard behavior of > *_free() functions is to check for null pointer. What do you think? Well, I have a mixed feeling about this. Allowing NULL sometimes makes the code easier. OTOH, caling snd_card_free() with NULL is really an unexpected situation, and if a driver does it, most likely it does something weird. So, at this moment, I would fix the caller side. But, it's not a final call, just my gut feeling. thanks, Takashi ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Should snd_card_free() check for null pointer? @ 2016-02-09 21:56 ` Takashi Iwai 0 siblings, 0 replies; 6+ messages in thread From: Takashi Iwai @ 2016-02-09 21:56 UTC (permalink / raw) To: Jerome Marchand; +Cc: Jaroslav Kysela, alsa-devel, linux-kernel On Tue, 09 Feb 2016 15:30:16 +0100, Jerome Marchand wrote: > > Hi, > > Before commit f24640648186b (ALSA: Use standard device refcount for card > accounting), snd_card_free() would return -EINVAL on a null pointer. Now > it ends up in a null pointer dereference. There is at least one driver > that can call snd_card_free() with null argument: saa7134_alsa. It can > easily be triggered by just inserting and removing the module (no need > to have the hardware). > I don't think that is a rule, but it seems that the standard behavior of > *_free() functions is to check for null pointer. What do you think? Well, I have a mixed feeling about this. Allowing NULL sometimes makes the code easier. OTOH, caling snd_card_free() with NULL is really an unexpected situation, and if a driver does it, most likely it does something weird. So, at this moment, I would fix the caller side. But, it's not a final call, just my gut feeling. thanks, Takashi ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Should snd_card_free() check for null pointer? 2016-02-09 21:56 ` Takashi Iwai @ 2016-02-10 7:41 ` Jerome Marchand -1 siblings, 0 replies; 6+ messages in thread From: Jerome Marchand @ 2016-02-10 7:41 UTC (permalink / raw) To: Takashi Iwai; +Cc: alsa-devel, linux-kernel ----- Original Message ----- > From: "Takashi Iwai" <tiwai@suse.de> > To: "Jerome Marchand" <jmarchan@redhat.com> > Cc: "Jaroslav Kysela" <perex@perex.cz>, alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org > Sent: Tuesday, February 9, 2016 10:56:39 PM > Subject: Re: Should snd_card_free() check for null pointer? > > On Tue, 09 Feb 2016 15:30:16 +0100, > Jerome Marchand wrote: > > > > Hi, > > > > Before commit f24640648186b (ALSA: Use standard device refcount for card > > accounting), snd_card_free() would return -EINVAL on a null pointer. Now > > it ends up in a null pointer dereference. There is at least one driver > > that can call snd_card_free() with null argument: saa7134_alsa. It can > > easily be triggered by just inserting and removing the module (no need > > to have the hardware). > > I don't think that is a rule, but it seems that the standard behavior of > > *_free() functions is to check for null pointer. What do you think? > > Well, I have a mixed feeling about this. Allowing NULL sometimes > makes the code easier. OTOH, caling snd_card_free() with NULL is > really an unexpected situation, and if a driver does it, most likely > it does something weird. > > So, at this moment, I would fix the caller side. But, it's not a > final call, just my gut feeling. I have no strong opinion either way and I have a patch that fixes saa7134 driver ready to be sent if that is your preference. Thanks, Jerome > > > thanks, > > Takashi > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Should snd_card_free() check for null pointer? @ 2016-02-10 7:41 ` Jerome Marchand 0 siblings, 0 replies; 6+ messages in thread From: Jerome Marchand @ 2016-02-10 7:41 UTC (permalink / raw) To: Takashi Iwai; +Cc: Jaroslav Kysela, alsa-devel, linux-kernel ----- Original Message ----- > From: "Takashi Iwai" <tiwai@suse.de> > To: "Jerome Marchand" <jmarchan@redhat.com> > Cc: "Jaroslav Kysela" <perex@perex.cz>, alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org > Sent: Tuesday, February 9, 2016 10:56:39 PM > Subject: Re: Should snd_card_free() check for null pointer? > > On Tue, 09 Feb 2016 15:30:16 +0100, > Jerome Marchand wrote: > > > > Hi, > > > > Before commit f24640648186b (ALSA: Use standard device refcount for card > > accounting), snd_card_free() would return -EINVAL on a null pointer. Now > > it ends up in a null pointer dereference. There is at least one driver > > that can call snd_card_free() with null argument: saa7134_alsa. It can > > easily be triggered by just inserting and removing the module (no need > > to have the hardware). > > I don't think that is a rule, but it seems that the standard behavior of > > *_free() functions is to check for null pointer. What do you think? > > Well, I have a mixed feeling about this. Allowing NULL sometimes > makes the code easier. OTOH, caling snd_card_free() with NULL is > really an unexpected situation, and if a driver does it, most likely > it does something weird. > > So, at this moment, I would fix the caller side. But, it's not a > final call, just my gut feeling. I have no strong opinion either way and I have a patch that fixes saa7134 driver ready to be sent if that is your preference. Thanks, Jerome > > > thanks, > > Takashi > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Should snd_card_free() check for null pointer? 2016-02-10 7:41 ` Jerome Marchand (?) @ 2016-02-10 8:45 ` Takashi Iwai -1 siblings, 0 replies; 6+ messages in thread From: Takashi Iwai @ 2016-02-10 8:45 UTC (permalink / raw) To: Jerome Marchand; +Cc: Jaroslav Kysela, alsa-devel, linux-kernel On Wed, 10 Feb 2016 08:41:38 +0100, Jerome Marchand wrote: > > ----- Original Message ----- > > From: "Takashi Iwai" <tiwai@suse.de> > > To: "Jerome Marchand" <jmarchan@redhat.com> > > Cc: "Jaroslav Kysela" <perex@perex.cz>, alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org > > Sent: Tuesday, February 9, 2016 10:56:39 PM > > Subject: Re: Should snd_card_free() check for null pointer? > > > > On Tue, 09 Feb 2016 15:30:16 +0100, > > Jerome Marchand wrote: > > > > > > Hi, > > > > > > Before commit f24640648186b (ALSA: Use standard device refcount for card > > > accounting), snd_card_free() would return -EINVAL on a null pointer. Now > > > it ends up in a null pointer dereference. There is at least one driver > > > that can call snd_card_free() with null argument: saa7134_alsa. It can > > > easily be triggered by just inserting and removing the module (no need > > > to have the hardware). > > > I don't think that is a rule, but it seems that the standard behavior of > > > *_free() functions is to check for null pointer. What do you think? > > > > Well, I have a mixed feeling about this. Allowing NULL sometimes > > makes the code easier. OTOH, caling snd_card_free() with NULL is > > really an unexpected situation, and if a driver does it, most likely > > it does something weird. > > > > So, at this moment, I would fix the caller side. But, it's not a > > final call, just my gut feeling. > > I have no strong opinion either way and I have a patch that fixes saa7134 > driver ready to be sent if that is your preference. Go ahead, let's fix saa7134 side for now. thanks, Takashi ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-02-10 8:45 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-02-09 14:30 Should snd_card_free() check for null pointer? Jerome Marchand 2016-02-09 21:56 ` Takashi Iwai 2016-02-09 21:56 ` Takashi Iwai 2016-02-10 7:41 ` Jerome Marchand 2016-02-10 7:41 ` Jerome Marchand 2016-02-10 8:45 ` Takashi Iwai
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.