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

[Jesper Dangaard Brouer <brouer@redhat.com>]

On Tue, 12 Nov 2019 09:32:10 -0800
"Jonathan Lemon" <jonathan.lemon@gmail.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