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 1C5A0D81113 for ; Mon, 12 Feb 2024 19:31:37 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=RsFlZUPCK5tfjcGpCqZo2NHJjqSu08amPQBiBuYlK0A=; c=relaxed/simple; d=groups.io; h=ARC-Seal:ARC-Message-Signature:ARC-Authentication-Results:From:To:CC:Subject:Thread-Topic:Thread-Index:Date:Message-ID:References:In-Reply-To:Accept-Language:msip_labels:MIME-Version:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type; s=20140610; t=1707766296; v=1; b=ge5ZEEvMGHnuGGekJcCPjdyiVQetWeUF4hLxMJyNf6zeq5CK5ecG3yBcjImfHzKzsOFlmLBJ 9Cn00aaChohb/emDluZOBU1/4PwVAYeO53oipBEY6fSxAWowkbFGmLaDzqZYG3jPBzohy8slzDg 2IcaJobLu8hk6aJiHKXkK8uU= X-Received: by 127.0.0.2 with SMTP id l52pYY7687511xaEJyBCpeJ4; Mon, 12 Feb 2024 11:31:36 -0800 X-Received: from SA9PR02CU001.outbound.protection.outlook.com (SA9PR02CU001.outbound.protection.outlook.com [40.93.193.7]) by mx.groups.io with SMTP id smtpd.web11.15943.1707766296038077712 for ; Mon, 12 Feb 2024 11:31:36 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=QwjycwtfAJnKphJNdEVPzsrbsb7B7s667tPyyUgBZC0w17wmik/2O62MhVrSZp9kFIR+IdFn9VNncAjvYDExyY58vyVyyDFfjTtytBzHNXUKCgExDyPy7MPK14vbVf5Tjc9rH3yJls2Gwlvr2uL33bDx1mRiRJrS7+NOyNeNNu9dY3Bqmip6gA0PwJHoFyLWNa70GgEWzz93JHOe9d+XmXcoxpUmUErTTBNlAldSi0kB4wuNWeQwvjdZ36owY3BfvYJj38n+mM/F+1wKtkxXe9xAPE1xDZ25WCzKgJMSKuD6zQOsSv9nf6yh1XryH3RG7GgvLeOwCS5xlUdx36KIeQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=fpvKZCoXvRh7RxzLMOCrUcpDOeUTpYUdNS73/aPJQyE=; b=ieB2XcYKSmJPHDJGkZGMQSil3zpnobCTxlBM1SjUT01TJjFTDaJU7KNnyuythQQ+uAlWx0oB6+3QAyaT6XnUWMWBzgSMs64eAZz8SUJiyL27BZnc5UwFhsLA4+gXwnV1J0+yi5NrhJwtcxDgh8fnZpoWLvvVuAyJPoBWNqU+qWP2lYr+5NH4IA2txOECpr+1hykeSFvrDmYO7t+M3QaFV2r/uee9K72gFWTawqpKl4kU+u6b+DqOsukgR48XdrBY9bGqEZ0SOQcYIycZai31UGjXYoZoFGH7ofTJ3ei5aTzLBcF5noF+ConRHi71nGLqWZpm1yzRafimYH0HhDyCgA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=microsoft.com; dmarc=pass action=none header.from=microsoft.com; dkim=pass header.d=microsoft.com; arc=none X-Received: from CY5PR21MB3684.namprd21.prod.outlook.com (2603:10b6:930:e::15) by SA0PR21MB1945.namprd21.prod.outlook.com (2603:10b6:806:e9::8) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7316.9; Mon, 12 Feb 2024 19:31:32 +0000 X-Received: from CY5PR21MB3684.namprd21.prod.outlook.com ([fe80::fd2b:5028:3c46:c23b]) by CY5PR21MB3684.namprd21.prod.outlook.com ([fe80::fd2b:5028:3c46:c23b%4]) with mapi id 15.20.7316.006; Mon, 12 Feb 2024 19:31:32 +0000 From: "Doug Flick via groups.io" To: Leif Lindholm , "Douglas Flick [MSFT]" CC: "devel@edk2.groups.io" , Saloni Kasbekar , Zachary Clark-williams , Andrew Fish , "Kinney, Michael D" Subject: Re: [edk2-devel] [PATCH 2/3] [edk2-stable202402] NetworkPkg: Dhcp6Dxe: Additional Code Cleanup Thread-Topic: [EXTERNAL] Re: [PATCH 2/3] [edk2-stable202402] NetworkPkg: Dhcp6Dxe: Additional Code Cleanup Thread-Index: AQHaXegOyQoygsSdYUaCN5nYo/bpebEHFnPq Date: Mon, 12 Feb 2024 19:31:32 +0000 Message-ID: References: <25e7367bd09b43a4f4d9e084ad5019dcd1f28446.1707534069.git.doug.edk2@gmail.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: msip_labels: MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Enabled=True;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SiteId=72f988bf-86f1-41af-91ab-2d7cd011db47;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SetDate=2024-02-12T19:24:43.8911054Z;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_ContentBits=0;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Method=Standard x-ms-publictraffictype: Email x-ms-traffictypediagnostic: CY5PR21MB3684:EE_|SA0PR21MB1945:EE_ x-ms-office365-filtering-correlation-id: bc95ae2a-624c-4e03-a529-08dc2c01381e x-ld-processed: 72f988bf-86f1-41af-91ab-2d7cd011db47,ExtAddr x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam-message-info: tUzsrEDP+mviD+J52/kmMaX8RQtnTIrIgYlw1kUvg692zi6kdkJPBZgXr9P6juALsykRhpRcBO/lgDQBQD3L06GyBam8UJiSgfsDYhmpt2tTixfPZRTEoFqFnSf69hxLBkTitNQrgjSjwf4F67mSkNffy0ZmAFtiXbKXJHsK/PB5WtNXnKkZKxrETIQrSGfHtcUyHuXhPV8Z4Q2CcerNpiLaWTtnHzBCEtAtM/uScRblvY+qAVTIbpYDekqQgxFcBT5x8XYb2GfN3tlKm5mOi0IZvAJQY19l7dBbTnrXgi0ytyDPEG8WCOjmzROyp2Opr8ohLW7xoTjOnwyjGFMHLI9Axu56NACVkQdcdmnBTVFs04pk/v5+PoTaBrj/ufHBo45M51TlyU/WCLxqN5iqZYKACUwLJiY4sP9p3Tp457VVAl4o3TJGL4+jaRlCzj0SNzQDRKeSys4MaXcNTg1CGsJN1Y4sYsuvU4WLLlCAP1Sm/rgNA72FpRHcAUVT+iIsMBrBPzdUcmLaGDZAqZJvETcw6e2YpNXT4tqqEVAhO0ZlrDg6kaZO2tyw0NgQggx/hpDwL/YqLLnx/h6TjyZ9C8nL9kQBi2wJGTP5+Zq2SzHPDSTQ4l5W1y/tcb9tyvZ4 x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?qXzDxbalm3oxRdXNioXIqa8971ye/u5psq0ZCRn3LxmuYtGLOmqJ7tPfnBGs?= =?us-ascii?Q?RsLMaPnbK70PIROZVItxEjTqJRXyRGQSmRPh9V7NdvuJByesW7/5DSCXnXob?= =?us-ascii?Q?RWLci2732W8dB3d8na5tMQnhC7CzrL+JQfycR+BhkJxpykglsaS40tsGWHVx?= =?us-ascii?Q?4H0rKhJXYnpvGpAjN7cxPthW28AVhU1H836qPkKdtdt+MvDbGU/ff/19/jOS?= =?us-ascii?Q?o60mDKkdAeOrv5LE0WEsfGVmXC6Td4F8f0pswZtIkQcxKX7fQBhz1igmGbIw?= =?us-ascii?Q?adFNH+CzKF2faZIBlgVs35k3KH9ZXpPWDbBiBpk4Y7XlZ0TaJPoKYDkzCedo?= =?us-ascii?Q?rkqBZhiJOQ3vE7qufiEVJTK3QiS7licNM/wVazv4fNhbvoR9EAJeoKZDu9MK?= =?us-ascii?Q?1QlKABwV6YuW/F+QmMyqliIqbbKBo8PnVl7k769YdUfQpsNaSPdDDWfzkx8a?= =?us-ascii?Q?LN9BeXzL3v4BDT9mLTWu8toIC1n4lVM0Bo1/Ky/WG+aVh1ZbgXzr2V/RBYOJ?= =?us-ascii?Q?AKC04xThvMxxHHqM5Ew+1s0cxlTheo0gg79mkX8bHTiSMLO7rOcRgD2w4LJA?= =?us-ascii?Q?rBTNh2bRifMOeQ9+NCYT7FFCWQQ8RwVt+o+ciTU9jX+NRS1I5aAhcM0j7sA+?= =?us-ascii?Q?iP2RzbjoMBwsVorm6vA9G5nmcxsykDQji4wY3JtdFoZfI+sU3Pqd6CiGglQt?= =?us-ascii?Q?YeUWsqAR8i6uhhC2HKo0YIdz7D7750urTs0wiYvx9RwdAFWvNfLmhHOTF79c?= =?us-ascii?Q?aqzMzIHy/hBNTGXAybv+r9wnX/CHws+7M2HDBeh6/24Lg5qFCJmJJZWqNCoA?= =?us-ascii?Q?0g3qmWUs8vtL7Xrm+D+C+uC3tHYvXaEu8aUv+zyTLW5LFmFxDX+AKTiGnT36?= =?us-ascii?Q?kapnYwkpBZTJ7tSui+NHcfvswnRNEdSrZO0QThC9B0B5RxmkQz2oVvcNMEoA?= =?us-ascii?Q?bJm63vaavy6I6Ekd6X07YnPK/S2Dq4Mnt0EFPcP6TfH/9SLtIR4bqUglus4w?= =?us-ascii?Q?zseUCNGMqCbegaYoiqf2Hpi5UjAx0jfvaGOFC8rLSm13vqyOHxl7jOoZyHGq?= =?us-ascii?Q?AVvxwI86+oduSa+ZYdnvSdreCm6jrbmQ2I+n/0ATaAUCcu/y+rwKtDaiOnLR?= =?us-ascii?Q?ZBauRNNIuJ5C3c1htkLEP9YqSG+37ekx1fGz2CoB8NeOuYQ5pFl7FI6AvsSc?= =?us-ascii?Q?ugjGpzgi//8Hcf/cZv2dzcFzNnxwcGmJZfRdOuAVTJsxTlqxwmP4TF8DEQwd?= =?us-ascii?Q?opdGbu3E2GJuJxOQbsVWUn61UM1t4ydUsizFx/tBXYiDXU/2qSJV41tWT7I7?= =?us-ascii?Q?49YO7G1U2e+OIQiKSdb3EsF8KtcdQTrUHqLcXMRvz8c55nuWM1M7ml63cC4g?= =?us-ascii?Q?ed3cJxfieoIgeK5UBLXPJKsBO7KQXejN8VHUFigA/iJMgriJPbtyzWyt7gNK?= =?us-ascii?Q?Sez0rC0wvg4OsHEc+QUxoV6aENj7L+aSbXS1F5Xk2qpyTRxitePxSjiAvkvs?= =?us-ascii?Q?+fVhMUDMzur63Rxr7LibNIL2gOjg2De3zCKWUxCYmG8wBcP99g+p955JYb58?= =?us-ascii?Q?9nsgA/n2HKr0TGvJzSmmlL6zssgZSbj5diqIOvKhjjhBHHzTTI58XXYAjjTU?= =?us-ascii?Q?aw=3D=3D?= MIME-Version: 1.0 X-OriginatorOrg: microsoft.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: CY5PR21MB3684.namprd21.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: bc95ae2a-624c-4e03-a529-08dc2c01381e X-MS-Exchange-CrossTenant-originalarrivaltime: 12 Feb 2024 19:31:32.7796 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 72f988bf-86f1-41af-91ab-2d7cd011db47 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: vekxU2kWE3gzSEY41M+0Nyw4b7V3YCtdh0InejaAO2S/s/5gan+6blHHZc3BRkeJ0j4D8H33tmG5zxokel5QPg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA0PR21MB1945 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,dougflick@microsoft.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: uVbTxJPQTyfsabl6ZoON8AFPx7686176AA= Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_CY5PR21MB3684CAA3EA4AC2D30BEA664FB3482CY5PR21MB3684namp_" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=ge5ZEEvM; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io; dmarc=pass (policy=none) header.from=groups.io; arc=reject ("signature check failed: fail, {[1] = sig:microsoft.com:reject}") --_000_CY5PR21MB3684CAA3EA4AC2D30BEA664FB3482CY5PR21MB3684namp_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable > If this was one of the packages I maintain, I would ask > you to break any "Additionally" out as a separate commit. Thats fair enough. I can go ahead and do that. > I see this exact pattern repeated identically 4 times in > this file. After this deletion. If we're cleaning this up, > someone should look into making a macro or static > helper function. That's a good suggestion > That's not just cleanup though, this is a bugfix in its own > right? As i= n, even if the code is currently laid out in a way > that doesn't trigger a= n issue, > Dhcp6AppendIaAddrOption () depends on this option > being up to date when called. Or am I overreacting? Ultimately, I think you're right. This issue was originally pointed on the = original mail thread but it was merged in before it could be corrected. Sho= uld I create a bugzilla for this and then attach that to the change? Douglas Flick ________________________________ From: Leif Lindholm Sent: Monday, February 12, 2024 11:16:44 AM To: Douglas Flick [MSFT] Cc: devel@edk2.groups.io ; Doug Flick ; Saloni Kasbekar ; Zachary Clark-will= iams ; Andrew Fish ; Kin= ney, Michael D Subject: [EXTERNAL] Re: [PATCH 2/3] [edk2-stable202402] NetworkPkg: Dhcp6Dx= e: Additional Code Cleanup [You don't often get email from quic_llindhol@quicinc.com. Learn why this i= s important at https://aka.ms/LearnAboutSenderIdentification ] On Fri, Feb 09, 2024 at 19:04:57 -0800, Douglas Flick [MSFT] wrote: > From: Doug Flick > > Removes duplicate check after merge > > > > > // > > // Verify the PacketCursor is within the packet > > // > > if ( (*PacketCursor < Packet->Dhcp6.Option) > > || (*PacketCursor >=3D Packet->Dhcp6.Option + (Packet->Size - > sizeof (EFI_DHCP6_HEADER)))) > > { > > return EFI_INVALID_PARAMETER; > > } > > > > Additionally Calculates the Packet->Length before appending If this was one of the packages I maintain, I would ask you to break any "Additionally" out as a separate commit. > Cc: Saloni Kasbekar > Cc: Zachary Clark-williams > > Cc: Andrew Fish > Cc: Leif Lindholm > Cc: Michael D Kinney > > Signed-off-by: Doug Flick [MSFT] > --- > NetworkPkg/Dhcp6Dxe/Dhcp6Utility.c | 20 ++++++-------------- > 1 file changed, 6 insertions(+), 14 deletions(-) > > diff --git a/NetworkPkg/Dhcp6Dxe/Dhcp6Utility.c b/NetworkPkg/Dhcp6Dxe/Dhc= p6Utility.c > index 705c665c519d..348602c4e1a8 100644 > --- a/NetworkPkg/Dhcp6Dxe/Dhcp6Utility.c > +++ b/NetworkPkg/Dhcp6Dxe/Dhcp6Utility.c > @@ -657,15 +657,6 @@ Dhcp6AppendOption ( > return EFI_BUFFER_TOO_SMALL; > } > > - // > - // Verify the PacketCursor is within the packet > - // > - if ( (*PacketCursor < Packet->Dhcp6.Option) > - || (*PacketCursor >=3D Packet->Dhcp6.Option + (Packet->Size - sizeo= f (EFI_DHCP6_HEADER)))) > - { > - return EFI_INVALID_PARAMETER; > - } > - I see this exact pattern repeated identically 4 times in this file. After this deletion. If we're cleaning this up, someone should look into making a macro or static helper function. > WriteUnaligned16 ((UINT16 *)*PacketCursor, OptType); > *PacketCursor +=3D DHCP6_SIZE_OF_OPT_CODE; > WriteUnaligned16 ((UINT16 *)*PacketCursor, OptLen); > @@ -929,6 +920,11 @@ Dhcp6AppendIaOption ( > *PacketCursor +=3D sizeof (T2); > } > > + // > + // Update the packet length > + // > + Packet->Length +=3D BytesNeeded; > + That's not just cleanup though, this is a bugfix in its own right? As in, even if the code is currently laid out in a way that doesn't trigger an issue, Dhcp6AppendIaAddrOption () depends on this option being up to date when called. Or am I overreacting? / Leif > // > // Fill all the addresses belong to the Ia > // > @@ -945,11 +941,6 @@ Dhcp6AppendIaOption ( > // > *Len =3D HTONS ((UINT16)(*PacketCursor - (UINT8 *)Len - 2)); > > - // > - // Update the packet length > - // > - Packet->Length +=3D BytesNeeded; > - > return EFI_SUCCESS; > } > > @@ -957,6 +948,7 @@ Dhcp6AppendIaOption ( > Append the appointed Elapsed time option to Buf, and move Buf to the e= nd. > > @param[in, out] Packet A pointer to the packet, on success Pack= et->Length > + will be updated. > @param[in, out] PacketCursor The pointer in the packet, on success Pa= cketCursor > will be moved to the end of the option. > @param[in] Instance The pointer to the Dhcp6 instance. > -- > 2.43.0 > -=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 (#115372): https://edk2.groups.io/g/devel/message/115372 Mute This Topic: https://groups.io/mt/104272127/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- --_000_CY5PR21MB3684CAA3EA4AC2D30BEA664FB3482CY5PR21MB3684namp_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable
> If this was one of the packages I maintain= , I would ask
> you to break any "Additionally" out as a separate commit.

