netdev - Re: net/core/sock.c lacks some SO_TIMESTAMPING

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <0d7cddc9-03fa-43db-a579-14f3e822615b@app.fastmail.com>
Date: Wed, 20 Dec 2023 09:43:45 +0000
From: "Arnd Bergmann" <arnd@...db.de>
To: "Willem de Bruijn" <willemdebruijn.kernel@...il.com>,
 "Thomas Lange" <thomas@...elatus.se>, Netdev <netdev@...r.kernel.org>,
 "Deepa Dinamani" <deepa.kernel@...il.com>
Cc: Jörn-Thorben Hinz <jthinz@...lbox.tu-berlin.de>
Subject: Re: net/core/sock.c lacks some SO_TIMESTAMPING_NEW support

On Wed, Dec 20, 2023, at 04:00, Willem de Bruijn wrote:
> Thomas Lange wrote:
>> diff --git a/net/core/sock.c b/net/core/sock.c
>> index 16584e2dd648..a56ec1d492c9 100644
>> --- a/net/core/sock.c
>> +++ b/net/core/sock.c
>> @@ -2821,6 +2821,7 @@ int __sock_cmsg_send(struct sock *sk, struct cmsghdr *cmsg,
>>                  sockc->mark = *(u32 *)CMSG_DATA(cmsg);
>>                  break;
>>          case SO_TIMESTAMPING_OLD:
>> +       case SO_TIMESTAMPING_NEW:
>>                  if (cmsg->cmsg_len != CMSG_LEN(sizeof(u32)))
>>                          return -EINVAL;
>> 
>> However, looking through the module, it seems that sk_getsockopt() has no
>> support for SO_TIMESTAMPING_NEW either, but sk_setsockopt() has.
>
> Good point. Adding the author to see if this was a simple oversight or
> there was a rationale at the time for leaving it out.

I'm fairly sure this was just a mistake on our side. For the cmsg case,
I think we just missed it because there is no corresponding SO_TIMESTAMP{,NS}
version of this, so it fell through the cracks.

In the patch above, I'm not entirely sure about what needs to happen
with the old/new format, i.e. the

   sock_valbool_flag(sk, SOCK_TSTAMP_NEW, optname == SO_TIMESTAMPING_NEW)

from setsockopt(). Is __sock_cmsg_send() allowed to turn on timestamping
without it being first enabled using setsockopt()? If so, I think
we need to set the flag here the same way that setsockopt does. If
not, then I think we instead should check that the old/new format
in the option sent via cmsg is the same that was set earlier with
setsockopt.

For the missing getsockopt, there was even a patch earlier this year
by Jörn-Thorben Hinz [1], but I failed to realize that we need patch
1/2 from his series regardless of patch 2/2.

     Arnd

[1] https://lore.kernel.org/lkml/20230703175048.151683-2-jthinz@mailbox.tu-berlin.de/