Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

libpldm: decode PLDM getversion response is not supporting multiple versions #5

Open
PriyangaRamasamy opened this issue Jun 7, 2019 · 0 comments
Open

libpldm: decode PLDM getversion response is not supporting multiple versions #5

PriyangaRamasamy opened this issue Jun 7, 2019 · 0 comments

Comments

@PriyangaRamasamy
Copy link
Contributor

In encode, the caller provides the size of the version. But in decode, version size is considered to be ver32_t as a fixed size.
RashmicaG pushed a commit to RashmicaG/pldm that referenced this issue Jul 6, 2023
@amboar
host-bmc: host_pdr_handler: Don't double-account for pldm_msg_hdr (#423)
81778fe 
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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@PriyangaRamasamy