[Intel-wired-lan] [bug report] ixgbevf: add VF IPsec offload code
Shannon Nelson
shannon.nelson at oracle.com
Tue Sep 4 15:32:39 UTC 2018
On 8/31/2018 10:29 AM, Dan Carpenter wrote:
> Hello Shannon Nelson,
>
> The patch 0062e7cc955e: "ixgbevf: add VF IPsec offload code" from Aug
> 13, 2018, leads to the following static checker warning:
>
> drivers/net/ethernet/intel/ixgbevf/mbx.c:244 ixgbevf_write_mbx_vf()
> warn: potential pointer math issue ('msg' is a 32 bit pointer)
>
> [ This is an unpublished smatch check. The warning message isn't very
> good. What it's trying to warn about here is the difference between
> ARRAY_SIZE() and sizeof(). - dan ]
Thanks for finding this, Dan. I'll send up a fix.
sln
>
> drivers/net/ethernet/intel/ixgbevf/ipsec.c
> 18 static int ixgbevf_ipsec_set_pf_sa(struct ixgbevf_adapter *adapter,
> 19 struct xfrm_state *xs)
> 20 {
> 21 u32 msgbuf[IXGBE_VFMAILBOX_SIZE] = { 0 };
> 22 struct ixgbe_hw *hw = &adapter->hw;
> 23 struct sa_mbx_msg *sam;
> 24 u16 msglen;
> 25 int ret;
> 26
> 27 /* send the important bits to the PF */
> 28 sam = (struct sa_mbx_msg *)(&msgbuf[1]);
> 29 sam->flags = xs->xso.flags;
> 30 sam->spi = xs->id.spi;
> 31 sam->proto = xs->id.proto;
> 32 sam->family = xs->props.family;
> 33
> 34 if (xs->props.family == AF_INET6)
> 35 memcpy(sam->addr, &xs->id.daddr.a6, sizeof(xs->id.daddr.a6));
> 36 else
> 37 memcpy(sam->addr, &xs->id.daddr.a4, sizeof(xs->id.daddr.a4));
> 38 memcpy(sam->key, xs->aead->alg_key, sizeof(sam->key));
> 39
> 40 msgbuf[0] = IXGBE_VF_IPSEC_ADD;
> 41 msglen = sizeof(*sam) + sizeof(msgbuf[0]);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 42
> 43 spin_lock_bh(&adapter->mbx_lock);
> 44
> 45 ret = hw->mbx.ops.write_posted(hw, msgbuf, msglen);
> ^^^^^^
> This is in byte terms but it should be in terms of sizeof(u32).
>
> 46 if (ret)
> 47 goto out;
> 48
> 49 msglen = sizeof(msgbuf[0]) * 2;
> 50 ret = hw->mbx.ops.read_posted(hw, msgbuf, msglen);
> ^^^^^^
> 51 if (ret)
> 52 goto out;
> 53
> 54 ret = (int)msgbuf[1];
> 55 if (msgbuf[0] & IXGBE_VT_MSGTYPE_NACK && ret >= 0)
> 56 ret = -1;
> 57
> 58 out:
> 59 spin_unlock_bh(&adapter->mbx_lock);
> 60
> 61 return ret;
> 62 }
> 63
> 64 /**
> 65 * ixgbevf_ipsec_del_pf_sa - ask the PF to delete an SA
> 66 * @adapter: board private structure
> 67 * @pfsa: sa index returned from PF when created, -1 for all
> 68 *
> 69 * Returns: 0 on success, or negative error code
> 70 **/
> 71 static int ixgbevf_ipsec_del_pf_sa(struct ixgbevf_adapter *adapter, int pfsa)
> 72 {
> 73 struct ixgbe_hw *hw = &adapter->hw;
> 74 u32 msgbuf[2];
> 75 int err;
> 76
> 77 memset(msgbuf, 0, sizeof(msgbuf));
> 78 msgbuf[0] = IXGBE_VF_IPSEC_DEL;
> 79 msgbuf[1] = (u32)pfsa;
> 80
> 81 spin_lock_bh(&adapter->mbx_lock);
> 82
> 83 err = hw->mbx.ops.write_posted(hw, msgbuf, sizeof(msgbuf));
> ^^^^^^^^^^^^^^
> This should be 2
>
> 84 if (err)
> 85 goto out;
> 86
> 87 err = hw->mbx.ops.read_posted(hw, msgbuf, sizeof(msgbuf));
> ^^^^^^^^^^^^^
> 2 elements here also. (This static checker warning is not published
> because it's low quality, so take it all with a grain of salt etc).
>
> 88 if (err)
> 89 goto out;
> 90
> 91 out:
> 92 spin_unlock_bh(&adapter->mbx_lock);
> 93 return err;
> 94 }
>
> regards,
> dan carpenter
>
More information about the Intel-wired-lan
mailing list