net: y2038-safe socket timeout options

PATCH In review Deepa Dinamani 2 participants 8 messages Jan 8, 2019

Subsystems (11)

Networking Network drivers PowerPC MIPS +7 more subsystems

lore

Share & tools

lore

Deepa Dinamani

author

2019-01-08 05:22 UTC #0 lore raw

cover net: y2038-safe socket timeout options

The series is aimed at adding y2038-safe timeout options:
SO_RCVTIMEO_NEW and SO_SNDTIMEO_NEW.

This is similar to the previous series adding y2038-safe SO_TIMESTAMP* options.

The series needs to be applied after the socket timestamp series:
https://lore.kernel.org/lkml/20190108032657.8331-1-deepa.kernel at gmail.com

Deepa Dinamani (3):
socket: Use old_timeval types for socket timeouts
socket: Rename SO_RCVTIMEO/ SO_SNDTIMEO with _OLD suffixes
sock: Add SO_RCVTIMEO_NEW and SO_SNDTIMEO_NEW

 arch/alpha/include/uapi/asm/socket.h   | 13 ++++-
 arch/mips/include/uapi/asm/socket.h    | 13 ++++-
 arch/parisc/include/uapi/asm/socket.h  | 13 ++++-
 arch/powerpc/include/uapi/asm/socket.h |  4 +-
 arch/sparc/include/uapi/asm/socket.h   | 13 ++++-
 fs/dlm/lowcomms.c                      |  4 +-
 include/uapi/asm-generic/socket.h      | 13 ++++-
 net/compat.c                           | 14 ++---
 net/core/sock.c                        | 78 +++++++++++++++++++-------
 net/vmw_vsock/af_vsock.c               |  4 +-
 10 files changed, 126 insertions(+), 43 deletions(-)

base-commit: a4983672f9ca4c8393f26b6b80710e6c78886b8c
prerequisite-patch-id: a03ec6afbdd328cd90557f7ee6675016a5f5c653
prerequisite-patch-id: 724d26c3036e6f3a38f810c2f10db3f7ddbf843b
prerequisite-patch-id: 14017867b6eb4d5231eec1b563edcd840a1be26e
prerequisite-patch-id: 8df0edfd9b973ff5aae91c7709c8223be096a5bc
prerequisite-patch-id: 9850ad48d41bf068f074c0dd3c7610fb7177c89f
prerequisite-patch-id: bd31f35bba11902d1cc3e8726492b54df34b5c59
prerequisite-patch-id: ea4b005c5ad188a4e0899d728357c114710a3a8e
prerequisite-patch-id: cc3ee912c1ee1ea502ca079de81236a467950501

Signature · 15 lines

2.17.1

Cc: ccaulfie at redhat.com
Cc: cluster-devel at redhat.com
Cc: davem at davemloft.net
Cc: deller at gmx.de
Cc: linux-alpha at vger.kernel.org
Cc: linux-arch at vger.kernel.org
Cc: linux-mips at vger.kernel.org
Cc: linux-parisc at vger.kernel.org
Cc: linuxppc-dev at lists.ozlabs.org
Cc: paulus at samba.org
Cc: ralf at linux-mips.org
Cc: rth at twiddle.net
Cc: sparclinux at vger.kernel.org

Deepa Dinamani

author

2019-01-08 05:22 UTC #1 lore raw

PATCH 1/3 socket: Use old_timeval types for socket timeouts

As part of y2038 solution, all internal uses of struct timeval are replaced by struct __kernel_old_timeval and struct compat_timeval by struct old_timeval32. Make socket timeouts use these new types.

This is mainly to be able to verify that the kernel build is y2038 safe when such non y2038 safe types are not supported anymore.

Signed-off-by Deepa Dinamani <[email protected]>

---
 net/compat.c             | 10 +++++-----
 net/core/sock.c          |  8 ++++----
 net/vmw_vsock/af_vsock.c |  4 ++--
 3 files changed, 11 insertions(+), 11 deletions(-)

net/compat.c +5 -5

