On incoming TCP reset, subflow closing could happen before error
propagation. That in turn could cause the socket error being ignored,
and a missing socket state transition, as reported by Daire-Byrne.

Address the issues explicitly checking for subflow socket error at
close time. To avoid code duplication, factor-out of __mptcp_error_report()
a new helper implementing the relevant bits.

Reported-by: Daire-Byrne
Fixes: 15cc10453398 ("mptcp: deliver ssk errors to msk")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
Note: the fixes tag need a real email address...
---
 net/mptcp/protocol.c | 63 ++++++++++++++++++++++++--------------------
 1 file changed, 34 insertions(+), 29 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 12d3b575ceba..7ab5391b2778 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -764,40 +764,44 @@ static bool __mptcp_ofo_queue(struct mptcp_sock *msk)
 	return moved;
 }
 
-void __mptcp_error_report(struct sock *sk)
+bool __mptcp_subflow_error_report(struct sock *sk, struct sock *ssk)
 {
-	struct mptcp_subflow_context *subflow;
-	struct mptcp_sock *msk = mptcp_sk(sk);
+	int err = sock_error(ssk);
+	int ssk_state;
 
-	mptcp_for_each_subflow(msk, subflow) {
-		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
-		int err = sock_error(ssk);
-		int ssk_state;
+	if (!err)
+		return false;
 
-		if (!err)
-			continue;
+	/* only propagate errors on fallen-back sockets or
+	 * on MPC connect
+	 */
+	if (sk->sk_state != TCP_SYN_SENT && !__mptcp_check_fallback(mptcp_sk(sk)))
+		return false;
 
-		/* only propagate errors on fallen-back sockets or
-		 * on MPC connect
-		 */
-		if (sk->sk_state != TCP_SYN_SENT && !__mptcp_check_fallback(msk))
-			continue;
+	/* We need to propagate only transition to CLOSE state.
+	 * Orphaned socket will see such state change via
+	 * subflow_sched_work_if_closed() and that path will properly
+	 * destroy the msk as needed.
+	 */
+	ssk_state = inet_sk_state_load(ssk);
+	if (ssk_state == TCP_CLOSE && !sock_flag(sk, SOCK_DEAD))
+		inet_sk_state_store(sk, ssk_state);
+	WRITE_ONCE(sk->sk_err, -err);
 
-		/* We need to propagate only transition to CLOSE state.
-		 * Orphaned socket will see such state change via
-		 * subflow_sched_work_if_closed() and that path will properly
-		 * destroy the msk as needed.
-		 */
-		ssk_state = inet_sk_state_load(ssk);
-		if (ssk_state == TCP_CLOSE && !sock_flag(sk, SOCK_DEAD))
-			inet_sk_state_store(sk, ssk_state);
-		WRITE_ONCE(sk->sk_err, -err);
-
-		/* This barrier is coupled with smp_rmb() in mptcp_poll() */
-		smp_wmb();
-		sk_error_report(sk);
-		break;
-	}
+	/* This barrier is coupled with smp_rmb() in mptcp_poll() */
+	smp_wmb();
+	sk_error_report(sk);
+	return true;
+}
+
+void __mptcp_error_report(struct sock *sk)
+{
+	struct mptcp_subflow_context *subflow;
+	struct mptcp_sock *msk = mptcp_sk(sk);
+
+	mptcp_for_each_subflow(msk, subflow)
+		if (__mptcp_subflow_error_report(sk, mptcp_subflow_tcp_sock(subflow)))
+			break;
 }
 
 /* In most cases we will be able to lock the mptcp socket.  If its already
@@ -2422,6 +2426,7 @@ static void __mptcp_close_ssk(struct sock *sk, struct sock *ssk,
 	}
 
 out_release:
+	__mptcp_subflow_error_report(sk, ssk);
 	release_sock(ssk);
 
 	sock_put(ssk);
-- 
2.41.0

Hi Paolo,

kernel test robot noticed the following build warnings:

[auto build test WARNING on next-20230821]
[also build test WARNING on v6.5-rc7]
[cannot apply to linus/master v6.5-rc7 v6.5-rc6 v6.5-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Paolo-Abeni/mptcp-move-__mptcp_error_report-in-protocol-c/20230822-154511
base:   next-20230821
patch link:    https://lore.kernel.org/r/065ed2c95b4ceef822232e4d0641427af6e27092.1692690157.git.pabeni%40redhat.com
patch subject: [PATCH mptcp-net 2/2] mptcp: process pending subflow error on close
config: sparc64-randconfig-r033-20230823 (https://download.01.org/0day-ci/archive/20230823/202308230746.DvdUqzMi-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20230823/202308230746.DvdUqzMi-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202308230746.DvdUqzMi-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> net/mptcp/protocol.c:767:6: warning: no previous prototype for '__mptcp_subflow_error_report' [-Wmissing-prototypes]
     767 | bool __mptcp_subflow_error_report(struct sock *sk, struct sock *ssk)
         |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~


vim +/__mptcp_subflow_error_report +767 net/mptcp/protocol.c

   766	
 > 767	bool __mptcp_subflow_error_report(struct sock *sk, struct sock *ssk)
   768	{
   769		int err = sock_error(ssk);
   770		int ssk_state;
   771	
   772		if (!err)
   773			return false;
   774	
   775		/* only propagate errors on fallen-back sockets or
   776		 * on MPC connect
   777		 */
   778		if (sk->sk_state != TCP_SYN_SENT && !__mptcp_check_fallback(mptcp_sk(sk)))
   779			return false;
   780	
   781		/* We need to propagate only transition to CLOSE state.
   782		 * Orphaned socket will see such state change via
   783		 * subflow_sched_work_if_closed() and that path will properly
   784		 * destroy the msk as needed.
   785		 */
   786		ssk_state = inet_sk_state_load(ssk);
   787		if (ssk_state == TCP_CLOSE && !sock_flag(sk, SOCK_DEAD))
   788			inet_sk_state_store(sk, ssk_state);
   789		WRITE_ONCE(sk->sk_err, -err);
   790	
   791		/* This barrier is coupled with smp_rmb() in mptcp_poll() */
   792		smp_wmb();
   793		sk_error_report(sk);
   794		return true;
   795	}
   796	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki