Message ID | d358a1ca10006d65c6d799e56d1abb7d17fda4a5.1461315621.git.sd@queasysnail.net |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
On 22.04.2016 11:28, Sabrina Dubroca wrote: > Fixes: c09440f7dcb3 ("macsec: introduce IEEE 802.1AE driver") > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > Signed-off-by: Sabrina Dubroca <sd@queasysnail.net> > Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org> > --- > drivers/net/macsec.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c > index 84d3e5ca8817..f691030ee3df 100644 > --- a/drivers/net/macsec.c > +++ b/drivers/net/macsec.c > @@ -1622,8 +1622,8 @@ static int macsec_add_rxsa(struct sk_buff *skb, struct genl_info *info) > } > > rx_sa = kmalloc(sizeof(*rx_sa), GFP_KERNEL); > - if (init_rx_sa(rx_sa, nla_data(tb_sa[MACSEC_SA_ATTR_KEY]), secy->key_len, > - secy->icv_len)) { > + if (!rx_sa || init_rx_sa(rx_sa, nla_data(tb_sa[MACSEC_SA_ATTR_KEY]), > + secy->key_len, secy->icv_len)) { > rtnl_unlock(); > return -ENOMEM; > } In case that kmalloc was successful and init_rx_sa failed, the allocated memory should be freed, shouldnt it?. Regards, Lino
2016-04-22, 11:35:03 +0200, Lino Sanfilippo wrote: > > > On 22.04.2016 11:28, Sabrina Dubroca wrote: > > Fixes: c09440f7dcb3 ("macsec: introduce IEEE 802.1AE driver") > > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > > Signed-off-by: Sabrina Dubroca <sd@queasysnail.net> > > Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org> > > --- > > drivers/net/macsec.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c > > index 84d3e5ca8817..f691030ee3df 100644 > > --- a/drivers/net/macsec.c > > +++ b/drivers/net/macsec.c > > @@ -1622,8 +1622,8 @@ static int macsec_add_rxsa(struct sk_buff *skb, struct genl_info *info) > > } > > > > rx_sa = kmalloc(sizeof(*rx_sa), GFP_KERNEL); > > - if (init_rx_sa(rx_sa, nla_data(tb_sa[MACSEC_SA_ATTR_KEY]), secy->key_len, > > - secy->icv_len)) { > > + if (!rx_sa || init_rx_sa(rx_sa, nla_data(tb_sa[MACSEC_SA_ATTR_KEY]), > > + secy->key_len, secy->icv_len)) { > > rtnl_unlock(); > > return -ENOMEM; > > } > > > In case that kmalloc was successful and init_rx_sa failed, the allocated memory should be freed, shouldnt it?. Yep, Lance pointed that out in v1: http://marc.info/?l=linux-netdev&m=146108796406155 But since we have the same code with init_tx_sa, I decided to fix both in a separate patch (7/9 in this set). Thanks,
On 22.04.2016 11:48, Sabrina Dubroca wrote: > 2016-04-22, 11:35:03 +0200, Lino Sanfilippo wrote: >> >> >> On 22.04.2016 11:28, Sabrina Dubroca wrote: >>> Fixes: c09440f7dcb3 ("macsec: introduce IEEE 802.1AE driver") >>> Reported-by: Dan Carpenter <dan.carpenter@oracle.com> >>> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net> >>> Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org> >>> --- >>> drivers/net/macsec.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c >>> index 84d3e5ca8817..f691030ee3df 100644 >>> --- a/drivers/net/macsec.c >>> +++ b/drivers/net/macsec.c >>> @@ -1622,8 +1622,8 @@ static int macsec_add_rxsa(struct sk_buff *skb, struct genl_info *info) >>> } >>> >>> rx_sa = kmalloc(sizeof(*rx_sa), GFP_KERNEL); >>> - if (init_rx_sa(rx_sa, nla_data(tb_sa[MACSEC_SA_ATTR_KEY]), secy->key_len, >>> - secy->icv_len)) { >>> + if (!rx_sa || init_rx_sa(rx_sa, nla_data(tb_sa[MACSEC_SA_ATTR_KEY]), >>> + secy->key_len, secy->icv_len)) { >>> rtnl_unlock(); >>> return -ENOMEM; >>> } >> >> >> In case that kmalloc was successful and init_rx_sa failed, the allocated memory should be freed, shouldnt it?. > > Yep, Lance pointed that out in v1: > http://marc.info/?l=linux-netdev&m=146108796406155 > > But since we have the same code with init_tx_sa, I decided to fix both > in a separate patch (7/9 in this set). > Ah ok, sorry about the noise then :) Regards, Lino
diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c index 84d3e5ca8817..f691030ee3df 100644 --- a/drivers/net/macsec.c +++ b/drivers/net/macsec.c @@ -1622,8 +1622,8 @@ static int macsec_add_rxsa(struct sk_buff *skb, struct genl_info *info) } rx_sa = kmalloc(sizeof(*rx_sa), GFP_KERNEL); - if (init_rx_sa(rx_sa, nla_data(tb_sa[MACSEC_SA_ATTR_KEY]), secy->key_len, - secy->icv_len)) { + if (!rx_sa || init_rx_sa(rx_sa, nla_data(tb_sa[MACSEC_SA_ATTR_KEY]), + secy->key_len, secy->icv_len)) { rtnl_unlock(); return -ENOMEM; }