Request to cherry-pick f00432063db1 to 5.10

Request to cherry-pick f00432063db1 to 5.10

[Meena Shanmugam]

Hi all,

The commit f00432063db1a0db484e85193eccc6845435b80e upstream (SUNRPC:
Ensure we flush any closed sockets before xs_xprt_free()) fixes
CVE-2022-28893, hence good candidate for stable trees.
The above commit depends on 3be232f(SUNRPC: Prevent immediate
close+reconnect)  and  89f4249(SUNRPC: Don't call connect() more than
once on a TCP socket). Commit 3be232f depends on commit
e26d9972720e(SUNRPC: Clean up scheduling of autoclose).

Commits e26d9972720e, 3be232f, f00432063db1 apply cleanly on 5.10
kernel. commit 89f4249 didn't apply cleanly. I have patch for 89f4249
below.

Thanks,
Meena

From: Trond Myklebust <trond.myklebust@hammerspace.com>
Date: Wed, 16 Mar 2022 19:10:43 -0400
Subject: [PATCH] SUNRPC: Don't call connect() more than once on a TCP socket

commit 89f42494f92f448747bd8a7ab1ae8b5d5520577d upstream.

Avoid socket state races due to repeated calls to ->connect() using the
same socket. If connect() returns 0 due to the connection having
completed, but we are in fact in a closing state, then we may leave the
XPRT_CONNECTING flag set on the transport.

