Re: [PATCH] can: kvaser_usb: hydra: help gcc-13 to figure out cmd_len

Jimmy Assarsson <extja@xxxxxxxxxx> · Mon, 19 Dec 2022 14:48:53 +0100

On 12/19/22 12:01, Marc Kleine-Budde wrote:
Debian's gcc-13 [1] throws the following error in
kvaser_usb_hydra_cmd_size():

[1] gcc version 13.0.0 20221214 (experimental) [master r13-4693-g512098a3316] (Debian 13-20221214-1)

| drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c:502:65: error:
| array subscript ‘struct kvaser_cmd_ext[0]’ is partly outside array
| bounds of ‘unsigned char[32]’ [-Werror=array-bounds=]
|   502 |                 ret = le16_to_cpu(((struct kvaser_cmd_ext *)cmd)->len);

kvaser_usb_hydra_cmd_size() returns the size of given command. It
depends on the command number (cmd->header.cmd_no). For extended
commands (cmd->header.cmd_no == CMD_EXTENDED) the above shown code is
executed.

Help gcc to recognize that this code path is not taken in all cases,
by calling kvaser_usb_hydra_cmd_size() directly after assigning the
command number.

Cc: Jimmy Assarsson <extja@xxxxxxxxxx>
Cc: Anssi Hannula <anssi.hannula@xxxxxxxxxx>
Fixes: aec5fb2268b7 ("can: kvaser_usb: Add support for Kvaser USB hydra family")
Signed-off-by: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx>
---

Hello,

Jimmy, Anssi can you test this patch on real hardware?

Hi Marc,

Yes. Now I've tested it and it's working.
Tested-by: Jimmy Assarsson <extja@xxxxxxxxxx>

Thanks for fixing this!

Best regards,
jimmy

regards,
Marc

  .../net/can/usb/kvaser_usb/kvaser_usb_hydra.c | 33 ++++++++++++++-----
  1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
