netdev - Re: [PATCH 0/3] PROTO_CMSG_DATA

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 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:   Sat, 12 Dec 2020 10:07:17 -0700
From:   Jens Axboe <axboe@...nel.dk>
To:     Victor Stewart <v@...etag.social>,
        io-uring <io-uring@...r.kernel.org>,
        Soheil Hassas Yeganeh <soheil@...gle.com>,
        netdev <netdev@...r.kernel.org>,
        Stefan Metzmacher <metze@...ba.org>
Subject: Re: [PATCH 0/3] PROTO_CMSG_DATA_ONLY for Datagram (UDP)

On 12/12/20 8:31 AM, Victor Stewart wrote:
> RE our conversation on the "[RFC 0/1] whitelisting UDP GSO and GRO
> cmsgs" thread...
> 
> https://lore.kernel.org/io-uring/CAM1kxwi5m6i8hrtkw7nZYoziPTD-Wp03+fcsUwh3CuSc=81kUQ@mail.gmail.com/
> 
> here are the patches we discussed.
> 
> Victor Stewart (3):
>    net/socket.c: add PROTO_CMSG_DATA_ONLY to __sys_sendmsg_sock
>    net/ipv4/af_inet.c: add PROTO_CMSG_DATA_ONLY to inet_dgram_ops
>    net/ipv6/af_inet6.c: add PROTO_CMSG_DATA_ONLY to inet6_dgram_ops
> 
>    net/ipv4/af_inet.c
>      |   1 +
>    net/ipv6/af_inet6.c
>     |   1 +
>    net/socket.c
>        |   8 +-
>    3 files changed, 7 insertions(+), 3 deletions(-)

Changes look fine to me, but a few comments:

- I'd order 1/3 as 3/3, that ordering makes more sense as at that point it
  could actually be used.

- For adding it to af_inet/af_inet6, you should write a better commit message
  on the reasoning for the change. Right now it just describes what the
  patch does (which is obvious from the change), not WHY it's done. Really
  goes for current 1/3 as well, commit messages need to be better in
  general.

I'd also CC Jann Horn on the series, he's the one that found an issue there
in the past and also acked the previous change on doing PROTO_CMSG_DATA_ONLY.

-- 
Jens Axboe