From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58518) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1afqUu-0002UB-W1 for qemu-devel@nongnu.org; Tue, 15 Mar 2016 11:03:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1afqUo-0007M4-E1 for qemu-devel@nongnu.org; Tue, 15 Mar 2016 11:03:00 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33433) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1afqUo-0007Lq-0L for qemu-devel@nongnu.org; Tue, 15 Mar 2016 11:02:54 -0400 Date: Tue, 15 Mar 2016 17:02:49 +0200 From: "Michael S. Tsirkin" Message-ID: <1458053975-2410-44-git-send-email-mst@redhat.com> References: <1458053975-2410-1-git-send-email-mst@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <1458053975-2410-1-git-send-email-mst@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: [Qemu-devel] [PULL v2 43/51] ipmi: remove IPMI_CHECK_CMD_LEN() macro List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Peter Maydell , Eduardo Habkost , Corey Minyard , =?us-ascii?B?PT9VVEYtOD9xP0M9QzM9QTlkcmljPTIwTGU9MjBHb2F0ZXI/PQ==?= , Marcel Apfelbaum , Greg Kurz From: C=E9dric Le Goater Most IPMI command handlers in the BMC simulator start with a call to the macro IPMI_CHECK_CMD_LEN() which verifies that a minimal number of arguments expected by the command are indeed available. To achieve this task, the macro implicitly uses local variables which is misleading in the code. This patch adds a 'cmd_len_min' attribute to the struct IPMICmdHandler defining the minimal number of arguments expected by the command and moves this check in the global command handler ipmi_sim_handle_command(). To clarify the checks being done on the received command, the patch introduces a helper ipmi_get_handler(). Signed-off-by: C=E9dric Le Goater Acked-by: Corey Minyard Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/ipmi/ipmi_bmc_sim.c | 164 +++++++++++++++++++++++++------------------= ------ 1 file changed, 84 insertions(+), 80 deletions(-) diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c index 51d234a..cbf2991 100644 --- a/hw/ipmi/ipmi_bmc_sim.c +++ b/hw/ipmi/ipmi_bmc_sim.c @@ -155,10 +155,15 @@ typedef struct IPMISensor { typedef struct IPMIBmcSim IPMIBmcSim; =20 #define MAX_NETFNS 64 -typedef void (*IPMICmdHandler)(IPMIBmcSim *s, - uint8_t *cmd, unsigned int cmd_len, - uint8_t *rsp, unsigned int *rsp_len, - unsigned int max_rsp_len); + +typedef struct IPMICmdHandler { + void (*cmd_handler)(IPMIBmcSim *s, + uint8_t *cmd, unsigned int cmd_len, + uint8_t *rsp, unsigned int *rsp_len, + unsigned int max_rsp_len); + unsigned int cmd_len_min; +} IPMICmdHandler; + typedef struct IPMINetfn { unsigned int cmd_nums; const IPMICmdHandler *cmd_handlers; @@ -269,13 +274,6 @@ struct IPMIBmcSim { rsp[(*rsp_len)++] =3D (b); \ } while (0) =20 -/* Verify that the received command is a certain length. */ -#define IPMI_CHECK_CMD_LEN(l) \ - if (cmd_len < l) { \ - rsp[2] =3D IPMI_CC_REQUEST_DATA_LENGTH_INVALID; \ - return; \ - } - /* Check that the reservation in the command is valid. */ #define IPMI_CHECK_RESERVATION(off, r) \ do { \ @@ -566,6 +564,28 @@ static int ipmi_register_netfn(IPMIBmcSim *s, unsign= ed int netfn, return 0; } =20 +static const IPMICmdHandler *ipmi_get_handler(IPMIBmcSim *ibs, + unsigned int netfn, + unsigned int cmd) +{ + const IPMICmdHandler *hdl; + + if (netfn & 1 || netfn >=3D MAX_NETFNS || !ibs->netfns[netfn / 2]) { + return NULL; + } + + if (cmd >=3D ibs->netfns[netfn / 2]->cmd_nums) { + return NULL; + } + + hdl =3D &ibs->netfns[netfn / 2]->cmd_handlers[cmd]; + if (!hdl->cmd_handler) { + return NULL; + } + + return hdl; +} + static void next_timeout(IPMIBmcSim *ibs) { int64_t next; @@ -586,11 +606,11 @@ static void ipmi_sim_handle_command(IPMIBmc *b, IPMIBmcSim *ibs =3D IPMI_BMC_SIMULATOR(b); IPMIInterface *s =3D ibs->parent.intf; IPMIInterfaceClass *k =3D IPMI_INTERFACE_GET_CLASS(s); - unsigned int netfn; uint8_t rsp[MAX_IPMI_MSG_SIZE]; unsigned int rsp_len_holder =3D 0; unsigned int *rsp_len =3D &rsp_len_holder; unsigned int max_rsp_len =3D sizeof(rsp); + const IPMICmdHandler *hdl; =20 /* Set up the response, set the low bit of NETFN. */ /* Note that max_rsp_len must be at least 3 */ @@ -619,18 +639,18 @@ static void ipmi_sim_handle_command(IPMIBmc *b, goto out; } =20 - netfn =3D cmd[0] >> 2; - - /* Odd netfns are not valid, make sure the command is registered */ - if ((netfn & 1) || !ibs->netfns[netfn / 2] || - (cmd[1] >=3D ibs->netfns[netfn / 2]->cmd_nums) |= | - (!ibs->netfns[netfn / 2]->cmd_handlers[cmd[1]]))= { + hdl =3D ipmi_get_handler(ibs, cmd[0] >> 2, cmd[1]); + if (!hdl) { rsp[2] =3D IPMI_CC_INVALID_CMD; goto out; } =20 - ibs->netfns[netfn / 2]->cmd_handlers[cmd[1]](ibs, cmd, cmd_len, rsp,= rsp_len, - max_rsp_len); + if (cmd_len < hdl->cmd_len_min) { + rsp[2] =3D IPMI_CC_REQUEST_DATA_LENGTH_INVALID; + goto out; + } + + hdl->cmd_handler(ibs, cmd, cmd_len, rsp, rsp_len, max_rsp_len); =20 out: k->handle_rsp(s, msg_id, rsp, *rsp_len); @@ -737,7 +757,6 @@ static void chassis_control(IPMIBmcSim *ibs, IPMIInterface *s =3D ibs->parent.intf; IPMIInterfaceClass *k =3D IPMI_INTERFACE_GET_CLASS(s); =20 - IPMI_CHECK_CMD_LEN(3); switch (cmd[2] & 0xf) { case 0: /* power down */ rsp[2] =3D k->do_hw_op(s, IPMI_POWEROFF_CHASSIS, 0); @@ -838,7 +857,6 @@ static void set_acpi_power_state(IPMIBmcSim *ibs, uint8_t *rsp, unsigned int *rsp_len, unsigned int max_rsp_len) { - IPMI_CHECK_CMD_LEN(4); ibs->acpi_power_state[0] =3D cmd[2]; ibs->acpi_power_state[1] =3D cmd[3]; } @@ -869,7 +887,6 @@ static void set_bmc_global_enables(IPMIBmcSim *ibs, uint8_t *rsp, unsigned int *rsp_len, unsigned int max_rsp_len) { - IPMI_CHECK_CMD_LEN(3); set_global_enables(ibs, cmd[2]); } =20 @@ -889,7 +906,6 @@ static void clr_msg_flags(IPMIBmcSim *ibs, IPMIInterface *s =3D ibs->parent.intf; IPMIInterfaceClass *k =3D IPMI_INTERFACE_GET_CLASS(s); =20 - IPMI_CHECK_CMD_LEN(3); ibs->msg_flags &=3D ~cmd[2]; k->set_atn(s, attn_set(ibs), attn_irq_enabled(ibs)); } @@ -976,15 +992,17 @@ static void send_msg(IPMIBmcSim *ibs, uint8_t *buf; uint8_t netfn, rqLun, rsLun, rqSeq; =20 - IPMI_CHECK_CMD_LEN(3); - if (cmd[2] !=3D 0) { /* We only handle channel 0 with no options */ rsp[2] =3D IPMI_CC_INVALID_DATA_FIELD; return; } =20 - IPMI_CHECK_CMD_LEN(10); + if (cmd_len < 10) { + rsp[2] =3D IPMI_CC_REQUEST_DATA_LENGTH_INVALID; + return; + } + if (cmd[3] !=3D 0x40) { /* We only emulate a MC at address 0x40. */ rsp[2] =3D 0x83; /* NAK on write */ @@ -1092,7 +1110,6 @@ static void set_watchdog_timer(IPMIBmcSim *ibs, IPMIInterfaceClass *k =3D IPMI_INTERFACE_GET_CLASS(s); unsigned int val; =20 - IPMI_CHECK_CMD_LEN(8); val =3D cmd[2] & 0x7; /* Validate use */ if (val =3D=3D 0 || val > 5) { rsp[2] =3D IPMI_CC_INVALID_DATA_FIELD; @@ -1217,7 +1234,6 @@ static void get_sdr(IPMIBmcSim *ibs, uint16_t nextrec; struct ipmi_sdr_header *sdrh; =20 - IPMI_CHECK_CMD_LEN(8); if (cmd[6]) { IPMI_CHECK_RESERVATION(2, ibs->sdr.reservation); } @@ -1271,7 +1287,6 @@ static void clear_sdr_rep(IPMIBmcSim *ibs, uint8_t *rsp, unsigned int *rsp_len, unsigned int max_rsp_len) { - IPMI_CHECK_CMD_LEN(8); IPMI_CHECK_RESERVATION(2, ibs->sdr.reservation); if (cmd[4] !=3D 'C' || cmd[5] !=3D 'L' || cmd[6] !=3D 'R') { rsp[2] =3D IPMI_CC_INVALID_DATA_FIELD; @@ -1330,7 +1345,6 @@ static void get_sel_entry(IPMIBmcSim *ibs, { unsigned int val; =20 - IPMI_CHECK_CMD_LEN(8); if (cmd[6]) { IPMI_CHECK_RESERVATION(2, ibs->sel.reservation); } @@ -1375,7 +1389,6 @@ static void add_sel_entry(IPMIBmcSim *ibs, uint8_t *rsp, unsigned int *rsp_len, unsigned int max_rsp_len) { - IPMI_CHECK_CMD_LEN(18); if (sel_add_event(ibs, cmd + 2)) { rsp[2] =3D IPMI_CC_OUT_OF_SPACE; return; @@ -1390,7 +1403,6 @@ static void clear_sel(IPMIBmcSim *ibs, uint8_t *rsp, unsigned int *rsp_len, unsigned int max_rsp_len) { - IPMI_CHECK_CMD_LEN(8); IPMI_CHECK_RESERVATION(2, ibs->sel.reservation); if (cmd[4] !=3D 'C' || cmd[5] !=3D 'L' || cmd[6] !=3D 'R') { rsp[2] =3D IPMI_CC_INVALID_DATA_FIELD; @@ -1434,7 +1446,6 @@ static void set_sel_time(IPMIBmcSim *ibs, uint32_t val; struct ipmi_time now; =20 - IPMI_CHECK_CMD_LEN(6); val =3D cmd[2] | (cmd[3] << 8) | (cmd[4] << 16) | (cmd[5] << 24); ipmi_gettime(&now); ibs->sel.time_offset =3D now.tv_sec - ((long) val); @@ -1447,7 +1458,6 @@ static void set_sensor_evt_enable(IPMIBmcSim *ibs, { IPMISensor *sens; =20 - IPMI_CHECK_CMD_LEN(4); if ((cmd[2] >=3D MAX_SENSORS) || !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) { rsp[2] =3D IPMI_CC_REQ_ENTRY_NOT_PRESENT; @@ -1499,7 +1509,6 @@ static void get_sensor_evt_enable(IPMIBmcSim *ibs, { IPMISensor *sens; =20 - IPMI_CHECK_CMD_LEN(3); if ((cmd[2] >=3D MAX_SENSORS) || !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) { rsp[2] =3D IPMI_CC_REQ_ENTRY_NOT_PRESENT; @@ -1520,7 +1529,6 @@ static void rearm_sensor_evts(IPMIBmcSim *ibs, { IPMISensor *sens; =20 - IPMI_CHECK_CMD_LEN(4); if ((cmd[2] >=3D MAX_SENSORS) || !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) { rsp[2] =3D IPMI_CC_REQ_ENTRY_NOT_PRESENT; @@ -1542,7 +1550,6 @@ static void get_sensor_evt_status(IPMIBmcSim *ibs, { IPMISensor *sens; =20 - IPMI_CHECK_CMD_LEN(3); if ((cmd[2] >=3D MAX_SENSORS) || !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) { rsp[2] =3D IPMI_CC_REQ_ENTRY_NOT_PRESENT; @@ -1564,7 +1571,6 @@ static void get_sensor_reading(IPMIBmcSim *ibs, { IPMISensor *sens; =20 - IPMI_CHECK_CMD_LEN(3); if ((cmd[2] >=3D MAX_SENSORS) || !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) { rsp[2] =3D IPMI_CC_REQ_ENTRY_NOT_PRESENT; @@ -1587,7 +1593,6 @@ static void set_sensor_type(IPMIBmcSim *ibs, IPMISensor *sens; =20 =20 - IPMI_CHECK_CMD_LEN(5); if ((cmd[2] >=3D MAX_SENSORS) || !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) { rsp[2] =3D IPMI_CC_REQ_ENTRY_NOT_PRESENT; @@ -1606,7 +1611,6 @@ static void get_sensor_type(IPMIBmcSim *ibs, IPMISensor *sens; =20 =20 - IPMI_CHECK_CMD_LEN(3); if ((cmd[2] >=3D MAX_SENSORS) || !IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) { rsp[2] =3D IPMI_CC_REQ_ENTRY_NOT_PRESENT; @@ -1619,10 +1623,10 @@ static void get_sensor_type(IPMIBmcSim *ibs, =20 =20 static const IPMICmdHandler chassis_cmds[] =3D { - [IPMI_CMD_GET_CHASSIS_CAPABILITIES] =3D chassis_capabilities, - [IPMI_CMD_GET_CHASSIS_STATUS] =3D chassis_status, - [IPMI_CMD_CHASSIS_CONTROL] =3D chassis_control, - [IPMI_CMD_GET_SYS_RESTART_CAUSE] =3D chassis_get_sys_restart_cause + [IPMI_CMD_GET_CHASSIS_CAPABILITIES] =3D { chassis_capabilities }, + [IPMI_CMD_GET_CHASSIS_STATUS] =3D { chassis_status }, + [IPMI_CMD_CHASSIS_CONTROL] =3D { chassis_control, 3 }, + [IPMI_CMD_GET_SYS_RESTART_CAUSE] =3D { chassis_get_sys_restart_cause= } }; static const IPMINetfn chassis_netfn =3D { .cmd_nums =3D ARRAY_SIZE(chassis_cmds), @@ -1630,13 +1634,13 @@ static const IPMINetfn chassis_netfn =3D { }; =20 static const IPMICmdHandler sensor_event_cmds[] =3D { - [IPMI_CMD_SET_SENSOR_EVT_ENABLE] =3D set_sensor_evt_enable, - [IPMI_CMD_GET_SENSOR_EVT_ENABLE] =3D get_sensor_evt_enable, - [IPMI_CMD_REARM_SENSOR_EVTS] =3D rearm_sensor_evts, - [IPMI_CMD_GET_SENSOR_EVT_STATUS] =3D get_sensor_evt_status, - [IPMI_CMD_GET_SENSOR_READING] =3D get_sensor_reading, - [IPMI_CMD_SET_SENSOR_TYPE] =3D set_sensor_type, - [IPMI_CMD_GET_SENSOR_TYPE] =3D get_sensor_type, + [IPMI_CMD_SET_SENSOR_EVT_ENABLE] =3D { set_sensor_evt_enable, 4 }, + [IPMI_CMD_GET_SENSOR_EVT_ENABLE] =3D { get_sensor_evt_enable, 3 }, + [IPMI_CMD_REARM_SENSOR_EVTS] =3D { rearm_sensor_evts, 4 }, + [IPMI_CMD_GET_SENSOR_EVT_STATUS] =3D { get_sensor_evt_status, 3 }, + [IPMI_CMD_GET_SENSOR_READING] =3D { get_sensor_reading, 3 }, + [IPMI_CMD_SET_SENSOR_TYPE] =3D { set_sensor_type, 5 }, + [IPMI_CMD_GET_SENSOR_TYPE] =3D { get_sensor_type, 3 }, }; static const IPMINetfn sensor_event_netfn =3D { .cmd_nums =3D ARRAY_SIZE(sensor_event_cmds), @@ -1644,22 +1648,22 @@ static const IPMINetfn sensor_event_netfn =3D { }; =20 static const IPMICmdHandler app_cmds[] =3D { - [IPMI_CMD_GET_DEVICE_ID] =3D get_device_id, - [IPMI_CMD_COLD_RESET] =3D cold_reset, - [IPMI_CMD_WARM_RESET] =3D warm_reset, - [IPMI_CMD_SET_ACPI_POWER_STATE] =3D set_acpi_power_state, - [IPMI_CMD_GET_ACPI_POWER_STATE] =3D get_acpi_power_state, - [IPMI_CMD_GET_DEVICE_GUID] =3D get_device_guid, - [IPMI_CMD_SET_BMC_GLOBAL_ENABLES] =3D set_bmc_global_enables, - [IPMI_CMD_GET_BMC_GLOBAL_ENABLES] =3D get_bmc_global_enables, - [IPMI_CMD_CLR_MSG_FLAGS] =3D clr_msg_flags, - [IPMI_CMD_GET_MSG_FLAGS] =3D get_msg_flags, - [IPMI_CMD_GET_MSG] =3D get_msg, - [IPMI_CMD_SEND_MSG] =3D send_msg, - [IPMI_CMD_READ_EVT_MSG_BUF] =3D read_evt_msg_buf, - [IPMI_CMD_RESET_WATCHDOG_TIMER] =3D reset_watchdog_timer, - [IPMI_CMD_SET_WATCHDOG_TIMER] =3D set_watchdog_timer, - [IPMI_CMD_GET_WATCHDOG_TIMER] =3D get_watchdog_timer, + [IPMI_CMD_GET_DEVICE_ID] =3D { get_device_id }, + [IPMI_CMD_COLD_RESET] =3D { cold_reset }, + [IPMI_CMD_WARM_RESET] =3D { warm_reset }, + [IPMI_CMD_SET_ACPI_POWER_STATE] =3D { set_acpi_power_state, 4 }, + [IPMI_CMD_GET_ACPI_POWER_STATE] =3D { get_acpi_power_state }, + [IPMI_CMD_GET_DEVICE_GUID] =3D { get_device_guid }, + [IPMI_CMD_SET_BMC_GLOBAL_ENABLES] =3D { set_bmc_global_enables, 3 }, + [IPMI_CMD_GET_BMC_GLOBAL_ENABLES] =3D { get_bmc_global_enables }, + [IPMI_CMD_CLR_MSG_FLAGS] =3D { clr_msg_flags, 3 }, + [IPMI_CMD_GET_MSG_FLAGS] =3D { get_msg_flags }, + [IPMI_CMD_GET_MSG] =3D { get_msg }, + [IPMI_CMD_SEND_MSG] =3D { send_msg, 3 }, + [IPMI_CMD_READ_EVT_MSG_BUF] =3D { read_evt_msg_buf }, + [IPMI_CMD_RESET_WATCHDOG_TIMER] =3D { reset_watchdog_timer }, + [IPMI_CMD_SET_WATCHDOG_TIMER] =3D { set_watchdog_timer, 8 }, + [IPMI_CMD_GET_WATCHDOG_TIMER] =3D { get_watchdog_timer }, }; static const IPMINetfn app_netfn =3D { .cmd_nums =3D ARRAY_SIZE(app_cmds), @@ -1667,18 +1671,18 @@ static const IPMINetfn app_netfn =3D { }; =20 static const IPMICmdHandler storage_cmds[] =3D { - [IPMI_CMD_GET_SDR_REP_INFO] =3D get_sdr_rep_info, - [IPMI_CMD_RESERVE_SDR_REP] =3D reserve_sdr_rep, - [IPMI_CMD_GET_SDR] =3D get_sdr, - [IPMI_CMD_ADD_SDR] =3D add_sdr, - [IPMI_CMD_CLEAR_SDR_REP] =3D clear_sdr_rep, - [IPMI_CMD_GET_SEL_INFO] =3D get_sel_info, - [IPMI_CMD_RESERVE_SEL] =3D reserve_sel, - [IPMI_CMD_GET_SEL_ENTRY] =3D get_sel_entry, - [IPMI_CMD_ADD_SEL_ENTRY] =3D add_sel_entry, - [IPMI_CMD_CLEAR_SEL] =3D clear_sel, - [IPMI_CMD_GET_SEL_TIME] =3D get_sel_time, - [IPMI_CMD_SET_SEL_TIME] =3D set_sel_time, + [IPMI_CMD_GET_SDR_REP_INFO] =3D { get_sdr_rep_info }, + [IPMI_CMD_RESERVE_SDR_REP] =3D { reserve_sdr_rep }, + [IPMI_CMD_GET_SDR] =3D { get_sdr, 8 }, + [IPMI_CMD_ADD_SDR] =3D { add_sdr }, + [IPMI_CMD_CLEAR_SDR_REP] =3D { clear_sdr_rep, 8 }, + [IPMI_CMD_GET_SEL_INFO] =3D { get_sel_info }, + [IPMI_CMD_RESERVE_SEL] =3D { reserve_sel }, + [IPMI_CMD_GET_SEL_ENTRY] =3D { get_sel_entry, 8 }, + [IPMI_CMD_ADD_SEL_ENTRY] =3D { add_sel_entry, 18 }, + [IPMI_CMD_CLEAR_SEL] =3D { clear_sel, 8 }, + [IPMI_CMD_GET_SEL_TIME] =3D { get_sel_time, 6 }, + [IPMI_CMD_SET_SEL_TIME] =3D { set_sel_time }, }; =20 static const IPMINetfn storage_netfn =3D { --=20 MST