[PATCH] vsock: Handle compat 32-bit timeout

[PATCH] vsock: Handle compat 32-bit timeout

[Richard Palethorpe]

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@suse.com>

---

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?

 net/vmw_vsock/af_vsock.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 3e02cc3b24f8..6e9232adf8d0 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -1616,7 +1616,16 @@ static int vsock_connectible_setsockopt(struct socket *sock,
 
 	case SO_VM_SOCKETS_CONNECT_TIMEOUT: {
 		struct __kernel_old_timeval tv;
-		COPY_IN(tv);
+		struct old_timeval32 tv32;
+
+		if (in_compat_syscall() && !COMPAT_USE_64BIT_TIME) {
+			COPY_IN(tv32);
+			tv.tv_sec = tv32.tv_sec;
+			tv.tv_usec = tv32.tv_usec;
+		} else {
+			COPY_IN(tv);
+		}
+
 		if (tv.tv_sec >= 0 && tv.tv_usec < USEC_PER_SEC &&
 		    tv.tv_sec < (MAX_SCHEDULE_TIMEOUT / HZ - 1)) {
 			vsk->connect_timeout = tv.tv_sec * HZ +
@@ -1694,11 +1703,20 @@ static int vsock_connectible_getsockopt(struct socket *sock,
 
 	case SO_VM_SOCKETS_CONNECT_TIMEOUT: {
 		struct __kernel_old_timeval tv;
+		struct old_timeval32 tv32;
+
 		tv.tv_sec = vsk->connect_timeout / HZ;
 		tv.tv_usec =
 		    (vsk->connect_timeout -
 		     tv.tv_sec * HZ) * (1000000 / HZ);
-		COPY_OUT(tv);
+
+		if (in_compat_syscall() && !COMPAT_USE_64BIT_TIME) {
+			tv32.tv_sec = (u32)tv.tv_sec;
+			tv32.tv_usec = (u32)tv.tv_usec;
+			COPY_OUT(tv32);
+		} else {
+			COPY_OUT(tv);
+		}
 		break;
 	}
 	default:
-- 
2.33.0

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

[Arnd Bergmann]

On Wed, Oct 6, 2021 at 9:48 AM Richard Palethorpe <rpalethorpe@suse.com> 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@suse.com>
>
> ---
>
> 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

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

[Richard Palethorpe]

Hello Arnd,

Arnd Bergmann <arnd@arndb.de> writes:

> On Wed, Oct 6, 2021 at 9:48 AM Richard Palethorpe <rpalethorpe@suse.com> 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@suse.com>
>>
>> ---
>>
>> 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.

Thanks!

>
> 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

Ah, yes, it will still be broken if libc is configured with 64bit
timeval only.

-- 
Thank you,
Richard.