index b6ff0899e424..cbc15f65033c 100644
@@ -351,8 +351,8 @@ static int do_set_attach_filter(struct socket *sock, int level, int optname,
 static int do_set_sock_timeout(struct socket *sock, int level,
 		int optname, char __user *optval, unsigned int optlen)
 {
-	struct compat_timeval __user *up = (struct compat_timeval __user *)optval;
-	struct timeval ktime;
+	struct old_timeval32 __user *up = (struct old_timeval32 __user *)optval;
+	struct __kernel_old_timeval ktime;
 	mm_segment_t old_fs;
 	int err;

@@ -420,12 +420,12 @@ COMPAT_SYSCALL_DEFINE5(setsockopt, int, fd, int, level, int, optname,
 static int do_get_sock_timeout(struct socket *sock, int level, int optname,
 		char __user *optval, int __user *optlen)
 {
-	struct compat_timeval __user *up;
-	struct timeval ktime;
+	struct old_timeval32 __user *up;
+	struct __kernel_old_timeval ktime;
 	mm_segment_t old_fs;
 	int len, err;
 
-	up = (struct compat_timeval __user *)optval;
+	up = (struct old_timeval32 __user *)optval;
 	if (get_user(len, optlen))
 		return -EFAULT;
 	if (len < sizeof(*up))

net/core/sock.c +4 -4

index 78df5228ffbd..af0fb33624e2 100644
@@ -337,7 +337,7 @@ EXPORT_SYMBOL(__sk_backlog_rcv);
 
 static int sock_set_timeout(long *timeo_p, char __user *optval, int optlen)
 {
-	struct timeval tv;
+	struct __kernel_old_timeval tv;
 
 	if (optlen < sizeof(tv))
 		return -EINVAL;

@@ -1113,7 +1113,7 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
 		int val;
 		u64 val64;
 		struct linger ling;
-		struct timeval tm;
+		struct __kernel_old_timeval tm;
 		struct sock_txtime txtime;
 	} v;

@@ -1223,7 +1223,7 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
 		break;
 
 	case SO_RCVTIMEO:
-		lv = sizeof(struct timeval);
+		lv = sizeof(struct __kernel_old_timeval);
 		if (sk->sk_rcvtimeo == MAX_SCHEDULE_TIMEOUT) {
 			v.tm.tv_sec = 0;
 			v.tm.tv_usec = 0;

@@ -1234,7 +1234,7 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
 		break;
 
 	case SO_SNDTIMEO:
-		lv = sizeof(struct timeval);
+		lv = sizeof(struct __kernel_old_timeval);
 		if (sk->sk_sndtimeo == MAX_SCHEDULE_TIMEOUT) {
 			v.tm.tv_sec = 0;
 			v.tm.tv_usec = 0;

net/vmw_vsock/af_vsock.c +2 -2

index 43a1dec08825..e1149bf1b57b 100644
@@ -1439,7 +1439,7 @@ static int vsock_stream_setsockopt(struct socket *sock,
 		break;
 
 	case SO_VM_SOCKETS_CONNECT_TIMEOUT: {
-		struct timeval tv;
+		struct __kernel_old_timeval tv;
 		COPY_IN(tv);
 		if (tv.tv_sec >= 0 && tv.tv_usec < USEC_PER_SEC &&
 		    tv.tv_sec < (MAX_SCHEDULE_TIMEOUT / HZ - 1)) {

@@ -1517,7 +1517,7 @@ static int vsock_stream_getsockopt(struct socket *sock,
 		break;
 
 	case SO_VM_SOCKETS_CONNECT_TIMEOUT: {
-		struct timeval tv;
+		struct __kernel_old_timeval tv;
 		tv.tv_sec = vsk->connect_timeout / HZ;
 		tv.tv_usec =
 		    (vsk->connect_timeout -

Signature · 1 line

2.17.1

Deepa Dinamani

author

2019-01-08 05:22 UTC #2 lore raw

PATCH 2/3 socket: Rename SO_RCVTIMEO/ SO_SNDTIMEO with _OLD suffixes

In reply to Deepa Dinamani

SO_RCVTIMEO and SO_SNDTIMEO socket options use struct timeval as the time format. struct timeval is not y2038 safe. The subsequent patches in the series add support for new socket timeout options with _NEW suffix that are y2038 safe. Rename the existing options with _OLD suffix forms so that the right option is enabled for userspace applications according to the architecture and time_t definition of libc.

Signed-off-by Deepa Dinamani <[email protected]>

Cc ccaulfie at redhat.com

Cc deller at gmx.de

Cc paulus at samba.org

Cc ralf at linux-mips.org

Cc rth at twiddle.net

Cc cluster-devel at redhat.com

Cc linuxppc-dev at lists.ozlabs.org

Cc linux-alpha at vger.kernel.org

Cc linux-arch at vger.kernel.org

Cc linux-mips at vger.kernel.org

Cc linux-parisc at vger.kernel.org

Cc sparclinux at vger.kernel.org

---
 arch/alpha/include/uapi/asm/socket.h   | 7 +++++--
 arch/mips/include/uapi/asm/socket.h    | 6 ++++--
 arch/parisc/include/uapi/asm/socket.h  | 6 ++++--
 arch/powerpc/include/uapi/asm/socket.h | 4 ++--
 arch/sparc/include/uapi/asm/socket.h   | 6 ++++--
 fs/dlm/lowcomms.c                      | 4 ++--
 include/net/sock.h                     | 4 ++--
 include/uapi/asm-generic/socket.h      | 6 ++++--
 net/compat.c                           | 4 ++--
 net/core/sock.c                        | 8 ++++----
 10 files changed, 33 insertions(+), 22 deletions(-)

arch/alpha/include/uapi/asm/socket.h +5 -2

index da08412bd49f..ea3ba981d8a0 100644
@@ -31,8 +31,8 @@
 #define SO_RCVBUFFORCE	0x100b
 #define	SO_RCVLOWAT	0x1010
 #define	SO_SNDLOWAT	0x1011
-#define	SO_RCVTIMEO	0x1012
-#define	SO_SNDTIMEO	0x1013
+#define	SO_RCVTIMEO_OLD	0x1012
+#define	SO_SNDTIMEO_OLD	0x1013
 #define SO_ACCEPTCONN	0x1014
 #define SO_PROTOCOL	0x1028
 #define SO_DOMAIN	0x1029

@@ -120,6 +120,9 @@
 
 #if !defined(__KERNEL__)
 
+#define	SO_RCVTIMEO SO_RCVTIMEO_OLD
+#define	SO_SNDTIMEO SO_SNDTIMEO_OLD
+
 #if __BITS_PER_LONG == 64
 #define SO_TIMESTAMP		SO_TIMESTAMP_OLD
 #define SO_TIMESTAMPNS		SO_TIMESTAMPNS_OLD

arch/mips/include/uapi/asm/socket.h +4 -2

index 1e48f67f1052..4dde20d64690 100644
@@ -39,8 +39,8 @@
 #define SO_RCVBUF	0x1002	/* Receive buffer. */
 #define SO_SNDLOWAT	0x1003	/* send low-water mark */
 #define SO_RCVLOWAT	0x1004	/* receive low-water mark */
-#define SO_SNDTIMEO	0x1005	/* send timeout */
-#define SO_RCVTIMEO	0x1006	/* receive timeout */
+#define SO_SNDTIMEO_OLD	0x1005	/* send timeout */
+#define SO_RCVTIMEO_OLD	0x1006	/* receive timeout */
 #define SO_ACCEPTCONN	0x1009
 #define SO_PROTOCOL	0x1028	/* protocol type */
 #define SO_DOMAIN	0x1029	/* domain/socket family */

@@ -130,6 +130,8 @@
 
 #if !defined(__KERNEL__)
 
+#define	SO_RCVTIMEO SO_RCVTIMEO_OLD
+#define	SO_SNDTIMEO SO_SNDTIMEO_OLD
 #if __BITS_PER_LONG == 64
 #define SO_TIMESTAMP		SO_TIMESTAMP_OLD
 #define SO_TIMESTAMPNS		SO_TIMESTAMPNS_OLD

arch/parisc/include/uapi/asm/socket.h +4 -2

index e8d6cf20f9a4..546937fa0d8b 100644
@@ -22,8 +22,8 @@
 #define SO_RCVBUFFORCE	0x100b
 #define SO_SNDLOWAT	0x1003
 #define SO_RCVLOWAT	0x1004
-#define SO_SNDTIMEO	0x1005
-#define SO_RCVTIMEO	0x1006
+#define SO_SNDTIMEO_OLD	0x1005
+#define SO_RCVTIMEO_OLD	0x1006
 #define SO_ERROR	0x1007
 #define SO_TYPE		0x1008
 #define SO_PROTOCOL	0x1028

@@ -111,6 +111,8 @@
 
 #if !defined(__KERNEL__)
 
+#define	SO_RCVTIMEO SO_RCVTIMEO_OLD
+#define	SO_SNDTIMEO SO_SNDTIMEO_OLD
 #if __BITS_PER_LONG == 64
 #define SO_TIMESTAMP		SO_TIMESTAMP_OLD
 #define SO_TIMESTAMPNS		SO_TIMESTAMPNS_OLD

arch/powerpc/include/uapi/asm/socket.h +2 -2

index 94de465e0920..12aa0c43e775 100644
@@ -11,8 +11,8 @@
 
 #define SO_RCVLOWAT	16
 #define SO_SNDLOWAT	17
-#define SO_RCVTIMEO	18
-#define SO_SNDTIMEO	19
+#define SO_RCVTIMEO_OLD	18
+#define SO_SNDTIMEO_OLD	19
 #define SO_PASSCRED	20
 #define SO_PEERCRED	21

arch/sparc/include/uapi/asm/socket.h +4 -2

index fc65bf6b6440..bdc396211627 100644
@@ -21,8 +21,8 @@
 #define SO_BSDCOMPAT    0x0400
 #define SO_RCVLOWAT     0x0800
 #define SO_SNDLOWAT     0x1000
-#define SO_RCVTIMEO     0x2000
-#define SO_SNDTIMEO     0x4000
+#define SO_RCVTIMEO_OLD     0x2000
+#define SO_SNDTIMEO_OLD     0x4000
 #define SO_ACCEPTCONN	0x8000
 
 #define SO_SNDBUF	0x1001

@@ -112,6 +112,8 @@
 
 #if !defined(__KERNEL__)
 
+#define	SO_RCVTIMEO	SO_RCVTIMEO_OLD
+#define	SO_SNDTIMEO	SO_SNDTIMEO_OLD
 #if __BITS_PER_LONG == 64
 #define SO_TIMESTAMP		SO_TIMESTAMP_OLD
 #define SO_TIMESTAMPNS		SO_TIMESTAMPNS_OLD

fs/dlm/lowcomms.c +2 -2

index 76976d6e50f9..c98ad9777ad9 100644
@@ -1089,12 +1089,12 @@ static void sctp_connect_to_sock(struct connection *con)
 	 * since O_NONBLOCK argument in connect() function does not work here,
 	 * then, we should restore the default value of this attribute.
 	 */
-	kernel_setsockopt(sock, SOL_SOCKET, SO_SNDTIMEO, (char *)&tv,
+	kernel_setsockopt(sock, SOL_SOCKET, SO_SNDTIMEO_OLD, (char *)&tv,
 			  sizeof(tv));
 	result = sock->ops->connect(sock, (struct sockaddr *)&daddr, addr_len,
 				   0);
 	memset(&tv, 0, sizeof(tv));
-	kernel_setsockopt(sock, SOL_SOCKET, SO_SNDTIMEO, (char *)&tv,
+	kernel_setsockopt(sock, SOL_SOCKET, SO_SNDTIMEO_OLD, (char *)&tv,
 			  sizeof(tv));
 
 	if (result == -EINPROGRESS)

include/net/sock.h +2 -2

index 6679f3c120b0..98965a9a2bf4 100644
@@ -292,8 +292,8 @@ struct sock_common {
   *	@sk_peer_pid: &struct pid for this socket's peer
   *	@sk_peer_cred: %SO_PEERCRED setting
   *	@sk_rcvlowat: %SO_RCVLOWAT setting
-  *	@sk_rcvtimeo: %SO_RCVTIMEO setting
-  *	@sk_sndtimeo: %SO_SNDTIMEO setting
+  *	@sk_rcvtimeo: %SO_RCVTIMEO_OLD setting
+  *	@sk_sndtimeo: %SO_SNDTIMEO_OLD setting
   *	@sk_txhash: computed flow hash for use on transmit
   *	@sk_filter: socket filtering instructions
   *	@sk_timer: sock cleanup timer

include/uapi/asm-generic/socket.h +4 -2

index 94e618a4a43f..9e370586fb19 100644
@@ -30,8 +30,8 @@
 #define SO_PEERCRED	17
 #define SO_RCVLOWAT	18
 #define SO_SNDLOWAT	19
-#define SO_RCVTIMEO	20
-#define SO_SNDTIMEO	21
+#define SO_RCVTIMEO_OLD	20
+#define SO_SNDTIMEO_OLD	21
 #endif
 
 /* Security levels - as per NRL IPv6 - don't actually do anything */

@@ -114,6 +114,8 @@
 
 #if !defined(__KERNEL__)
 
+#define	SO_RCVTIMEO SO_RCVTIMEO_OLD
+#define	SO_SNDTIMEO SO_SNDTIMEO_OLD
 #if __BITS_PER_LONG == 64 || (defined(__x86_64__) && defined(__ILP32__))
 /* on 64-bit and x32, avoid the ?: operator */
 #define SO_TIMESTAMP		SO_TIMESTAMP_OLD

net/compat.c +2 -2

index cbc15f65033c..19e047f70f64 100644
@@ -378,7 +378,7 @@ static int compat_sock_setsockopt(struct socket *sock, int level, int optname,
 		return do_set_attach_filter(sock, level, optname,
 					    optval, optlen);
 	if (!COMPAT_USE_64BIT_TIME &&
-	    (optname == SO_RCVTIMEO || optname == SO_SNDTIMEO))
+	    (optname == SO_RCVTIMEO_OLD || optname == SO_SNDTIMEO_OLD))
 		return do_set_sock_timeout(sock, level, optname, optval, optlen);
 
 	return sock_setsockopt(sock, level, optname, optval, optlen);

@@ -450,7 +450,7 @@ static int compat_sock_getsockopt(struct socket *sock, int level, int optname,
 				char __user *optval, int __user *optlen)
 {
 	if (!COMPAT_USE_64BIT_TIME &&
-	    (optname == SO_RCVTIMEO || optname == SO_SNDTIMEO))
+	    (optname == SO_RCVTIMEO_OLD || optname == SO_SNDTIMEO_OLD))
 		return do_get_sock_timeout(sock, level, optname, optval, optlen);
 	return sock_getsockopt(sock, level, optname, optval, optlen);
 }

net/core/sock.c +4 -4

index af0fb33624e2..42914ca3186c 100644
@@ -889,11 +889,11 @@ int sock_setsockopt(struct socket *sock, int level, int optname,
 			sk->sk_rcvlowat = val ? : 1;
 		break;
 
-	case SO_RCVTIMEO:
+	case SO_RCVTIMEO_OLD:
 		ret = sock_set_timeout(&sk->sk_rcvtimeo, optval, optlen);
 		break;
 
-	case SO_SNDTIMEO:
+	case SO_SNDTIMEO_OLD:
 		ret = sock_set_timeout(&sk->sk_sndtimeo, optval, optlen);
 		break;

@@ -1222,7 +1222,7 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
 		v.val = sk->sk_tsflags;
 		break;
 
-	case SO_RCVTIMEO:
+	case SO_RCVTIMEO_OLD:
 		lv = sizeof(struct __kernel_old_timeval);
 		if (sk->sk_rcvtimeo == MAX_SCHEDULE_TIMEOUT) {
 			v.tm.tv_sec = 0;

@@ -1233,7 +1233,7 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
 		}
 		break;
 
-	case SO_SNDTIMEO:
+	case SO_SNDTIMEO_OLD:
 		lv = sizeof(struct __kernel_old_timeval);
 		if (sk->sk_sndtimeo == MAX_SCHEDULE_TIMEOUT) {
 			v.tm.tv_sec = 0;

Signature · 1 line

2.17.1

Arnd Bergmann

maintainer

2019-01-08 20:03 UTC #3 lore raw

Re: [PATCH 2/3] socket: Rename SO_RCVTIMEO/ SO_SNDTIMEO with _OLD suffixes

Quoted 10 lines · Deepa Dinamani

SO_RCVTIMEO and SO_SNDTIMEO socket options use struct timeval as the time format. struct timeval is not y2038 safe. The subsequent patches in the series add support for new socket timeout options with _NEW suffix that are y2038 safe. Rename the existing options with _OLD suffix forms so that the right option is enabled for userspace applications according to the architecture and time_t definition of libc.
Signed-off-by Deepa Dinamani <[email protected]>

Looks good overall. A few minor concerns:

The description above makes it sound like there is a bug with y2038-safety in this particular interface, which I think is just not what you meant, as the change is only needed for compatiblity with new C libraries that work around the y2038 problem in general by changing their timeval definition.

Quoted 9 lines

fs/dlm/lowcomms.c +0 -1

index 76976d6e50f9..c98ad9777ad9 100644
@@ -1089,12 +1089,12 @@ static void sctp_connect_to_sock(struct connection *con)
         * since O_NONBLOCK argument in connect() function does not work here,
         * then, we should restore the default value of this attribute.
         */
-       kernel_setsockopt(sock, SOL_SOCKET, SO_SNDTIMEO, (char *)&tv,

fs/dlm/lowcomms.c static void sctp_connect_to_sock(struct connection *con) · line 1100

Quoted 10 lines

fs/dlm/lowcomms.c +2 -1

+       kernel_setsockopt(sock, SOL_SOCKET, SO_SNDTIMEO_OLD, (char *)&tv,
                          sizeof(tv));
        result = sock->ops->connect(sock, (struct sockaddr *)&daddr, addr_len,
                                   0);
        memset(&tv, 0, sizeof(tv));
-       kernel_setsockopt(sock, SOL_SOCKET, SO_SNDTIMEO, (char *)&tv,
+       kernel_setsockopt(sock, SOL_SOCKET, SO_SNDTIMEO_OLD, (char *)&tv,
                          sizeof(tv));
 
        if (result == -EINPROGRESS)

It took me a bit to realize there that this is safe as well even if we don't use SO_SNDTIMEO_NEW, for the same reason.

Quoted 12 lines

net/compat.c +1 -1

@@ -378,7 +378,7 @@ static int compat_sock_setsockopt(struct socket *sock, int level, int optname,
                return do_set_attach_filter(sock, level, optname,
                                            optval, optlen);
        if (!COMPAT_USE_64BIT_TIME &&
-           (optname == SO_RCVTIMEO || optname == SO_SNDTIMEO))
+           (optname == SO_RCVTIMEO_OLD || optname == SO_SNDTIMEO_OLD))
                return do_set_sock_timeout(sock, level, optname, optval, optlen);
 
        return sock_setsockopt(sock, level, optname, optval, optlen);

net/compat.c static int compat_sock_getsockopt(struct socket *sock, int level, int optname, · line 456

Quoted 8 lines

net/compat.c +1 -1

@@ -450,7 +450,7 @@ static int compat_sock_getsockopt(struct socket *sock, int level, int optname,
                                char __user *optval, int __user *optlen)
 {
        if (!COMPAT_USE_64BIT_TIME &&
-           (optname == SO_RCVTIMEO || optname == SO_SNDTIMEO))
+           (optname == SO_RCVTIMEO_OLD || optname == SO_SNDTIMEO_OLD))
                return do_get_sock_timeout(sock, level, optname, optval, optlen);
        return sock_getsockopt(sock, level, optname, optval, optlen);
 }

I looked at the original code and noticed that it's horrible, which of course is not your fault, but I wonder if we should just fix it now to avoid that get_fs()/set_fs() hack, since that code mostly implements what you also have in your patch 3 (which is done more nicely).

I'll follow up with a patch to demonstrate what I mean here. Your third patch will then just have to add another code path so we can handle all of old_timespec32 (for existing 32-bit user space), __kernel_old_timespec (for sparc64) and __kernel_sock_timeval (for everything else).

Arnd

Arnd Bergmann

maintainer

2019-01-08 20:09 UTC #4 lore raw

PATCH 1/1 socket: move compat timeout handling into sock.c

This is a cleanup to prepare for the addition of 64-bit time_t in O_SNDTIMEO/O_RCVTIMEO. The existing compat handler seems unnecessarily complex and error-prone, moving it all into the main setsockopt()/getsockopt() implementation requires half as much code and is easier to extend.

32-bit user space can now use old_timeval32 on both 32-bit and 64-bit machines, while 64-bit code can use __old_kernel_timeval.

Signed-off-by Arnd Bergmann <[email protected]>

---
 net/compat.c    | 66 +------------------------------------------------
 net/core/sock.c | 65 +++++++++++++++++++++++++++++++-----------------
 2 files changed, 44 insertions(+), 87 deletions(-)

net/compat.c +1 ~~-65~~

index 959d1c51826d..ce8f6e8cdcd2 100644
@@ -348,28 +348,6 @@ static int do_set_attach_filter(struct socket *sock, int level, int optname,
 			      sizeof(struct sock_fprog));
 }
 
-static int do_set_sock_timeout(struct socket *sock, int level,
-		int optname, char __user *optval, unsigned int optlen)
-{
-	struct compat_timeval __user *up = (struct compat_timeval __user *)optval;
-	struct timeval ktime;
-	mm_segment_t old_fs;
-	int err;
-
-	if (optlen < sizeof(*up))
-		return -EINVAL;
-	if (!access_ok(up, sizeof(*up)) ||
-	    __get_user(ktime.tv_sec, &up->tv_sec) ||
-	    __get_user(ktime.tv_usec, &up->tv_usec))
-		return -EFAULT;
-	old_fs = get_fs();
-	set_fs(KERNEL_DS);
-	err = sock_setsockopt(sock, level, optname, (char *)&ktime, sizeof(ktime));
-	set_fs(old_fs);
-
-	return err;
-}
-
 static int compat_sock_setsockopt(struct socket *sock, int level, int optname,
 				char __user *optval, unsigned int optlen)
 {

@@ -377,10 +355,6 @@ static int compat_sock_setsockopt(struct socket *sock, int level, int optname,
 	    optname == SO_ATTACH_REUSEPORT_CBPF)
 		return do_set_attach_filter(sock, level, optname,
 					    optval, optlen);
-	if (!COMPAT_USE_64BIT_TIME &&
-	    (optname == SO_RCVTIMEO || optname == SO_SNDTIMEO))
-		return do_set_sock_timeout(sock, level, optname, optval, optlen);
-
 	return sock_setsockopt(sock, level, optname, optval, optlen);
 }

@@ -417,44 +391,6 @@ COMPAT_SYSCALL_DEFINE5(setsockopt, int, fd, int, level, int, optname,
 	return __compat_sys_setsockopt(fd, level, optname, optval, optlen);
 }
 
-static int do_get_sock_timeout(struct socket *sock, int level, int optname,
-		char __user *optval, int __user *optlen)
-{
-	struct compat_timeval __user *up;
-	struct timeval ktime;
-	mm_segment_t old_fs;
-	int len, err;
-
-	up = (struct compat_timeval __user *) optval;
-	if (get_user(len, optlen))
-		return -EFAULT;
-	if (len < sizeof(*up))
-		return -EINVAL;
-	len = sizeof(ktime);
-	old_fs = get_fs();
-	set_fs(KERNEL_DS);
-	err = sock_getsockopt(sock, level, optname, (char *) &ktime, &len);
-	set_fs(old_fs);
-
-	if (!err) {
-		if (put_user(sizeof(*up), optlen) ||
-		    !access_ok(up, sizeof(*up)) ||
-		    __put_user(ktime.tv_sec, &up->tv_sec) ||
-		    __put_user(ktime.tv_usec, &up->tv_usec))
-			err = -EFAULT;
-	}
-	return err;
-}
-
-static int compat_sock_getsockopt(struct socket *sock, int level, int optname,
-				char __user *optval, int __user *optlen)
-{
-	if (!COMPAT_USE_64BIT_TIME &&
-	    (optname == SO_RCVTIMEO || optname == SO_SNDTIMEO))
-		return do_get_sock_timeout(sock, level, optname, optval, optlen);
-	return sock_getsockopt(sock, level, optname, optval, optlen);
-}
-
 int compat_sock_get_timestamp(struct sock *sk, struct timeval __user *userstamp)
 {
 	struct compat_timeval __user *ctv;

@@ -527,7 +463,7 @@ static int __compat_sys_getsockopt(int fd, int level, int optname,
 		}
 
 		if (level == SOL_SOCKET)
-			err = compat_sock_getsockopt(sock, level,
+			err = sock_getsockopt(sock, level,
 					optname, optval, optlen);
 		else if (sock->ops->compat_getsockopt)
 			err = sock->ops->compat_getsockopt(sock, level,

net/core/sock.c +43 ~~-22~~

89 lines not on this page · raw message

Signature · 1 line

2.20.0

Deepa Dinamani

author Acked-by on v1 1/1

2019-01-08 21:29 UTC #5 lore raw

Re: [PATCH] socket: move compat timeout handling into sock.c

Quoted 7 lines · Arnd Bergmann

This is a cleanup to prepare for the addition of 64-bit time_t in O_SNDTIMEO/O_RCVTIMEO. The existing compat handler seems unnecessarily complex and error-prone, moving it all into the main setsockopt()/getsockopt() implementation requires half as much code and is easier to extend.

Quoted 5 lines

32-bit user space can now use old_timeval32 on both 32-bit and 64-bit machines, while 64-bit code can use __old_kernel_timeval.
Signed-off-by Arnd Bergmann <[email protected]>

This will make the other series so much nicer. Thank you.

Acked-by Deepa Dinamani <[email protected]>

Deepa Dinamani

author

2019-01-08 21:19 UTC #6 lore raw

Re: [PATCH 2/3] socket: Rename SO_RCVTIMEO/ SO_SNDTIMEO with _OLD suffixes

In reply to Arnd Bergmann

Quoted 13 lines · Arnd Bergmann

Quoted 10 lines · Deepa Dinamani
SO_RCVTIMEO and SO_SNDTIMEO socket options use struct timeval as the time format. struct timeval is not y2038 safe. The subsequent patches in the series add support for new socket timeout options with _NEW suffix that are y2038 safe. Rename the existing options with _OLD suffix forms so that the right option is enabled for userspace applications according to the architecture and time_t definition of libc.
Signed-off-by Deepa Dinamani <[email protected]>

Quoted 7 lines

Looks good overall. A few minor concerns:
The description above makes it sound like there is a bug with y2038-safety in this particular interface, which I think is just not what you meant, as the change is only needed for compatiblity with new C libraries that work around the y2038 problem in general by changing their timeval definition.

Right, there is y2038 safety issue, just the libc part that needs to be handled. I will fix the commit text.

Quoted 18 lines

fs/dlm/lowcomms.c +2 -2

index 76976d6e50f9..c98ad9777ad9 100644
@@ -1089,12 +1089,12 @@ static void sctp_connect_to_sock(struct connection *con)
         * since O_NONBLOCK argument in connect() function does not work here,
         * then, we should restore the default value of this attribute.
         */
-       kernel_setsockopt(sock, SOL_SOCKET, SO_SNDTIMEO, (char *)&tv,
+       kernel_setsockopt(sock, SOL_SOCKET, SO_SNDTIMEO_OLD, (char *)&tv,
                          sizeof(tv));
        result = sock->ops->connect(sock, (struct sockaddr *)&daddr, addr_len,
                                   0);
        memset(&tv, 0, sizeof(tv));
-       kernel_setsockopt(sock, SOL_SOCKET, SO_SNDTIMEO, (char *)&tv,
+       kernel_setsockopt(sock, SOL_SOCKET, SO_SNDTIMEO_OLD, (char *)&tv,
                          sizeof(tv));

Quoted 4 lines

fs/dlm/lowcomms.c static void sctp_connect_to_sock(struct connection *con) · line 1100
Quoted 1 line
fs/dlm/lowcomms.c +0 -0
        if (result == -EINPROGRESS)
It took me a bit to realize there that this is safe as well even if we don't use SO_SNDTIMEO_NEW, for the same reason.

Correct.

Quoted 10 lines

net/compat.c +1 -1

@@ -378,7 +378,7 @@ static int compat_sock_setsockopt(struct socket *sock, int level, int optname,
                return do_set_attach_filter(sock, level, optname,
                                            optval, optlen);
        if (!COMPAT_USE_64BIT_TIME &&
-           (optname == SO_RCVTIMEO || optname == SO_SNDTIMEO))
+           (optname == SO_RCVTIMEO_OLD || optname == SO_SNDTIMEO_OLD))
                return do_set_sock_timeout(sock, level, optname, optval, optlen);

Quoted 15 lines

net/compat.c static int compat_sock_getsockopt(struct socket *sock, int level, int optname, · line 456
Quoted 10 lines
net/compat.c +1 -1
        return sock_setsockopt(sock, level, optname, optval, optlen);
@@ -450,7 +450,7 @@ static int compat_sock_getsockopt(struct socket *sock, int level, int optname,
                                char __user *optval, int __user *optlen)
 {
        if (!COMPAT_USE_64BIT_TIME &&
-           (optname == SO_RCVTIMEO || optname == SO_SNDTIMEO))
+           (optname == SO_RCVTIMEO_OLD || optname == SO_SNDTIMEO_OLD))
                return do_get_sock_timeout(sock, level, optname, optval, optlen);
        return sock_getsockopt(sock, level, optname, optval, optlen);
 }
I looked at the original code and noticed that it's horrible, which of course is not your fault, but I wonder if we should just fix it now to avoid that get_fs()/set_fs() hack, since that code mostly implements what you also have in your patch 3 (which is done more nicely).

I did think of getting rid of set_fs()/ get_fs() here.
But, I wasn't sure as the maintainers seemed to prefer to leave to the old code as is in the other series for timestamps.

Quoted 4 lines

I'll follow up with a patch to demonstrate what I mean here. Your third patch will then just have to add another code path so we can handle all of old_timespec32 (for existing 32-bit user space), __kernel_old_timespec (for sparc64) and __kernel_sock_timeval (for everything else).

Cool, I will rebase on top of your patch.

Thanks,
Deepa

Deepa Dinamani

author

2019-01-08 05:22 UTC #7 lore raw

PATCH 3/3 sock: Add SO_RCVTIMEO_NEW and SO_SNDTIMEO_NEW

In reply to Deepa Dinamani

Add new socket timeout options that are y2038 safe.

Signed-off-by Deepa Dinamani <[email protected]>

Cc ccaulfie at redhat.com

Cc davem at davemloft.net

Cc deller at gmx.de

Cc paulus at samba.org

Cc ralf at linux-mips.org

Cc rth at twiddle.net

Cc cluster-devel at redhat.com

Cc linuxppc-dev at lists.ozlabs.org

Cc linux-alpha at vger.kernel.org

Cc linux-arch at vger.kernel.org

Cc linux-mips at vger.kernel.org

Cc linux-parisc at vger.kernel.org

Cc sparclinux at vger.kernel.org

---
 arch/alpha/include/uapi/asm/socket.h  | 12 +++--
 arch/mips/include/uapi/asm/socket.h   | 11 ++++-
 arch/parisc/include/uapi/asm/socket.h | 11 ++++-
 arch/sparc/include/uapi/asm/socket.h  | 11 ++++-
 include/net/sock.h                    |  4 +-
 include/uapi/asm-generic/socket.h     | 11 ++++-
 net/core/sock.c                       | 64 +++++++++++++++++++++------
 7 files changed, 98 insertions(+), 26 deletions(-)

arch/alpha/include/uapi/asm/socket.h +9 -3

index ea3ba981d8a0..3d800d5d3d5d 100644
@@ -118,19 +118,25 @@
 #define SO_TIMESTAMPNS_NEW       63
 #define SO_TIMESTAMPING_NEW      64
 
-#if !defined(__KERNEL__)
+#define SO_RCVTIMEO_NEW          65
+#define SO_SNDTIMEO_NEW          66
 
-#define	SO_RCVTIMEO SO_RCVTIMEO_OLD
-#define	SO_SNDTIMEO SO_SNDTIMEO_OLD
+#if !defined(__KERNEL__)
 
 #if __BITS_PER_LONG == 64
 #define SO_TIMESTAMP		SO_TIMESTAMP_OLD
 #define SO_TIMESTAMPNS		SO_TIMESTAMPNS_OLD
 #define SO_TIMESTAMPING	SO_TIMESTAMPING_OLD
+
+#define	SO_RCVTIMEO SO_RCVTIMEO_OLD
+#define	SO_SNDTIMEO SO_SNDTIMEO_OLD
 #else
 #define SO_TIMESTAMP (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMP_OLD : SO_TIMESTAMP_NEW)
 #define SO_TIMESTAMPNS (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMPNS_OLD : SO_TIMESTAMPNS_NEW)
 #define SO_TIMESTAMPING (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMPING_OLD : SO_TIMESTAMPING_NEW)
+
+#define SO_RCVTIMEO (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_RCVTIMEO_OLD : SO_RCVTIMEO_NEW)
+#define SO_SNDTIMEO (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_SNDTIMEO_OLD : SO_SNDTIMEO_NEW)
 #endif
 
 #define SCM_TIMESTAMP          SO_TIMESTAMP

arch/mips/include/uapi/asm/socket.h +9 -2

index 4dde20d64690..5a7f9010c090 100644
@@ -128,18 +128,25 @@
 #define SO_TIMESTAMPNS_NEW       63
 #define SO_TIMESTAMPING_NEW      64
 
+#define SO_RCVTIMEO_NEW          65
+#define SO_SNDTIMEO_NEW          66
+
 #if !defined(__KERNEL__)
 
-#define	SO_RCVTIMEO SO_RCVTIMEO_OLD
-#define	SO_SNDTIMEO SO_SNDTIMEO_OLD
 #if __BITS_PER_LONG == 64
 #define SO_TIMESTAMP		SO_TIMESTAMP_OLD
 #define SO_TIMESTAMPNS		SO_TIMESTAMPNS_OLD
 #define SO_TIMESTAMPING	SO_TIMESTAMPING_OLD
+
+#define        SO_RCVTIMEO             SO_RCVTIMEO_OLD
+#define        SO_SNDTIMEO             SO_SNDTIMEO_OLD
 #else
 #define SO_TIMESTAMP (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMP_OLD : SO_TIMESTAMP_NEW)
 #define SO_TIMESTAMPNS (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMPNS_OLD : SO_TIMESTAMPNS_NEW)
 #define SO_TIMESTAMPING (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMPING_OLD : SO_TIMESTAMPING_NEW)
+
+#define        SO_RCVTIMEO (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_RCVTIMEO_OLD : SO_RCVTIMEO_NEW)
+#define        SO_SNDTIMEO (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_SNDTIMEO_OLD : SO_SNDTIMEO_NEW)
 #endif
 
 #define SCM_TIMESTAMP          SO_TIMESTAMP

arch/parisc/include/uapi/asm/socket.h +9 -2

index 546937fa0d8b..bd35de5b4666 100644
@@ -109,18 +109,25 @@
 #define SO_TIMESTAMPNS_NEW       0x4038
 #define SO_TIMESTAMPING_NEW      0x4039
 
+#define SO_RCVTIMEO_NEW          0x4040
+#define SO_SNDTIMEO_NEW          0x4041
+
 #if !defined(__KERNEL__)
 
-#define	SO_RCVTIMEO SO_RCVTIMEO_OLD
-#define	SO_SNDTIMEO SO_SNDTIMEO_OLD
 #if __BITS_PER_LONG == 64
 #define SO_TIMESTAMP		SO_TIMESTAMP_OLD
 #define SO_TIMESTAMPNS		SO_TIMESTAMPNS_OLD
 #define SO_TIMESTAMPING	SO_TIMESTAMPING_OLD
+
+#define	SO_RCVTIMEO SO_RCVTIMEO_OLD
+#define	SO_SNDTIMEO SO_SNDTIMEO_OLD
 #else
 #define SO_TIMESTAMP (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMP_OLD : SO_TIMESTAMP_NEW)
 #define SO_TIMESTAMPNS (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMPNS_OLD : SO_TIMESTAMPNS_NEW)
 #define SO_TIMESTAMPING (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMPING_OLD : SO_TIMESTAMPING_NEW)
+
+#define        SO_RCVTIMEO (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_RCVTIMEO_OLD : SO_RCVTIMEO_NEW)
+#define        SO_SNDTIMEO (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_SNDTIMEO_OLD : SO_SNDTIMEO_NEW)
 #endif
 
 #define SCM_TIMESTAMP          SO_TIMESTAMP

arch/sparc/include/uapi/asm/socket.h +9 -2

index bdc396211627..5a5b073c3299 100644
@@ -110,18 +110,25 @@
 #define SO_TIMESTAMPNS_NEW       0x0042
 #define SO_TIMESTAMPING_NEW      0x0043
 
+#define SO_RCVTIMEO_NEW          0x0044
+#define SO_SNDTIMEO_NEW          0x0045
+
 #if !defined(__KERNEL__)
 
-#define	SO_RCVTIMEO	SO_RCVTIMEO_OLD
-#define	SO_SNDTIMEO	SO_SNDTIMEO_OLD
 #if __BITS_PER_LONG == 64
 #define SO_TIMESTAMP		SO_TIMESTAMP_OLD
 #define SO_TIMESTAMPNS		SO_TIMESTAMPNS_OLD
 #define SO_TIMESTAMPING	SO_TIMESTAMPING_OLD
+
+#define	SO_RCVTIMEO SO_RCVTIMEO_OLD
+#define	SO_SNDTIMEO SO_SNDTIMEO_OLD
 #else
 #define SO_TIMESTAMP (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMP_OLD : SO_TIMESTAMP_NEW)
 #define SO_TIMESTAMPNS (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMPNS_OLD : SO_TIMESTAMPNS_NEW)
 #define SO_TIMESTAMPING (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMPING_OLD : SO_TIMESTAMPING_NEW)
+
+#define        SO_RCVTIMEO (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_RCVTIMEO_OLD : SO_RCVTIMEO_NEW)
+#define        SO_SNDTIMEO (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_SNDTIMEO_OLD : SO_SNDTIMEO_NEW)
 #endif

include/net/sock.h +2 -2

index 98965a9a2bf4..6679f3c120b0 100644
@@ -292,8 +292,8 @@ struct sock_common {
   *	@sk_peer_pid: &struct pid for this socket's peer
   *	@sk_peer_cred: %SO_PEERCRED setting
   *	@sk_rcvlowat: %SO_RCVLOWAT setting
-  *	@sk_rcvtimeo: %SO_RCVTIMEO_OLD setting
-  *	@sk_sndtimeo: %SO_SNDTIMEO_OLD setting
+  *	@sk_rcvtimeo: %SO_RCVTIMEO setting
+  *	@sk_sndtimeo: %SO_SNDTIMEO setting
   *	@sk_txhash: computed flow hash for use on transmit
   *	@sk_filter: socket filtering instructions
   *	@sk_timer: sock cleanup timer

include/uapi/asm-generic/socket.h +9 -2

index 9e370586fb19..5b4da6eacc9f 100644
@@ -112,19 +112,26 @@
 #define SO_TIMESTAMPNS_NEW       63
 #define SO_TIMESTAMPING_NEW      64
 
+#define SO_RCVTIMEO_NEW          65
+#define SO_SNDTIMEO_NEW          66
+
 #if !defined(__KERNEL__)
 
-#define	SO_RCVTIMEO SO_RCVTIMEO_OLD
-#define	SO_SNDTIMEO SO_SNDTIMEO_OLD
 #if __BITS_PER_LONG == 64 || (defined(__x86_64__) && defined(__ILP32__))
 /* on 64-bit and x32, avoid the ?: operator */
 #define SO_TIMESTAMP		SO_TIMESTAMP_OLD
 #define SO_TIMESTAMPNS		SO_TIMESTAMPNS_OLD
 #define SO_TIMESTAMPING	SO_TIMESTAMPING_OLD
+
+#define	SO_RCVTIMEO SO_RCVTIMEO_OLD
+#define	SO_SNDTIMEO SO_SNDTIMEO_OLD
 #else
 #define SO_TIMESTAMP (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMP_OLD : SO_TIMESTAMP_NEW)
 #define SO_TIMESTAMPNS (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMPNS_OLD : SO_TIMESTAMPNS_NEW)
 #define SO_TIMESTAMPING (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_TIMESTAMPING_OLD : SO_TIMESTAMPING_NEW)
+
+#define        SO_RCVTIMEO (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_RCVTIMEO_OLD : SO_RCVTIMEO_NEW)
+#define        SO_SNDTIMEO (sizeof(time_t) == sizeof(__kernel_long_t) ? SO_SNDTIMEO_OLD : SO_SNDTIMEO_NEW)
 #endif
 
 #define SCM_TIMESTAMP          SO_TIMESTAMP

net/core/sock.c +51 ~~-13~~

108 lines not on this page · raw message

Signature · 1 line

2.17.1