netdev - Re: [RFC 0/1] whitelisting UDP GSO and GRO cmsgs

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]

Date:   Mon, 30 Nov 2020 10:15:19 -0500
From:   Soheil Hassas Yeganeh <soheil@...gle.com>
To:     Stefan Metzmacher <metze@...ba.org>
Cc:     Victor Stewart <v@...etag.social>,
        io-uring <io-uring@...r.kernel.org>,
        Luke Hsiao <lukehsiao@...gle.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jann Horn <jannh@...gle.com>, Arjun Roy <arjunroy@...gle.com>,
        netdev <netdev@...r.kernel.org>, Jens Axboe <axboe@...nel.dk>
Subject: Re: [RFC 0/1] whitelisting UDP GSO and GRO cmsgs

On Mon, Nov 30, 2020 at 10:05 AM Stefan Metzmacher <metze@...ba.org> wrote:
>
> Hi Soheil,
>
> > Thank you for CCing us.
> >
> > The reason for PROTO_CMSG_DATA_ONLY is explained in the paragraph
> > above in the commit message.  PROTO_CMSG_DATA_ONLY is basically to
> > allow-list a protocol that is guaranteed not to have the privilege
> > escalation in https://crbug.com/project-zero/1975.  TCP doesn't have
> > that issue, and I believe UDP doesn't have that issue either (but
> > please audit and confirm that with +Jann Horn).
> >
> > If you couldn't find any non-data CMSGs for UDP, you should just add
> > PROTO_CMSG_DATA_ONLY to inet dgram sockets instead of introducing
> > __sys_whitelisted_cmsghdrs as Stefan mentioned.
>
> Was there a specific reason why you only added the PROTO_CMSG_DATA_ONLY check
> in __sys_recvmsg_sock(), but not in __sys_sendmsg_sock()?

We only needed this for recvmsg(MSG_ERRQUEUE) to support transmit
zerocopy.  So, we took a more conservative approach and didn't add it
for sendmsg().

I believe it should be fine to add it for TCP sendmsg, because for
SO_MARK we check the user's capability:

if (!ns_capable(sock_net(sk)->user_ns, CAP_NET_ADMIN))
          return -EPERM;

I believe udp_sendmsg() is sane too and I cannot spot any issue there.

> metze
>
>
>