Reported-by: Enrico Scholz <enrico.scholz@sigma-chemnitz.de>
Fixes: 3be232f11a3c ("SUNRPC: Prevent immediate close+reconnect")
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
[meenashanmugam: Backported to 5.10: Fixed merge conflict in
xs_tcp_setup_socket]
Signed-off-by: Meena Shanmugam <meena.shanmugam@google.com>
---
 include/linux/sunrpc/xprtsock.h |  1 +
 net/sunrpc/xprtsock.c           | 21 +++++++++++----------
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/include/linux/sunrpc/xprtsock.h b/include/linux/sunrpc/xprtsock.h
index 8c2a712cb242..689062afdd61 100644
--- a/include/linux/sunrpc/xprtsock.h
+++ b/include/linux/sunrpc/xprtsock.h
@@ -89,5 +89,6 @@ struct sock_xprt {
 #define XPRT_SOCK_WAKE_WRITE (5)
 #define XPRT_SOCK_WAKE_PENDING (6)
 #define XPRT_SOCK_WAKE_DISCONNECT (7)
+#define XPRT_SOCK_CONNECT_SENT (8)

 #endif /* _LINUX_SUNRPC_XPRTSOCK_H */
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 60c58eb9a456..33a81f9703b1 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -2260,10 +2260,14 @@ static void xs_tcp_setup_socket(struct
work_struct *work)
  struct rpc_xprt *xprt = &transport->xprt;
  int status = -EIO;

- if (!sock) {
- sock = xs_create_sock(xprt, transport,
- xs_addr(xprt)->sa_family, SOCK_STREAM,
- IPPROTO_TCP, true);
+ if (xprt_connected(xprt))
+ goto out;
+ if (test_and_clear_bit(XPRT_SOCK_CONNECT_SENT,
+        &transport->sock_state) ||
+     !sock) {
+ xs_reset_transport(transport);
+ sock = xs_create_sock(xprt, transport, xs_addr(xprt)->sa_family,
+       SOCK_STREAM, IPPROTO_TCP, true);
  if (IS_ERR(sock)) {
  status = PTR_ERR(sock);
  goto out;
@@ -2294,6 +2298,7 @@ static void xs_tcp_setup_socket(struct work_struct *work)
  break;
  case 0:
  case -EINPROGRESS:
+ set_bit(XPRT_SOCK_CONNECT_SENT, &transport->sock_state);
  case -EALREADY:
  xprt_unlock_connect(xprt, transport);
  return;
@@ -2345,13 +2350,9 @@ static void xs_connect(struct rpc_xprt *xprt,
struct rpc_task *task)

  WARN_ON_ONCE(!xprt_lock_connect(xprt, task, transport));

- if (transport->sock != NULL && !xprt_connecting(xprt)) {
+ if (transport->sock != NULL) {
  dprintk("RPC:       xs_connect delayed xprt %p for %lu "
- "seconds\n",
- xprt, xprt->reestablish_timeout / HZ);
-
- /* Start by resetting any existing state */
- xs_reset_transport(transport);
+ "seconds\n", xprt, xprt->reestablish_timeout / HZ);

  delay = xprt_reconnect_delay(xprt);
  xprt_reconnect_backoff(xprt, XS_TCP_INIT_REEST_TO);
-- 
2.36.0.512.ge40c2bad7a-goog

Re: Request to cherry-pick f00432063db1 to 5.10

[Bagas Sanjaya]

On Tue, May 10, 2022 at 07:33:23PM -0700, Meena Shanmugam wrote:
> From: Trond Myklebust <trond.myklebust@hammerspace.com>
> Date: Wed, 16 Mar 2022 19:10:43 -0400
> Subject: [PATCH] SUNRPC: Don't call connect() more than once on a TCP socket
> 
> commit 89f42494f92f448747bd8a7ab1ae8b5d5520577d upstream.
> 
> Avoid socket state races due to repeated calls to ->connect() using the
> same socket. If connect() returns 0 due to the connection having
> completed, but we are in fact in a closing state, then we may leave the
> XPRT_CONNECTING flag set on the transport.
> 
> Reported-by: Enrico Scholz <enrico.scholz@sigma-chemnitz.de>
> Fixes: 3be232f11a3c ("SUNRPC: Prevent immediate close+reconnect")
> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> [meenashanmugam: Backported to 5.10: Fixed merge conflict in
> xs_tcp_setup_socket]
> Signed-off-by: Meena Shanmugam <meena.shanmugam@google.com>
> ---
>  include/linux/sunrpc/xprtsock.h |  1 +
>  net/sunrpc/xprtsock.c           | 21 +++++++++++----------
>  2 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/include/linux/sunrpc/xprtsock.h b/include/linux/sunrpc/xprtsock.h
> index 8c2a712cb242..689062afdd61 100644
> --- a/include/linux/sunrpc/xprtsock.h
> +++ b/include/linux/sunrpc/xprtsock.h
> @@ -89,5 +89,6 @@ struct sock_xprt {
>  #define XPRT_SOCK_WAKE_WRITE (5)
>  #define XPRT_SOCK_WAKE_PENDING (6)
>  #define XPRT_SOCK_WAKE_DISCONNECT (7)
> +#define XPRT_SOCK_CONNECT_SENT (8)
> 
>  #endif /* _LINUX_SUNRPC_XPRTSOCK_H */
> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> index 60c58eb9a456..33a81f9703b1 100644
> --- a/net/sunrpc/xprtsock.c
> +++ b/net/sunrpc/xprtsock.c
> @@ -2260,10 +2260,14 @@ static void xs_tcp_setup_socket(struct
> work_struct *work)
>   struct rpc_xprt *xprt = &transport->xprt;
>   int status = -EIO;
> 
> - if (!sock) {
> - sock = xs_create_sock(xprt, transport,
> - xs_addr(xprt)->sa_family, SOCK_STREAM,
> - IPPROTO_TCP, true);
> + if (xprt_connected(xprt))
> + goto out;
> + if (test_and_clear_bit(XPRT_SOCK_CONNECT_SENT,
> +        &transport->sock_state) ||
> +     !sock) {
> + xs_reset_transport(transport);
> + sock = xs_create_sock(xprt, transport, xs_addr(xprt)->sa_family,
> +       SOCK_STREAM, IPPROTO_TCP, true);
>   if (IS_ERR(sock)) {
>   status = PTR_ERR(sock);
>   goto out;
> @@ -2294,6 +2298,7 @@ static void xs_tcp_setup_socket(struct work_struct *work)
>   break;
>   case 0:
>   case -EINPROGRESS:
> + set_bit(XPRT_SOCK_CONNECT_SENT, &transport->sock_state);
>   case -EALREADY:
>   xprt_unlock_connect(xprt, transport);
>   return;
> @@ -2345,13 +2350,9 @@ static void xs_connect(struct rpc_xprt *xprt,
> struct rpc_task *task)
> 
>   WARN_ON_ONCE(!xprt_lock_connect(xprt, task, transport));
> 
> - if (transport->sock != NULL && !xprt_connecting(xprt)) {
> + if (transport->sock != NULL) {
>   dprintk("RPC:       xs_connect delayed xprt %p for %lu "
> - "seconds\n",
> - xprt, xprt->reestablish_timeout / HZ);
> -
> - /* Start by resetting any existing state */
> - xs_reset_transport(transport);
> + "seconds\n", xprt, xprt->reestablish_timeout / HZ);
> 
>   delay = xprt_reconnect_delay(xprt);
>   xprt_reconnect_backoff(xprt, XS_TCP_INIT_REEST_TO);
> -- 
> 2.36.0.512.ge40c2bad7a-goog

Hi Meena,

The backport above didn't apply due to whitespace mangling (maybe your
mailer?). So I had to manually apply it.

Here's the proper backport:

-- >8 --

From 2ec4a7d1668a53a1f46d3413bdec6ff9cc0458a3 Mon Sep 17 00:00:00 2001
From: Bagas Sanjaya <bagasdotme@gmail.com>
Date: Thu, 12 May 2022 12:53:07 +0700
Subject: [PATCH] SUNRPC: Don't call connect() more than once on a TCP socket

From: Trond Myklebust <trond.myklebust@hammerspace.com>

commit 89f42494f92f448747bd8a7ab1ae8b5d5520577d upstream.

Avoid socket state races due to repeated calls to ->connect() using the
same socket. If connect() returns 0 due to the connection having
completed, but we are in fact in a closing state, then we may leave the
XPRT_CONNECTING flag set on the transport.

Reported-by: Enrico Scholz <enrico.scholz@sigma-chemnitz.de>
Fixes: 3be232f11a3cc9 ("SUNRPC: Prevent immediate close+reconnect")
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
[meenashanmugam: Backported to 5.10: Fixed merge conflict in
xs_tcp_setup_socket]
Signed-off-by: Meena Shanmugam <meena.shanmugam@google.com>
[Bagas: Fix patch corruption by manually applying the backport]
Signed-off-by: Bagas Sanjaya <bagasdotme@gmail.com>
---
 include/linux/sunrpc/xprtsock.h |  1 +
 net/sunrpc/xprtsock.c           | 22 ++++++++++++----------
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/include/linux/sunrpc/xprtsock.h b/include/linux/sunrpc/xprtsock.h
index 8c2a712cb24202..689062afdd610d 100644
--- a/include/linux/sunrpc/xprtsock.h
+++ b/include/linux/sunrpc/xprtsock.h
@@ -89,5 +89,6 @@ struct sock_xprt {
 #define XPRT_SOCK_WAKE_WRITE	(5)
 #define XPRT_SOCK_WAKE_PENDING	(6)
 #define XPRT_SOCK_WAKE_DISCONNECT	(7)
+#define XPRT_SOCK_CONNECT_SENT	(8)
 
 #endif /* _LINUX_SUNRPC_XPRTSOCK_H */
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 60c58eb9a456f1..c9975a5c769be9 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -2260,10 +2260,15 @@ static void xs_tcp_setup_socket(struct work_struct *work)
 	struct rpc_xprt *xprt = &transport->xprt;
 	int status = -EIO;
 
-	if (!sock) {
-		sock = xs_create_sock(xprt, transport,
-				xs_addr(xprt)->sa_family, SOCK_STREAM,
-				IPPROTO_TCP, true);
+	if (xprt_connected(xprt))
+		goto out;
+
+	if (test_and_clear_bit(XPRT_SOCK_CONNECT_SENT),
+	    &transport->sock_state || !sock) {
+		xs_reset_transport(transport);
+		sock = xs_create_sock(xprt, transport, xs_addr(xprt)->sa_family,
+				      SOCK_STREAM, IPPROTO_TCP, true);
+
 		if (IS_ERR(sock)) {
 			status = PTR_ERR(sock);
 			goto out;
@@ -2294,6 +2299,7 @@ static void xs_tcp_setup_socket(struct work_struct *work)
 		break;
 	case 0:
 	case -EINPROGRESS:
+		set_bit(XPRT_SOCK_CONNECT_SENT, &transport->sock_state);
 	case -EALREADY:
 		xprt_unlock_connect(xprt, transport);
 		return;
@@ -2345,13 +2351,9 @@ static void xs_connect(struct rpc_xprt *xprt, struct rpc_task *task)
 
 	WARN_ON_ONCE(!xprt_lock_connect(xprt, task, transport));
 
-	if (transport->sock != NULL && !xprt_connecting(xprt)) {
+	if (transport->sock != NULL) {
 		dprintk("RPC:       xs_connect delayed xprt %p for %lu "
-				"seconds\n",
-				xprt, xprt->reestablish_timeout / HZ);
-
-		/* Start by resetting any existing state */
-		xs_reset_transport(transport);
+			"seconds\n", xprt, xprt->reestablish_timeout / HZ);
 
 		delay = xprt_reconnect_delay(xprt);
 		xprt_reconnect_backoff(xprt, XS_TCP_INIT_REEST_TO);
-- 
An old man doll... just what I always wanted! - Clara

Next time, configure your mailer/MUA to not mangle whitespaces. For sending
patches, it is preferred to use git-send-email(1).

FYI, CC Dexter Rivera since he's also requesting the backport [1].

[1]: https://lore.kernel.org/stable/CAG5dfppH0s-ujBjXyJJ62mGiJRiKz1NOYBPYOx3A1550Z8X7Mg@mail.gmail.com/

-- 
An old man doll... just what I always wanted! - Clara

Re: Request to cherry-pick f00432063db1 to 5.10

[Greg KH]

On Tue, May 10, 2022 at 07:33:23PM -0700, Meena Shanmugam wrote:
> Hi all,
> 
> The commit f00432063db1a0db484e85193eccc6845435b80e upstream (SUNRPC:
> Ensure we flush any closed sockets before xs_xprt_free()) fixes
> CVE-2022-28893, hence good candidate for stable trees.
> The above commit depends on 3be232f(SUNRPC: Prevent immediate
> close+reconnect)  and  89f4249(SUNRPC: Don't call connect() more than
> once on a TCP socket). Commit 3be232f depends on commit
> e26d9972720e(SUNRPC: Clean up scheduling of autoclose).
> 
> Commits e26d9972720e, 3be232f, f00432063db1 apply cleanly on 5.10
> kernel. commit 89f4249 didn't apply cleanly. I have patch for 89f4249
> below.

We also need this for 5.15.y first, before we can apply it to 5.10.y.
Can you provide a working backport for that tree as well?

And as others pointed out, your patch is totally corrupted and can not
be used, please fix your email client.

thanks,

greg k-h

Re: Request to cherry-pick f00432063db1 to 5.10

[Meena Shanmugam]

On Thu, May 12, 2022 at 9:23 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Tue, May 10, 2022 at 07:33:23PM -0700, Meena Shanmugam wrote:
> > Hi all,
> >
> > The commit f00432063db1a0db484e85193eccc6845435b80e upstream (SUNRPC:
> > Ensure we flush any closed sockets before xs_xprt_free()) fixes
> > CVE-2022-28893, hence good candidate for stable trees.
> > The above commit depends on 3be232f(SUNRPC: Prevent immediate
> > close+reconnect)  and  89f4249(SUNRPC: Don't call connect() more than
> > once on a TCP socket). Commit 3be232f depends on commit
> > e26d9972720e(SUNRPC: Clean up scheduling of autoclose).
> >
> > Commits e26d9972720e, 3be232f, f00432063db1 apply cleanly on 5.10
> > kernel. commit 89f4249 didn't apply cleanly. I have patch for 89f4249
> > below.
>
> We also need this for 5.15.y first, before we can apply it to 5.10.y.
> Can you provide a working backport for that tree as well?
>
> And as others pointed out, your patch is totally corrupted and can not
> be used, please fix your email client.
>
> thanks,
>
> greg k-h

For 5.15.y commit f00432063db1a0db484e85193eccc6845435b80e((SUNRPC:
Ensure we flush any closed sockets before xs_xprt_free())) applies
cleanly. The depend patch
3be232f(SUNRPC: Prevent immediate close+reconnect) also applies
cleanly. Patch  89f4249
(SUNRPC: Don't call connect() more than once on a TCP socket) is
already present in 5.15.34 onwards.

Sorry about the patch corruption, I will fix it.

Thanks,
Meena

Re: Request to cherry-pick f00432063db1 to 5.10

[Greg KH]

On Thu, May 12, 2022 at 10:38:04AM -0700, Meena Shanmugam wrote:
> On Thu, May 12, 2022 at 9:23 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Tue, May 10, 2022 at 07:33:23PM -0700, Meena Shanmugam wrote:
> > > Hi all,
> > >
> > > The commit f00432063db1a0db484e85193eccc6845435b80e upstream (SUNRPC:
> > > Ensure we flush any closed sockets before xs_xprt_free()) fixes
> > > CVE-2022-28893, hence good candidate for stable trees.
> > > The above commit depends on 3be232f(SUNRPC: Prevent immediate
> > > close+reconnect)  and  89f4249(SUNRPC: Don't call connect() more than
> > > once on a TCP socket). Commit 3be232f depends on commit
> > > e26d9972720e(SUNRPC: Clean up scheduling of autoclose).
> > >
> > > Commits e26d9972720e, 3be232f, f00432063db1 apply cleanly on 5.10
> > > kernel. commit 89f4249 didn't apply cleanly. I have patch for 89f4249
> > > below.
> >
> > We also need this for 5.15.y first, before we can apply it to 5.10.y.
> > Can you provide a working backport for that tree as well?
> >
> > And as others pointed out, your patch is totally corrupted and can not
> > be used, please fix your email client.
> >
> > thanks,
> >
> > greg k-h
> 
> For 5.15.y commit f00432063db1a0db484e85193eccc6845435b80e((SUNRPC:
> Ensure we flush any closed sockets before xs_xprt_free())) applies
> cleanly. The depend patch
> 3be232f(SUNRPC: Prevent immediate close+reconnect) also applies
> cleanly. Patch  89f4249
> (SUNRPC: Don't call connect() more than once on a TCP socket) is
> already present in 5.15.34 onwards.
> 
> Sorry about the patch corruption, I will fix it.

Sorry, but this did not work out at all, I get build errors when
attempting it for 5.10.y:

  CC [M]  net/sunrpc/xprtsock.o
net/sunrpc/xprtsock.c: In function ‘xs_tcp_setup_socket’:
net/sunrpc/xprtsock.c:2276:13: error: too few arguments to function ‘test_and_clear_bit’
 2276 |         if (test_and_clear_bit(XPRT_SOCK_CONNECT_SENT),
      |             ^~~~~~~~~~~~~~~~~~
In file included from ./arch/x86/include/asm/bitops.h:391,
                 from ./include/linux/bitops.h:29,
                 from ./include/linux/kernel.h:12,
                 from ./include/asm-generic/bug.h:20,
                 from ./arch/x86/include/asm/bug.h:93,
                 from ./include/linux/bug.h:5,
                 from ./include/linux/mmdebug.h:5,
                 from ./include/linux/gfp.h:5,
                 from ./include/linux/slab.h:15,
                 from net/sunrpc/xprtsock.c:24:
./include/asm-generic/bitops/instrumented-atomic.h:81:20: note: declared here
   81 | static inline bool test_and_clear_bit(long nr, volatile unsigned long *addr)
      |                    ^~~~~~~~~~~~~~~~~~
net/sunrpc/xprtsock.c:2276:55: warning: left-hand operand of comma expression has no effect [-Wunused-value]
 2276 |         if (test_and_clear_bit(XPRT_SOCK_CONNECT_SENT),
      |                                                       ^
net/sunrpc/xprtsock.c:2312:17: warning: this statement may fall through [-Wimplicit-fallthrough=]
 2312 |                 set_bit(XPRT_SOCK_CONNECT_SENT, &transport->sock_state);
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
net/sunrpc/xprtsock.c:2313:9: note: here
 2313 |         case -EALREADY:
      |         ^~~~
make[2]: *** [scripts/Makefile.build:280: net/sunrpc/xprtsock.o] Error 1
make[1]: *** [scripts/Makefile.build:497: net/sunrpc] Error 2


And I am not quite sure what order you want me to apply things for 5.15.y.

So please, send me a properly backported series of patches for this for 5.15.y
and 5.10.y and I will be glad to pick them up.  Right now I'm just confused as
this was obviously not tested at all :(

thanks,

greg k-h

[PATCH] SUNRPC: Don't call connect() more than once on a TCP socket

[Meena Shanmugam]

From: Trond Myklebust <trond.myklebust@hammerspace.com>

commit 89f42494f92f448747bd8a7ab1ae8b5d5520577d upstream.

Avoid socket state races due to repeated calls to ->connect() using the
same socket. If connect() returns 0 due to the connection having
completed, but we are in fact in a closing state, then we may leave the
XPRT_CONNECTING flag set on the transport.

Reported-by: Enrico Scholz <enrico.scholz@sigma-chemnitz.de>
Fixes: 3be232f11a3c ("SUNRPC: Prevent immediate close+reconnect")
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
[meenashanmugam: Backported to 5.10: Fixed merge conflict in xs_tcp_setup_socket]
Signed-off-by: Meena Shanmugam <meenashanmugam@google.com>
---
 include/linux/sunrpc/xprtsock.h |  1 +
 net/sunrpc/xprtsock.c           | 21 +++++++++++----------
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/include/linux/sunrpc/xprtsock.h b/include/linux/sunrpc/xprtsock.h
index 8c2a712cb242..689062afdd61 100644
--- a/include/linux/sunrpc/xprtsock.h
+++ b/include/linux/sunrpc/xprtsock.h
@@ -89,5 +89,6 @@ struct sock_xprt {
 #define XPRT_SOCK_WAKE_WRITE	(5)
 #define XPRT_SOCK_WAKE_PENDING	(6)
 #define XPRT_SOCK_WAKE_DISCONNECT	(7)
+#define XPRT_SOCK_CONNECT_SENT	(8)
 
 #endif /* _LINUX_SUNRPC_XPRTSOCK_H */
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 60c58eb9a456..33a81f9703b1 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -2260,10 +2260,14 @@ static void xs_tcp_setup_socket(struct work_struct *work)
 	struct rpc_xprt *xprt = &transport->xprt;
 	int status = -EIO;
 
-	if (!sock) {
-		sock = xs_create_sock(xprt, transport,
-				xs_addr(xprt)->sa_family, SOCK_STREAM,
-				IPPROTO_TCP, true);
+	if (xprt_connected(xprt))
+		goto out;
+	if (test_and_clear_bit(XPRT_SOCK_CONNECT_SENT,
+			       &transport->sock_state) ||
+	    !sock) {
+		xs_reset_transport(transport);
+		sock = xs_create_sock(xprt, transport, xs_addr(xprt)->sa_family,
+				      SOCK_STREAM, IPPROTO_TCP, true);
 		if (IS_ERR(sock)) {
 			status = PTR_ERR(sock);
 			goto out;
@@ -2294,6 +2298,7 @@ static void xs_tcp_setup_socket(struct work_struct *work)
 		break;
 	case 0:
 	case -EINPROGRESS:
+		set_bit(XPRT_SOCK_CONNECT_SENT, &transport->sock_state);
 	case -EALREADY:
 		xprt_unlock_connect(xprt, transport);
 		return;
@@ -2345,13 +2350,9 @@ static void xs_connect(struct rpc_xprt *xprt, struct rpc_task *task)
 
 	WARN_ON_ONCE(!xprt_lock_connect(xprt, task, transport));
 
-	if (transport->sock != NULL && !xprt_connecting(xprt)) {
+	if (transport->sock != NULL) {
 		dprintk("RPC:       xs_connect delayed xprt %p for %lu "
-				"seconds\n",
-				xprt, xprt->reestablish_timeout / HZ);
-
-		/* Start by resetting any existing state */
-		xs_reset_transport(transport);
+			"seconds\n", xprt, xprt->reestablish_timeout / HZ);
 
 		delay = xprt_reconnect_delay(xprt);
 		xprt_reconnect_backoff(xprt, XS_TCP_INIT_REEST_TO);
-- 
2.36.0.512.ge40c2bad7a-goog

Re: Request to cherry-pick f00432063db1 to 5.10

[Meena Shanmugam]

I tested my original patch(which was corrupted by email client).
When the patch is manually backported to fix white space, the patch
was fixed wrongly :(
I sent my original patch again for 5.10.y without any corruption.
Sorry for the inconvenience caused.

For 5.15.y, this is the cherry-pick order:

3be232f11a3cc9b0ef0795e39fa11bdb8e422a06(SUNRPC: Prevent immediate
close+reconnect)
f00432063db1a0db484e85193eccc6845435b80e(SUNRPC: Ensure we flush any
closed sockets before xs_xprt_free())

Thanks,
Meena

On Fri, May 13, 2022 at 1:25 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Thu, May 12, 2022 at 10:38:04AM -0700, Meena Shanmugam wrote:
> > On Thu, May 12, 2022 at 9:23 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Tue, May 10, 2022 at 07:33:23PM -0700, Meena Shanmugam wrote:
> > > > Hi all,
> > > >
> > > > The commit f00432063db1a0db484e85193eccc6845435b80e upstream (SUNRPC:
> > > > Ensure we flush any closed sockets before xs_xprt_free()) fixes
> > > > CVE-2022-28893, hence good candidate for stable trees.
> > > > The above commit depends on 3be232f(SUNRPC: Prevent immediate
> > > > close+reconnect)  and  89f4249(SUNRPC: Don't call connect() more than
> > > > once on a TCP socket). Commit 3be232f depends on commit
> > > > e26d9972720e(SUNRPC: Clean up scheduling of autoclose).
> > > >
> > > > Commits e26d9972720e, 3be232f, f00432063db1 apply cleanly on 5.10
> > > > kernel. commit 89f4249 didn't apply cleanly. I have patch for 89f4249
> > > > below.
> > >
> > > We also need this for 5.15.y first, before we can apply it to 5.10.y.
> > > Can you provide a working backport for that tree as well?
> > >
> > > And as others pointed out, your patch is totally corrupted and can not
> > > be used, please fix your email client.
> > >
> > > thanks,
> > >
> > > greg k-h
> >
> > For 5.15.y commit f00432063db1a0db484e85193eccc6845435b80e((SUNRPC:
> > Ensure we flush any closed sockets before xs_xprt_free())) applies
> > cleanly. The depend patch
> > 3be232f(SUNRPC: Prevent immediate close+reconnect) also applies
> > cleanly. Patch  89f4249
> > (SUNRPC: Don't call connect() more than once on a TCP socket) is
> > already present in 5.15.34 onwards.
> >
> > Sorry about the patch corruption, I will fix it.
>
> Sorry, but this did not work out at all, I get build errors when
> attempting it for 5.10.y:
>
>   CC [M]  net/sunrpc/xprtsock.o
> net/sunrpc/xprtsock.c: In function ‘xs_tcp_setup_socket’:
> net/sunrpc/xprtsock.c:2276:13: error: too few arguments to function ‘test_and_clear_bit’
>  2276 |         if (test_and_clear_bit(XPRT_SOCK_CONNECT_SENT),
>       |             ^~~~~~~~~~~~~~~~~~
> In file included from ./arch/x86/include/asm/bitops.h:391,
>                  from ./include/linux/bitops.h:29,
>                  from ./include/linux/kernel.h:12,
>                  from ./include/asm-generic/bug.h:20,
>                  from ./arch/x86/include/asm/bug.h:93,
>                  from ./include/linux/bug.h:5,
>                  from ./include/linux/mmdebug.h:5,
>                  from ./include/linux/gfp.h:5,
>                  from ./include/linux/slab.h:15,
>                  from net/sunrpc/xprtsock.c:24:
> ./include/asm-generic/bitops/instrumented-atomic.h:81:20: note: declared here
>    81 | static inline bool test_and_clear_bit(long nr, volatile unsigned long *addr)
>       |                    ^~~~~~~~~~~~~~~~~~
> net/sunrpc/xprtsock.c:2276:55: warning: left-hand operand of comma expression has no effect [-Wunused-value]
>  2276 |         if (test_and_clear_bit(XPRT_SOCK_CONNECT_SENT),
>       |                                                       ^
> net/sunrpc/xprtsock.c:2312:17: warning: this statement may fall through [-Wimplicit-fallthrough=]
>  2312 |                 set_bit(XPRT_SOCK_CONNECT_SENT, &transport->sock_state);
>       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> net/sunrpc/xprtsock.c:2313:9: note: here
>  2313 |         case -EALREADY:
>       |         ^~~~
> make[2]: *** [scripts/Makefile.build:280: net/sunrpc/xprtsock.o] Error 1
> make[1]: *** [scripts/Makefile.build:497: net/sunrpc] Error 2
>
>
> And I am not quite sure what order you want me to apply things for 5.15.y.
>
> So please, send me a properly backported series of patches for this for 5.15.y
> and 5.10.y and I will be glad to pick them up.  Right now I'm just confused as
> this was obviously not tested at all :(
>
> thanks,
>
> greg k-h

Re: Request to cherry-pick f00432063db1 to 5.10

[Meena Shanmugam]

On Fri, May 13, 2022 at 1:25 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Thu, May 12, 2022 at 10:38:04AM -0700, Meena Shanmugam wrote:
> > On Thu, May 12, 2022 at 9:23 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Tue, May 10, 2022 at 07:33:23PM -0700, Meena Shanmugam wrote:
> > > > Hi all,
> > > >
> > > > The commit f00432063db1a0db484e85193eccc6845435b80e upstream (SUNRPC:
> > > > Ensure we flush any closed sockets before xs_xprt_free()) fixes
> > > > CVE-2022-28893, hence good candidate for stable trees.
> > > > The above commit depends on 3be232f(SUNRPC: Prevent immediate
> > > > close+reconnect)  and  89f4249(SUNRPC: Don't call connect() more than
> > > > once on a TCP socket). Commit 3be232f depends on commit
> > > > e26d9972720e(SUNRPC: Clean up scheduling of autoclose).
> > > >
> > > > Commits e26d9972720e, 3be232f, f00432063db1 apply cleanly on 5.10
> > > > kernel. commit 89f4249 didn't apply cleanly. I have patch for 89f4249
> > > > below.
> > >
> > > We also need this for 5.15.y first, before we can apply it to 5.10.y.
> > > Can you provide a working backport for that tree as well?
> > >
> > > And as others pointed out, your patch is totally corrupted and can not
> > > be used, please fix your email client.
> > >
> > > thanks,
> > >
> > > greg k-h
> >
> > For 5.15.y commit f00432063db1a0db484e85193eccc6845435b80e((SUNRPC:
> > Ensure we flush any closed sockets before xs_xprt_free())) applies
> > cleanly. The depend patch
> > 3be232f(SUNRPC: Prevent immediate close+reconnect) also applies
> > cleanly. Patch  89f4249
> > (SUNRPC: Don't call connect() more than once on a TCP socket) is
> > already present in 5.15.34 onwards.
> >
> > Sorry about the patch corruption, I will fix it.
>
> Sorry, but this did not work out at all, I get build errors when
> attempting it for 5.10.y:
>
>   CC [M]  net/sunrpc/xprtsock.o
> net/sunrpc/xprtsock.c: In function ‘xs_tcp_setup_socket’:
> net/sunrpc/xprtsock.c:2276:13: error: too few arguments to function ‘test_and_clear_bit’
>  2276 |         if (test_and_clear_bit(XPRT_SOCK_CONNECT_SENT),
>       |             ^~~~~~~~~~~~~~~~~~
> In file included from ./arch/x86/include/asm/bitops.h:391,
>                  from ./include/linux/bitops.h:29,
>                  from ./include/linux/kernel.h:12,
>                  from ./include/asm-generic/bug.h:20,
>                  from ./arch/x86/include/asm/bug.h:93,
>                  from ./include/linux/bug.h:5,
>                  from ./include/linux/mmdebug.h:5,
>                  from ./include/linux/gfp.h:5,
>                  from ./include/linux/slab.h:15,
>                  from net/sunrpc/xprtsock.c:24:
> ./include/asm-generic/bitops/instrumented-atomic.h:81:20: note: declared here
>    81 | static inline bool test_and_clear_bit(long nr, volatile unsigned long *addr)
>       |                    ^~~~~~~~~~~~~~~~~~
> net/sunrpc/xprtsock.c:2276:55: warning: left-hand operand of comma expression has no effect [-Wunused-value]
>  2276 |         if (test_and_clear_bit(XPRT_SOCK_CONNECT_SENT),
>       |                                                       ^
> net/sunrpc/xprtsock.c:2312:17: warning: this statement may fall through [-Wimplicit-fallthrough=]
>  2312 |                 set_bit(XPRT_SOCK_CONNECT_SENT, &transport->sock_state);
>       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> net/sunrpc/xprtsock.c:2313:9: note: here
>  2313 |         case -EALREADY:
>       |         ^~~~
> make[2]: *** [scripts/Makefile.build:280: net/sunrpc/xprtsock.o] Error 1
> make[1]: *** [scripts/Makefile.build:497: net/sunrpc] Error 2
>
>
> And I am not quite sure what order you want me to apply things for 5.15.y.
>
> So please, send me a properly backported series of patches for this for 5.15.y
> and 5.10.y and I will be glad to pick them up.  Right now I'm just confused as
> this was obviously not tested at all :(
>
> thanks,
>
> greg k-h

I tested my original patch(which was corrupted by email client).
When the patch is manually backported to fix white space, the patch
was fixed wrongly :(
I sent my original patch again for 5.10.y without any corruption.
Sorry for the inconvenience caused.

For 5.15.y, this is the cherry-pick order:

3be232f11a3cc9b0ef0795e39fa11bdb8e422a06(SUNRPC: Prevent immediate
close+reconnect)
f00432063db1a0db484e85193eccc6845435b80e(SUNRPC: Ensure we flush any
closed sockets before xs_xprt_free())

Thanks,
Meena

Re: [PATCH] SUNRPC: Don't call connect() more than once on a TCP socket

[Greg KH]

On Fri, May 13, 2022 at 05:59:59PM +0000, Meena Shanmugam wrote:
> From: Trond Myklebust <trond.myklebust@hammerspace.com>
> 
> commit 89f42494f92f448747bd8a7ab1ae8b5d5520577d upstream.
> 
> Avoid socket state races due to repeated calls to ->connect() using the
> same socket. If connect() returns 0 due to the connection having
> completed, but we are in fact in a closing state, then we may leave the
> XPRT_CONNECTING flag set on the transport.
> 
> Reported-by: Enrico Scholz <enrico.scholz@sigma-chemnitz.de>
> Fixes: 3be232f11a3c ("SUNRPC: Prevent immediate close+reconnect")
> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> [meenashanmugam: Backported to 5.10: Fixed merge conflict in xs_tcp_setup_socket]
> Signed-off-by: Meena Shanmugam <meenashanmugam@google.com>
> ---
>  include/linux/sunrpc/xprtsock.h |  1 +
>  net/sunrpc/xprtsock.c           | 21 +++++++++++----------
>  2 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/include/linux/sunrpc/xprtsock.h b/include/linux/sunrpc/xprtsock.h
> index 8c2a712cb242..689062afdd61 100644
> --- a/include/linux/sunrpc/xprtsock.h
> +++ b/include/linux/sunrpc/xprtsock.h
> @@ -89,5 +89,6 @@ struct sock_xprt {
>  #define XPRT_SOCK_WAKE_WRITE	(5)
>  #define XPRT_SOCK_WAKE_PENDING	(6)
>  #define XPRT_SOCK_WAKE_DISCONNECT	(7)
> +#define XPRT_SOCK_CONNECT_SENT	(8)
>  
>  #endif /* _LINUX_SUNRPC_XPRTSOCK_H */
> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> index 60c58eb9a456..33a81f9703b1 100644
> --- a/net/sunrpc/xprtsock.c
> +++ b/net/sunrpc/xprtsock.c
> @@ -2260,10 +2260,14 @@ static void xs_tcp_setup_socket(struct work_struct *work)
>  	struct rpc_xprt *xprt = &transport->xprt;
>  	int status = -EIO;
>  
> -	if (!sock) {
> -		sock = xs_create_sock(xprt, transport,
> -				xs_addr(xprt)->sa_family, SOCK_STREAM,
> -				IPPROTO_TCP, true);
> +	if (xprt_connected(xprt))
> +		goto out;
> +	if (test_and_clear_bit(XPRT_SOCK_CONNECT_SENT,
> +			       &transport->sock_state) ||
> +	    !sock) {
> +		xs_reset_transport(transport);
> +		sock = xs_create_sock(xprt, transport, xs_addr(xprt)->sa_family,
> +				      SOCK_STREAM, IPPROTO_TCP, true);
>  		if (IS_ERR(sock)) {
>  			status = PTR_ERR(sock);
>  			goto out;
> @@ -2294,6 +2298,7 @@ static void xs_tcp_setup_socket(struct work_struct *work)
>  		break;
>  	case 0:
>  	case -EINPROGRESS:
> +		set_bit(XPRT_SOCK_CONNECT_SENT, &transport->sock_state);
>  	case -EALREADY:
>  		xprt_unlock_connect(xprt, transport);
>  		return;
> @@ -2345,13 +2350,9 @@ static void xs_connect(struct rpc_xprt *xprt, struct rpc_task *task)
>  
>  	WARN_ON_ONCE(!xprt_lock_connect(xprt, task, transport));
>  
> -	if (transport->sock != NULL && !xprt_connecting(xprt)) {
> +	if (transport->sock != NULL) {
>  		dprintk("RPC:       xs_connect delayed xprt %p for %lu "
> -				"seconds\n",
> -				xprt, xprt->reestablish_timeout / HZ);
> -
> -		/* Start by resetting any existing state */
> -		xs_reset_transport(transport);
> +			"seconds\n", xprt, xprt->reestablish_timeout / HZ);
>  
>  		delay = xprt_reconnect_delay(xprt);
>  		xprt_reconnect_backoff(xprt, XS_TCP_INIT_REEST_TO);
> -- 
> 2.36.0.512.ge40c2bad7a-goog
> 

This should be a patch series, not just this one commit, right?

What are _ALL_ of the commits you want to see applied for 5.10.y?

thanks,

greg k-h

[PATCH 0/4] Request to cherry-pick f00432063db1 to 5.10

[Meena Shanmugam]

The commit f00432063db1a0db484e85193eccc6845435b80e upstream (SUNRPC:
Ensure we flush any closed sockets before xs_xprt_free()) fixes
CVE-2022-28893, hence good candidate for stable trees.
The above commit depends on 3be232f(SUNRPC: Prevent immediate
close+reconnect)  and  89f4249(SUNRPC: Don't call connect() more than
once on a TCP socket). Commit 3be232f depends on commit
e26d9972720e(SUNRPC: Clean up scheduling of autoclose).

Commits e26d9972720e, 3be232f, f00432063db1 apply cleanly on 5.10
kernel. commit 89f4249 didn't apply cleanly. This patch series includes
all the commits required for back porting f00432063db1.

Trond Myklebust (4):
  SUNRPC: Clean up scheduling of autoclose
  SUNRPC: Prevent immediate close+reconnect
  SUNRPC: Don't call connect() more than once on a TCP socket
  SUNRPC: Ensure we flush any closed sockets before xs_xprt_free()

 fs/file_table.c                 |  1 +
 include/linux/sunrpc/xprtsock.h |  1 +
 include/trace/events/sunrpc.h   |  1 -
 net/sunrpc/xprt.c               | 36 ++++++++++++++++-----------------
 net/sunrpc/xprtsock.c           | 35 +++++++++++++++++++++-----------
 5 files changed, 43 insertions(+), 31 deletions(-)

-- 
2.36.0.550.gb090851708-goog

[PATCH 1/4] SUNRPC: Clean up scheduling of autoclose

[Meena Shanmugam]

From: Trond Myklebust <trond.myklebust@hammerspace.com>

commit e26d9972720e2484f44cdd94ca4e31cc372ed2ed upstream.

Consolidate duplicated code in xprt_force_disconnect() and
xprt_conditional_disconnect().

Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
Signed-off-by: Meena Shanmugam <meenashanmugam@google.com>
---
 net/sunrpc/xprt.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
index 6bc225d64d23..a6bb957084f7 100644
--- a/net/sunrpc/xprt.c
+++ b/net/sunrpc/xprt.c
@@ -731,6 +731,20 @@ void xprt_disconnect_done(struct rpc_xprt *xprt)
 }
 EXPORT_SYMBOL_GPL(xprt_disconnect_done);
 
+/**
+ * xprt_schedule_autoclose_locked - Try to schedule an autoclose RPC call
+ * @xprt: transport to disconnect
+ */
+static void xprt_schedule_autoclose_locked(struct rpc_xprt *xprt)
+{
+	set_bit(XPRT_CLOSE_WAIT, &xprt->state);
+	if (test_and_set_bit(XPRT_LOCKED, &xprt->state) == 0)
+		queue_work(xprtiod_workqueue, &xprt->task_cleanup);
+	else if (xprt->snd_task && !test_bit(XPRT_SND_IS_COOKIE, &xprt->state))
+		rpc_wake_up_queued_task_set_status(&xprt->pending,
+						   xprt->snd_task, -ENOTCONN);
+}
+
 /**
  * xprt_force_disconnect - force a transport to disconnect
  * @xprt: transport to disconnect
@@ -742,13 +756,7 @@ void xprt_force_disconnect(struct rpc_xprt *xprt)
 
 	/* Don't race with the test_bit() in xprt_clear_locked() */
 	spin_lock(&xprt->transport_lock);
-	set_bit(XPRT_CLOSE_WAIT, &xprt->state);
-	/* Try to schedule an autoclose RPC call */
-	if (test_and_set_bit(XPRT_LOCKED, &xprt->state) == 0)
-		queue_work(xprtiod_workqueue, &xprt->task_cleanup);
-	else if (xprt->snd_task && !test_bit(XPRT_SND_IS_COOKIE, &xprt->state))
-		rpc_wake_up_queued_task_set_status(&xprt->pending,
-						   xprt->snd_task, -ENOTCONN);
+	xprt_schedule_autoclose_locked(xprt);
 	spin_unlock(&xprt->transport_lock);
 }
 EXPORT_SYMBOL_GPL(xprt_force_disconnect);
@@ -788,11 +796,7 @@ void xprt_conditional_disconnect(struct rpc_xprt *xprt, unsigned int cookie)
 		goto out;
 	if (test_bit(XPRT_CLOSING, &xprt->state))
 		goto out;
-	set_bit(XPRT_CLOSE_WAIT, &xprt->state);
-	/* Try to schedule an autoclose RPC call */
-	if (test_and_set_bit(XPRT_LOCKED, &xprt->state) == 0)
-		queue_work(xprtiod_workqueue, &xprt->task_cleanup);
-	xprt_wake_pending_tasks(xprt, -EAGAIN);
+	xprt_schedule_autoclose_locked(xprt);
 out:
 	spin_unlock(&xprt->transport_lock);
 }
-- 
2.36.0.550.gb090851708-goog

[PATCH 2/4] SUNRPC: Prevent immediate close+reconnect

[Meena Shanmugam]

From: Trond Myklebust <trond.myklebust@hammerspace.com>

commit 3be232f11a3cc9b0ef0795e39fa11bdb8e422a06 upstream.

If we have already set up the socket and are waiting for it to connect,
then don't immediately close and retry.

Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
Signed-off-by: Meena Shanmugam <meenashanmugam@google.com>
---
 net/sunrpc/xprt.c     | 3 ++-
 net/sunrpc/xprtsock.c | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
index a6bb957084f7..af0159459c75 100644
--- a/net/sunrpc/xprt.c
+++ b/net/sunrpc/xprt.c
@@ -737,7 +737,8 @@ EXPORT_SYMBOL_GPL(xprt_disconnect_done);
  */
 static void xprt_schedule_autoclose_locked(struct rpc_xprt *xprt)
 {
-	set_bit(XPRT_CLOSE_WAIT, &xprt->state);
+	if (test_and_set_bit(XPRT_CLOSE_WAIT, &xprt->state))
+		return;
 	if (test_and_set_bit(XPRT_LOCKED, &xprt->state) == 0)
 		queue_work(xprtiod_workqueue, &xprt->task_cleanup);
 	else if (xprt->snd_task && !test_bit(XPRT_SND_IS_COOKIE, &xprt->state))
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index bd123f1d0923..60c58eb9a456 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -2345,7 +2345,7 @@ static void xs_connect(struct rpc_xprt *xprt, struct rpc_task *task)
 
 	WARN_ON_ONCE(!xprt_lock_connect(xprt, task, transport));
 
-	if (transport->sock != NULL) {
+	if (transport->sock != NULL && !xprt_connecting(xprt)) {
 		dprintk("RPC:       xs_connect delayed xprt %p for %lu "
 				"seconds\n",
 				xprt, xprt->reestablish_timeout / HZ);
-- 
2.36.0.550.gb090851708-goog

[PATCH 3/4] SUNRPC: Don't call connect() more than once on a TCP socket

[Meena Shanmugam]

From: Trond Myklebust <trond.myklebust@hammerspace.com>

commit 89f42494f92f448747bd8a7ab1ae8b5d5520577d upstream.

Avoid socket state races due to repeated calls to ->connect() using the
same socket. If connect() returns 0 due to the connection having
completed, but we are in fact in a closing state, then we may leave the
XPRT_CONNECTING flag set on the transport.

Reported-by: Enrico Scholz <enrico.scholz@sigma-chemnitz.de>
Fixes: 3be232f11a3c ("SUNRPC: Prevent immediate close+reconnect")
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
[meenashanmugam: Backported to 5.10: Fixed merge conflict in xs_tcp_setup_socket]
Signed-off-by: Meena Shanmugam <meenashanmugam@google.com>
---
 include/linux/sunrpc/xprtsock.h |  1 +
 net/sunrpc/xprtsock.c           | 21 +++++++++++----------
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/include/linux/sunrpc/xprtsock.h b/include/linux/sunrpc/xprtsock.h
index 8c2a712cb242..689062afdd61 100644
--- a/include/linux/sunrpc/xprtsock.h
+++ b/include/linux/sunrpc/xprtsock.h
@@ -89,5 +89,6 @@ struct sock_xprt {
 #define XPRT_SOCK_WAKE_WRITE	(5)
 #define XPRT_SOCK_WAKE_PENDING	(6)
 #define XPRT_SOCK_WAKE_DISCONNECT	(7)
+#define XPRT_SOCK_CONNECT_SENT	(8)
 
 #endif /* _LINUX_SUNRPC_XPRTSOCK_H */
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 60c58eb9a456..33a81f9703b1 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -2260,10 +2260,14 @@ static void xs_tcp_setup_socket(struct work_struct *work)
 	struct rpc_xprt *xprt = &transport->xprt;
 	int status = -EIO;
 
-	if (!sock) {
-		sock = xs_create_sock(xprt, transport,
-				xs_addr(xprt)->sa_family, SOCK_STREAM,
-				IPPROTO_TCP, true);
+	if (xprt_connected(xprt))
+		goto out;
+	if (test_and_clear_bit(XPRT_SOCK_CONNECT_SENT,
+			       &transport->sock_state) ||
+	    !sock) {
+		xs_reset_transport(transport);
+		sock = xs_create_sock(xprt, transport, xs_addr(xprt)->sa_family,
+				      SOCK_STREAM, IPPROTO_TCP, true);
 		if (IS_ERR(sock)) {
 			status = PTR_ERR(sock);
 			goto out;
@@ -2294,6 +2298,7 @@ static void xs_tcp_setup_socket(struct work_struct *work)
 		break;
 	case 0:
 	case -EINPROGRESS:
+		set_bit(XPRT_SOCK_CONNECT_SENT, &transport->sock_state);
 	case -EALREADY:
 		xprt_unlock_connect(xprt, transport);
 		return;
@@ -2345,13 +2350,9 @@ static void xs_connect(struct rpc_xprt *xprt, struct rpc_task *task)
 
 	WARN_ON_ONCE(!xprt_lock_connect(xprt, task, transport));
 
-	if (transport->sock != NULL && !xprt_connecting(xprt)) {
+	if (transport->sock != NULL) {
 		dprintk("RPC:       xs_connect delayed xprt %p for %lu "
-				"seconds\n",
-				xprt, xprt->reestablish_timeout / HZ);
-
-		/* Start by resetting any existing state */
-		xs_reset_transport(transport);
+			"seconds\n", xprt, xprt->reestablish_timeout / HZ);
 
 		delay = xprt_reconnect_delay(xprt);
 		xprt_reconnect_backoff(xprt, XS_TCP_INIT_REEST_TO);
-- 
2.36.0.550.gb090851708-goog

[PATCH 4/4] SUNRPC: Ensure we flush any closed sockets before xs_xprt_free()

[Meena Shanmugam]

From: Trond Myklebust <trond.myklebust@hammerspace.com>

commit f00432063db1a0db484e85193eccc6845435b80e upstream.

We must ensure that all sockets are closed before we call xprt_free()
and release the reference to the net namespace. The problem is that
calling fput() will defer closing the socket until delayed_fput() gets
called.
Let's fix the situation by allowing rpciod and the transport teardown
code (which runs on the system wq) to call __fput_sync(), and directly
close the socket.

Reported-by: Felix Fu <foyjog@gmail.com>
Acked-by: Al Viro <viro@zeniv.linux.org.uk>
Fixes: a73881c96d73 ("SUNRPC: Fix an Oops in udp_poll()")
Cc: stable@vger.kernel.org # 5.1.x: 3be232f11a3c: SUNRPC: Prevent immediate close+reconnect
Cc: stable@vger.kernel.org # 5.1.x: 89f42494f92f: SUNRPC: Don't call connect() more than once on a TCP socket
Cc: stable@vger.kernel.org # 5.1.x
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
Signed-off-by: Meena Shanmugam <meenashanmugam@google.com>
---
 fs/file_table.c               |  1 +
 include/trace/events/sunrpc.h |  1 -
 net/sunrpc/xprt.c             |  7 +------
 net/sunrpc/xprtsock.c         | 16 +++++++++++++---
 4 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/fs/file_table.c b/fs/file_table.c
index fbd45a1a0e7e..12aaa04ac5b7 100644
--- a/fs/file_table.c
+++ b/fs/file_table.c
@@ -377,6 +377,7 @@ void __fput_sync(struct file *file)
 }
 
 EXPORT_SYMBOL(fput);
+EXPORT_SYMBOL(__fput_sync);
 
 void __init files_init(void)
 {
diff --git a/include/trace/events/sunrpc.h b/include/trace/events/sunrpc.h
index ed1bbac004d5..8220369ee610 100644
--- a/include/trace/events/sunrpc.h
+++ b/include/trace/events/sunrpc.h
@@ -1006,7 +1006,6 @@ DEFINE_RPC_XPRT_LIFETIME_EVENT(connect);
 DEFINE_RPC_XPRT_LIFETIME_EVENT(disconnect_auto);
 DEFINE_RPC_XPRT_LIFETIME_EVENT(disconnect_done);
 DEFINE_RPC_XPRT_LIFETIME_EVENT(disconnect_force);
-DEFINE_RPC_XPRT_LIFETIME_EVENT(disconnect_cleanup);
 DEFINE_RPC_XPRT_LIFETIME_EVENT(destroy);
 
 DECLARE_EVENT_CLASS(rpc_xprt_event,
diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
index af0159459c75..13d5323f8098 100644
--- a/net/sunrpc/xprt.c
+++ b/net/sunrpc/xprt.c
@@ -886,12 +886,7 @@ void xprt_connect(struct rpc_task *task)
 	if (!xprt_lock_write(xprt, task))
 		return;
 
-	if (test_and_clear_bit(XPRT_CLOSE_WAIT, &xprt->state)) {
-		trace_xprt_disconnect_cleanup(xprt);
-		xprt->ops->close(xprt);
-	}
-
-	if (!xprt_connected(xprt)) {
+	if (!xprt_connected(xprt) && !test_bit(XPRT_CLOSE_WAIT, &xprt->state)) {
 		task->tk_rqstp->rq_connect_cookie = xprt->connect_cookie;
 		rpc_sleep_on_timeout(&xprt->pending, task, NULL,
 				xprt_request_timeout(task->tk_rqstp));
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 33a81f9703b1..71c0ec9eaf0b 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -871,7 +871,7 @@ static int xs_local_send_request(struct rpc_rqst *req)
 
 	/* Close the stream if the previous transmission was incomplete */
 	if (xs_send_request_was_aborted(transport, req)) {
-		xs_close(xprt);
+		xprt_force_disconnect(xprt);
 		return -ENOTCONN;
 	}
 
@@ -909,7 +909,7 @@ static int xs_local_send_request(struct rpc_rqst *req)
 			-status);
 		fallthrough;
 	case -EPIPE:
-		xs_close(xprt);
+		xprt_force_disconnect(xprt);
 		status = -ENOTCONN;
 	}
 
@@ -1191,6 +1191,16 @@ static void xs_reset_transport(struct sock_xprt *transport)
 
 	if (sk == NULL)
 		return;
+	/*
+	 * Make sure we're calling this in a context from which it is safe
+	 * to call __fput_sync(). In practice that means rpciod and the
+	 * system workqueue.
+	 */
+	if (!(current->flags & PF_WQ_WORKER)) {
+		WARN_ON_ONCE(1);
+		set_bit(XPRT_CLOSE_WAIT, &xprt->state);
+		return;
+	}
 
 	if (atomic_read(&transport->xprt.swapper))
 		sk_clear_memalloc(sk);
@@ -1214,7 +1224,7 @@ static void xs_reset_transport(struct sock_xprt *transport)
 	mutex_unlock(&transport->recv_mutex);
 
 	trace_rpc_socket_close(xprt, sock);
-	fput(filp);
+	__fput_sync(filp);
 
 	xprt_disconnect_done(xprt);
 }
-- 
2.36.0.550.gb090851708-goog

Re: [PATCH 0/4] Request to cherry-pick f00432063db1 to 5.10

[Bagas Sanjaya]

On Sat, May 14, 2022 at 05:34:49AM +0000, Meena Shanmugam wrote:
> The commit f00432063db1a0db484e85193eccc6845435b80e upstream (SUNRPC:
> Ensure we flush any closed sockets before xs_xprt_free()) fixes
> CVE-2022-28893, hence good candidate for stable trees.
> The above commit depends on 3be232f(SUNRPC: Prevent immediate
> close+reconnect)  and  89f4249(SUNRPC: Don't call connect() more than
> once on a TCP socket). Commit 3be232f depends on commit
> e26d9972720e(SUNRPC: Clean up scheduling of autoclose).
> 
> Commits e26d9972720e, 3be232f, f00432063db1 apply cleanly on 5.10
> kernel. commit 89f4249 didn't apply cleanly. This patch series includes
> all the commits required for back porting f00432063db1.
> 

Hi Meena,

I can't speaking about the code (as I'm not subject-expert here), but I
would like to give you suggestions:

  - When sending backported patch series, always prefix the subject with
    "[PATCH x.y]", where x.y is the stable version the backport is made
    against.
  - Abbreviated commit hash should be at least 12 (or my favorite, 14) characters long.
  - Commit identifier should be in format "%h (\"%s\")".
  - As always, DON'T DO top-posting, DO interleaved reply and reply
    below the quoted original message.

Trond and Dexter, any comments or ACKs?

Thanks.

-- 
An old man doll... just what I always wanted! - Clara

Re: Request to cherry-pick f00432063db1 to 5.10

[Greg KH]

On Fri, May 13, 2022 at 11:14:51AM -0700, Meena Shanmugam wrote:
> On Fri, May 13, 2022 at 1:25 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Thu, May 12, 2022 at 10:38:04AM -0700, Meena Shanmugam wrote:
> > > On Thu, May 12, 2022 at 9:23 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > On Tue, May 10, 2022 at 07:33:23PM -0700, Meena Shanmugam wrote:
> > > > > Hi all,
> > > > >
> > > > > The commit f00432063db1a0db484e85193eccc6845435b80e upstream (SUNRPC:
> > > > > Ensure we flush any closed sockets before xs_xprt_free()) fixes
> > > > > CVE-2022-28893, hence good candidate for stable trees.
> > > > > The above commit depends on 3be232f(SUNRPC: Prevent immediate
> > > > > close+reconnect)  and  89f4249(SUNRPC: Don't call connect() more than
> > > > > once on a TCP socket). Commit 3be232f depends on commit
> > > > > e26d9972720e(SUNRPC: Clean up scheduling of autoclose).
> > > > >
> > > > > Commits e26d9972720e, 3be232f, f00432063db1 apply cleanly on 5.10
> > > > > kernel. commit 89f4249 didn't apply cleanly. I have patch for 89f4249
> > > > > below.
> > > >
> > > > We also need this for 5.15.y first, before we can apply it to 5.10.y.
> > > > Can you provide a working backport for that tree as well?
> > > >
> > > > And as others pointed out, your patch is totally corrupted and can not
> > > > be used, please fix your email client.
> > > >
> > > > thanks,
> > > >
> > > > greg k-h
> > >
> > > For 5.15.y commit f00432063db1a0db484e85193eccc6845435b80e((SUNRPC:
> > > Ensure we flush any closed sockets before xs_xprt_free())) applies
> > > cleanly. The depend patch
> > > 3be232f(SUNRPC: Prevent immediate close+reconnect) also applies
> > > cleanly. Patch  89f4249
> > > (SUNRPC: Don't call connect() more than once on a TCP socket) is
> > > already present in 5.15.34 onwards.
> > >
> > > Sorry about the patch corruption, I will fix it.
> >
> > Sorry, but this did not work out at all, I get build errors when
> > attempting it for 5.10.y:
> >
> >   CC [M]  net/sunrpc/xprtsock.o
> > net/sunrpc/xprtsock.c: In function ‘xs_tcp_setup_socket’:
> > net/sunrpc/xprtsock.c:2276:13: error: too few arguments to function ‘test_and_clear_bit’
> >  2276 |         if (test_and_clear_bit(XPRT_SOCK_CONNECT_SENT),
> >       |             ^~~~~~~~~~~~~~~~~~
> > In file included from ./arch/x86/include/asm/bitops.h:391,
> >                  from ./include/linux/bitops.h:29,
> >                  from ./include/linux/kernel.h:12,
> >                  from ./include/asm-generic/bug.h:20,
> >                  from ./arch/x86/include/asm/bug.h:93,
> >                  from ./include/linux/bug.h:5,
> >                  from ./include/linux/mmdebug.h:5,
> >                  from ./include/linux/gfp.h:5,
> >                  from ./include/linux/slab.h:15,
> >                  from net/sunrpc/xprtsock.c:24:
> > ./include/asm-generic/bitops/instrumented-atomic.h:81:20: note: declared here
> >    81 | static inline bool test_and_clear_bit(long nr, volatile unsigned long *addr)
> >       |                    ^~~~~~~~~~~~~~~~~~
> > net/sunrpc/xprtsock.c:2276:55: warning: left-hand operand of comma expression has no effect [-Wunused-value]
> >  2276 |         if (test_and_clear_bit(XPRT_SOCK_CONNECT_SENT),
> >       |                                                       ^
> > net/sunrpc/xprtsock.c:2312:17: warning: this statement may fall through [-Wimplicit-fallthrough=]
> >  2312 |                 set_bit(XPRT_SOCK_CONNECT_SENT, &transport->sock_state);
> >       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > net/sunrpc/xprtsock.c:2313:9: note: here
> >  2313 |         case -EALREADY:
> >       |         ^~~~
> > make[2]: *** [scripts/Makefile.build:280: net/sunrpc/xprtsock.o] Error 1
> > make[1]: *** [scripts/Makefile.build:497: net/sunrpc] Error 2
> >
> >
> > And I am not quite sure what order you want me to apply things for 5.15.y.
> >
> > So please, send me a properly backported series of patches for this for 5.15.y
> > and 5.10.y and I will be glad to pick them up.  Right now I'm just confused as
> > this was obviously not tested at all :(
> >
> > thanks,
> >
> > greg k-h
> 
> I tested my original patch(which was corrupted by email client).
> When the patch is manually backported to fix white space, the patch
> was fixed wrongly :(
> I sent my original patch again for 5.10.y without any corruption.
> Sorry for the inconvenience caused.
> 
> For 5.15.y, this is the cherry-pick order:
> 
> 3be232f11a3cc9b0ef0795e39fa11bdb8e422a06(SUNRPC: Prevent immediate
> close+reconnect)

Already in 5.15.34.

> f00432063db1a0db484e85193eccc6845435b80e(SUNRPC: Ensure we flush any
> closed sockets before xs_xprt_free())

Now queued up, thanks.

greg k-h

Re: [PATCH 0/4] Request to cherry-pick f00432063db1 to 5.10

[Greg KH]

On Sat, May 14, 2022 at 03:47:38PM +0700, Bagas Sanjaya wrote:
> On Sat, May 14, 2022 at 05:34:49AM +0000, Meena Shanmugam wrote:
> > The commit f00432063db1a0db484e85193eccc6845435b80e upstream (SUNRPC:
> > Ensure we flush any closed sockets before xs_xprt_free()) fixes
> > CVE-2022-28893, hence good candidate for stable trees.
> > The above commit depends on 3be232f(SUNRPC: Prevent immediate
> > close+reconnect)  and  89f4249(SUNRPC: Don't call connect() more than
> > once on a TCP socket). Commit 3be232f depends on commit
> > e26d9972720e(SUNRPC: Clean up scheduling of autoclose).
> > 
> > Commits e26d9972720e, 3be232f, f00432063db1 apply cleanly on 5.10
> > kernel. commit 89f4249 didn't apply cleanly. This patch series includes
> > all the commits required for back porting f00432063db1.
> > 
> 
> Hi Meena,
> 
> I can't speaking about the code (as I'm not subject-expert here), but I
> would like to give you suggestions:
> 
>   - When sending backported patch series, always prefix the subject with
>     "[PATCH x.y]", where x.y is the stable version the backport is made
>     against.
>   - Abbreviated commit hash should be at least 12 (or my favorite, 14) characters long.
>   - Commit identifier should be in format "%h (\"%s\")".
>   - As always, DON'T DO top-posting, DO interleaved reply and reply
>     below the quoted original message.

Yes, that would have been better, but I figured it out from this series,
it wasn't that hard.

Now all queued up, thanks!

greg k-h

Re: [PATCH 3/4] SUNRPC: Don't call connect() more than once on a TCP socket

[Greg KH]

On Sat, May 14, 2022 at 05:34:52AM +0000, Meena Shanmugam wrote:
> From: Trond Myklebust <trond.myklebust@hammerspace.com>
> 
> commit 89f42494f92f448747bd8a7ab1ae8b5d5520577d upstream.
> 
> Avoid socket state races due to repeated calls to ->connect() using the
> same socket. If connect() returns 0 due to the connection having
> completed, but we are in fact in a closing state, then we may leave the
> XPRT_CONNECTING flag set on the transport.
> 
> Reported-by: Enrico Scholz <enrico.scholz@sigma-chemnitz.de>
> Fixes: 3be232f11a3c ("SUNRPC: Prevent immediate close+reconnect")
> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> [meenashanmugam: Backported to 5.10: Fixed merge conflict in xs_tcp_setup_socket]
> Signed-off-by: Meena Shanmugam <meenashanmugam@google.com>
> ---
>  include/linux/sunrpc/xprtsock.h |  1 +
>  net/sunrpc/xprtsock.c           | 21 +++++++++++----------
>  2 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/include/linux/sunrpc/xprtsock.h b/include/linux/sunrpc/xprtsock.h
> index 8c2a712cb242..689062afdd61 100644
> --- a/include/linux/sunrpc/xprtsock.h
> +++ b/include/linux/sunrpc/xprtsock.h
> @@ -89,5 +89,6 @@ struct sock_xprt {
>  #define XPRT_SOCK_WAKE_WRITE	(5)
>  #define XPRT_SOCK_WAKE_PENDING	(6)
>  #define XPRT_SOCK_WAKE_DISCONNECT	(7)
> +#define XPRT_SOCK_CONNECT_SENT	(8)
>  
>  #endif /* _LINUX_SUNRPC_XPRTSOCK_H */
> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> index 60c58eb9a456..33a81f9703b1 100644
> --- a/net/sunrpc/xprtsock.c
> +++ b/net/sunrpc/xprtsock.c
> @@ -2260,10 +2260,14 @@ static void xs_tcp_setup_socket(struct work_struct *work)
>  	struct rpc_xprt *xprt = &transport->xprt;
>  	int status = -EIO;
>  
> -	if (!sock) {
> -		sock = xs_create_sock(xprt, transport,
> -				xs_addr(xprt)->sa_family, SOCK_STREAM,
> -				IPPROTO_TCP, true);
> +	if (xprt_connected(xprt))
> +		goto out;
> +	if (test_and_clear_bit(XPRT_SOCK_CONNECT_SENT,
> +			       &transport->sock_state) ||
> +	    !sock) {
> +		xs_reset_transport(transport);
> +		sock = xs_create_sock(xprt, transport, xs_addr(xprt)->sa_family,
> +				      SOCK_STREAM, IPPROTO_TCP, true);
>  		if (IS_ERR(sock)) {
>  			status = PTR_ERR(sock);
>  			goto out;
> @@ -2294,6 +2298,7 @@ static void xs_tcp_setup_socket(struct work_struct *work)
>  		break;
>  	case 0:
>  	case -EINPROGRESS:
> +		set_bit(XPRT_SOCK_CONNECT_SENT, &transport->sock_state);
>  	case -EALREADY:
>  		xprt_unlock_connect(xprt, transport);
>  		return;
> @@ -2345,13 +2350,9 @@ static void xs_connect(struct rpc_xprt *xprt, struct rpc_task *task)
>  
>  	WARN_ON_ONCE(!xprt_lock_connect(xprt, task, transport));
>  
> -	if (transport->sock != NULL && !xprt_connecting(xprt)) {
> +	if (transport->sock != NULL) {
>  		dprintk("RPC:       xs_connect delayed xprt %p for %lu "
> -				"seconds\n",
> -				xprt, xprt->reestablish_timeout / HZ);
> -
> -		/* Start by resetting any existing state */
> -		xs_reset_transport(transport);
> +			"seconds\n", xprt, xprt->reestablish_timeout / HZ);
>  
>  		delay = xprt_reconnect_delay(xprt);
>  		xprt_reconnect_backoff(xprt, XS_TCP_INIT_REEST_TO);
> -- 
> 2.36.0.550.gb090851708-goog
> 

This commit added a build warning.  Always properly test your changes,
they can not add warnings.

I've fixed it up by hand now.

thanks,

greg k-h

Re: [PATCH 3/4] SUNRPC: Don't call connect() more than once on a TCP socket

[Meena Shanmugam]

On Mon, May 16, 2022 at 2:34 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> This commit added a build warning.  Always properly test your changes,
> they can not add warnings.
>
> I've fixed it up by hand now.
>
> thanks,
>
> greg k-h

Sorry about that. Thank you for fixing it.

Thanks,
Meena