From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: philmd@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Thu, 26 Sep 2019 05:42:40 -0700 Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id DD6C62026F for ; Thu, 26 Sep 2019 12:42:39 +0000 (UTC) Received: by mail-wm1-f71.google.com with SMTP id 124so4092219wmz.1 for ; Thu, 26 Sep 2019 05:42:39 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:openpgp:message-id :date:user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=gPoNozXdhVb0anZq8CHcCv27OHpQj8oAxm/VMq6L0OU=; b=RTX8prMyMzSDwHfLAnfQWKeoRWm+MZF0SyxE7zladuJ7yjXKLSDRTaZ3IKqLd946CU 0uxEnYm1IJr7zHdoSiscGZhj75xzt3poICP7hMJ6tE0QpCcVg/NY64HbtSbZ0SMXE22o 4nOG7Ce46SUm2fHRgVoNV1h3V+F3GyBzQoOrDcDb3x0IxdROrq0yofZ/RTyrgxIuRNOm KlCpI4MEGzzLv6uO7SWWN6PGaU1EKFyKh33ExDVuSx809Dvgyiwhaku3i6YbZkE3TLR/ POa/eNrN/d/ySUnTSJdW/f4cLbIuDLF5RY36Yr5RE9383ScBY8fxiDWmx3jsI/QXvmgU ajaw== X-Gm-Message-State: APjAAAWmjYDBCSTjeYkuvDbrZE2YXFFC+B9sYgincR8GIkN1nj3sPhLy QwIZ7HOQUezPmo5eRt/FfywqbMtGgCyhuvnCLdvFGjCXqkXe5B8o29hwJRnJd1UbkVf0KTAzlRY /XT2tI9D0AFl+EQ== X-Received: by 2002:a05:6000:2:: with SMTP id h2mr2681968wrx.309.1569501758632; Thu, 26 Sep 2019 05:42:38 -0700 (PDT) X-Google-Smtp-Source: APXvYqxVGkiXLhp2QE2zhThDDVi1e2v6g1NV90XI7KpTF6isoxn1/07sL3rKQ/+sqqhAnxovVUHigA== X-Received: by 2002:a05:6000:2:: with SMTP id h2mr2681946wrx.309.1569501758384; Thu, 26 Sep 2019 05:42:38 -0700 (PDT) Received: from [192.168.1.35] (240.red-88-21-68.staticip.rima-tde.net. [88.21.68.240]) by smtp.gmail.com with ESMTPSA id v6sm4010669wma.24.2019.09.26.05.42.37 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 26 Sep 2019 05:42:37 -0700 (PDT) Subject: Re: [edk2-devel] [PATCH 19/35] NetworkPkg: fix CloseProtocol & UninstallMultipleProtocolInterfaces calls To: devel@edk2.groups.io, lersek@redhat.com Cc: Jiaxin Wu , Siyuan Fu References: <20190917194935.24322-1-lersek@redhat.com> <20190917194935.24322-20-lersek@redhat.com> From: =?UTF-8?B?UGhpbGlwcGUgTWF0aGlldS1EYXVkw6k=?= Openpgp: id=89C1E78F601EE86C867495CBA2A3FD6EDEADC0DE; url=http://pgp.mit.edu/pks/lookup?op=get&search=0xA2A3FD6EDEADC0DE Message-ID: Date: Thu, 26 Sep 2019 14:42:36 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.0 MIME-Version: 1.0 In-Reply-To: <20190917194935.24322-20-lersek@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: quoted-printable Hi Laszlo, On 9/17/19 9:49 PM, Laszlo Ersek wrote: > Both the "ControllerHandle" parameter of CloseProtocol() Maybe worth adding "of type EFI_CLOSE_PROTOCOL" > and the "Handle" > parameter of UninstallMultipleProtocolInterfaces()=20 "of type EFI_UNINSTALL_MULTIPLE_PROTOCOL_INTERFACES" have type EFI_HANDLE, > not (EFI_HANDLE*). >=20 > This patch fixes actual bugs. The issues have been dormant likely becau= se > they are on error paths. (Or, in case of TlsAuthConfigDxe, because the > driver is unloaded likely very infrequently.) >=20 > Cc: Jiaxin Wu > Cc: Siyuan Fu > Signed-off-by: Laszlo Ersek > --- >=20 > Notes: > build-tested only >=20 > NetworkPkg/DnsDxe/DnsDriver.c | 4 ++-- > NetworkPkg/IScsiDxe/IScsiConfig.c | 2 +- > NetworkPkg/Ip4Dxe/Ip4Driver.c | 2 +- > NetworkPkg/Ip6Dxe/Ip6Driver.c | 2 +- > NetworkPkg/Mtftp4Dxe/Mtftp4Driver.c | 2 +- > NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.c | 2 +- > 6 files changed, 7 insertions(+), 7 deletions(-) >=20 > diff --git a/NetworkPkg/DnsDxe/DnsDriver.c b/NetworkPkg/DnsDxe/DnsDrive= r.c > index 94d072159a4d..ad007da8b7d6 100644 > --- a/NetworkPkg/DnsDxe/DnsDriver.c > +++ b/NetworkPkg/DnsDxe/DnsDriver.c > @@ -1145,7 +1145,7 @@ Dns4ServiceBindingCreateChild ( > DnsSb->ConnectUdp->UdpHandle, > &gEfiUdp4ProtocolGuid, > gDns4DriverBinding.DriverBindingHandle, > - ChildHandle > + *ChildHandle EFI_CLOSE_PROTOCOL, OK. > ); > =20 > gBS->UninstallMultipleProtocolInterfaces ( > @@ -1388,7 +1388,7 @@ Dns6ServiceBindingCreateChild ( > DnsSb->ConnectUdp->UdpHandle, > &gEfiUdp6ProtocolGuid, > gDns6DriverBinding.DriverBindingHandle, > - ChildHandle > + *ChildHandle EFI_CLOSE_PROTOCOL, OK. > ); > =20 > gBS->UninstallMultipleProtocolInterfaces ( > diff --git a/NetworkPkg/IScsiDxe/IScsiConfig.c b/NetworkPkg/IScsiDxe/IS= csiConfig.c > index b876da7f5ccd..d773849fd3b0 100644 > --- a/NetworkPkg/IScsiDxe/IScsiConfig.c > +++ b/NetworkPkg/IScsiDxe/IScsiConfig.c > @@ -3852,7 +3852,7 @@ IScsiConfigFormInit ( > ); > if (CallbackInfo->RegisteredHandle =3D=3D NULL) { > gBS->UninstallMultipleProtocolInterfaces ( > - &CallbackInfo->DriverHandle, > + CallbackInfo->DriverHandle, EFI_UNINSTALL_MULTIPLE_PROTOCOL_INTERFACES, OK. > &gEfiDevicePathProtocolGuid, > &mIScsiHiiVendorDevicePath, > &gEfiHiiConfigAccessProtocolGuid, > diff --git a/NetworkPkg/Ip4Dxe/Ip4Driver.c b/NetworkPkg/Ip4Dxe/Ip4Drive= r.c > index ebd4dec1dfe4..62be8b681a18 100644 > --- a/NetworkPkg/Ip4Dxe/Ip4Driver.c > +++ b/NetworkPkg/Ip4Dxe/Ip4Driver.c > @@ -891,7 +891,7 @@ Ip4ServiceBindingCreateChild ( > ); > if (EFI_ERROR (Status)) { > gBS->UninstallMultipleProtocolInterfaces ( > - ChildHandle, > + *ChildHandle, EFI_UNINSTALL_MULTIPLE_PROTOCOL_INTERFACES, OK. > &gEfiIp4ProtocolGuid, > &IpInstance->Ip4Proto, > NULL > diff --git a/NetworkPkg/Ip6Dxe/Ip6Driver.c b/NetworkPkg/Ip6Dxe/Ip6Drive= r.c > index 7dc9e45af7b6..63d8428dbced 100644 > --- a/NetworkPkg/Ip6Dxe/Ip6Driver.c > +++ b/NetworkPkg/Ip6Dxe/Ip6Driver.c > @@ -888,7 +888,7 @@ Ip6ServiceBindingCreateChild ( > ); > if (EFI_ERROR (Status)) { > gBS->UninstallMultipleProtocolInterfaces ( > - ChildHandle, > + *ChildHandle, EFI_UNINSTALL_MULTIPLE_PROTOCOL_INTERFACES, OK. > &gEfiIp6ProtocolGuid, > &IpInstance->Ip6Proto, > NULL > diff --git a/NetworkPkg/Mtftp4Dxe/Mtftp4Driver.c b/NetworkPkg/Mtftp4Dxe= /Mtftp4Driver.c > index ae9e65544a86..06c4e202d3ef 100644 > --- a/NetworkPkg/Mtftp4Dxe/Mtftp4Driver.c > +++ b/NetworkPkg/Mtftp4Dxe/Mtftp4Driver.c > @@ -592,7 +592,7 @@ Mtftp4ServiceBindingCreateChild ( > MtftpSb->ConnectUdp->UdpHandle, > &gEfiUdp4ProtocolGuid, > gMtftp4DriverBinding.DriverBindingHandle, > - ChildHandle > + *ChildHandle EFI_CLOSE_PROTOCOL, OK. > ); > goto ON_ERROR; > } > diff --git a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.c b/NetworkPk= g/TlsAuthConfigDxe/TlsAuthConfigDxe.c > index 18ee763002b4..c0870ab9979c 100644 > --- a/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.c > +++ b/NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.c > @@ -39,7 +39,7 @@ TlsAuthConfigDxeUnload ( > ASSERT (PrivateData->Signature =3D=3D TLS_AUTH_CONFIG_PRIVATE_DATA_S= IGNATURE); > =20 > gBS->UninstallMultipleProtocolInterfaces ( > - &ImageHandle, > + ImageHandle, EFI_UNINSTALL_MULTIPLE_PROTOCOL_INTERFACES, OK. > &gEfiCallerIdGuid, > PrivateData, > NULL >=20 I'd have split this patch in 2 for easier review (one fixing EFI_CLOSE_PROTOCOL, another fixing EFI_UNINSTALL_MULTIPLE_PROTOCOL_INTERFACES). As it or split: Reviewed-by: Philippe Mathieu-Daud=C3=A9 Regards, Phil.