| Message ID | 1531926420-25488-1-git-send-email-srikanth.h@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 18 Jul 2018 17:07:00 +0200, Srikanth K H wrote: > > If the timer object is created without the card for entries > "SNDRV_TIMER_CLASS_CARD" and "SNDRV_TIMER_CLASS_PCM", then > while reading the sound info entry in function > "snd_timer_proc_read" the card information is directly > dereferenced without checking for NULL and hence kernel > panic occur. So as preventive measure while the creating > the sound timer object is created the card information > availability is checked for the mentioned entries and > returned error if its NULL. > > Signed-off-by: Srikanth K H <srikanth.h@samsung.com> > --- > sound/core/timer.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/sound/core/timer.c b/sound/core/timer.c > index c7be4f1..06f734f 100644 > --- a/sound/core/timer.c > +++ b/sound/core/timer.c > @@ -883,6 +883,11 @@ int snd_timer_new(struct snd_card *card, char *id, struct snd_timer_id *tid, > > if (snd_BUG_ON(!tid)) > return -EINVAL; > + if (tid->dev_class == SNDRV_TIMER_CLASS_CARD || > + tid->dev_class == SNDRV_TIMER_CLASS_PCM) { > + if (WARN_ON(!card)) > + return -EINVAL; > + } > if (rtimer) > *rtimer = NULL; > timer = kzalloc(sizeof(*timer), GFP_KERNEL); > @@ -1192,12 +1197,10 @@ static void snd_timer_proc_read(struct snd_info_entry *entry, > break; > case SNDRV_TIMER_CLASS_CARD: > snd_iprintf(buffer, "C%i-%i: ", > - timer->card ? timer->card->number : -1, > - timer->tmr_device); > + timer->card->number, timer->tmr_device); > break; > case SNDRV_TIMER_CLASS_PCM: > - snd_iprintf(buffer, "P%i-%i-%i: ", > - timer->card ? timer->card->number : -1, > + snd_iprintf(buffer, "P%i-%i-%i: ", timer->card->number, > timer->tmr_device, timer->tmr_subdevice); > break; The checks in proc are moot if we guarantee the non-NULL card at snd_timer_new() in the above. So it's not about fixing in sound module. It serves right. Your patch would add a sanity check to catch a buggy code in the caller side. thanks, Takashi
> The checks in proc are moot if we guarantee the non-NULL card at > snd_timer_new() in the above. > So it's not about fixing in sound module. It serves right. Your > patch would add a sanity check to catch a buggy code in the caller > side. You are right Takashi, as you said this changes will catch buggy code in caller side. Thank you for your respose. Thanks, Srikanth
diff --git a/sound/core/timer.c b/sound/core/timer.c index c7be4f1..06f734f 100644 --- a/sound/core/timer.c +++ b/sound/core/timer.c @@ -883,6 +883,11 @@ int snd_timer_new(struct snd_card *card, char *id, struct snd_timer_id *tid, if (snd_BUG_ON(!tid)) return -EINVAL; + if (tid->dev_class == SNDRV_TIMER_CLASS_CARD || + tid->dev_class == SNDRV_TIMER_CLASS_PCM) { + if (WARN_ON(!card)) + return -EINVAL; + } if (rtimer) *rtimer = NULL; timer = kzalloc(sizeof(*timer), GFP_KERNEL); @@ -1192,12 +1197,10 @@ static void snd_timer_proc_read(struct snd_info_entry *entry, break; case SNDRV_TIMER_CLASS_CARD: snd_iprintf(buffer, "C%i-%i: ", - timer->card ? timer->card->number : -1, - timer->tmr_device); + timer->card->number, timer->tmr_device); break; case SNDRV_TIMER_CLASS_PCM: - snd_iprintf(buffer, "P%i-%i-%i: ", - timer->card ? timer->card->number : -1, + snd_iprintf(buffer, "P%i-%i-%i: ", timer->card->number, timer->tmr_device, timer->tmr_subdevice); break; default:
If the timer object is created without the card for entries "SNDRV_TIMER_CLASS_CARD" and "SNDRV_TIMER_CLASS_PCM", then while reading the sound info entry in function "snd_timer_proc_read" the card information is directly dereferenced without checking for NULL and hence kernel panic occur. So as preventive measure while the creating the sound timer object is created the card information availability is checked for the mentioned entries and returned error if its NULL. Signed-off-by: Srikanth K H <srikanth.h@samsung.com> --- sound/core/timer.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)