SO_PEERSEC protections in sk_getsockopt()?

[Alexei Starovoitov]

On Fri, Oct 7, 2022 at 10:43 AM Paul Moore <paul at paul-moore.com> wrote:
>
> On Wed, Oct 5, 2022 at 4:44 PM Paul Moore <paul at paul-moore.com> wrote:
> >
> > Hi Martin,
> >
> > In commit 4ff09db1b79b ("bpf: net: Change sk_getsockopt() to take the
> > sockptr_t argument") I see you wrapped the getsockopt value/len
> > pointers with sockptr_t and in the SO_PEERSEC case you pass the
> > sockptr_t:user field to avoid having to update the LSM hook and
> > implementations.  I think that's fine, especially as you note that
> > eBPF does not support fetching the SO_PEERSEC information, but I think
> > it would be good to harden this case to prevent someone from calling
> > sk_getsockopt(SO_PEERSEC) with kernel pointers.  What do you think of
> > something like this?
> >
> >   static int sk_getsockopt(...)
> >   {
> >     /* ... */
> >     case SO_PEERSEC:
> >       if (optval.is_kernel || optlen.is_kernel)
> >         return -EINVAL;
> >       return security_socket_getpeersec_stream(...);
> >     /* ... */
> >   }
>
> Any thoughts on this Martin, Alexei?  It would be nice to see this
> fixed soon ...

'fixed' ?
I don't see any bug.
Maybe WARN_ON_ONCE can be added as a precaution, but also dubious value.