Re: macsec: introduce IEEE 802.1AE driver

Sabrina Dubroca
2016-03-16T10:22:44Z

Hello Dan,

2016-03-16, 00:12:41 +0300, Dan Carpenter wrote:
> Hello Sabrina Dubroca,
>
> The patch c09440f7dcb3: "macsec: introduce IEEE 802.1AE driver" from
> Mar 11, 2016, leads to the following Smatch warnings:
>
> drivers/net/macsec.c:714 macsec_encrypt()
> warn: 'skb->dev' held on error path.
>
> drivers/net/macsec.c
> 711 dev_hold(skb->dev);
> ^^^^^^^^^^^^^^^^^^
> 712 ret = crypto_aead_encrypt(req);
> 713 if (ret = -EINPROGRESS) {
> 714 return ERR_PTR(ret);
>
> Need to dev_put() before returning.

In this case, no. We want to hold a reference on the netdevice, and
it is released by the crypto callback (macsec_encrypt_done(), or
macsec_decrypt_done() in the second case).

[snip second case]

> drivers/net/macsec.c:1624 macsec_add_rxsa()
> error: potential null dereference 'rx_sa'. (kmalloc returns null)

Right, I'm missing a NULL check here. I'll send a fix.

> 1616 }
> 1617
> 1618 rx_sa = rtnl_dereference(rx_sc->sa[assoc_num]);
> 1619 if (rx_sa) {
> 1620 rtnl_unlock();
> 1621 return -EBUSY;
> 1622 }
> 1623
> 1624 rx_sa = kmalloc(sizeof(*rx_sa), GFP_KERNEL);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 1625 if (init_rx_sa(rx_sa, nla_data(tb_sa[MACSEC_SA_ATTR_KEY]), secy->key_len,
> ^^^^^
> Dereferenced inside function.
>
> 1626 secy->icv_len)) {
> 1627 rtnl_unlock();
> 1628 return -ENOMEM;
> 1629 }
> 1630

Thanks for the report!

--
Sabrina