Hardfault triggered in SMBus::block_read

1

I ran into a hardfault when reading Smart Battery data with SMBus::block_read.

The fault is triggered in line 126 of
/Firmware/src/lib/drivers/smbus/SMBus.cpp
when memcpy writes beyond the end of the buffer “data” provided by the caller.

In my case the smbus device returned a byte count larger than the requested length (13 instead of 8 bytes).

I fixed this by adding a check for the length limit before the memcpy call, see

Should I open a pull request?

2

You could but you might want to take a look at the code.
memcpy(data, &rx_data[4], byte_count);
should be
memcpy(data, &rx_data[4], cpy_len);
Or your change will not do anything.

3

Yes let’s fix this, can you create a PR?

4

I’ll review jimdgit’s hint, then PR.

5

Thanks for the catch @jimdgit! Seems I made that mistake copying over from my working code.

6

@bkueng: added PR #15789

7

You might also consider this but of code:

memcpy(data, &rx_data[4], (byte_count > length) ? length : byte_count);

  • no temp variable required.
  • localizes the code and makes it more obvious the purpose of the code.
  • easier to read.
    Of course we should really try and find out why too many bytes are returned, but I can see why that might not be feasible.