netdev - Re: [net-next PATCH] page_pool: do not release pool until inflight == 0.

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: <20191113110823.0e1186a5@carbon>
Date:   Wed, 13 Nov 2019 11:08:23 +0100
From:   Jesper Dangaard Brouer <brouer@...hat.com>
To:     "Jonathan Lemon" <jonathan.lemon@...il.com>
Cc:     "Alexei Starovoitov" <ast@...com>, netdev@...r.kernel.org,
        davem@...emloft.net, "Kernel Team" <Kernel-team@...com>,
        ilias.apalodimas@...aro.org, brouer@...hat.com
Subject: Re: [net-next PATCH] page_pool: do not release pool until inflight
 == 0.

On Tue, 12 Nov 2019 09:32:10 -0800
"Jonathan Lemon" <jonathan.lemon@...il.com> wrote:

> On 12 Nov 2019, at 9:23, Alexei Starovoitov wrote:
> 
> > On 11/12/19 8:48 AM, Jesper Dangaard Brouer wrote:  
> >>> The trace_page_pool_state_release() does not dereference pool, it 
> >>> just
> >>> reports the pointer value, so there shouldn't be any use-after-free.  
> >> In the tracepoint we can still dereference the pool object pointer.
> >> This is made easier via using bpftrace for example see[1] (and with 
> >> BTF
> >> this will become more common to do so).  
> >
> > bpf tracing progs cannot assume that the pointer is valid.
> > The program can remember a kernel pointer in a map and then
> > access it days later.
> > Like kretprobe on kfree_skb(). The skb is freed. 100% use-after-free.
> > Such bpf program is broken and won't be reading meaningful values,
> > but it won't crash the kernel.
> >
> > On the other side we should not be passing pointers to freed objects
> > into tracepoints. That just wrong.
> > May be simply move that questionable tracepoint?  
> 
> Yes, move and simplify it.  I believe this patch should resolve the 
> issue, it just reports pages entering/exiting the pool, without
> trying to access the counters - the counters are reported through the
> inflight tracepoint.

Sorry, I don't like loosing the counter.  I have a plan for using these
counters in a bpftrace script.  (Worst case I might be able to live
without the counters).  

The basic idea is to use these tracepoints to detect if we leak
DMA-mappings. I'll try write the bpftrace script today, and
see it I can live without the counter.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer