All of lore.kernel.org
 help / color / mirror / Atom feed
* how is the bpfilter sockopt processing supposed to work
@ 2020-07-17  5:52 Christoph Hellwig
  2020-07-17 16:13 ` Alexei Starovoitov
  0 siblings, 1 reply; 5+ messages in thread
From: Christoph Hellwig @ 2020-07-17  5:52 UTC (permalink / raw)
  To: Alexei Starovoitov; +Cc: David S. Miller, netdev, bpf

Hi Alexei,

I've just been auditing the sockopt code, and bpfilter looks really
odd.  Both getsockopts and setsockopt eventually end up
in__bpfilter_process_sockopt, which then passes record to the
userspace helper containing the address of the optval buffer.
Which depending on bpf-cgroup might be in user or kernel space.
But even if it is in userspace it would be in a different process
than the bpfiler helper.  What makes all this work?

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: how is the bpfilter sockopt processing supposed to work
  2020-07-17  5:52 how is the bpfilter sockopt processing supposed to work Christoph Hellwig
@ 2020-07-17 16:13 ` Alexei Starovoitov
  2020-07-17 16:25   ` Christoph Hellwig
  0 siblings, 1 reply; 5+ messages in thread
From: Alexei Starovoitov @ 2020-07-17 16:13 UTC (permalink / raw)
  To: Christoph Hellwig, Stanislav Fomichev
  Cc: Alexei Starovoitov, David S. Miller, Network Development, bpf

On Thu, Jul 16, 2020 at 10:52 PM Christoph Hellwig <hch@lst.de> wrote:
>
> Hi Alexei,
>
> I've just been auditing the sockopt code, and bpfilter looks really
> odd.  Both getsockopts and setsockopt eventually end up
> in__bpfilter_process_sockopt, which then passes record to the
> userspace helper containing the address of the optval buffer.
> Which depending on bpf-cgroup might be in user or kernel space.
> But even if it is in userspace it would be in a different process
> than the bpfiler helper.  What makes all this work?

Hmm. Good point. bpfilter assumes user addresses. It will break
if bpf cgroup sockopt messes with it.
We had a different issue with bpf-cgroup-sockopt and iptables in the past.
Probably the easiest way forward is to special case this particular one.
With your new series is there a way to tell in bpfilter_ip_get_sockopt()
whether addr is kernel or user? And if it's the kernel just return with error.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: how is the bpfilter sockopt processing supposed to work
  2020-07-17 16:13 ` Alexei Starovoitov
@ 2020-07-17 16:25   ` Christoph Hellwig
  2020-07-17 17:28     ` Alexei Starovoitov
  0 siblings, 1 reply; 5+ messages in thread
From: Christoph Hellwig @ 2020-07-17 16:25 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Christoph Hellwig, Stanislav Fomichev, Alexei Starovoitov,
	David S. Miller, Network Development, bpf

On Fri, Jul 17, 2020 at 09:13:07AM -0700, Alexei Starovoitov wrote:
> On Thu, Jul 16, 2020 at 10:52 PM Christoph Hellwig <hch@lst.de> wrote:
> >
> > Hi Alexei,
> >
> > I've just been auditing the sockopt code, and bpfilter looks really
> > odd.  Both getsockopts and setsockopt eventually end up
> > in__bpfilter_process_sockopt, which then passes record to the
> > userspace helper containing the address of the optval buffer.
> > Which depending on bpf-cgroup might be in user or kernel space.
> > But even if it is in userspace it would be in a different process
> > than the bpfiler helper.  What makes all this work?
> 
> Hmm. Good point. bpfilter assumes user addresses. It will break
> if bpf cgroup sockopt messes with it.
> We had a different issue with bpf-cgroup-sockopt and iptables in the past.
> Probably the easiest way forward is to special case this particular one.
> With your new series is there a way to tell in bpfilter_ip_get_sockopt()
> whether addr is kernel or user? And if it's the kernel just return with error.

