netdev - Re: [PATCH net-next] loopback: clear pfmemalloc on outgoing skb's

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <1485993427.6360.170.camel@edumazet-glaptop3.roam.corp.google.com>
Date:   Wed, 01 Feb 2017 15:57:07 -0800
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Josef Bacik <jbacik@...com>
Cc:     netdev@...r.kernel.org, kernel-team@...com,
        "David S . Miller" <davem@...emloft.net>
Subject: Re: [PATCH net-next] loopback: clear pfmemalloc on outgoing skb's

On Wed, 2017-02-01 at 15:38 -0800, Eric Dumazet wrote:
> On Wed, 2017-02-01 at 16:04 -0500, Josef Bacik wrote:
> > I was seeing random disconnects while testing NBD over loopback.  This turned
> > out to be because NBD sets pfmemalloc on it's socket, however the receiving side
> > is a user space application so does not have pfmemalloc set on its socket.  This
> > means that sk_filter_trim_cap will simply drop this packet, under the assumption
> > that the other side will simply retransmit.  Well we do retransmit, and then the
> > packet is just dropped again for the same reason.  To keep this from happening
> > simply clear skb->pfmemalloc on transmit so that we don't drop the packet on the
> > receive side.
> > 
> > Signed-off-by: Josef Bacik <jbacik@...com>
> > ---
> >  drivers/net/loopback.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c
> > index 1e05b7c..13c9126 100644
> > --- a/drivers/net/loopback.c
> > +++ b/drivers/net/loopback.c
> > @@ -81,6 +81,13 @@ static netdev_tx_t loopback_xmit(struct sk_buff *skb,
> >  	 */
> >  	skb_dst_force(skb);
> >  
> > +	/* If our transmitter was a pfmemalloc socket we need to clear
> > +	 * pfmemalloc here, otherwise the receiving socket may not be
> > +	 * pfmemalloc, and if this is a tcp packet then it'll get dropped and
> > +	 * all traffic will halt.
> > +	 */
> > +	skb->pfmemalloc = false;
> > +
> 
> I am not sure this is a proper fix.
> 
> Presumably if the socket was able to store packets in its write queue,
> fact that it sends it to loopback or an Ethernet link should not matter.
> 
> Only in RX path the pfmemalloc thing is really important.
> 
> So I would rather not set skb->pfmemalloc for skbs allocated for the
> write queue, and more exactly the fast clone.
> 
> This would actually speed up the stack a bit.
> 
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 734c71468b013838516cfe8c744dcd0e797a6e2b..f91b81340dc5be80e0c26f9835d9192f35b75ad7 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -271,7 +271,7 @@ struct sk_buff *__alloc_skb(unsigned int size, gfp_t gfp_mask,
>                 atomic_set(&fclones->fclone_ref, 1);
>  
>                 fclones->skb2.fclone = SKB_FCLONE_CLONE;
> -               fclones->skb2.pfmemalloc = pfmemalloc;
> +               fclones->skb2.pfmemalloc = 0;


It turns out this part was not needed in current kernel, because
__copy_skb_header() will copy pfmemalloc, since it is included in the
headers_start/headers_end section of skb.

So this patch is not solving your issue.