Re: [PATCH RFC] bpf: Add support for reading user pointers

[Joel Fernandes]

On Sun, May 05, 2019 at 12:04:24PM +0100, Qais Yousef wrote:
> On 05/03/19 09:49, Joel Fernandes wrote:
> > On Fri, May 03, 2019 at 01:12:34PM +0100, Qais Yousef wrote:
> > > Hi Joel
> > > 
> > > On 05/02/19 16:49, Joel Fernandes (Google) wrote:
> > > > The eBPF based opensnoop tool fails to read the file path string passed
> > > > to the do_sys_open function. This is because it is a pointer to
> > > > userspace address and causes an -EFAULT when read with
> > > > probe_kernel_read. This is not an issue when running the tool on x86 but
> > > > is an issue on arm64. This patch adds a new bpf function call based
> > > 
> > > I just did an experiment and if I use Android 4.9 kernel I indeed fail to see
> > > PATH info when running opensnoop. But if I run on 5.1-rc7 opensnoop behaves
> > > correctly on arm64.
> > > 
> > > My guess either a limitation that was fixed on later kernel versions or Android
> > > kernel has some strict option/modifications that make this fail?
> > 
> > Thanks a lot for checking, yes I was testing 4.9 kernel with this patch (pixel 3).
> > 
> > I am not sure what has changed since then, but I still think it is a good
> > idea to make the code more robust against such future issues anyway. In
> > particular, we learnt with extensive discussions that user/kernel pointers
> > are not necessarily distinguishable purely based on their address.
> 
> Yes I wasn't arguing against that. But the commit message is misleading or
> needs more explanation at least. I tried 4.9.y stable and arm64 worked on that
> too. Why do you think it's an arm64 problem?

Well it is broken on at least on at least one arm64 device and the patch I
sent fixes it. We know that the bpf is using wrong kernel API so why not fix
it? Are you saying we should not fix it like in this patch? Or do you have
another fix in mind?

thanks.


> 
> --
> Qais Yousef