From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 15B82C2BB3F for ; Wed, 15 Nov 2023 19:47:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235277AbjKOTrj (ORCPT ); Wed, 15 Nov 2023 14:47:39 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36710 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235281AbjKOTri (ORCPT ); Wed, 15 Nov 2023 14:47:38 -0500 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CE9DFB8 for ; Wed, 15 Nov 2023 11:47:35 -0800 (PST) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 0D9A7C433C9; Wed, 15 Nov 2023 19:47:34 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1700077655; bh=E48yjiBHWX8Q9kZUFZlca2qPUc5XFnt8ppUTbCyrAZU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=gDTAXsV3k2k0CdozBcSM4O1X00vl+NDYdhVgKwZ4x8/PCWgEdkCQmGLu5h38vHgTm hMudSg+saziJcWXznrPAAwQlWpd3pnnaBqJ5ZPPQlklNa+TTwGu9xBqGrAfugkRylB H3+ax0C2ugA2WsXMEr243YRpldrJsF1fP7PsPwEc= From: Greg Kroah-Hartman To: stable@vger.kernel.org Cc: Greg Kroah-Hartman , patches@lists.linux.dev, Jia-Ju Bai , Sasha Levin Subject: [PATCH 6.6 436/603] usb: dwc2: fix possible NULL pointer dereference caused by driver concurrency Date: Wed, 15 Nov 2023 14:16:21 -0500 Message-ID: <20231115191643.007912624@linuxfoundation.org> X-Mailer: git-send-email 2.42.1 In-Reply-To: <20231115191613.097702445@linuxfoundation.org> References: <20231115191613.097702445@linuxfoundation.org> User-Agent: quilt/0.67 X-stable: review X-Patchwork-Hint: ignore MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org 6.6-stable review patch. If anyone has any objections, please let me know. ------------------ From: Jia-Ju Bai [ Upstream commit ef307bc6ef04e8c1ea843231db58e3afaafa9fa6 ] In _dwc2_hcd_urb_enqueue(), "urb->hcpriv = NULL" is executed without holding the lock "hsotg->lock". In _dwc2_hcd_urb_dequeue(): spin_lock_irqsave(&hsotg->lock, flags); ... if (!urb->hcpriv) { dev_dbg(hsotg->dev, "## urb->hcpriv is NULL ##\n"); goto out; } rc = dwc2_hcd_urb_dequeue(hsotg, urb->hcpriv); // Use urb->hcpriv ... out: spin_unlock_irqrestore(&hsotg->lock, flags); When _dwc2_hcd_urb_enqueue() and _dwc2_hcd_urb_dequeue() are concurrently executed, the NULL check of "urb->hcpriv" can be executed before "urb->hcpriv = NULL". After urb->hcpriv is NULL, it can be used in the function call to dwc2_hcd_urb_dequeue(), which can cause a NULL pointer dereference. This possible bug is found by an experimental static analysis tool developed by myself. This tool analyzes the locking APIs to extract function pairs that can be concurrently executed, and then analyzes the instructions in the paired functions to identify possible concurrency bugs including data races and atomicity violations. The above possible bug is reported, when my tool analyzes the source code of Linux 6.5. To fix this possible bug, "urb->hcpriv = NULL" should be executed with holding the lock "hsotg->lock". After using this patch, my tool never reports the possible bug, with the kernelconfiguration allyesconfig for x86_64. Because I have no associated hardware, I cannot test the patch in runtime testing, and just verify it according to the code logic. Fixes: 33ad261aa62b ("usb: dwc2: host: spinlock urb_enqueue") Signed-off-by: Jia-Ju Bai Link: https://lore.kernel.org/r/20230926024404.832096-1-baijiaju@buaa.edu.cn Signed-off-by: Greg Kroah-Hartman Signed-off-by: Sasha Levin --- drivers/usb/dwc2/hcd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index 657f1f659ffaf..35c7a4df8e717 100644 --- a/drivers/usb/dwc2/hcd.c +++ b/drivers/usb/dwc2/hcd.c @@ -4769,8 +4769,8 @@ static int _dwc2_hcd_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, if (qh_allocated && qh->channel && qh->channel->qh == qh) qh->channel->qh = NULL; fail2: - spin_unlock_irqrestore(&hsotg->lock, flags); urb->hcpriv = NULL; + spin_unlock_irqrestore(&hsotg->lock, flags); kfree(qtd); fail1: if (qh_allocated) { -- 2.42.0