From:   Pawel Laszczak <pawell@cadence.com>
To:     Zhou Qingyang <zhou1615@umn.edu>
CC:     "kjlu@umn.edu" <kjlu@umn.edu>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Peter Chen <peter.chen@nxp.com>,
        "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH] usb: cdnsp: Fix a NULL pointer dereference in
 cdnsp_endpoint_init()
Thread-Topic: [PATCH] usb: cdnsp: Fix a NULL pointer dereference in
 cdnsp_endpoint_init()
Thread-Index: AQHX5g+EScFJGN+g50SE4yK10NJi+qwdPKog
Date:   Wed, 1 Dec 2021 07:28:06 +0000
Message-ID: <BYAPR07MB5381BE5297FA4905E8BD40C0DD689@BYAPR07MB5381.namprd07.prod.outlook.com>
References: <20211130172700.206650-1-zhou1615@umn.edu>
In-Reply-To: <20211130172700.206650-1-zhou1615@umn.edu>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?mggnva7Wd9fTKVGv12xJyQJeBid+9kdRkDywQz8w/kCKs0cv8R2jw/4eaGn4?=
 =?us-ascii?Q?Ei8FOUAdHvfDkE/+ECc4EbM3DQw5D8k0Mo8ZtPnWJc0sdLVO3+7htjCq9x7B?=
 =?us-ascii?Q?EmQU/vDEcIULpqzm0n/DTq6LN8WhU6Apzjyimp/6a/GrZY3Muk8eASLt6R1h?=
 =?us-ascii?Q?NO5W7hNW3ASNUjqBqdpoIHc2QNYmkJtaBkPH7LXwpHZmWUml4gigJJh3URTd?=
 =?us-ascii?Q?3i/hSqERZG798hVP2mL2ug+D6NRmMfKjufMcYxm9RihfpmU478AJ1T8dnwA9?=
 =?us-ascii?Q?hMBVBHA08KHhtll8j57MpbmEOdXG2rYqGSO0ctstrbxROlghKv+AtZeytZ7p?=
 =?us-ascii?Q?NKnGIh8SsM0plf5TJ2sOujC3i/T2ugZUQmPawT7hvz+m2cM5HHyg+9k/pHDY?=
 =?us-ascii?Q?tYhFed3flMtRX+jwpn7w/c33M6KDFkco/TNXAJG2sXQQVMSwhQDxcC0pBuXP?=
 =?us-ascii?Q?33N7jRIgm8+daNqgT/f2ZrQ1q9/2PzEPFRZ7cUd3aPmbAnVGRdkuoAx7AU04?=
 =?us-ascii?Q?8l1aCeBmbYYhNE/i1YGxr5HOjP9HluydVJoLFSZCT+pgqoExJoRSVKHj0OFt?=
 =?us-ascii?Q?NeUc7RIvG1Dowsi0m7QIqEuMVvRbB9BnzAN96YiT80H/WuWdVQD/gx7tjXzw?=
 =?us-ascii?Q?yXhrf6RqhTX8L8w8d1y39iw7ToSrdiWC1PFrfCEetASdts/Z2FCVyHJxrkkw?=
 =?us-ascii?Q?Xws2hLEGfnoBwMgZ9orKllIBIJLJ3fbVXgD3ABedj5vP/mVEj73c1yZulKSb?=
 =?us-ascii?Q?iLtBBhkot52wXqQVnXZIOpB1yjZMZAu3uaWTtnuRT+FA9WsGIVPyVkFbEheu?=
 =?us-ascii?Q?+iDvoNyBHndUFNQF6s/vG7y8t/ddHKjOVFAHkVRXlNhqNWMV6wpo30EEyK4e?=
 =?us-ascii?Q?g+bhLJEQaIeik4TwPfIYMSXjELNpbdrhArctooniYfvEeBfzlaG4ZgDT6w37?=
 =?us-ascii?Q?VSBcDyUataTuYjGphTkKXgKreKHTYoyXU9Ri0ukPiCDOPct7Y5fGYl2rTwxX?=
 =?us-ascii?Q?vyJSdlXuLE26jxciZsoAJYWvhjwcJw8fKjdrmfUsA4aSORzLhe5yr+yLC/+6?=
 =?us-ascii?Q?ppaFZMQ8inFN8ugWpZEZ45otQHNi+JbsOPLfuWpqOMYSO5X8XjeCKAw2Zb1I?=
 =?us-ascii?Q?yM+wPJPFL9T0yTyXeFOC1jH4Ml+8uSGcNYMlyePykAIsYLJsRzj033jZkam2?=
 =?us-ascii?Q?i2rcmteT4ywk6El8zczgmfB7X+QGmB00kPtqF4KwAhzup6nxglj5cAO1mVG7?=
 =?us-ascii?Q?bLLDDxxao7g7uile/G0QiG9GE+eDduiG9z1y0QEbqM38pGqUY7uIyy9AQ4rl?=
 =?us-ascii?Q?Sayb22nzWS7J+ADuC1avQhTrExHODffFKO7agOdKdSAk5+Igv4+lOQF7+602?=
 =?us-ascii?Q?R6A3Bm1YxlDLI1/IZ4HZCoCfH1D46IsY+l9ucMK8rRKkA5VC0pg9nSE5I5D6?=
 =?us-ascii?Q?R0t/xlxFVfwbZG29XRL3aS+6MkgS5pHGOzmTZ7AqfzURxOLKBYbdIQS7GVbt?=
 =?us-ascii?Q?U0r79Sm8jwyYpHvdOeavBe4Pjm6XjXJ78vkVkJARUnxW+67W1wxRP1cq08kk?=
 =?us-ascii?Q?pZobWn4qJmb+K1YEfxRvgXGMi2gvmtRCWw0EAHc1od+OU9AuHN6u9BAb6zNe?=
 =?us-ascii?Q?/GwSrgIkzEdY9dVW8Us+I7mkZtoCtYx8PC5YLI2eb1cbCbN63gROI7itiNYd?=
 =?us-ascii?Q?tQDFxQ=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: cadence.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BYAPR07MB5381.namprd07.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 903b1f6f-e611-42cb-8660-08d9b49c1e1c