Thats fair enough. I can go ahead and do that.

&= gt; I see this exact pattern repeated identically 4 times in
&= gt; this file.&= nbsp;After thi= s deletion. If we're cleaning this up,
> someone shoul= d look into making a macro or static
> helper f= unction.

That's a good sugg= estion

> That's not just cleanup though, = this is a bugfix in its own > right? As in, even if the code is currently laid out in a way > that doesn't = ;trigger an issue= ,
> Dhcp6AppendIaAddrOption () depends on= this option
> being up to date when called. Or am I overreacting?

Ultimately, I think you're right. This iss= ue was originally pointed on the original mail thread but it was merged in before it could be corrected. Should I cr= eate a bugzilla for this and then attach that to the change? 

Douglas Flick
From: Leif Lindholm <qui= c_llindhol@quicinc.com>
Sent: Monday, February 12, 2024 11:16:44 AM
To: Douglas Flick [MSFT] <doug.edk2@gmail.com>
Cc: devel@edk2.groups.io <devel@edk2.groups.io>; Doug Flick &l= t;dougflick@microsoft.com>; Saloni Kasbekar <saloni.kasbekar@intel.co= m>; Zachary Clark-williams <zachary.clark-williams@intel.com>; And= rew Fish <afish@apple.com>; Kinney, Michael D <michael.d.kinney@in= tel.com>
Subject: [EXTERNAL] Re: [PATCH 2/3] [edk2-stable202402] NetworkPkg: = Dhcp6Dxe: Additional Code Cleanup
 
