2019-06-30, 22:05:41 -0400, Willem de Bruijn wrote:
> On Sun, Jun 30, 2019 at 4:48 PM Andreas Steinmetz <a...@domdv.de> wrote:
> >
> > At this point, skb could only be a valid pointer, so this patch does
> > not introduce any functional change.
>
> Previously, macsec_post_decrypt could be called on the original skb if
> the initial condition was false and macsec_decrypt is skipped. That
> was probably unintended. Either way, then this is a functional change,
> and perhaps a bugfix?
Ouch, I missed that when Andreas sent me that patch before. No, it is
actually intended. If we skip macsec_decrypt(), we should still
account for that packet in the InPktsUnchecked/InPktsDelayed
counters. That's in Figure 10-5 in the standard.
Thanks for catching this, Willem. That patch should only move the
IS_ERR(skb) case under the block where macsec_decrypt() is called, but
not move the call to macsec_post_decrypt().
> > Signed-off-by: Andreas Steinmetz <a...@domdv.de>
> >
> > --- a/drivers/net/macsec.c 2019-06-30 22:05:17.785683634 +0200
> > +++ b/drivers/net/macsec.c 2019-06-30 22:05:20.526171178 +0200
> > @@ -1205,21 +1205,22 @@
> >
> > /* Disabled && !changed text => skip validation */
> > if (hdr->tci_an & MACSEC_TCI_C ||
> > - secy->validate_frames != MACSEC_VALIDATE_DISABLED)
> > + secy->validate_frames != MACSEC_VALIDATE_DISABLED) {
> > skb = macsec_decrypt(skb, dev, rx_sa, sci, secy);
> >
> > - if (IS_ERR(skb)) {
> > - /* the decrypt callback needs the reference */
> > - if (PTR_ERR(skb) != -EINPROGRESS) {
> > - macsec_rxsa_put(rx_sa);
> > - macsec_rxsc_put(rx_sc);
> > + if (IS_ERR(skb)) {
> > + /* the decrypt callback needs the reference */
> > + if (PTR_ERR(skb) != -EINPROGRESS) {
> > + macsec_rxsa_put(rx_sa);
> > + macsec_rxsc_put(rx_sc);
> > + }
> > + rcu_read_unlock();
> > + return RX_HANDLER_CONSUMED;
> > }
> > - rcu_read_unlock();
> > - return RX_HANDLER_CONSUMED;
> > - }
> >
> > - if (!macsec_post_decrypt(skb, secy, pn))
> > - goto drop;
> > + if (!macsec_post_decrypt(skb, secy, pn))
> > + goto drop;
> > + }
> >
> > deliver:
> > macsec_finalize_skb(skb, secy->icv_len,
> >
--
Sabrina
