| Message ID | 20240426122511.547755-1-quic_kuruva@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] ALSA: usb-audio: endpoint: Prevent NULL pointer deference in snd_usb_endpoint_close | expand |
On Fri, 26 Apr 2024 14:25:11 +0200, Rajashekar kuruva wrote: > > When multiple plug-in and plug-out events occur, > there is a risk of encountering a NULL pointer dereference > leading to a kernel panic during a headset use-case. > this issue arises in the snd_usb_endpoint_close function Such a scenario can't happen: ep->iface_ref is changed only in chip->mutex lock, hence it can't be NULL there. thanks, Takashi
On 4/26/2024 6:13 PM, Takashi Iwai wrote: > On Fri, 26 Apr 2024 14:25:11 +0200, > Rajashekar kuruva wrote: >> When multiple plug-in and plug-out events occur, >> there is a risk of encountering a NULL pointer dereference >> leading to a kernel panic during a headset use-case. >> this issue arises in the snd_usb_endpoint_close function > Such a scenario can't happen: ep->iface_ref is changed only in > chip->mutex lock, hence it can't be NULL there. > > > thanks, > > Takashi Hi Takashi, Actually we are facing NULL pointer deference while running headset case when i checked call trace the last running function is snd_usb_endpoint_close where iface_ref and clock_ref both are 0x0 [75703.933104][T10585] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000004 [75703.933113][T10585] [RB/E]rb_sreason_str_set: sreason_str set NULL pointer dereference [75703.933116][T10585] Mem abort info: [75703.933117][T10585] ESR = 0x0000000096000005 [75703.933119][T10585] EC = 0x25: DABT (current EL), IL = 32 bits [75703.933120][T10585] SET = 0, FnV = 0 [75703.933121][T10585] EA = 0, S1PTW = 0 [75703.933123][T10585] FSC = 0x05: level 1 translation fault [75703.933124][T10585] Data abort info: [75703.933124][T10585] ISV = 0, ISS = 0x00000005 [75703.933125][T10585] CM = 0, WnR = 0 … [75703.933676][T10585] CPU: 3 PID: 10585 Comm: kworker/u17:0 Tainted: G S W OE 6.1.43-android14-11-ga2fa77d36d26-ab11204829 #1 [75703.933697][T10585] pstate: 62400005 (nZCv daif +PAN -UAO +TCO -DIT -SSBS BTYPE=--) [75703.933700][T10585] pc : snd_usb_endpoint_close+0x30/0x104 [75703.933721][T10585] lr : snd_usb_endpoint_close+0x28/0x104 [75703.933724][T10585] sp : ffffffc04b2bb740 [75703.933725][T10585] x29: ffffffc04b2bb740 x28: ffffff8024e3ba78 x27: ffffffd266e91da0 [75703.933728][T10585] x26: ffffffc04b2bb7a8 x25: ffffff89bec5be00 x24: 00000000ffffffea [75703.933730][T10585] x23: 0000000000000002 x22: ffffff885d568008 x21: ffffff8024e3ba78 [75703.933732][T10585] x20: ffffff885d568000 x19: ffffff8024e3bb18 x18: ffffffd26db2d140 [75703.933734][T10585] x17: 00000000f01b0818 x16: 00000000f01b0818 x15: 0000000000000008 [75703.933736][T10585] x14: ffffff8a3e2b5780 x13: ffffff8a3e2b5780 x12: ffffffd26cbd2770 [75703.933738][T10585] x11: 0000000000000001 x10: ffffff8984320000 x9 : 4f43b86e946b4e00 [75703.933740][T10585] x8 : 0000000000000000 x7 : 0000000000000001 x6 : fffffffdef8e8b70 [75703.933742][T10585] x5 : 0000000000000001 x4 : 0000000000000000 x3 : ffffff8024e3bb28 [75703.933743][T10585] x2 : 00000001011fa7c9 x1 : ffffffc04b2bb680 x0 : 0000000000000000 [75703.933746][T10585] Call trace: [75703.933747][T10585] snd_usb_endpoint_close+0x30/0x104 That's why I have added the NULL pointer check Thanks, Rajashekar K.
Hi Takashi, I'm checking on who is calling snd_usb_endpoint_close, without opened endpoint is created. As its rarely reproducible, its taking time. I'll replay you back. Thanks, Rajashekar K. On 4/29/2024 12:27 PM, Takashi Iwai wrote: > On Mon, 29 Apr 2024 08:23:27 +0200, > Rajashekar Kuruva (Temp) wrote: >> >> On 4/26/2024 6:13 PM, Takashi Iwai wrote: >>> On Fri, 26 Apr 2024 14:25:11 +0200, >>> Rajashekar kuruva wrote: >>>> When multiple plug-in and plug-out events occur, >>>> there is a risk of encountering a NULL pointer dereference >>>> leading to a kernel panic during a headset use-case. >>>> this issue arises in the snd_usb_endpoint_close function >>> Such a scenario can't happen: ep->iface_ref is changed only in >>> chip->mutex lock, hence it can't be NULL there. >>> >>> >>> thanks, >>> >>> Takashi >> Hi Takashi, >> >> Actually we are facing NULL pointer deference while running headset >> case when i checked call trace the last running function is >> snd_usb_endpoint_close where iface_ref and clock_ref both are 0x0 >> >> [75703.933104][T10585] Unable to handle kernel NULL pointer >> dereference at virtual address 0000000000000004 >> [75703.933113][T10585] [RB/E]rb_sreason_str_set: sreason_str set NULL >> pointer dereference >> [75703.933116][T10585] Mem abort info: >> [75703.933117][T10585] ESR = 0x0000000096000005 >> [75703.933119][T10585] EC = 0x25: DABT (current EL), IL = 32 bits >> [75703.933120][T10585] SET = 0, FnV = 0 >> [75703.933121][T10585] EA = 0, S1PTW = 0 >> [75703.933123][T10585] FSC = 0x05: level 1 translation fault >> [75703.933124][T10585] Data abort info: >> [75703.933124][T10585] ISV = 0, ISS = 0x00000005 >> [75703.933125][T10585] CM = 0, WnR = 0 >> … >> [75703.933676][T10585] CPU: 3 PID: 10585 Comm: kworker/u17:0 Tainted: >> G S W OE 6.1.43-android14-11-ga2fa77d36d26-ab11204829 #1 >> [75703.933697][T10585] pstate: 62400005 (nZCv daif +PAN -UAO +TCO -DIT >> -SSBS BTYPE=--) >> [75703.933700][T10585] pc : snd_usb_endpoint_close+0x30/0x104 >> [75703.933721][T10585] lr : snd_usb_endpoint_close+0x28/0x104 >> [75703.933724][T10585] sp : ffffffc04b2bb740 >> [75703.933725][T10585] x29: ffffffc04b2bb740 x28: ffffff8024e3ba78 >> x27: ffffffd266e91da0 >> [75703.933728][T10585] x26: ffffffc04b2bb7a8 x25: ffffff89bec5be00 >> x24: 00000000ffffffea >> [75703.933730][T10585] x23: 0000000000000002 x22: ffffff885d568008 >> x21: ffffff8024e3ba78 >> [75703.933732][T10585] x20: ffffff885d568000 x19: ffffff8024e3bb18 >> x18: ffffffd26db2d140 >> [75703.933734][T10585] x17: 00000000f01b0818 x16: 00000000f01b0818 >> x15: 0000000000000008 >> [75703.933736][T10585] x14: ffffff8a3e2b5780 x13: ffffff8a3e2b5780 >> x12: ffffffd26cbd2770 >> [75703.933738][T10585] x11: 0000000000000001 x10: ffffff8984320000 x9 >> : 4f43b86e946b4e00 >> [75703.933740][T10585] x8 : 0000000000000000 x7 : 0000000000000001 x6 >> : fffffffdef8e8b70 >> [75703.933742][T10585] x5 : 0000000000000001 x4 : 0000000000000000 x3 >> : ffffff8024e3bb28 >> [75703.933743][T10585] x2 : 00000001011fa7c9 x1 : ffffffc04b2bb680 x0 >> : 0000000000000000 >> [75703.933746][T10585] Call trace: >> [75703.933747][T10585] snd_usb_endpoint_close+0x30/0x104 > Who is actually calling snd_usb_endpoint_close()? > I guess that's rather a bug in the call pattern, not the code in > USB-audio driver itself. > > snd_usb_endpoint_close() is supposed to be called only for a really > opened endpoint. So, if any, it's rather a race (or a bug) in the > caller side, and it should be addressed there instead. > > > thanks, > > Takashi
On 4/29/2024 12:27 PM, Takashi Iwai wrote: > On Mon, 29 Apr 2024 08:23:27 +0200, > Rajashekar Kuruva (Temp) wrote: >> >> On 4/26/2024 6:13 PM, Takashi Iwai wrote: >>> On Fri, 26 Apr 2024 14:25:11 +0200, >>> Rajashekar kuruva wrote: >>>> When multiple plug-in and plug-out events occur, >>>> there is a risk of encountering a NULL pointer dereference >>>> leading to a kernel panic during a headset use-case. >>>> this issue arises in the snd_usb_endpoint_close function >>> Such a scenario can't happen: ep->iface_ref is changed only in >>> chip->mutex lock, hence it can't be NULL there. >>> >>> >>> thanks, >>> >>> Takashi >> Hi Takashi, >> >> Actually we are facing NULL pointer deference while running headset >> case when i checked call trace the last running function is >> snd_usb_endpoint_close where iface_ref and clock_ref both are 0x0 >> >> [75703.933104][T10585] Unable to handle kernel NULL pointer >> dereference at virtual address 0000000000000004 >> [75703.933113][T10585] [RB/E]rb_sreason_str_set: sreason_str set NULL >> pointer dereference >> [75703.933116][T10585] Mem abort info: >> [75703.933117][T10585] ESR = 0x0000000096000005 >> [75703.933119][T10585] EC = 0x25: DABT (current EL), IL = 32 bits >> [75703.933120][T10585] SET = 0, FnV = 0 >> [75703.933121][T10585] EA = 0, S1PTW = 0 >> [75703.933123][T10585] FSC = 0x05: level 1 translation fault >> [75703.933124][T10585] Data abort info: >> [75703.933124][T10585] ISV = 0, ISS = 0x00000005 >> [75703.933125][T10585] CM = 0, WnR = 0 >> … >> [75703.933676][T10585] CPU: 3 PID: 10585 Comm: kworker/u17:0 Tainted: >> G S W OE 6.1.43-android14-11-ga2fa77d36d26-ab11204829 #1 >> [75703.933697][T10585] pstate: 62400005 (nZCv daif +PAN -UAO +TCO -DIT >> -SSBS BTYPE=--) >> [75703.933700][T10585] pc : snd_usb_endpoint_close+0x30/0x104 >> [75703.933721][T10585] lr : snd_usb_endpoint_close+0x28/0x104 >> [75703.933724][T10585] sp : ffffffc04b2bb740 >> [75703.933725][T10585] x29: ffffffc04b2bb740 x28: ffffff8024e3ba78 >> x27: ffffffd266e91da0 >> [75703.933728][T10585] x26: ffffffc04b2bb7a8 x25: ffffff89bec5be00 >> x24: 00000000ffffffea >> [75703.933730][T10585] x23: 0000000000000002 x22: ffffff885d568008 >> x21: ffffff8024e3ba78 >> [75703.933732][T10585] x20: ffffff885d568000 x19: ffffff8024e3bb18 >> x18: ffffffd26db2d140 >> [75703.933734][T10585] x17: 00000000f01b0818 x16: 00000000f01b0818 >> x15: 0000000000000008 >> [75703.933736][T10585] x14: ffffff8a3e2b5780 x13: ffffff8a3e2b5780 >> x12: ffffffd26cbd2770 >> [75703.933738][T10585] x11: 0000000000000001 x10: ffffff8984320000 x9 >> : 4f43b86e946b4e00 >> [75703.933740][T10585] x8 : 0000000000000000 x7 : 0000000000000001 x6 >> : fffffffdef8e8b70 >> [75703.933742][T10585] x5 : 0000000000000001 x4 : 0000000000000000 x3 >> : ffffff8024e3bb28 >> [75703.933743][T10585] x2 : 00000001011fa7c9 x1 : ffffffc04b2bb680 x0 >> : 0000000000000000 >> [75703.933746][T10585] Call trace: >> [75703.933747][T10585] snd_usb_endpoint_close+0x30/0x104 > Who is actually calling snd_usb_endpoint_close()? > I guess that's rather a bug in the call pattern, not the code in > USB-audio driver itself. > > snd_usb_endpoint_close() is supposed to be called only for a really > opened endpoint. So, if any, it's rather a race (or a bug) in the > caller side, and it should be addressed there instead. > > > thanks, > > Takashi Hi Takashi, I'm checking on who is calling snd_usb_endpoint_close, without opened endpoint is created. As its rarely reproducible, its taking time. I'll replay you back. Thanks, Rajashekar K.
> On Mon, 29 Apr 2024 08:23:27 +0200, > Rajashekar Kuruva (Temp) wrote: >> >> On 4/26/2024 6:13 PM, Takashi Iwai wrote: >>> On Fri, 26 Apr 2024 14:25:11 +0200, >>> Rajashekar kuruva wrote: >>>> When multiple plug-in and plug-out events occur, >>>> there is a risk of encountering a NULL pointer dereference >>>> leading to a kernel panic during a headset use-case. >>>> this issue arises in the snd_usb_endpoint_close function >>> Such a scenario can't happen: ep->iface_ref is changed only in >>> chip->mutex lock, hence it can't be NULL there. >>> >>> >>> thanks, >>> >>> Takashi >> Hi Takashi, >> >> Actually we are facing NULL pointer deference while running headset >> case when i checked call trace the last running function is >> snd_usb_endpoint_close where iface_ref and clock_ref both are 0x0 >> >> [75703.933104][T10585] Unable to handle kernel NULL pointer >> dereference at virtual address 0000000000000004 >> [75703.933113][T10585] [RB/E]rb_sreason_str_set: sreason_str set NULL >> pointer dereference >> [75703.933116][T10585] Mem abort info: >> [75703.933117][T10585] ESR = 0x0000000096000005 >> [75703.933119][T10585] EC = 0x25: DABT (current EL), IL = 32 bits >> [75703.933120][T10585] SET = 0, FnV = 0 >> [75703.933121][T10585] EA = 0, S1PTW = 0 >> [75703.933123][T10585] FSC = 0x05: level 1 translation fault >> [75703.933124][T10585] Data abort info: >> [75703.933124][T10585] ISV = 0, ISS = 0x00000005 >> [75703.933125][T10585] CM = 0, WnR = 0 >> … >> [75703.933676][T10585] CPU: 3 PID: 10585 Comm: kworker/u17:0 Tainted: >> G S W OE 6.1.43-android14-11-ga2fa77d36d26-ab11204829 #1 >> [75703.933697][T10585] pstate: 62400005 (nZCv daif +PAN -UAO +TCO -DIT >> -SSBS BTYPE=--) >> [75703.933700][T10585] pc : snd_usb_endpoint_close+0x30/0x104 >> [75703.933721][T10585] lr : snd_usb_endpoint_close+0x28/0x104 >> [75703.933724][T10585] sp : ffffffc04b2bb740 >> [75703.933725][T10585] x29: ffffffc04b2bb740 x28: ffffff8024e3ba78 >> x27: ffffffd266e91da0 >> [75703.933728][T10585] x26: ffffffc04b2bb7a8 x25: ffffff89bec5be00 >> x24: 00000000ffffffea >> [75703.933730][T10585] x23: 0000000000000002 x22: ffffff885d568008 >> x21: ffffff8024e3ba78 >> [75703.933732][T10585] x20: ffffff885d568000 x19: ffffff8024e3bb18 >> x18: ffffffd26db2d140 >> [75703.933734][T10585] x17: 00000000f01b0818 x16: 00000000f01b0818 >> x15: 0000000000000008 >> [75703.933736][T10585] x14: ffffff8a3e2b5780 x13: ffffff8a3e2b5780 >> x12: ffffffd26cbd2770 >> [75703.933738][T10585] x11: 0000000000000001 x10: ffffff8984320000 x9 >> : 4f43b86e946b4e00 >> [75703.933740][T10585] x8 : 0000000000000000 x7 : 0000000000000001 x6 >> : fffffffdef8e8b70 >> [75703.933742][T10585] x5 : 0000000000000001 x4 : 0000000000000000 x3 >> : ffffff8024e3bb28 >> [75703.933743][T10585] x2 : 00000001011fa7c9 x1 : ffffffc04b2bb680 x0 >> : 0000000000000000 >> [75703.933746][T10585] Call trace: >> [75703.933747][T10585] snd_usb_endpoint_close+0x30/0x104 > Who is actually calling snd_usb_endpoint_close()? > I guess that's rather a bug in the call pattern, not the code in > USB-audio driver itself. > > snd_usb_endpoint_close() is supposed to be called only for a really > opened endpoint. So, if any, it's rather a race (or a bug) in the > caller side, and it should be addressed there instead. > > > thanks, > > Takashi Hi Takashi, I'm checking on who is calling snd_usb_endpoint_close, without opened endpoint is created. As its rarely reproducible, its taking time. I'll replay you back. --Rajashekar K.
diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 8f65349a06d3..0e3101b7e392 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -950,7 +950,7 @@ void snd_usb_endpoint_close(struct snd_usb_audio *chip, usb_audio_dbg(chip, "Closing EP 0x%x (count %d)\n", ep->ep_num, ep->opened); - if (!--ep->iface_ref->opened && + if (ep->iface_ref && !--ep->iface_ref->opened && !(chip->quirk_flags & QUIRK_FLAG_IFACE_SKIP_CLOSE)) endpoint_set_interface(chip, ep, false);
When multiple plug-in and plug-out events occur, there is a risk of encountering a NULL pointer dereference leading to a kernel panic during a headset use-case. this issue arises in the snd_usb_endpoint_close function To avoid check if ep->iface_ref is not null before decrementing its opened count. If ep->iface_ref is null, we skip the decrement and the subsequent logic. Signed-off-by: Rajashekar kuruva <quic_kuruva@quicinc.com> --- sound/usb/endpoint.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)