Re: [RFC net-next v1 4/5] net: macsec: introduce mdo_insert_tx_tag

[Radu Pirea (OSS)]

On 12.08.2023 21:07, Simon Horman wrote:
> On Fri, Aug 11, 2023 at 06:32:48PM +0300, Radu Pirea (NXP OSS) wrote:
> > @@ -4137,6 +4211,11 @@ static int macsec_newlink(struct net *net, struct net_device *dev,
> >   			if (err)
> >   				goto del_dev;
> >   		}
> > +
> > +		dev->needed_headroom -= MACSEC_NEEDED_HEADROOM;
> > +		dev->needed_headroom += ops->needed_headroom;
> > +		dev->needed_tailroom -= MACSEC_NEEDED_TAILROOM;
> > +		dev->needed_tailroom += ops->needed_tailroom;
>
> Hi Radu,
>
> Just above the beginning of this hunk it is assumed that ops may be NULL.
> However, here it is dereferenced unconditionally. Is this safe?

Of course it's not safe.
Thank you.

> Flagged by Smatch.
>
> >   	}
> >   
> >   	err = register_macsec_dev(real_dev, dev);
> > diff --git a/include/net/macsec.h b/include/net/macsec.h
> > index 33dc7f2aa42e..a988249d9608 100644
> > --- a/include/net/macsec.h
> > +++ b/include/net/macsec.h
> > @@ -272,6 +272,7 @@ struct macsec_context {
> >   		struct macsec_rx_sa_stats *rx_sa_stats;
> >   		struct macsec_dev_stats  *dev_stats;
> >   	} stats;
> > +	struct sk_buff *skb;
>
> Not strictly related to this patch,
> but it would be nice to update the kernel doc for this
> structure so that it's fields are documented.

Agreed. I will update the documentation.

--
Radu P.