Bug 1951480

Description of problem:

A customer reported that since upgrading his systems from RHEL7 to RHEL8.3, he could see "ipmitool sol activate" command crash constantly when trying to access the remote console.

Coredump analysis below:
-------- 8< ---------------- 8< ---------------- 8< ---------------- 8< --------
Core was generated by `ipmitool -I lanplus -L OPERATOR -H <IP ADDR> -U user-id1 -P XXXXXXXXX sol a'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  __memmove_ssse3 () at ../sysdeps/x86_64/multiarch/memcpy-ssse3.S:2831
2831        movdqu    0x40(%rsi), %xmm4
(gdb) up
#1  0x000055c9c74c1d15 in memmove (__len=<optimized out>, __src=0x55c9c772b816 <rsp+22>, 
    __dest=0x55c9c772b801 <rsp+1>) at /usr/include/bits/string_fortified.h:40
warning: Source file is more recent than executable.
40      return __builtin___memmove_chk (__dest, __src, __len, __bos0 (__dest));
(gdb) up
#2  ipmi_lan_poll_single (intf=intf@entry=0x55c9c7728720 <ipmi_lanplus_intf>) at lanplus.c:819
819                    memmove(rsp->data, rsp->data + offset, extra_data_length);
(gdb) list
814                 * rsp->data_len becomes the length of that data
815                 */
816                extra_data_length = payload_size - (offset - payload_start) - 1;
817                if (extra_data_length) {
818                    rsp->data_len = extra_data_length;
819                    memmove(rsp->data, rsp->data + offset, extra_data_length);
820                } else {
821                    rsp->data_len = 0;
822                }
823            }
(gdb) p rsp->data_len 
$1 = 117440512
-------- 8< ---------------- 8< ---------------- 8< ---------------- 8< --------

From above, the extra_data_length is just huge, it cannot be correct. This causes unallocated memory to be reached.
Below is the source code from RHEL 8.3.0 codebase (src/plugins/lanplus/lanplus.c file):
-------- 8< ---------------- 8< ---------------- 8< ---------------- 8< --------
 811                         /*
 812                          * Good packet.  Shift response data to start of array.
 813                          * rsp->data becomes the variable length IPMI response data
 814                          * rsp->data_len becomes the length of that data
 815                          */
 816                         extra_data_length = payload_size - (offset - payload_start) - 1;
 817                         if (extra_data_length) {
 818                                 rsp->data_len = extra_data_length;
 819                                 memmove(rsp->data, rsp->data + offset, extra_data_length);
 820                         } else {
 821                                 rsp->data_len = 0;
 822                         }
-------- 8< ---------------- 8< ---------------- 8< ---------------- 8< --------

On line 817 we see that the condition is entered (because extra_data_length != 0) and the memmove() happens.
Similar code is seen on the RHEL 7.9 codebase.

However, checking the Upstream code, it's different: the condition is more robust:
-------- 8< ---------------- 8< ---------------- 8< ---------------- 8< --------
 781                         /*
 782                          * Good packet.  Shift response data to start of array.
 783                          * rsp->data becomes the variable length IPMI response data
 784                          * rsp->data_len becomes the length of that data
 785                          */
 786                         extra_data_length = payload_size - (offset - payload_start) - 1;
 787                         if (extra_data_length > 0) {
 788                                 rsp->data_len = extra_data_length;
 789                                 memmove(rsp->data, rsp->data + offset, extra_data_length);
 790                                 offset = 0;
 791                                 payload_start = 0;
 792                                 payload_size = extra_data_length;
 793                         } else {
 794                                 rsp->data_len = 0;
 795                         }
-------- 8< ---------------- 8< ---------------- 8< ---------------- 8< --------

The latter was brought by commit below:
-------- 8< ---------------- 8< ---------------- 8< ---------------- 8< --------
commit 9ec2232321a7bca7e1fb8f939d071f12c8dfa7fd
Author: pjdhpe <44778156+pjdhpe.github.com>
Date:   Wed Nov 28 07:27:29 2018 -0600

    lanplus: Fix segfault for truncated dcmi response

    On occasion a dcmi power reading will return error C6, and a
    truncated response payload. As the decrypted payload is shorter
    than the expected length, lanplus_decrypt_aes_cbc_128() adjusts
    the payload_size downward by one byte. In ipmi_lan_poll_single()
    the calculation to determine if the payload size has increased
    erroniously sets extra_data_length to -1, with a subsequent
    segv when calling a memmove to shift response data.
    The fix is to check for a positive value in the extra_data_length.

    Resolves ipmitool/ipmitool#72
-------- 8< ---------------- 8< ---------------- 8< ---------------- 8< --------

We need this commit to be backported, I gave a test package to the customer and he confirmed it fixes the issue.

Version-Release number of selected component (if applicable):

ipmitool-1.8.18-17.el8_3


How reproducible:

Always on customer systems