netdev - Re: [PATCH] Truncate UTS_RELEASE for rxrpc version

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <223250.1685052554@warthog.procyon.org.uk>
Date: Thu, 25 May 2023 23:09:14 +0100
From: David Howells <dhowells@...hat.com>
To: Kenny Ho <Kenny.Ho@....com>
Cc: dhowells@...hat.com, David Laight <David.Laight@...lab.com>,
    Jakub Kicinski <kuba@...nel.org>, Andrew Lunn <andrew@...n.ch>,
    Marc Dionne <marc.dionne@...istor.com>,
    "Kenny
 Ho" <y2kenny@...il.com>,
    "David S.
 Miller" <davem@...emloft.net>,
    Eric Dumazet <edumazet@...gle.com>,
    "Paolo
 Abeni" <pabeni@...hat.com>,
    "linux-afs@...ts.infradead.org" <linux-afs@...ts.infradead.org>,
    "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
    "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
    alexander.deucher@....com
Subject: Re: [PATCH] Truncate UTS_RELEASE for rxrpc version

Kenny Ho <Kenny.Ho@....com> wrote:

> @@ -30,6 +28,7 @@ static void rxrpc_send_version_request(struct rxrpc_local *local,
>  	struct sockaddr_rxrpc srx;
>  	struct msghdr msg;
>  	struct kvec iov[2];
> +	static char rxrpc_version_string[65];
>  	size_t len;
>  	int ret;
>  

That's not thread-safe.  If you have multiple endpoints each one of them could
be overwriting the string at the same time.  We can't guarantee that one
wouldn't corrupt the other.

There's also no need to reprint it every time; just once during module init
will do.  How about the attached patch instead?

David
---
rxrpc: Truncate UTS_RELEASE for rxrpc version

UTS_RELEASE has a maximum length of 64 which can cause rxrpc_version to
exceed the 65 byte message limit.

Per the rx spec[1]: "If a server receives a packet with a type value of 13,
and the client-initiated flag set, it should respond with a 65-byte payload
containing a string that identifies the version of AFS software it is
running."

The current implementation causes a compile error when WERROR is turned on
and/or UTS_RELEASE exceeds the length of 49 (making the version string more
than 64 characters).

Fix this by generating the string during module initialisation and limiting
the UTS_RELEASE segment of the string does not exceed 49 chars.  We need to
make sure that the 64 bytes includes "linux-" at the front and " AF_RXRPC" at
the back as this may be used in pattern matching.

Fixes: 44ba06987c0b ("RxRPC: Handle VERSION Rx protocol packets")
Reported-by: Kenny Ho <Kenny.Ho@....com>
Link: https://lore.kernel.org/r/20230523223944.691076-1-Kenny.Ho@amd.com/
Signed-off-by: David Howells <dhowells@...hat.com>
Link: https://web.mit.edu/kolya/afs/rx/rx-spec [1]
cc: Marc Dionne <marc.dionne@...istor.com>
cc: Andrew Lunn <andrew@...n.ch>
cc: David Laight <David.Laight@...LAB.COM>
cc: "David S. Miller" <davem@...emloft.net>
cc: Eric Dumazet <edumazet@...gle.com>
cc: Jakub Kicinski <kuba@...nel.org>
cc: Paolo Abeni <pabeni@...hat.com>
cc: linux-afs@...ts.infradead.org
cc: netdev@...r.kernel.org
---
 net/rxrpc/af_rxrpc.c    |    1 +
 net/rxrpc/ar-internal.h |    1 +
 net/rxrpc/local_event.c |   11 ++++++++++-
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/net/rxrpc/af_rxrpc.c b/net/rxrpc/af_rxrpc.c
index 31f738d65f1c..da0b3b5157d5 100644
--- a/net/rxrpc/af_rxrpc.c
+++ b/net/rxrpc/af_rxrpc.c
@@ -980,6 +980,7 @@ static int __init af_rxrpc_init(void)
 	BUILD_BUG_ON(sizeof(struct rxrpc_skb_priv) > sizeof_field(struct sk_buff, cb));
 
 	ret = -ENOMEM;
+	rxrpc_gen_version_string();
 	rxrpc_call_jar = kmem_cache_create(
 		"rxrpc_call_jar", sizeof(struct rxrpc_call), 0,
 		SLAB_HWCACHE_ALIGN, NULL);
diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index 5d44dc08f66d..e8e14c6f904d 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -1068,6 +1068,7 @@ int rxrpc_get_server_data_key(struct rxrpc_connection *, const void *, time64_t,
 /*
  * local_event.c
  */
+void rxrpc_gen_version_string(void);
 void rxrpc_send_version_request(struct rxrpc_local *local,
 				struct rxrpc_host_header *hdr,
 				struct sk_buff *skb);
diff --git a/net/rxrpc/local_event.c b/net/rxrpc/local_event.c
index 5e69ea6b233d..993c69f97488 100644
--- a/net/rxrpc/local_event.c
+++ b/net/rxrpc/local_event.c
@@ -16,7 +16,16 @@
 #include <generated/utsrelease.h>
 #include "ar-internal.h"
 
-static const char rxrpc_version_string[65] = "linux-" UTS_RELEASE " AF_RXRPC";
+static char rxrpc_version_string[65]; // "linux-" UTS_RELEASE " AF_RXRPC";
+
+/*
+ * Generate the VERSION packet string.
+ */
+void rxrpc_gen_version_string(void)
+{
+	snprintf(rxrpc_version_string, sizeof(rxrpc_version_string),
+		 "linux-%.49s AF_RXRPC", UTS_RELEASE);
+}
 
 /*
  * Reply to a version request