lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Fri, 26 Apr 2019 13:28:44 -0600
From:   Captain Wiggum <captwiggum@...il.com>
To:     Peter Oskolkov <posk@...gle.com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        stable@...r.kernel.org, netdev@...r.kernel.org,
        Peter Oskolkov <posk@...k.io>,
        David Miller <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Sasha Levin <sashal@...nel.org>, Lars Persson <lists@...h.nu>
Subject: Re: [PATCH 4.14 stable v2 1/5] ipv6: frags: fix a lockdep false positive

Sorry for the late reply.
I am starting the TAHI test on the 4.14 v2 patch today and reply with
results before Mon morning.
The test is long running so we let it run overnight.

On Tue, Apr 23, 2019 at 11:48 AM Peter Oskolkov <posk@...gle.com> wrote:
>
> From: Eric Dumazet <edumazet@...gle.com>
>
> [ Upstream commit 415787d7799f4fccbe8d49cb0b8e5811be6b0389 ]
>
> lockdep does not know that the locks used by IPv4 defrag
> and IPv6 reassembly units are of different classes.
>
> It complains because of following chains :
>
> 1) sch_direct_xmit()        (lock txq->_xmit_lock)
>     dev_hard_start_xmit()
>      xmit_one()
>       dev_queue_xmit_nit()
>        packet_rcv_fanout()
>         ip_check_defrag()
>          ip_defrag()
>           spin_lock()     (lock frag queue spinlock)
>
> 2) ip6_input_finish()
>     ipv6_frag_rcv()       (lock frag queue spinlock)
>      ip6_frag_queue()
>       icmpv6_param_prob() (lock txq->_xmit_lock at some point)
>
> We could add lockdep annotations, but we also can make sure IPv6
> calls icmpv6_param_prob() only after the release of the frag queue spinlock,
> since this naturally makes frag queue spinlock a leaf in lock hierarchy.
>
> Signed-off-by: Eric Dumazet <edumazet@...gle.com>
> Signed-off-by: David S. Miller <davem@...emloft.net>
> ---
>  net/ipv6/reassembly.c | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
> index 2a8c680b67cd..f75e9e711c31 100644
> --- a/net/ipv6/reassembly.c
> +++ b/net/ipv6/reassembly.c
> @@ -170,7 +170,8 @@ fq_find(struct net *net, __be32 id, const struct ipv6hdr *hdr, int iif)
>  }
>
>  static int ip6_frag_queue(struct frag_queue *fq, struct sk_buff *skb,
> -                          struct frag_hdr *fhdr, int nhoff)
> +                         struct frag_hdr *fhdr, int nhoff,
> +                         u32 *prob_offset)
>  {
>         struct sk_buff *prev, *next;
>         struct net_device *dev;
> @@ -186,11 +187,7 @@ static int ip6_frag_queue(struct frag_queue *fq, struct sk_buff *skb,
>                         ((u8 *)(fhdr + 1) - (u8 *)(ipv6_hdr(skb) + 1)));
>
>         if ((unsigned int)end > IPV6_MAXPLEN) {
> -               __IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)),
> -                               IPSTATS_MIB_INHDRERRORS);
> -               icmpv6_param_prob(skb, ICMPV6_HDR_FIELD,
> -                                 ((u8 *)&fhdr->frag_off -
> -                                  skb_network_header(skb)));
> +               *prob_offset = (u8 *)&fhdr->frag_off - skb_network_header(skb);
>                 return -1;
>         }
>
> @@ -221,10 +218,7 @@ static int ip6_frag_queue(struct frag_queue *fq, struct sk_buff *skb,
>                         /* RFC2460 says always send parameter problem in
>                          * this case. -DaveM
>                          */
> -                       __IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)),
> -                                       IPSTATS_MIB_INHDRERRORS);
> -                       icmpv6_param_prob(skb, ICMPV6_HDR_FIELD,
> -                                         offsetof(struct ipv6hdr, payload_len));
> +                       *prob_offset = offsetof(struct ipv6hdr, payload_len);
>                         return -1;
>                 }
>                 if (end > fq->q.len) {
> @@ -536,15 +530,22 @@ static int ipv6_frag_rcv(struct sk_buff *skb)
>         iif = skb->dev ? skb->dev->ifindex : 0;
>         fq = fq_find(net, fhdr->identification, hdr, iif);
>         if (fq) {
> +               u32 prob_offset = 0;
>                 int ret;
>
>                 spin_lock(&fq->q.lock);
>
>                 fq->iif = iif;
> -               ret = ip6_frag_queue(fq, skb, fhdr, IP6CB(skb)->nhoff);
> +               ret = ip6_frag_queue(fq, skb, fhdr, IP6CB(skb)->nhoff,
> +                                    &prob_offset);
>
>                 spin_unlock(&fq->q.lock);
>                 inet_frag_put(&fq->q);
> +               if (prob_offset) {
> +                       __IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)),
> +                                       IPSTATS_MIB_INHDRERRORS);
> +                       icmpv6_param_prob(skb, ICMPV6_HDR_FIELD, prob_offset);
> +               }
>                 return ret;
>         }
>
> --
> 2.21.0.593.g511ec345e18-goog
>

Powered by blists - more mailing lists