AhciDumpPortStatus doesn't fully populate all the fields of the parameter EFI_ATA_STATUS_BLOCK *AtaStatusBlock #3

luonanh · 2014-04-08T18:41:06Z

AhciDumpPortStatus in MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c doesn't fully populate ALL the fields of the parameter EFI_ATA_STATUS_BLOCK *AtaStatusBlock. As a result, utilities depend on the return status may give wrong information. Is this work as designed and consumers will implement this themselves or is this a bug?

Thanks,
Anh Luong

Before the merger of the authenticated and non-authenticated variable drivers (commit fa0737a), we had to match the varstore header GUID in "OvmfPkg/VarStore.fdf.inc" to SECURE_BOOT_ENABLE, because the opposite GUID would cause either driver to fail an assertion. The header structures for individual variables residing in the varstore were different (VARIABLE_HEADER vs. AUTHENTICATED_VARIABLE_HEADER), and each driver could only handle its own, so this GUID enforcement was necessary. Since the unification of the variable driver however, it treats (a) variable store format, and (b) AuthVariableLib instance as independent characteristics; it can always manipulate variable stores with both header types. All variations boot now; the difference is whether authenticated variables, and special variables computed from them (like SecureBoot) are supported at runtime: variable store non-auth auth and SB header GUID AuthVariableLib variables variables -- --------------------- ------------------- -> --------- ----------- 1 Variable SecurityPkg/... supported unsupported 2 Variable AuthVariableLibNull supported unsupported 3 AuthenticatedVariable SecurityPkg/... supported supported 4 AuthenticatedVariable AuthVariableLibNull supported unsupported At the moment, SECURE_BOOT_ENABLE selects between cases #2 (FALSE) and #3 (TRUE). That is, it controls both the varstore header GUID in "OvmfPkg/VarStore.fdf.inc", and the AuthVariableLib resolution in the DSC files. Exploiting the unified driver's flexibility, we can simplify "OvmfPkg/VarStore.fdf.inc" by picking the AuthenticatedVariable GUID as a constant, and letting SECURE_BOOT_ENABLE control only the AuthVariableLib resolution. This amounts to SECURE_BOOT_ENABLE choosing between cases #3 (TRUE) and #4 (FALSE), with identical results as before. Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Star Zeng <star.zeng@intel.com> Ref: http://thread.gmane.org/gmane.comp.bios.edk2.devel/7319/focus=7344 Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Jordan Justen <jordan.l.justen@intel.com> Reviewed-by: Star Zeng <star.zeng@intel.com>

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4536 Bug Overview: PixieFail Bug tianocore#3 CVE-2023-45231 CVSS 6.5 : CVSS:3.1/AV:A/AC:L/PR:N/UI:N/S:U/C:H/I:N/A:N CWE-125 Out-of-bounds Read Out-of-bounds read when handling a ND Redirect message with truncated options Change Overview: Adds a check to prevent truncated options from being parsed + // + // Cannot process truncated options. + // Cannot process options with a length of 0 as there is no Type field. + // + if (OptionLen < sizeof (IP6_OPTION_HEADER)) { + return FALSE; + } Cc: Saloni Kasbekar <saloni.kasbekar@intel.com> Cc: Zachary Clark-williams <zachary.clark-williams@intel.com> Signed-off-by: Doug Flick [MSFT] <doug.edk2@gmail.com>

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4536 Bug Overview: PixieFail Bug tianocore#3 CVE-2023-45231 CVSS 6.5 : CVSS:3.1/AV:A/AC:L/PR:N/UI:N/S:U/C:H/I:N/A:N CWE-125 Out-of-bounds Read Out-of-bounds read when handling a ND Redirect message with truncated options Change Overview: Adds a check to prevent truncated options from being parsed + // + // Cannot process truncated options. + // Cannot process options with a length of 0 as there is no Type field. + // + if (OptionLen < sizeof (IP6_OPTION_HEADER)) { + return FALSE; + } Cc: Saloni Kasbekar <saloni.kasbekar@intel.com> Cc: Zachary Clark-williams <zachary.clark-williams@intel.com> Signed-off-by: Doug Flick [MSFT] <doug.edk2@gmail.com> Reviewed-by: Saloni Kasbekar <saloni.kasbekar@intel.com>

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4536 Bug Overview: PixieFail Bug #3 CVE-2023-45231 CVSS 6.5 : CVSS:3.1/AV:A/AC:L/PR:N/UI:N/S:U/C:H/I:N/A:N CWE-125 Out-of-bounds Read Out-of-bounds read when handling a ND Redirect message with truncated options Change Overview: Adds a check to prevent truncated options from being parsed + // + // Cannot process truncated options. + // Cannot process options with a length of 0 as there is no Type field. + // + if (OptionLen < sizeof (IP6_OPTION_HEADER)) { + return FALSE; + } Cc: Saloni Kasbekar <saloni.kasbekar@intel.com> Cc: Zachary Clark-williams <zachary.clark-williams@intel.com> Signed-off-by: Doug Flick [MSFT] <doug.edk2@gmail.com> Reviewed-by: Saloni Kasbekar <saloni.kasbekar@intel.com>

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4536 Bug Overview: PixieFail Bug tianocore#3 CVE-2023-45231 CVSS 6.5 : CVSS:3.1/AV:A/AC:L/PR:N/UI:N/S:U/C:H/I:N/A:N CWE-125 Out-of-bounds Read Out-of-bounds read when handling a ND Redirect message with truncated options Change Overview: Adds a check to prevent truncated options from being parsed + // + // Cannot process truncated options. + // Cannot process options with a length of 0 as there is no Type field. + // + if (OptionLen < sizeof (IP6_OPTION_HEADER)) { + return FALSE; + } Bug 4457168 Cc: Saloni Kasbekar <saloni.kasbekar@intel.com> Cc: Zachary Clark-williams <zachary.clark-williams@intel.com> Signed-off-by: Doug Flick [MSFT] <doug.edk2@gmail.com> Reviewed-by: Saloni Kasbekar <saloni.kasbekar@intel.com> Change-Id: Ib705772399deddbfdb871e5b52c1ae02fad67621 Reviewed-on: https://git-master.nvidia.com/r/c/3rdparty/edk2/+/3073484 Reviewed-by: svcacv <svcacv@nvidia.com> Reviewed-by: svc-sw-mobile-l4t <svc-sw-mobile-l4t@nvidia.com> Reviewed-by: Jeff Brasen <jbrasen@nvidia.com> GVS: Gerrit_Virtual_Submit <buildbot_gerritrpt@nvidia.com> Tested-by: Jeff Brasen <jbrasen@nvidia.com>