[You don't often get email from quic_llindhol@quic= inc.com. Learn why this is important at https://aka.ms/Le= arnAboutSenderIdentification ]

On Fri, Feb 09, 2024 at 19:04:57 -0800, Douglas Flick [MSFT] wrote:
> From: Doug Flick <dougflick@microsoft.com>
>
> Removes duplicate check after merge
>
> >
> >  //
> >  // Verify the PacketCursor is within the packet
> >  //
> >  if (  (*PacketCursor < Packet->Dhcp6.Option)
> >     || (*PacketCursor >=3D Packet->Dhcp= 6.Option + (Packet->Size -
> sizeof (EFI_DHCP6_HEADER))))
> >  {
> >    return EFI_INVALID_PARAMETER;
> >  }
> >
>
> Additionally Calculates the Packet->Length before appending

If this was one of the packages I maintain, I would ask you to break
any "Additionally" out as a separate commit.

> Cc: Saloni Kasbekar <saloni.kasbekar@intel.com>
> Cc: Zachary Clark-williams <zachary.clark-williams@intel.com> >
> Cc: Andrew Fish <afish@apple.com>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>
> Signed-off-by: Doug Flick [MSFT] <doug.edk2@gmail.com>
> ---
>  NetworkPkg/Dhcp6Dxe/Dhcp6Utility.c | 20 ++++++--------------
>  1 file changed, 6 insertions(+), 14 deletions(-)
>
> diff --git a/NetworkPkg/Dhcp6Dxe/Dhcp6Utility.c b/NetworkPkg/Dhcp6Dxe/= Dhcp6Utility.c
> index 705c665c519d..348602c4e1a8 100644
> --- a/NetworkPkg/Dhcp6Dxe/Dhcp6Utility.c
> +++ b/NetworkPkg/Dhcp6Dxe/Dhcp6Utility.c
> @@ -657,15 +657,6 @@ Dhcp6AppendOption (
>      return EFI_BUFFER_TOO_SMALL;
>    }
>
> -  //
> -  // Verify the PacketCursor is within the packet
> -  //
> -  if (  (*PacketCursor < Packet->Dhcp6.Option)
> -     || (*PacketCursor >=3D Packet->Dhcp6.O= ption + (Packet->Size - sizeof (EFI_DHCP6_HEADER))))
> -  {
> -    return EFI_INVALID_PARAMETER;
> -  }
> -

I see this exact pattern repeated identically 4 times in this
file. After this deletion.
If we're cleaning this up, someone should look into making a macro or
static helper function.

>    WriteUnaligned16 ((UINT16 *)*PacketCursor, OptType);=
>    *PacketCursor +=3D DHCP6_SIZE_OF_OPT_CODE;
>    WriteUnaligned16 ((UINT16 *)*PacketCursor, OptLen);<= br> > @@ -929,6 +920,11 @@ Dhcp6AppendIaOption (
>      *PacketCursor +=3D sizeof (T2);
>    }
>
> +  //
> +  // Update the packet length
> +  //
> +  Packet->Length +=3D BytesNeeded;
> +

That's not just cleanup though, this is a bugfix in its own right?
As in, even if the code is currently laid out in a way that doesn't
trigger an issue, Dhcp6AppendIaAddrOption () depends on this option
being up to date when called.
Or am I overreacting?

/
    Leif

>    //
>    // Fill all the addresses belong to the Ia
>    //
> @@ -945,11 +941,6 @@ Dhcp6AppendIaOption (
>    //
>    *Len =3D HTONS ((UINT16)(*PacketCursor - (UINT8 *)Le= n - 2));
>
> -  //
> -  // Update the packet length
> -  //
> -  Packet->Length +=3D BytesNeeded;
> -
>    return EFI_SUCCESS;
>  }
>
> @@ -957,6 +948,7 @@ Dhcp6AppendIaOption (
>    Append the appointed Elapsed time option to Buf, and= move Buf to the end.
>
>    @param[in, out] Packet     =    A pointer to the packet, on success Packet->Length
> +           &nb= sp;            =         will be updated.
>    @param[in, out] PacketCursor  The pointer in th= e packet, on success PacketCursor
>            = ;            &n= bsp;         will be moved to the e= nd of the option.
>    @param[in]      Instance&nb= sp;     The pointer to the Dhcp6 instance.
> --
> 2.43.0
>
_._,_._,_
Groups.io Links:

=20 You receive all messages sent to this group. =20 =20

View/Reply Online (#115372) | =20 | Mute= This Topic | New Topic
Your Subscriptio= n | Contact Group Owner | Unsubscribe [rebecca@openfw.io]

_._,_._,_
--_000_CY5PR21MB3684CAA3EA4AC2D30BEA664FB3482CY5PR21MB3684namp_--