index f688124d6d66..ef341c4254fc 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
@@ -545,6 +545,7 @@ static int kvaser_usb_hydra_send_simple_cmd(struct kvaser_usb *dev,
  					    u8 cmd_no, int channel)
  {
  	struct kvaser_cmd *cmd;
+	size_t cmd_len;
  	int err;
  
  	cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
@@ -552,6 +553,7 @@ static int kvaser_usb_hydra_send_simple_cmd(struct kvaser_usb *dev,
  		return -ENOMEM;
  
  	cmd->header.cmd_no = cmd_no;
+	cmd_len = kvaser_usb_hydra_cmd_size(cmd);
  	if (channel < 0) {
  		kvaser_usb_hydra_set_cmd_dest_he
  				(cmd, KVASER_USB_HYDRA_HE_ADDRESS_ILLEGAL);
@@ -568,7 +570,7 @@ static int kvaser_usb_hydra_send_simple_cmd(struct kvaser_usb *dev,
  	kvaser_usb_hydra_set_cmd_transid
  				(cmd, kvaser_usb_hydra_get_next_transid(dev));
  
-	err = kvaser_usb_send_cmd(dev, cmd, kvaser_usb_hydra_cmd_size(cmd));
+	err = kvaser_usb_send_cmd(dev, cmd, cmd_len);
  	if (err)
  		goto end;
  
@@ -584,6 +586,7 @@ kvaser_usb_hydra_send_simple_cmd_async(struct kvaser_usb_net_priv *priv,
  {
  	struct kvaser_cmd *cmd;
  	struct kvaser_usb *dev = priv->dev;
+	size_t cmd_len;
  	int err;
  
  	cmd = kzalloc(sizeof(*cmd), GFP_ATOMIC);
@@ -591,14 +594,14 @@ kvaser_usb_hydra_send_simple_cmd_async(struct kvaser_usb_net_priv *priv,
  		return -ENOMEM;
  
  	cmd->header.cmd_no = cmd_no;
+	cmd_len = kvaser_usb_hydra_cmd_size(cmd);
  
  	kvaser_usb_hydra_set_cmd_dest_he
  		(cmd, dev->card_data.hydra.channel_to_he[priv->channel]);
  	kvaser_usb_hydra_set_cmd_transid
  				(cmd, kvaser_usb_hydra_get_next_transid(dev));
  
-	err = kvaser_usb_send_cmd_async(priv, cmd,
-					kvaser_usb_hydra_cmd_size(cmd));
+	err = kvaser_usb_send_cmd_async(priv, cmd, cmd_len);
  	if (err)
  		kfree(cmd);
  
@@ -742,6 +745,7 @@ static int kvaser_usb_hydra_get_single_capability(struct kvaser_usb *dev,
  {
  	struct kvaser_usb_dev_card_data *card_data = &dev->card_data;
  	struct kvaser_cmd *cmd;
+	size_t cmd_len;
  	u32 value = 0;
  	u32 mask = 0;
  	u16 cap_cmd_res;
@@ -753,13 +757,14 @@ static int kvaser_usb_hydra_get_single_capability(struct kvaser_usb *dev,
  		return -ENOMEM;
  
  	cmd->header.cmd_no = CMD_GET_CAPABILITIES_REQ;
+	cmd_len = kvaser_usb_hydra_cmd_size(cmd);
  	cmd->cap_req.cap_cmd = cpu_to_le16(cap_cmd_req);
  
  	kvaser_usb_hydra_set_cmd_dest_he(cmd, card_data->hydra.sysdbg_he);
  	kvaser_usb_hydra_set_cmd_transid
  				(cmd, kvaser_usb_hydra_get_next_transid(dev));
  
-	err = kvaser_usb_send_cmd(dev, cmd, kvaser_usb_hydra_cmd_size(cmd));
+	err = kvaser_usb_send_cmd(dev, cmd, cmd_len);
  	if (err)
  		goto end;
  
@@ -1578,6 +1583,7 @@ static int kvaser_usb_hydra_get_busparams(struct kvaser_usb_net_priv *priv,
  	struct kvaser_usb *dev = priv->dev;
  	struct kvaser_usb_net_hydra_priv *hydra = priv->sub_priv;
  	struct kvaser_cmd *cmd;
+	size_t cmd_len;
  	int err;
  
  	if (!hydra)
@@ -1588,6 +1594,7 @@ static int kvaser_usb_hydra_get_busparams(struct kvaser_usb_net_priv *priv,
  		return -ENOMEM;
  
  	cmd->header.cmd_no = CMD_GET_BUSPARAMS_REQ;
+	cmd_len = kvaser_usb_hydra_cmd_size(cmd);
  	kvaser_usb_hydra_set_cmd_dest_he
  		(cmd, dev->card_data.hydra.channel_to_he[priv->channel]);
  	kvaser_usb_hydra_set_cmd_transid
@@ -1597,7 +1604,7 @@ static int kvaser_usb_hydra_get_busparams(struct kvaser_usb_net_priv *priv,
  
  	reinit_completion(&priv->get_busparams_comp);
  
-	err = kvaser_usb_send_cmd(dev, cmd, kvaser_usb_hydra_cmd_size(cmd));
+	err = kvaser_usb_send_cmd(dev, cmd, cmd_len);
  	if (err)
  		return err;
  
@@ -1624,6 +1631,7 @@ static int kvaser_usb_hydra_set_bittiming(const struct net_device *netdev,
  	struct kvaser_cmd *cmd;
  	struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
  	struct kvaser_usb *dev = priv->dev;
+	size_t cmd_len;
  	int err;
  
  	cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
@@ -1631,6 +1639,7 @@ static int kvaser_usb_hydra_set_bittiming(const struct net_device *netdev,
  		return -ENOMEM;
  
  	cmd->header.cmd_no = CMD_SET_BUSPARAMS_REQ;
+	cmd_len = kvaser_usb_hydra_cmd_size(cmd);
  	memcpy(&cmd->set_busparams_req.busparams_nominal, busparams,
  	       sizeof(cmd->set_busparams_req.busparams_nominal));
  
@@ -1639,7 +1648,7 @@ static int kvaser_usb_hydra_set_bittiming(const struct net_device *netdev,
  	kvaser_usb_hydra_set_cmd_transid
  				(cmd, kvaser_usb_hydra_get_next_transid(dev));
  
-	err = kvaser_usb_send_cmd(dev, cmd, kvaser_usb_hydra_cmd_size(cmd));
+	err = kvaser_usb_send_cmd(dev, cmd, cmd_len);
  
  	kfree(cmd);
  
@@ -1652,6 +1661,7 @@ static int kvaser_usb_hydra_set_data_bittiming(const struct net_device *netdev,
  	struct kvaser_cmd *cmd;
  	struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
  	struct kvaser_usb *dev = priv->dev;
+	size_t cmd_len;
  	int err;
  
  	cmd = kzalloc(sizeof(*cmd), GFP_KERNEL);
@@ -1659,6 +1669,7 @@ static int kvaser_usb_hydra_set_data_bittiming(const struct net_device *netdev,
  		return -ENOMEM;
  
  	cmd->header.cmd_no = CMD_SET_BUSPARAMS_FD_REQ;
+	cmd_len = kvaser_usb_hydra_cmd_size(cmd);
  	memcpy(&cmd->set_busparams_req.busparams_data, busparams,
  	       sizeof(cmd->set_busparams_req.busparams_data));
  
@@ -1676,7 +1687,7 @@ static int kvaser_usb_hydra_set_data_bittiming(const struct net_device *netdev,
  	kvaser_usb_hydra_set_cmd_transid
  				(cmd, kvaser_usb_hydra_get_next_transid(dev));
  
-	err = kvaser_usb_send_cmd(dev, cmd, kvaser_usb_hydra_cmd_size(cmd));
+	err = kvaser_usb_send_cmd(dev, cmd, cmd_len);
  
  	kfree(cmd);
  
@@ -1804,6 +1815,7 @@ static int kvaser_usb_hydra_get_software_info(struct kvaser_usb *dev)
  static int kvaser_usb_hydra_get_software_details(struct kvaser_usb *dev)
  {
  	struct kvaser_cmd *cmd;
+	size_t cmd_len;
  	int err;
  	u32 flags;
  	struct kvaser_usb_dev_card_data *card_data = &dev->card_data;
@@ -1813,6 +1825,7 @@ static int kvaser_usb_hydra_get_software_details(struct kvaser_usb *dev)
  		return -ENOMEM;
  
  	cmd->header.cmd_no = CMD_GET_SOFTWARE_DETAILS_REQ;
+	cmd_len = kvaser_usb_hydra_cmd_size(cmd);
  	cmd->sw_detail_req.use_ext_cmd = 1;
  	kvaser_usb_hydra_set_cmd_dest_he
  				(cmd, KVASER_USB_HYDRA_HE_ADDRESS_ILLEGAL);
@@ -1820,7 +1833,7 @@ static int kvaser_usb_hydra_get_software_details(struct kvaser_usb *dev)
  	kvaser_usb_hydra_set_cmd_transid
  				(cmd, kvaser_usb_hydra_get_next_transid(dev));
  
-	err = kvaser_usb_send_cmd(dev, cmd, kvaser_usb_hydra_cmd_size(cmd));
+	err = kvaser_usb_send_cmd(dev, cmd, cmd_len);
  	if (err)
  		goto end;
  
@@ -1938,6 +1951,7 @@ static int kvaser_usb_hydra_set_opt_mode(const struct kvaser_usb_net_priv *priv)
  {
  	struct kvaser_usb *dev = priv->dev;
  	struct kvaser_cmd *cmd;
+	size_t cmd_len;
  	int err;
  
  	if ((priv->can.ctrlmode &
@@ -1953,6 +1967,7 @@ static int kvaser_usb_hydra_set_opt_mode(const struct kvaser_usb_net_priv *priv)
  		return -ENOMEM;
  
  	cmd->header.cmd_no = CMD_SET_DRIVERMODE_REQ;
+	cmd_len = kvaser_usb_hydra_cmd_size(cmd);
  	kvaser_usb_hydra_set_cmd_dest_he
  		(cmd, dev->card_data.hydra.channel_to_he[priv->channel]);
  	kvaser_usb_hydra_set_cmd_transid
@@ -1962,7 +1977,7 @@ static int kvaser_usb_hydra_set_opt_mode(const struct kvaser_usb_net_priv *priv)
  	else
  		cmd->set_ctrlmode.mode = KVASER_USB_HYDRA_CTRLMODE_NORMAL;
  
-	err = kvaser_usb_send_cmd(dev, cmd, kvaser_usb_hydra_cmd_size(cmd));
+	err = kvaser_usb_send_cmd(dev, cmd, cmd_len);
  	kfree(cmd);
  
  	return err;