This patch does not impact functionality. It aims to clarify the synchronization flow between the BSP and APs to enhance code readability and understanding: Steps tianocore#6 and tianocore#11 are the basic synchronization requirements for all cases. Steps tianocore#1 and tianocore#2 are additional requirements if the MmCpuSyncModeTradition mode is selected. Steps tianocore#1, tianocore#2, tianocore#3, tianocore#4, tianocore#5, tianocore#7, tianocore#8, tianocore#9, and tianocore#10 are additional requirements if the system needs to configure the MTRR. Steps tianocore#9 and tianocore#10 are additional requirements if the system needs to support the mSmmDebugAgentSupport. Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>

This patch does not impact functionality. It aims to clarify the synchronization flow between the BSP and APs to enhance code readability and understanding: Steps tianocore#6 and tianocore#11 are the basic synchronization requirements for all cases. Steps tianocore#1 is additional requirements if the MmCpuSyncModeTradition mode is selected. Steps tianocore#1, tianocore#2, tianocore#3, tianocore#4, tianocore#5, tianocore#7, tianocore#8, tianocore#9, and tianocore#10 are additional requirements if the system needs to configure the MTRR. Steps tianocore#9 and tianocore#10 are additional requirements if the system needs to support the mSmmDebugAgentSupport. Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>

This patch does not impact functionality. It aims to clarify the synchronization flow between the BSP and APs to enhance code readability and understanding: Steps #6 and #11 are the basic synchronization requirements for all cases. Steps #1 is additional requirements if the MmCpuSyncModeTradition mode is selected. Steps #1, #2, #3, #4, #5, #7, #8, #9, and #10 are additional requirements if the system needs to configure the MTRR. Steps #9 and #10 are additional requirements if the system needs to support the mSmmDebugAgentSupport. Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>

This patch does not impact functionality. It aims to clarify the synchronization flow between the BSP and APs to enhance code readability and understanding: Steps tianocore#6 and tianocore#11 are the basic synchronization requirements for all cases. Steps tianocore#1 is additional requirements if the MmCpuSyncModeTradition mode is selected. Steps tianocore#1, tianocore#2, tianocore#3, tianocore#4, tianocore#5, tianocore#7, tianocore#8, tianocore#9, and tianocore#10 are additional requirements if the system needs to configure the MTRR. Steps tianocore#9 and tianocore#10 are additional requirements if the system needs to support the mSmmDebugAgentSupport. Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>

lersek added the MdeModulePkg label Feb 2, 2016

mdkinney removed the MdeModulePkg label Nov 12, 2019

mdkinney closed this as completed Nov 29, 2022

jyao1 mentioned this issue Feb 3, 2023

Enable Tdx measurement in OvmfPkgX64 #3993

Merged

mdkinney mentioned this issue Feb 14, 2024

Bug 4683 unit test unexpected exception may be seen as PASS condition #5345

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AhciDumpPortStatus doesn't fully populate all the fields of the parameter EFI_ATA_STATUS_BLOCK *AtaStatusBlock #3

AhciDumpPortStatus doesn't fully populate all the fields of the parameter EFI_ATA_STATUS_BLOCK *AtaStatusBlock #3

luonanh commented Apr 8, 2014

AhciDumpPortStatus doesn't fully populate all the fields of the parameter EFI_ATA_STATUS_BLOCK *AtaStatusBlock #3

AhciDumpPortStatus doesn't fully populate all the fields of the parameter EFI_ATA_STATUS_BLOCK *AtaStatusBlock #3

Comments

luonanh commented Apr 8, 2014