Skip to content

Commit

Permalink
host-bmc: host_pdr_handler: Don't double-account for pldm_msg_hdr (#423)
Browse files Browse the repository at this point in the history
Removing the size of `struct pldm_msg_hdr` from the buffer size is
already accounted for in pldmd/pldmd.cpp:processRxMsg().

The double accounting leads to a failure to decode the message in
libpldm as the buffer appears too short. With assertions enabled in
libpldm (as it's currently not feasible to disable them) we arrive at
the following abort and (abbreviated) backtrace:

```
    Program terminated with signal SIGABRT, Aborted.
    #0  __pthread_kill_implementation (threadid=<optimized out>, signo=6, no_tid=<optimized out>) at pthread_kill.c:44
    44      pthread_kill.c: No such file or directory.
    #0  __pthread_kill_implementation (threadid=<optimized out>, signo=6, no_tid=<optimized out>) at pthread_kill.c:44
    openbmc#1  0x76403a14 in __GI_raise (sig=sig@entry=6) at /usr/src/debug/glibc/2.36-r0/sysdeps/posix/raise.c:26
    openbmc#2  0x763ee100 in __GI_abort () at abort.c:79
    openbmc#3  0x763fcd34 in __assert_fail_base (fmt=0x20 <error: Cannot access memory at address 0x20>, assertion=0x76cd70c0 "ctx->remaining >= 0",
        assertion@entry=0x76cd708c "/usr/src/debug/libpldm/0.2.0+git999-r1/src/msgbuf.h",
        file=0x76cd708c "/usr/src/debug/libpldm/0.2.0+git999-r1/src/msgbuf.h", file@entry=0x20 <error: Cannot access memory at address 0x20>,
        line=line@entry=188, function=0x76cd75b8 <__PRETTY_FUNCTION__.11.lto_priv.0> "pldm_msgbuf_extract_uint8", function@entry=0x76f06e80 "")
        at assert.c:92
    openbmc#4  0x763fcdd8 in __GI___assert_fail (assertion=0x76cd708c "/usr/src/debug/libpldm/0.2.0+git999-r1/src/msgbuf.h",
        file=0x20 <error: Cannot access memory at address 0x20>, line=line@entry=188, function=0x76f06e80 "") at assert.c:101
    openbmc#5  0x76cc6998 in pldm_msgbuf_extract_uint8 (dst=<optimized out>, ctx=<optimized out>)
        at /usr/src/debug/libpldm/0.2.0+git999-r1/src/msgbuf.h:188
    openbmc#6  0x76cc95e0 in pldm_msgbuf_extract_uint8 (dst=<optimized out>, ctx=<optimized out>)
        at /usr/src/debug/libpldm/0.2.0+git999-r1/src/msgbuf.h:63
    openbmc#7  decode_get_state_sensor_readings_resp (msg=<optimized out>, payload_length=<optimized out>, completion_code=<optimized out>,
        comp_sensor_count=<optimized out>, field=0x7edfb5e4) at /usr/src/debug/libpldm/0.2.0+git999-r1/src/platform.c:796
    openbmc#8  0x76e20adc in pldm::HostPDRHandler::getPresentStateBySensorReadigs(unsigned char const&, unsigned short, unsigned short, unsigned short, unsigned short, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, unsigned short)::{lambda(unsigned char, pldm_msg const*, unsigned int)openbmc#1}::operator()(unsigned char, pldm_msg const*, unsigned int) const [clone .constprop.0] (
        __closure=0x2459a20, response=0x20cf93a, respMsgLen=6)
        at /usr/src/debug/pldm/1.0+gitAUTOINC+14305952d9-r1/host-bmc/host_pdr_handler.cpp:1508
    ...
```

Due to optimisations the line called out in frame 7 is not correct.
Instead execution has moved beyond extraction of the completion code and
into extraction of the sensor reading data. However, the payload length
passed to decode_get_state_sensor_readings_resp() claims the buffer
terminates before the last required field of the sensor reading data,
giving rise to the assertion.

Tested: Booted a p10bmc system with the patch applied, plus a
        downstream patch rebased on openbmc/libpldm@5dc025719dc3 ("pdr:
        Add pldm_pdr_find_container_id_range_exclude() API")


Change-Id: I8c9d610ff7c96d63061f3d9d1437035a8da5bfa5

Signed-off-by: Andrew Jeffery <[email protected]>
  • Loading branch information
amboar authored Jun 26, 2023
1 parent 77d66f9 commit 81778fe
Showing 1 changed file with 1 addition and 2 deletions.
3 changes: 1 addition & 2 deletions host-bmc/host_pdr_handler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1506,8 +1506,7 @@ void HostPDRHandler::getPresentStateBySensorReadigs(
std::array<get_sensor_state_field, 8> stateField{};
auto responsePtr = reinterpret_cast<const struct pldm_msg*>(response);
auto rc = decode_get_state_sensor_readings_resp(
responsePtr, respMsgLen - sizeof(pldm_msg_hdr), &cc, &sensorCnt,
stateField.data());
responsePtr, respMsgLen, &cc, &sensorCnt, stateField.data());
if (rc != PLDM_SUCCESS || cc != PLDM_SUCCESS)
{
std::cerr << "Faile to decode get state sensor readings resp, "
Expand Down

0 comments on commit 81778fe

Please sign in to comment.