Now the logic of the function is less convoluted, drop some redundant
casts, variable initialisations and move the MiddleCount definition
into the only block where it's used.
Signed-off-by: Leif Lindholm <leif.lindholm@oss.qualcomm.com>
A multi-part request can consist of a Start, zero-to-several Middle,
and an End packet. For what I can only assume was an attempt to
confuse the enemy, SsifWriteRequest () handled this by setting up
three separate loops.
Rewrite this as a single loop in order to reduce confusion for
revewers and compilers.
Signed-off-by: Leif Lindholm <leif.lindholm@oss.qualcomm.com>
From the IPMI v2.0 (April 21, 2015 E7 Markup) specification,
Table 22-12, Get System Interface Capabilities Command:
"Input message size in bytes. (1 based.)"
...
"A BMC that supports multi-part Start and End would return
a value from 33 to 64. A BMC that supports multi-part with Middle
transactions would return a value from 65 to 255."
Yet the comment in the existing code describes this as being a
counter of the number of middle packets, with Start and End packets
counting outside of that. This seems very incorrect to me.
Address this, and simplify the code, by using the already existing
global variable mMaxRequestSize. Update the debug error message
text to reflect the functional change.
Signed-off-by: Leif Lindholm <leif.lindholm@oss.qualcomm.com>
For some reason this module was created "backwards", with .inf files
in subdirectories, as well as identical apart from failes to keep in
sync source files for Dxe and Pei drivers.
Flip the thing the right way around and delete the duplicated files.
Signed-off-by: Leif Lindholm <leif.lindholm@oss.qualcomm.com>
While reviewing PR #12035, I found this function a bit overcomplicated,
with a bunch of live-coded integers, so I reworked it for improved
human and compiler readability.
Signed-off-by: Leif Lindholm <leif.lindholm@oss.qualcomm.com>
In preparation for the MarkdownLintCheck plugin being added to the
repo, this change defaults the plugin to `AuditOnly` mode in each
package. This allows package maintainers to enable the plugin as they
see fit.
Continuous-integration-options: PatchCheck.ignore-multi-package
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Fix vulnerabilities and a memory leak in the IPMI Blob Transfer driver:
- Prevent integer underflow and OOB read by validating response size before
accessing CompletionCode, OEN, and CRC.
- Prevent buffer overflow by ensuring BMC response size does not exceed the
caller's buffer capacity, avoiding potential memory overwriting.
- Fix memory leak by freeing IpmiResponseData and IpmiSendData.
Additionally, fix unit test failures caused by the changes and pre-existing bugs:
- Allocate a safe fixed size (70 bytes) for IpmiResponseData in
IpmiBlobTransferSendIpmi to prevent heap overflow if the BMC returns
more data than expected. Add validation to check if returned size
exceeds this allocation.
- Modify IpmiBlobTransferStat and IpmiBlobTransferSessionStat to only
copy MetaDataLen bytes of metadata instead of always copying the max
size (64 bytes), preventing buffer overflow when caller allocates
a smaller buffer.
- Add missing ASSERT(FALSE) in IpmiBlobTransferStat parameter validation.
- Fix IpmiBlobTransferWriteMeta parameter validation to allow 0-length
writes (Data can be NULL when WriteLength is 0), which was causing
test failure.
- Fix syntax errors in unit tests where arrays were assigned values
after declaration using brace-enclosed lists.
- Fix sizeof misuse on macros representing size values in unit tests,
which caused too small allocations and buffer overflows in mock setups.
- Fix queue order in OpenValidResponse test to match the actual call
order (GetCount, Enumerate, Open) and free all mock buffers.
- Remove unnecessary MockIpmiSubmitCommand calls from invalid buffer
tests to prevent mock queue leaks to subsequent tests.
- Fix memory leaks of ExpectedMetadata in unit tests by changing them
to static arrays.
Signed-off-by: Phineas Su <pohaosu@google.com>
In HelperManageabilityPayLoadDebugPrint(), Page256 is only assigned
inside an if block within the while loop but is accessed outside of
it, causing an uninitialized variable warning.
Initialize Page256 to zero before the while loop to fix this.
Signed-off-by: Tuan Phan <tuan.phan@oss.qualcomm.com>
In SsifWriteRequest(), MiddleCount is only assigned inside an if
block but is accessed in a subsequent loop, causing an uninitialized
variable warning.
Initialize MiddleCount to zero at declaration to fix this.
Signed-off-by: Tuan Phan <tuan.phan@oss.qualcomm.com>
When building ManageabilityPkg with `-a X64 -t CLANGDWARF`, error
message is throwed:
BaseManageabilityTransportHelper.c:462:3: error:
'__builtin_ms_va_start' used in System V ABI function
462 | VA_START (Marker, Format);
| ^
Functions that call VA_START()/VA_END() must be declared with EFIAPI. The
function HelperManageabilityDebugPrint() should also be declared with
EFIAPI to ensure compliance with Microsoft X64 calling convention for
CLANG.
Signed-off-by: Qihang Gao <gaoqihang@loongson.cn>
Replace the hardcoded IPMI_KCS_TIMEOUT_5_SEC macro in WaitStatusSet() and
WaitStatusClear() with PcdKcsStatusCheckTimeout so platform firmware can
tune the KCS timeout without recompiling the library.
- ManageabilityPkg.dec: declare PcdKcsStatusCheckTimeout (UINT64, 5000000 us)
- KcsCommon.c: use PcdGet64(PcdKcsStatusCheckTimeout); add DEBUG on timeout
- ManageabilityTransportKcs.h: remove now-unused IPMI_KCS_TIMEOUT_5_SEC macro
- BaseManageabilityTransportKcs.inf, Dxe/DxeManageabilityTransportKcs.inf:
add PcdKcsStatusCheckTimeout to [FixedPcd]
Co-authored-by: Abner Chang <abner.chang@amd.com>
Signed-off-by: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>
ManageabilityPkg is migrated from edk2-platform repository.
The last commit ID of ManageabilityPkg on edk2-platform is
<4538a1b94c0e55d58e3841b2d640df7da0a3b149>.
Signed-off-by: Abner Chang <abner.chang@amd.com>
