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

When writing KCS status register the bmc should write Status reg before writing to ODR #1

Open
crgeddes opened this issue Jan 22, 2020 · 0 comments
Open

When writing KCS status register the bmc should write Status reg before writing to ODR #1

crgeddes opened this issue Jan 22, 2020 · 0 comments

Comments

@crgeddes
Copy link
Contributor

crgeddes commented Jan 22, 2020 •
edited
Loading

I think we are leaving ourselves open to a race condition if we set the ODR first. Setting the ODR will trigger the OBF bit in the status register which could trigger the host to read right away. If this happened faster than the bmc writes the status then we could miss whatever special status was trying to be written. The function I am referencing is in astlpc.c :

static int mctp_astlpc_kcs_set_status(struct mctp_binding_astlpc *astlpc,
    uint8_t status)
bradbishop pushed a commit that referenced this issue Mar 10, 2020
@amboar
test_seq: Clean up after each test case
c68d96e 
Free all allocated memory to avoid false-positive leak reports.
Resolves:

    ==11106==ERROR: LeakSanitizer: detected memory leaks

    Indirect leak of 12288 byte(s) in 3 object(s) allocated from:
        #0 0x7f92a1347f1e in __interceptor_realloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10df1e)
        #1 0x7f92a1207065 in __mctp_realloc /home/andrew/src/openbmc/libmctp/alloc.c:48
        #2 0x7f92a1201979 in mctp_msg_ctx_add_pkt /home/andrew/src/openbmc/libmctp/core.c:213
        #3 0x7f92a1204a09 in mctp_bus_rx /home/andrew/src/openbmc/libmctp/core.c:383
        #4 0x55ff747e1655 in mctp_binding_test_rx_raw tests/test-utils.c:50
        #5 0x55ff747df302 in run_one_test tests/test_seq.c:122
        #6 0x55ff747df6d6 in main tests/test_seq.c:141
        #7 0x7f92a06b21e2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x271e2)

    Indirect leak of 2208 byte(s) in 4 object(s) allocated from:
        #0 0x7f92a1347ae8 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10dae8)
        #1 0x7f92a1206cf0 in __mctp_alloc /home/andrew/src/openbmc/libmctp/alloc.c:28
        #2 0x7f92a1201edc in mctp_init /home/andrew/src/openbmc/libmctp/core.c:234
        #3 0x55ff747e1738 in mctp_test_stack_init tests/test-utils.c:63
        #4 0x55ff747def62 in run_one_test tests/test_seq.c:111
        #5 0x55ff747df6d6 in main tests/test_seq.c:141
        #6 0x7f92a06b21e2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x271e2)

    Indirect leak of 224 byte(s) in 4 object(s) allocated from:
        #0 0x7f92a1347ae8 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10dae8)
        #1 0x7f92a1206cf0 in __mctp_alloc /home/andrew/src/openbmc/libmctp/alloc.c:28
        #2 0x55ff747e1169 in mctp_binding_test_init tests/test-utils.c:27
        #3 0x55ff747e17b1 in mctp_test_stack_init tests/test-utils.c:66
        #4 0x55ff747def62 in run_one_test tests/test_seq.c:111
        #5 0x55ff747df6d6 in main tests/test_seq.c:141
        #6 0x7f92a06b21e2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x271e2)

    Indirect leak of 160 byte(s) in 4 object(s) allocated from:
        #0 0x7f92a1347ae8 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10dae8)
        #1 0x7f92a1206cf0 in __mctp_alloc /home/andrew/src/openbmc/libmctp/alloc.c:28
        #2 0x7f92a120255f in mctp_register_bus /home/andrew/src/openbmc/libmctp/core.c:277
        #3 0x55ff747e16eb in mctp_binding_test_register_bus tests/test-utils.c:56
        #4 0x55ff747e18d5 in mctp_test_stack_init tests/test-utils.c:69
        #5 0x55ff747def62 in run_one_test tests/test_seq.c:111
        #6 0x55ff747df6d6 in main tests/test_seq.c:141
        #7 0x7f92a06b21e2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x271e2)

