On Thu, Jan 25, 2024 at 11:06 PM, Doug Flick wrote:

<snip>
@@ -607,35 +616,95 @@ Dhcp6AppendOption (
// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+=
=0D
//=0D
=0D
- ASSERT (OptLen !=3D 0);=0D
+ //=0D
+ // Verify the arguments are valid=0D
+ //=0D
+ if (Packet =3D=3D NULL) {=0D
+ return EFI_INVALID_PARAMETER;=0D
+ }=0D
=0D
- WriteUnaligned16 ((UINT16 *)Buf, OptType);=0D
- Buf +=3D 2;=0D
- WriteUnaligned16 ((UINT16 *)Buf, OptLen);=0D
- Buf +=3D 2;=0D
- CopyMem (Buf, Data, NTOHS (OptLen));=0D
- Buf +=3D NTOHS (OptLen);=0D
+ if ((PacketCursor =3D=3D NULL) || (*PacketCursor =3D=3D NULL)) {=0D
+ return EFI_INVALID_PARAMETER;=0D
+ }=0D
=0D
- return Buf;=0D
+ if (Data =3D=3D NULL) {=0D
+ return EFI_INVALID_PARAMETER;=0D
+ }=0D
+=0D
+ if (OptLen =3D=3D 0) {=0D
+ return EFI_INVALID_PARAMETER;=0D
+ }=0D
+=0D
+ //=0D
+ // Verify the PacketCursor is within the packet=0D
+ //=0D
+ if ( (*PacketCursor < Packet->Dhcp6.Option)=0D
+ || (*PacketCursor >=3D Packet->Dhcp6.Option + (Packet->Size - sizeof =
(EFI_DHCP6_HEADER))))=0D
+ {=0D
+ return EFI_INVALID_PARAMETER;=0D
+ }=0D
+=0D
+ //=0D
+ // Calculate the bytes needed for the option=0D
+ //=0D
+ BytesNeeded =3D DHCP6_SIZE_OF_COMBINED_CODE_AND_LEN + NTOHS (OptLen);=0D
+=0D
+ //=0D
+ // Space remaining in the packet=0D
+ //=0D
+ Length =3D Packet->Size - Packet->Length;=0D
+ if (Length < BytesNeeded) {=0D
+ return EFI_BUFFER_TOO_SMALL;=0D
+ }=0D
+=0D
+ //=0D
+ // Verify the PacketCursor is within the packet=0D
+ //=0D
+ if ( (*PacketCursor < Packet->Dhcp6.Option)=0D
+ || (*PacketCursor >=3D Packet->Dhcp6.Option + (Packet->Size - sizeof =
(EFI_DHCP6_HEADER))))=0D
+ {=0D
+ return EFI_INVALID_PARAMETER;=0D
+ }=0D

This check is unnecessary; the same check is done above.

<snip>
@@ -766,35 +934,51 @@ Dhcp6AppendIaOption (
//=0D
for (Index =3D 0; Index < Ia->IaAddressCount; Index++) {=0D
AddrOpt =3D (UINT8 *)Ia->IaAddress + Index * sizeof (EFI_DHCP6_IA_ADDR=
ESS);=0D
- Buf =3D Dhcp6AppendIaAddrOption (Buf, (EFI_DHCP6_IA_ADDRESS *)Addr=
Opt, MessageType);=0D
+ Status =3D Dhcp6AppendIaAddrOption (Packet, PacketCursor, (EFI_DHCP6_=
IA_ADDRESS *)AddrOpt, MessageType);=0D
+ if (EFI_ERROR (Status)) {=0D
+ return Status;=0D
+ }=0D
}=0D
=0D
//=0D
// Fill the value of Ia option length=0D
//=0D
- *Len =3D HTONS ((UINT16)(Buf - (UINT8 *)Len - 2));=0D
+ *Len =3D HTONS ((UINT16)(*PacketCursor - (UINT8 *)Len - 2));=0D
=0D
- return Buf;=0D
+ //=0D
+ // Update the packet length=0D
+ //=0D
+ Packet->Length +=3D BytesNeeded;=0D

Shouldn't we update Packet->Length before calling Dhcp6AppendIaAddrOption
as it needs to know how much space is left in the Packet?

+=0D
+ return EFI_SUCCESS;=0D
}=0D
=0D
/**=0D
Append the appointed Elapsed time option to Buf, and move Buf to the end=
.=0D
=0D
- @param[in, out] Buf The pointer to the position to append.=0D
+ @param[in, out] Packet A pointer to the packet, on success Packet=
->Length=0D

Nit: Missing "will be updated." like other function header comments.

+ @param[in, out] PacketCursor The pointer in the packet, on success Pack=
etCursor=0D
+ will be moved to the end of the option.=0D
@param[in] Instance The pointer to the Dhcp6 instance.=0D
@param[out] Elapsed The pointer to the elapsed time value in=0D
- the generated packet.=0D
+ the generated packet.=0D
=0D
- @return Buf The position to append the next Ia option.=
=0D
+ @retval EFI_INVALID_PARAMETER An argument provided to the function was=
invalid=0D
+ @retval EFI_BUFFER_TOO_SMALL The buffer is too small to append the op=
tion.=0D
+ @retval EFI_SUCCESS The option is appended successfully.=0D
=0D
**/=0D
-UINT8 *=0D
+EFI_STATUS=0D
Dhcp6AppendETOption (=0D
- IN OUT UINT8 *Buf,=0D
- IN DHCP6_INSTANCE *Instance,=0D
- OUT UINT16 **Elapsed=0D
+ IN OUT EFI_DHCP6_PACKET *Packet,=0D
+ IN OUT UINT8 **PacketCursor,=0D
+ IN DHCP6_INSTANCE *Instance,=0D
+ OUT UINT16 **Elapsed=0D
)=0D
{=0D
+ UINT32 BytesNeeded;=0D
+ UINT32 Length;=0D
+=0D
//=0D
// The format of elapsed time option:=0D
//=0D

Apologies for any poor formatting, this is my first time posting on this
mailing list and I'm using the web interface.

_._,_._,_

Groups.io Links:

You receive all messages sent to this group.

_._,_._,_