From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id C775F9416D2 for ; Mon, 9 Oct 2023 11:37:46 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=7qxyVFoVrLpP0O6+gvrWJh+htLI5psGTm6VNcOzLpIU=; c=relaxed/simple; d=groups.io; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type:Content-Transfer-Encoding; s=20140610; t=1696851465; v=1; b=cunqmRtKHwthz944vRDG4WgdK1MZi0HFkj22Zh2dA/qotzdEHEVXAAGILDUoTCA7M7OpW0OW AWIKOi+sMy2PcL+xuRBwrwc2AdcDH5zeQgKF6EM81yMWNwIrhmFkBiy6w34F0qGfq5w3r9Ph+GD 67e8AXSO5CEYluqNmciSSslQ= X-Received: by 127.0.0.2 with SMTP id RdLbYY7687511xHnMdP2HTeR; Mon, 09 Oct 2023 04:37:45 -0700 X-Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mx.groups.io with SMTP id smtpd.web11.58923.1696851464550455293 for ; Mon, 09 Oct 2023 04:37:44 -0700 X-Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-447-yRdit9-sNciywaUuJ4R4HQ-1; Mon, 09 Oct 2023 07:37:40 -0400 X-MC-Unique: yRdit9-sNciywaUuJ4R4HQ-1 X-Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.rdu2.redhat.com [10.11.54.7]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 42140805B32; Mon, 9 Oct 2023 11:37:40 +0000 (UTC) X-Received: from [10.39.192.114] (unknown [10.39.192.114]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 61273140E962; Mon, 9 Oct 2023 11:37:39 +0000 (UTC) Message-ID: <561c61aa-dc5f-e8ff-2683-bc8502b4d03a@redhat.com> Date: Mon, 9 Oct 2023 13:37:38 +0200 MIME-Version: 1.0 Subject: Re: [edk2-devel] [PATCH v2 1/2] MdeModulePkg/Bus/Usb/UsbMouseDxe: Fix REVERSE_INULL Coverity issue To: devel@edk2.groups.io, rsingh@ventanamicro.com Cc: Hao A Wu , Ray Ni , Veeresh Sangolli References: <20231009112832.225861-1-rsingh@ventanamicro.com> <20231009112832.225861-2-rsingh@ventanamicro.com> From: "Laszlo Ersek" In-Reply-To: <20231009112832.225861-2-rsingh@ventanamicro.com> X-Scanned-By: MIMEDefang 3.1 on 10.11.54.7 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,lersek@redhat.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: w5ZnpiajSafpCsFfcgEbB7Gux7686176AA= Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=cunqmRtK; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=redhat.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io On 10/9/23 13:28, Ranbir Singh wrote: > From: Ranbir Singh >=20 > The function USBMouseDriverBindingStart do have >=20 > ASSERT (UsbMouseDevice !=3D NULL); >=20 > after AllocateZeroPool, but it is applicable only in DEBUG mode. > In RELEASE mode, if for whatever reasons UsbMouseDevice is NULL > at this point, the code proceeds to dereference "UsbMouseDevice" > afterwards which will lead to CRASH. >=20 > Hence, for safety add NULL pointer checks always. >=20 > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D4222 >=20 > Cc: Hao A Wu > Cc: Ray Ni > Co-authored-by: Veeresh Sangolli > Signed-off-by: Ranbir Singh > Signed-off-by: Ranbir Singh > --- > MdeModulePkg/Bus/Usb/UsbMouseDxe/UsbMouse.c | 4 ++++ > 1 file changed, 4 insertions(+) >=20 > diff --git a/MdeModulePkg/Bus/Usb/UsbMouseDxe/UsbMouse.c b/MdeModulePkg/B= us/Usb/UsbMouseDxe/UsbMouse.c > index 451d4b934f4c..621d09713b24 100644 > --- a/MdeModulePkg/Bus/Usb/UsbMouseDxe/UsbMouse.c > +++ b/MdeModulePkg/Bus/Usb/UsbMouseDxe/UsbMouse.c > @@ -161,6 +161,10 @@ USBMouseDriverBindingStart ( > =20 > UsbMouseDevice =3D AllocateZeroPool (sizeof (USB_MOUSE_DEV)); > ASSERT (UsbMouseDevice !=3D NULL); > + if (UsbMouseDevice =3D=3D NULL) { > + Status =3D EFI_OUT_OF_RESOURCES; > + goto ErrorExit; > + } > =20 > UsbMouseDevice->UsbIo =3D UsbIo; > UsbMouseDevice->Signature =3D USB_MOUSE_DEV_SIGNATURE; With this (good) fix, it's better to remove the ASSERT() altogether, in my opinion! The assert was a cop-out, and now you are replacing it with proper error checking (and cleanup, too), so there's no need for the ASSERT(). It's not an "invariant" that AllocateZeroPool must succeed, so whenever it fails, it's not an invariant violation (programming error). With the ASSERT() dropped: Reviewed-by: Laszlo Ersek -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#109458): https://edk2.groups.io/g/devel/message/109458 Mute This Topic: https://groups.io/mt/101849997/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/19134562= 12/xyzzy [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-