On Wed, 18 Jul 2018 12:58:20 +0200, Srikanth Korangala Hari wrote: > > > >> > >> Signed-off-by: Srikanth K H <srikant...@samsung.com> > > >What does this fix, and above all, why is this needed? > > Hi, > > When the sound driver creates the timer without sound card object, then while > reading the sound info entry the timer object’s card information is > dereferenced without checking for NULL pointer which will result for kernel > panic. I tried to simulate this scenario and got below call stack, > [ 36.668] E/DEVKMSG (P 0, T 0): Unable to handle kernel NULL pointer > dereference at virtual address 00000000 > [ 36.668] E/DEVKMSG (P 0, T 0): pgd = e52f0000 > [ 36.668] E/DEVKMSG (P 0, T 0): [00000000] *pgd=00000000 > [ 36.668] E/DEVKMSG (P 0, T 0): Internal error: Oops: 5 [#1] PREEMPT > SMP ARM > [ 36.668] E/DEVKMSG (P 0, T 0): Modules linked in: > [ 36.668] E/DEVKMSG (P 0, T 0): CPU: 1 PID: 1258 Comm: cat Tainted: G > W 3.10.65-00121-g83e9b9b-dirty #54-Tizen > [ 36.668] E/DEVKMSG (P 0, T 0): task: e653aec0 ti: e52ec000 task.ti: > e52ec000 > [ 36.668] E/DEVKMSG (P 0, T 0): PC is at > snd_timer_proc_read+0x104/0x278 > [ 36.668] E/DEVKMSG (P 0, T 0): LR is at > snd_timer_proc_read+0xec/0x278 > [ 36.668] E/DEVKMSG (P 0, T 0): pc : [<c0527cfc>] lr : > [<c0527ce4>] psr: 60040013\x0asp : e52eded0 ip : 00000000 fp : 10624dd3 > [ 36.668] E/DEVKMSG (P 0, T 0): r10: c08ded6c r9 : e49e3bd8 r8 : > c074f518 > [ 36.668] E/DEVKMSG (P 0, T 0): r7 : c0afbae4 r6 : eb95a000 r5 : > e49e3240 r4 : eb257e00 > [ 36.668] E/DEVKMSG (P 0, T 0): r3 : 00000000 r2 : 00000000 r1 : > c0987cd7 r0 : e49e3240 > [ 36.668] E/DEVKMSG (P 0, T 0): Flags: nZCv IRQs on FIQs on Mode > SVC_32 ISA ARM Segment user > [ 36.668] E/DEVKMSG (P 0, T 0): Control: 10c53c7d Table: a52f006a > DAC: 00000015 > > Hence this is a preventive patch to avoid kernel panic in case if the card > object passed to timer function is NULL. This would not happen in normal > case, but in case of buggy scenario this would results in kernel panic rather > than graceful exit.
The timer->card must be associated with both entries of SNDRV_TIMER_CLASS_CARD and SNDRV_TIMER_CLASS_PCM. IOW, if a timer object is created without the card but for these classes, it's already a bug. Papering over it doesn't give any benefits. At most it should be with WARN_ON(), but I guess here is a wrong place to add the check. The check should be done at the object creation time instead. thanks, Takashi
