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

msg param in mctp_message_tx should be const ptr and ptr to const #4

Open
crgeddes opened this issue Jun 1, 2020 · 0 comments
Open

msg param in mctp_message_tx should be const ptr and ptr to const #4

crgeddes opened this issue Jun 1, 2020 · 0 comments

Comments

@crgeddes
Copy link
Contributor

crgeddes commented Jun 1, 2020

libmctp/core.c

Lines 610 to 611 in ba6727e

int mctp_message_tx(struct mctp *mctp, mctp_eid_t eid,
void *msg, size_t msg_len)

I appears that the value stored at the pointer and the value of the pointer itself are never altered so they should be labeled as const.
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