On Fri, Nov 30, 2018 at 5:43 PM Deepa Dinamani deepa.kernel@gmail.com wrote:
On Sun, Nov 25, 2018 at 6:33 AM Willem de Bruijn willemdebruijn.kernel@gmail.com wrote:
On Sun, Nov 25, 2018 at 12:28 AM Deepa Dinamani deepa.kernel@gmail.com wrote:
if (type == SO_TIMESTAMP_NEW || type == SO_TIMESTAMPNS_NEW)sock_set_flag(sk, SOCK_TSTAMP_NEW);elsesock_reset_flag(sk, SOCK_TSTAMP_NEW);if adding a boolean whether the socket uses new or old-style timestamps, perhaps fail hard if a process tries to set a new-style option while an old-style is already set and vice versa. Also include SO_TIMESTAMPING_NEW as it toggles the same option.
I do not think this is a problem. Consider this example, if there is a user application with updated socket timestamps is linking into a library that is yet to be updated.
Also consider applications that do not use libraries.
Arnd and I talked about this. We thought that the new options should behave like the already existing options. The patch already does this. Eg: Today if we set SO_TIMESTAMP and then try to switch to SO_TIMESTAMPNS then there is no fail.
Do you still want a hard fail?
I do think that it is preferable. In general, and in this specific case.
We have had had many bug reports from syzkaller where the fuzzer manages to trigger unexpected behavior by combining two APIs that were never intended to be used together.
However inane the combination may be, once an API is published, we cannot simply add an EINVAL and stop supporting it. So it is safer to explicitly block unsafe combinations from the start. If there is a legitimate use it is always possible to loosen that restriction later.
I don't see any sensible use for mixing both the old and the new interface on the same socket.
That said, just a suggestion.