Signed-off-by: Andrew Jeffery <[email protected]>
Change-Id: I7fbc729740357eb89b679dd51bbf03d6735a75dc
bradbishop pushed a commit that referenced this issue Mar 10, 2020
@amboar
test_eid: Clean up after test case
7c73801 
Free all allocated memory to avoid false-positive leak reports.
Resolves:

    ==10460==ERROR: LeakSanitizer: detected memory leaks

    Indirect leak of 552 byte(s) in 1 object(s) allocated from:
        #0 0x7f3e1a574ae8 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10dae8)
        #1 0x7f3e1a433cf0 in __mctp_alloc /home/andrew/src/openbmc/libmctp/alloc.c:28
        #2 0x7f3e1a42eedc in mctp_init /home/andrew/src/openbmc/libmctp/core.c:234
        #3 0x55f705f81268 in mctp_test_stack_init tests/test-utils.c:63
        #4 0x55f705f7edff in main tests/test_eid.c:51
        #5 0x7f3e198df1e2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x271e2)

    Indirect leak of 56 byte(s) in 1 object(s) allocated from:
        #0 0x7f3e1a574ae8 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10dae8)
        #1 0x7f3e1a433cf0 in __mctp_alloc /home/andrew/src/openbmc/libmctp/alloc.c:28
        #2 0x55f705f80c99 in mctp_binding_test_init tests/test-utils.c:27
        #3 0x55f705f812e1 in mctp_test_stack_init tests/test-utils.c:66
        #4 0x55f705f7edff in main tests/test_eid.c:51
        #5 0x7f3e198df1e2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x271e2)

    Indirect leak of 40 byte(s) in 1 object(s) allocated from:
        #0 0x7f3e1a574ae8 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10dae8)
        #1 0x7f3e1a433cf0 in __mctp_alloc /home/andrew/src/openbmc/libmctp/alloc.c:28
        #2 0x7f3e1a42f55f in mctp_register_bus /home/andrew/src/openbmc/libmctp/core.c:277
        #3 0x55f705f8121b in mctp_binding_test_register_bus tests/test-utils.c:56
        #4 0x55f705f81405 in mctp_test_stack_init tests/test-utils.c:69
        #5 0x55f705f7edff in main tests/test_eid.c:51
        #6 0x7f3e198df1e2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x271e2)

Signed-off-by: Andrew Jeffery <[email protected]>
Change-Id: I34eae31219d4a31e8388d180be746ae8b7ec5c04
bradbishop pushed a commit that referenced this issue Mar 10, 2020
@amboar
test_astlpc: Clean up after test case
11b7e92 
Free all allocated memory to avoid false-positive leak reports.

Resolves:

    ==11807==ERROR: LeakSanitizer: detected memory leaks

    Indirect leak of 552 byte(s) in 1 object(s) allocated from:
        #0 0x7f74718fbae8 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10dae8)
        #1 0x7f74717bacf0 in __mctp_alloc /home/andrew/src/openbmc/libmctp/alloc.c:28
        #2 0x7f74717b5edc in mctp_init /home/andrew/src/openbmc/libmctp/core.c:234
        #3 0x56157917c7ee in main tests/test_astlpc.c:160
        #4 0x7f7470c661e2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x271e2)

    Indirect leak of 384 byte(s) in 1 object(s) allocated from:
        #0 0x7f74718fbae8 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10dae8)
        #1 0x7f74717bacf0 in __mctp_alloc /home/andrew/src/openbmc/libmctp/alloc.c:28
        #2 0x7f74717c2cd0 in __mctp_astlpc_init /home/andrew/src/openbmc/libmctp/astlpc.c:364
        #3 0x7f74717c31f0 in mctp_astlpc_init_ops /home/andrew/src/openbmc/libmctp/astlpc.c:388
        #4 0x56157917c88f in main tests/test_astlpc.c:165
        #5 0x7f7470c661e2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x271e2)

    Indirect leak of 40 byte(s) in 1 object(s) allocated from:
        #0 0x7f74718fbae8 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10dae8)
        #1 0x7f74717bacf0 in __mctp_alloc /home/andrew/src/openbmc/libmctp/alloc.c:28
        #2 0x7f74717b655f in mctp_register_bus /home/andrew/src/openbmc/libmctp/core.c:277
        #3 0x56157917c903 in main tests/test_astlpc.c:167
        #4 0x7f7470c661e2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x271e2)

    Indirect leak of 32 byte(s) in 1 object(s) allocated from:
        #0 0x7f74718fbae8 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10dae8)
        #1 0x7f74717bacf0 in __mctp_alloc /home/andrew/src/openbmc/libmctp/alloc.c:28
        #2 0x7f74717c34b2 in mctp_astlpc_init_ops /home/andrew/src/openbmc/libmctp/astlpc.c:400
        #3 0x56157917c88f in main tests/test_astlpc.c:165
        #4 0x7f7470c661e2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x271e2)

