Received-SPF: pass (google.com: domain of linux-kernel+bounces-161828-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Mon, 29 Apr 2024 08:57:12 +0200
Message-ID: <87o79s1ws7.wl-tiwai@suse.de>
From: Takashi Iwai <tiwai@suse.de>
To: "Rajashekar Kuruva (Temp)" <quic_kuruva@quicinc.com>
Cc: <srinivas.kandagatla@linaro.org>,
	<mathias.nyman@intel.com>,
	<perex@perex.cz>,
	<conor+dt@kernel.org>,
	<corbet@lwn.net>,
	<lgirdwood@gmail.com>,
	<andersson@kernel.org>,
	<krzysztof.kozlowski+dt@linaro.org>,
	<gregkh@linuxfoundation.org>,
	<Thinh.Nguyen@synopsys.com>,
	<broonie@kernel.org>,
	<bgoswami@quicinc.com>,
	<tiwai@suse.com>,
	<robh@kernel.org>,
	<konrad.dybcio@linaro.org>,
	<linux-kernel@vger.kernel.org>,
	<devicetree@vger.kernel.org>,
	<linux-sound@vger.kernel.org>,
	<linux-usb@vger.kernel.org>,
	<linux-arm-msm@vger.kernel.org>,
	<linux-doc@vger.kernel.org>,
	<alsa-devel@alsa-project.org>
Subject: Re: [PATCH] [RFC PATCH] ALSA: usb-audio: endpoint: Prevent NULL pointer deference in snd_usb_endpoint_close
In-Reply-To: <64ed9496-577c-4f31-b061-9f3dcaca4b26@quicinc.com>
References: <20240426122511.547755-1-quic_kuruva@quicinc.com>
	<877cgks399.wl-tiwai@suse.de>
	<64ed9496-577c-4f31-b061-9f3dcaca4b26@quicinc.com>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) Emacs/27.2 Mule/6.0
Precedence: bulk
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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