From nobody Mon May 6 05:52:14 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of groups.io designates 66.175.222.12 as permitted sender) client-ip=66.175.222.12; envelope-from=bounce+27952+56699+1787277+3901457@groups.io; helo=web01.groups.io; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.12 as permitted sender) smtp.mailfrom=bounce+27952+56699+1787277+3901457@groups.io; dmarc=fail(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1585616095; cv=none; d=zohomail.com; s=zohoarc; b=QpFeUvt7WQVc+kh4QbErsL3iCVBCz+s/Eo3tIK21/db2PaLkUNyysg4HHPWZyqIFPyqE7gbZy97fdd9SmYK16GzYvQIw3HCMOV+9bML+XsjgCm69Aa4CDb/XBfPYDhPMNRUbQC9Xj3vtkUB4wS2s7GUVZiLrABSmFcqpmGIf19o= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1585616095; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:List-Id:List-Unsubscribe:MIME-Version:Message-ID:Reply-To:Sender:Subject:To; bh=lukcRIgB5rv0/k3AuHYQauaI0OwwIhJlb/FUc8Bn4YI=; b=lUwUK7YTiGDg/eeM2vB9CmIvcaxyG6BKf2avpIax9nFRchcW84kkL8LpWqPDyEPPubZR3NXuPXh6lptiKoRj++TtHWBnrD9zGjjgc+tNwYWJvUAbmzVloDBwuMMo0FxMRj9dNDvO7ApyaeStwanAa9SWoFJ5hwPp5+QDsZn/LAo= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.12 as permitted sender) smtp.mailfrom=bounce+27952+56699+1787277+3901457@groups.io; dmarc=fail header.from= (p=none dis=none) header.from= Received: from web01.groups.io (web01.groups.io [66.175.222.12]) by mx.zohomail.com with SMTPS id 1585616095228130.85735332764432; Mon, 30 Mar 2020 17:54:55 -0700 (PDT) Return-Path: X-Received: by 127.0.0.2 with SMTP id xw4LYY1788612xvgsLMhB2Ri; Mon, 30 Mar 2020 17:54:54 -0700 X-Received: from us-smtp-delivery-74.mimecast.com (us-smtp-delivery-74.mimecast.com [63.128.21.74]) by mx.groups.io with SMTP id smtpd.web12.364.1585615680095690210 for ; Mon, 30 Mar 2020 17:48:00 -0700 X-Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-76-n0QyHsMrOm-3lZ8xb5oU6g-1; Mon, 30 Mar 2020 20:47:54 -0400 X-MC-Unique: n0QyHsMrOm-3lZ8xb5oU6g-1 X-Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id AE5948017CC; Tue, 31 Mar 2020 00:47:52 +0000 (UTC) X-Received: from lacos-laptop-7.usersys.redhat.com (ovpn-112-191.ams2.redhat.com [10.36.112.191]) by smtp.corp.redhat.com (Postfix) with ESMTP id 604005C1C5; Tue, 31 Mar 2020 00:47:51 +0000 (UTC) From: "Laszlo Ersek" To: edk2-devel-groups-io Cc: Jiaxin Wu , Maciej Rabeda , =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= , Siyuan Fu Subject: [edk2-devel] [PATCH] NetworkPkg/UefiPxeBcDxe: handle competing DHCP servers (more) gracefully Date: Tue, 31 Mar 2020 02:47:49 +0200 Message-Id: <20200331004749.16128-1-lersek@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Precedence: Bulk List-Unsubscribe: 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 X-Gm-Message-State: 6R5vbQNuiG8wLbLMybnmPfqRx1787277AA= Content-Transfer-Encoding: quoted-printable DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1585616094; bh=lukcRIgB5rv0/k3AuHYQauaI0OwwIhJlb/FUc8Bn4YI=; h=Cc:Content-Type:Date:From:Reply-To:Subject:To; b=ZSr/vrJBOnlLsqRjW6IJqvXHlUdOXSAPCHcYWP/YOGuTmuR6c8PAotYv/Iz6DGf7zBC FVrK0nHcAtPmoLIxqj57a2ni+Z0EMQoZbaBbBMfekEWE7TWKG3Kq1KskF7yVXguIPMfdA y7cj5SJdGl4IrbiHAzxrZMHaLC8TBgfhMo8= X-ZohoMail-DKIM: pass (identity @groups.io) Content-Type: text/plain; charset="utf-8" When DHCP is misconfigured on a network segment, such that two DHCP servers attempt to reply to requests (and therefore race with each other), the edk2 PXE client can confuse itself. In PxeBcDhcp4BootInfo() / PxeBcDhcp6BootInfo(), the client may refer to a DHCP reply packet as an "earlier" packet from the "same" DHCP server, when in reality both packets are unrelated, and arrive from different DHCP servers. While the edk2 PXE client can do nothing to fix this, it should at least not ASSERT() -- ASSERT() is for catching programming errors (violations of invariants that are under the control of the programmer). ASSERT()s should in particular not refer to external data (such as network packets). What's more, in RELEASE builds, we get NULL pointer references. Check the problem conditions with actual "if"s, and return EFI_PROTOCOL_ERROR. This will trickle out to PxeBcLoadBootFile(), and be reported as "PXE-E99: Unexpected network error". Cc: Jiaxin Wu Cc: Maciej Rabeda Cc: Philippe Mathieu-Daud=C3=A9 Cc: Siyuan Fu Signed-off-by: Laszlo Ersek Reviewed-by: Maciej Rabeda Reviewed-by: Siyuan Fu --- Notes: Repo: https://pagure.io/lersek/edk2.git Branch: dhcp_assert NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c | 30 ++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c b/NetworkPkg/UefiPxeBcDxe/= PxeBcBoot.c index 10bbb06f7593..d062a526077b 100644 --- a/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c +++ b/NetworkPkg/UefiPxeBcDxe/PxeBcBoot.c @@ -482,7 +482,20 @@ PxeBcDhcp4BootInfo ( Cache4 =3D &Private->DhcpAck.Dhcp4; } =20 - ASSERT (Cache4->OptList[PXEBC_DHCP4_TAG_INDEX_BOOTFILE] !=3D NULL); + if (Cache4->OptList[PXEBC_DHCP4_TAG_INDEX_BOOTFILE] =3D=3D NULL) { + // + // This should never happen in a correctly configured DHCP / PXE + // environment. One misconfiguration that can cause it is two DHCP ser= vers + // mistakenly running on the same network segment at the same time, and + // racing each other in answering DHCP requests. Thus, the DHCP packets + // that the edk2 PXE client considers "belonging together" may actuall= y be + // entirely independent, coming from two (competing) DHCP servers. + // + // Try to deal with this gracefully. Note that this check is not + // comprehensive, as we don't try to identify all such errors. + // + return EFI_PROTOCOL_ERROR; + } =20 // // Parse the boot server address. @@ -612,7 +625,20 @@ PxeBcDhcp6BootInfo ( Cache6 =3D &Private->DhcpAck.Dhcp6; } =20 - ASSERT (Cache6->OptList[PXEBC_DHCP6_IDX_BOOT_FILE_URL] !=3D NULL); + if (Cache6->OptList[PXEBC_DHCP6_IDX_BOOT_FILE_URL] =3D=3D NULL) { + // + // This should never happen in a correctly configured DHCP / PXE + // environment. One misconfiguration that can cause it is two DHCP ser= vers + // mistakenly running on the same network segment at the same time, and + // racing each other in answering DHCP requests. Thus, the DHCP packets + // that the edk2 PXE client considers "belonging together" may actuall= y be + // entirely independent, coming from two (competing) DHCP servers. + // + // Try to deal with this gracefully. Note that this check is not + // comprehensive, as we don't try to identify all such errors. + // + return EFI_PROTOCOL_ERROR; + } =20 // // Set the station address to IP layer. --=20 2.19.1.3.g30247aa5d201 -=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 (#56699): https://edk2.groups.io/g/devel/message/56699 Mute This Topic: https://groups.io/mt/72667954/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-