Signed-off-by: Andrew Jeffery <[email protected]>
Change-Id: I3f67e48b22948e18aea91d6fa28135e528268bc1
bradbishop pushed a commit that referenced this issue Mar 10, 2020
@amboar
test_serial: Clean up after test case
f0315a3 
Free all allocated memory to avoid false-positives from leak sanitizers.
Resolves:

    ==15807==ERROR: LeakSanitizer: detected memory leaks

    Indirect leak of 4096 byte(s) in 1 object(s) allocated from:
        #0 0x7fb35b934f1e in __interceptor_realloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10df1e)
        #1 0x7fb35b7f3065 in __mctp_realloc /home/andrew/src/openbmc/libmctp/alloc.c:48
        #2 0x7fb35b7ed979 in mctp_msg_ctx_add_pkt /home/andrew/src/openbmc/libmctp/core.c:213
        #3 0x7fb35b7f0a09 in mctp_bus_rx /home/andrew/src/openbmc/libmctp/core.c:383
        #4 0x7fb35b7f467d in mctp_serial_finish_packet /home/andrew/src/openbmc/libmctp/serial.c:169
        #5 0x7fb35b7f6071 in mctp_rx_consume_one /home/andrew/src/openbmc/libmctp/serial.c:254
        #6 0x7fb35b7f6409 in mctp_rx_consume /home/andrew/src/openbmc/libmctp/serial.c:271
        #7 0x7fb35b7f668a in mctp_serial_read /home/andrew/src/openbmc/libmctp/serial.c:288
        #8 0x559680986cc7 in main tests/test_serial.c:113
        #9 0x7fb35ac9e1e2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x271e2)

    Indirect leak of 1384 byte(s) in 1 object(s) allocated from:
        #0 0x7fb35b934ae8 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10dae8)
        #1 0x7fb35b7f2cf0 in __mctp_alloc /home/andrew/src/openbmc/libmctp/alloc.c:28
        #2 0x7fb35b7f6cad in mctp_serial_init /home/andrew/src/openbmc/libmctp/serial.c:343
        #3 0x559680986553 in main tests/test_serial.c:99
        #4 0x7fb35ac9e1e2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x271e2)

    Indirect leak of 552 byte(s) in 1 object(s) allocated from:
        #0 0x7fb35b934ae8 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10dae8)
        #1 0x7fb35b7f2cf0 in __mctp_alloc /home/andrew/src/openbmc/libmctp/alloc.c:28
        #2 0x7fb35b7ededc in mctp_init /home/andrew/src/openbmc/libmctp/core.c:234
        #3 0x559680986485 in main tests/test_serial.c:96
        #4 0x7fb35ac9e1e2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x271e2)

    Indirect leak of 40 byte(s) in 1 object(s) allocated from:
        #0 0x7fb35b934ae8 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10dae8)
        #1 0x7fb35b7f2cf0 in __mctp_alloc /home/andrew/src/openbmc/libmctp/alloc.c:28
        #2 0x7fb35b7ee55f in mctp_register_bus /home/andrew/src/openbmc/libmctp/core.c:277
        #3 0x559680986b64 in main tests/test_serial.c:105
        #4 0x7fb35ac9e1e2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x271e2)

Signed-off-by: Andrew Jeffery <[email protected]>
Change-Id: I9f95104545cc6f633ee134fffc90cb0ea2c46eeb
bradbishop pushed a commit that referenced this issue Nov 6, 2020
@amboar
core: Fix large packet buffer overrun
c2b833e 
The astlpc binding allows negotiation of Tx/Rx region sizes, but the
packet accumulator assumed packet sizes were at most 4096 bytes.  Avoid
buffer overflow by allocating at least the length of the inbound packet
if we have not yet initialised the packet buffer.

Fixes:

=================================================================
==42296==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x621000002500 at pc 0x7ff8a22235ce bp 0x7ffd47469750 sp 0x7ffd47468ef8
WRITE of size 8192 at 0x621000002500 thread T0
    #0 0x7ff8a22235cd in __interceptor_memcpy (/usr/lib/x86_64-linux-gnu/libasan.so.6+0x3a5cd)
    #1 0x7ff8a21ac78b in memcpy /usr/include/x86_64-linux-gnu/bits/string_fortified.h:34
    #2 0x7ff8a21ac78b in mctp_msg_ctx_add_pkt /home/andrew/src/openbmc/libmctp/core.c:237
    #3 0x7ff8a21af245 in mctp_bus_rx /home/andrew/src/openbmc/libmctp/core.c:495
    #4 0x56458d3f9648 in mctp_astlpc_rx_start astlpc.c:813
    #5 0x56458d3f9648 in mctp_astlpc_poll astlpc.c:931
    #6 0x56458d3fc1f4 in astlpc_test_send_large_packet tests/test_astlpc.c:1111
    #7 0x56458d3efc86 in main tests/test_astlpc.c:1185
    #8 0x7ff8a165dcb1 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x28cb1)
    #9 0x56458d3efe7d in _start (/home/andrew/src/openbmc/libmctp/tests/.libs/test_astlpc+0x17e7d)

0x621000002500 is located 0 bytes to the right of 4096-byte region [0x621000001500,0x621000002500)
allocated by thread T0 here:
    #0 0x7ff8a22998d0 in __interceptor_realloc (/usr/lib/x86_64-linux-gnu/libasan.so.6+0xb08d0)
    #1 0x7ff8a21b0533 in __mctp_realloc /home/andrew/src/openbmc/libmctp/alloc.c:48

SUMMARY: AddressSanitizer: heap-buffer-overflow (/usr/lib/x86_64-linux-gnu/libasan.so.6+0x3a5cd) in __interceptor_memcpy
Shadow bytes around the buggy address:
  0x0c427fff8450: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c427fff8460: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c427fff8470: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c427fff8480: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c427fff8490: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c427fff84a0:[fa]fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c427fff84b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c427fff84c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c427fff84d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c427fff84e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c427fff84f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==42296==ABORTING

Signed-off-by: Andrew Jeffery <[email protected]>
Change-Id: I9d39090cb9246ec2f6c06942d4f2a91fe0df0202
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
@crgeddes