X-MS-Exchange-CrossTenant-originalarrivaltime: 01 Dec 2021 07:28:06.0920
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: d36035c5-6ce6-4662-a3dc-e762e61ae4c9
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: GjmYK+bbj0MI+Z+UkZXI6Fv07YkN9WJ1zJey/rgnZxaeXBKW4dJTqo6Jzf9/XDQZ+NCNBRFu2Z01G0MoMAfhgYkZg+7eHkN/nvzP7rp5f14=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR07MB6546
X-Proofpoint-GUID: 1nzhVR7F0MOTt2d05GEGu-xdiSRL2UVP
X-Proofpoint-ORIG-GUID: 1nzhVR7F0MOTt2d05GEGu-xdiSRL2UVP
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.790,Hydra:6.0.425,FMLib:17.0.607.475
 definitions=2021-11-30_10,2021-11-28_01,2020-04-07_01
X-Proofpoint-Spam-Details: rule=outbound_check_notspam policy=outbound_check score=0 adultscore=0
 suspectscore=0 phishscore=0 malwarescore=0 lowpriorityscore=0
 mlxlogscore=999 mlxscore=0 impostorscore=0 priorityscore=1501 spamscore=0
 clxscore=1011 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2110150000 definitions=main-2112010042
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

>
>In cdnsp_endpoint_init(), cdnsp_ring_alloc() is assigned to pep->ring
>and there is a dereference of it in cdnsp_endpoint_init(), which could
>lead to a NULL pointer dereference on failure of cdnsp_ring_alloc().
>
>Fix this bug by adding a check of pep->ring.
>
>This bug was found by a static analyzer. The analysis employs
>differential checking to identify inconsistent security operations
>(e.g., checks or kfrees) between two code paths and confirms that the
>inconsistent operations are not recovered in the current function or
>the callers, so they constitute bugs.
>
>Note that, as a bug found by static analysis, it can be a false
>positive or hard to trigger. Multiple researchers have cross-reviewed
>the bug.
>
>Builds with CONFIG_USB_CDNSP_GADGET=3Dy show no new warnings,
>and our static analyzer no longer warns about this code.
>
>Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence USBSSP DR=
D Driver")
>Signed-off-by: Zhou Qingyang <zhou1615@umn.edu>
>---
> drivers/usb/cdns3/cdnsp-mem.c | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/drivers/usb/cdns3/cdnsp-mem.c b/drivers/usb/cdns3/cdnsp-mem.c
>index ad9aee3f1e39..97866bfb2da9 100644
>--- a/drivers/usb/cdns3/cdnsp-mem.c
>+++ b/drivers/usb/cdns3/cdnsp-mem.c
>@@ -987,6 +987,9 @@ int cdnsp_endpoint_init(struct cdnsp_device *pdev,
>
> 	/* Set up the endpoint ring. */
> 	pep->ring =3D cdnsp_ring_alloc(pdev, 2, ring_type, max_packet, mem_flags=
);
>+	if (!pep->ring)
>+		return -ENOMEM;
>+
> 	pep->skip =3D false;
>
> 	/* Fill the endpoint context */
>--
>2.25.1


Acked-by: Pawel Laszczak <pawell@cadence.com>

--

Thanks,
Pawel Laszczak