Date
1 - 4 of 4
[PATCH v1 2/3] MdeModulePkg/PciBusDxe: Fix possible uninitialized use
|
Sergei Dmitrouk <sergei@...>
If the function gets invalid value for the `ResizableBarOp` parameter
and asserts are disabled, `Bit` can be used uninitialized. Cc: Jian J Wang <jian.j.wang@...> Cc: Hao A Wu <hao.a.wu@...> Cc: Ray Ni <ray.ni@...> Signed-off-by: Sergei Dmitrouk <sergei@...> --- MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c index 6bba28367165..de601713a53b 100644 --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c @@ -1781,6 +1781,11 @@ PciProgramResizableBar ( } else if (ResizableBarOp == PciResizableBarMin) { Bit = LowBitSet64(Capabilities); } else { + // + // Set Bit to avoid uninitialized use when built without assertions. + // + Bit = 0; + ASSERT ((ResizableBarOp == PciResizableBarMax) || (ResizableBarOp == PciResizableBarMin)); } -- 2.17.6
|
|
|
|
Ni, Ray
How about below fix? I think it might be simpler to understand and doesn't introduce unnecessary logic to handle impossible case:
toggle quoted messageShow quoted text
if (ResizableBarOp == PciResizableBarMax) { Bit = HighBitSet64(Capabilities); } else { ASSERT (ResizableBarOp == PciResizableBarMin); Bit = LowBitSet64(Capabilities); }
-----Original Message-----
|
|
|
|
Sergei Dmitrouk <sergei@...>
If the function gets invalid value for the `ResizableBarOp` parameter
and asserts are disabled, `Bit` can be used uninitialized. Cc: Jian J Wang <jian.j.wang@...> Cc: Hao A Wu <hao.a.wu@...> Cc: Ray Ni <ray.ni@...> Signed-off-by: Sergei Dmitrouk <sergei@...> --- Notes: v2: - simplify if-statement to avoid unused branches MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c index 6bba28367165..4caac56f1dcd 100644 --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c @@ -1778,10 +1778,9 @@ PciProgramResizableBar ( if (ResizableBarOp == PciResizableBarMax) { Bit = HighBitSet64(Capabilities); - } else if (ResizableBarOp == PciResizableBarMin) { + } else { + ASSERT (ResizableBarOp == PciResizableBarMin); Bit = LowBitSet64(Capabilities); - } else { - ASSERT ((ResizableBarOp == PciResizableBarMax) || (ResizableBarOp == PciResizableBarMin)); } ASSERT (Bit >= 0); -- 2.17.6
|
|
|
|
Wu, Hao A
toggle quoted messageShow quoted text
-----Original Message----- Hello, Since the V1 is a patch series, I would suggest to send the whole series for V2 changes (even if other patches are unchanged). With this handled: Reviewed-by: Hao A Wu <hao.a.wu@...> Best Regards, Hao Wu
|
|
|