Re: [PATCH] vsock: Handle compat 32-bit timeout

[Arnd Bergmann]

On Wed, Oct 6, 2021 at 9:48 AM Richard Palethorpe <rpalethorpe@xxxxxxxx> wrote:
>
> Allow 32-bit timevals to be used with a 64-bit kernel.
>
> This allows the LTP regression test vsock01 to run without
> modification in 32-bit compat mode.
>
> Fixes: fe0c72f3db11 ("socket: move compat timeout handling into sock.c")
> Signed-off-by: Richard Palethorpe <rpalethorpe@xxxxxxxx>
>
> ---
>
> This is one of those fixes where I am not sure if we should just
> change the test instead. Because it's not clear if someone is likely
> to use vsock's in 32-bit compat mode?

We try very hard to ensure that compat mode works for every interface,
so it should be fixed in the kernel. Running compat mode is common
on memory-restricted machines, e.g. on cloud platforms and on deeply
embedded systems.

However, I think fixing the SO_VM_SOCKETS_CONNECT_TIMEOUT
to support 64-bit timeouts would actually be more important here. I think
what you need to do is to define the macro the same way
as the SO_TIMESTAMP one:

#define SO_RCVTIMEO (sizeof(time_t) == sizeof(__kernel_long_t) ? \
             SO_RCVTIMEO_OLD : SO_RCVTIMEO_NEW)
#define SO_TIMESTAMP (sizeof(time_t) == sizeof(__kernel_long_t) ? \
             SO_TIMESTAMP_OLD : SO_TIMESTAMP_NEW)
...

to ensure that user space picks an interface that matches the
user space definition of 'struct timeval'.

Your change looks correct otherwise, but I think you should first
add the new interface for 64-bit timeouts, since that likely changes
the code in a way that makes your current patch no longer the
best way to write it.

       Arnd