Re: [edk2-devel] [PATCH v3 01/12] MdeModulePkg/SmbiosDxe: Fix pointer and buffer overflow CodeQL alerts

2023-02-14 Thread Michael Kubacki
I know the second case was missed, that will be updated. I agree calculating the remaining buffer space is more straightforward here without the library so I'll go with that approach in a v4 of the series. Thanks for the detailed feedback. On 2/14/2023 9:11 AM, Gerd Hoffmann wrote: Hi,

Re: [edk2-devel] [PATCH v3 01/12] MdeModulePkg/SmbiosDxe: Fix pointer and buffer overflow CodeQL alerts

2023-02-14 Thread Gerd Hoffmann
Hi, > [ ... details snipped ... ] > > I'd prefer it if the code were updated to avoid SafeUintnAdd() altogether. > But if not, then at a minimum the redundant check should be removed, and the > calculation involving Smbios.Hdr->Length should also be updated to use > SafeUintnAdd(). Fully

Re: [edk2-devel] [PATCH v3 01/12] MdeModulePkg/SmbiosDxe: Fix pointer and buffer overflow CodeQL alerts

2023-02-14 Thread Michael Kubacki
Either approach works for me. I understand the desire to avoid code bloat that comes with the library. The most common classes of issues I see at the moment are asserts being misused for error handling (which is significant), general issues with integer conversion/evaluation, and unsafe

Re: [edk2-devel] [PATCH v3 01/12] MdeModulePkg/SmbiosDxe: Fix pointer and buffer overflow CodeQL alerts

2023-02-14 Thread Michael Brown
On 14/02/2023 13:01, Gerd Hoffmann wrote: On Mon, Feb 13, 2023 at 04:15:30PM +, Michael Brown wrote: On 13/02/2023 15:48, Michael Kubacki wrote: @@ -1608,9 +1610,12 @@ ParseAndAddExistingSmbiosTable ( // // Make sure not to access memory beyond SmbiosEnd // -if

Re: [edk2-devel] [PATCH v3 01/12] MdeModulePkg/SmbiosDxe: Fix pointer and buffer overflow CodeQL alerts

2023-02-14 Thread Gerd Hoffmann
On Mon, Feb 13, 2023 at 04:15:30PM +, Michael Brown wrote: > On 13/02/2023 15:48, Michael Kubacki wrote: > > @@ -1608,9 +1610,12 @@ ParseAndAddExistingSmbiosTable ( > > // > > // Make sure not to access memory beyond SmbiosEnd > > // > > -if ((Smbios.Raw + sizeof

Re: [edk2-devel] [PATCH v3 01/12] MdeModulePkg/SmbiosDxe: Fix pointer and buffer overflow CodeQL alerts

2023-02-13 Thread Michael Brown
On 13/02/2023 15:48, Michael Kubacki wrote: @@ -1608,9 +1610,12 @@ ParseAndAddExistingSmbiosTable ( // // Make sure not to access memory beyond SmbiosEnd // -if ((Smbios.Raw + sizeof (SMBIOS_STRUCTURE) > SmbiosEnd.Raw) || -(Smbios.Raw + sizeof (SMBIOS_STRUCTURE) <