Yes, I can send a fix.  But how do even the user space addressed work?
If some random process calls getsockopt or setsockopt, how does the
bpfilter user mode helper attach to its address space?

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: how is the bpfilter sockopt processing supposed to work
  2020-07-17 16:25   ` Christoph Hellwig
@ 2020-07-17 17:28     ` Alexei Starovoitov
  2020-07-20  8:25       ` David Laight
  0 siblings, 1 reply; 5+ messages in thread
From: Alexei Starovoitov @ 2020-07-17 17:28 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Stanislav Fomichev, Alexei Starovoitov, David S. Miller,
	Network Development, bpf

On Fri, Jul 17, 2020 at 9:25 AM Christoph Hellwig <hch@lst.de> wrote:
>
> On Fri, Jul 17, 2020 at 09:13:07AM -0700, Alexei Starovoitov wrote:
> > On Thu, Jul 16, 2020 at 10:52 PM Christoph Hellwig <hch@lst.de> wrote:
> > >
> > > Hi Alexei,
> > >
> > > I've just been auditing the sockopt code, and bpfilter looks really
> > > odd.  Both getsockopts and setsockopt eventually end up
> > > in__bpfilter_process_sockopt, which then passes record to the
> > > userspace helper containing the address of the optval buffer.
> > > Which depending on bpf-cgroup might be in user or kernel space.
> > > But even if it is in userspace it would be in a different process
> > > than the bpfiler helper.  What makes all this work?
> >
> > Hmm. Good point. bpfilter assumes user addresses. It will break
> > if bpf cgroup sockopt messes with it.
> > We had a different issue with bpf-cgroup-sockopt and iptables in the past.
> > Probably the easiest way forward is to special case this particular one.
> > With your new series is there a way to tell in bpfilter_ip_get_sockopt()
> > whether addr is kernel or user? And if it's the kernel just return with error.
>
> Yes, I can send a fix.  But how do even the user space addressed work?
> If some random process calls getsockopt or setsockopt, how does the
> bpfilter user mode helper attach to its address space?

The actual bpfilter processing is in two patches that we didn't land:
https://lore.kernel.org/patchwork/patch/902785/
https://lore.kernel.org/patchwork/patch/902783/
UMD is using process_vm_readv().
The target process is waiting for the sockopt syscall to return,
so from the toctou perspective it's the same as the kernel doing copy_from_user.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: how is the bpfilter sockopt processing supposed to work
  2020-07-17 17:28     ` Alexei Starovoitov
@ 2020-07-20  8:25       ` David Laight
  0 siblings, 0 replies; 5+ messages in thread
From: David Laight @ 2020-07-20  8:25 UTC (permalink / raw)
  To: 'Alexei Starovoitov', Christoph Hellwig
  Cc: Stanislav Fomichev, Alexei Starovoitov, David S. Miller,
	Network Development, bpf

From: Alexei Starovoitov
> Sent: 17 July 2020 18:29
> On Fri, Jul 17, 2020 at 9:25 AM Christoph Hellwig <hch@lst.de> wrote:
> >
> > On Fri, Jul 17, 2020 at 09:13:07AM -0700, Alexei Starovoitov wrote:
> > > On Thu, Jul 16, 2020 at 10:52 PM Christoph Hellwig <hch@lst.de> wrote:
> > > >
> > > > Hi Alexei,
> > > >
> > > > I've just been auditing the sockopt code, and bpfilter looks really
> > > > odd.  Both getsockopts and setsockopt eventually end up
> > > > in__bpfilter_process_sockopt, which then passes record to the
> > > > userspace helper containing the address of the optval buffer.
> > > > Which depending on bpf-cgroup might be in user or kernel space.
> > > > But even if it is in userspace it would be in a different process
> > > > than the bpfiler helper.  What makes all this work?
> > >
> > > Hmm. Good point. bpfilter assumes user addresses. It will break
> > > if bpf cgroup sockopt messes with it.
> > > We had a different issue with bpf-cgroup-sockopt and iptables in the past.
> > > Probably the easiest way forward is to special case this particular one.
> > > With your new series is there a way to tell in bpfilter_ip_get_sockopt()
> > > whether addr is kernel or user? And if it's the kernel just return with error.
> >
> > Yes, I can send a fix.  But how do even the user space addressed work?
> > If some random process calls getsockopt or setsockopt, how does the
> > bpfilter user mode helper attach to its address space?
> 
> The actual bpfilter processing is in two patches that we didn't land:
> https://lore.kernel.org/patchwork/patch/902785/
> https://lore.kernel.org/patchwork/patch/902783/
> UMD is using process_vm_readv().
> The target process is waiting for the sockopt syscall to return,
> so from the toctou perspective it's the same as the kernel doing copy_from_user.

You need to be doing the user-spaces accesses from the target process's
context.
If it is waiting for the syscall to return then you aren't running
it its context.

There is also at least one sockopt that has an embedded pointer.
So even if you've copied the sockopt buffer into kernel space
you are doomed.
If the request has come from a kernel thread with kernel buffers
then the embedded buffer will be kernel.
(This is probably more common for ioctl() requests.)

I'm not sure there is any sane was to handle this.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-07-20  8:25 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-17  5:52 how is the bpfilter sockopt processing supposed to work Christoph Hellwig
2020-07-17 16:13 ` Alexei Starovoitov
2020-07-17 16:25   ` Christoph Hellwig
2020-07-17 17:28     ` Alexei Starovoitov
2020-07-20  8:25       ` David Laight

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.