On Thu, 12 Feb 2026 at 17:41, Corey Minyard <[email protected]> wrote:
>
> From: Yunpeng Yang <[email protected]>
>
> The "Set LAN Configuration Parameters" IPMI command is added to the
> `ipmi_bmc_sim` device to support dynamically setting fake LAN channel
> configurations. With the fake LAN channel enabled, inside the guest OS,
> tools such as `ipmitool` can be used to modify the configurations.
>
> Signed-off-by: Yunpeng Yang <[email protected]>
> Message-ID: <[email protected]>
> Signed-off-by: Corey Minyard <[email protected]>
> ---
> hw/ipmi/ipmi_bmc_sim.c | 110 +++++++++++++++++++++++++++
> tests/qtest/ipmi-kcs-test.c | 143 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 253 insertions(+)
Hi; Coverity notices a minor nit in this commit (CID 1644838):
> +/*
> + * Request data (from cmd[2] on):
> + * bytes meaning
> + * 1 [bits 3:0] channel number
> + * 2 parameter selector
> + * [3:N] configuration parameter data (from cmd[4] on)
> + */
> +static void set_lan_config(IPMIBmcSim *ibs,
> + uint8_t *cmd, unsigned int cmd_len,
> + RspBuffer *rsp)
> +{
> + uint8_t channel;
> + uint8_t *param; /* pointer to configuration parameter data */
> + unsigned int param_len;
> +
> + if (ibs->lan.channel == 0) {
> + /* LAN channel disabled. Fail as if this command were not defined. */
> + rsp_buffer_set_error(rsp, IPMI_CC_INVALID_CMD);
> + return;
> + }
> + if (cmd_len < 5) {
> + rsp_buffer_set_error(rsp, IPMI_CC_REQUEST_DATA_LENGTH_INVALID);
> + return;
> + }
Here we check cmd_len is not less than 5...
> + channel = cmd[2] & 0xf;
> + param = cmd + 4;
> + param_len = cmd_len - 4;
...so here, param_len must be at least 1 (worst case is 5 - 4 == 1)...
> +
> + if (!IPMI_BMC_CHANNEL_IS_LAN(ibs, channel)) {
> + rsp_buffer_set_error(rsp, IPMI_CC_INVALID_DATA_FIELD);
> + return;
> + }
> +
> + switch (cmd[3]) {
> + case IPMI_BMC_LAN_CFG_PARAM_IP_ADDR:
> + if (param_len < NBYTES_IP) {
> + rsp_buffer_set_error(rsp, IPMI_CC_REQUEST_DATA_LENGTH_INVALID);
> + return;
> + }
> + memcpy(ibs->lan.ipaddr, param, NBYTES_IP);
> + break;
> +
> + case IPMI_BMC_LAN_CFG_PARAM_IP_ADDR_SOURCE:
> + if (param_len < 1) {
...but here we check param_len < 1, which can never be true...
> + rsp_buffer_set_error(rsp, IPMI_CC_REQUEST_DATA_LENGTH_INVALID);
...so this if() body is dead code.
> + return;
> + }
> + if (!IPMI_BMC_LAN_CFG_IS_VALID_IP_SOURCE(*param)) {
> + rsp_buffer_set_error(rsp, IPMI_CC_INVALID_DATA_FIELD);
> + return;
> + }
> + ibs->lan.ipsrc = *param;
> + break;
Possibilities:
* if it's valid to have a command with no parameters, then
the initial cmd_len check should probably be "< 4"
* should the handling for each command check that the parameter
data is exactly the expected length, i.e. no trailing junk?
thanks
-- PMM
