Message-ID: <20250331133315.117666-1-wangzhaolong1@huawei.com>
Date: Mon, 31 Mar 2025 21:33:12 +0800
From: Wang Zhaolong <wangzhaolong1@...wei.com>
To: <sfrench@...ba.org>, <linkinjeon@...nel.org>
CC: <linux-cifs@...r.kernel.org>, <samba-technical@...ts.samba.org>,
	<linux-kernel@...r.kernel.org>, <chengzhihao1@...wei.com>,
	<wangzhaolong1@...wei.com>, <yi.zhang@...wei.com>, <yangerkun@...wei.com>
Subject: [PATCH next 0/3] smb: client: Improve IO parameter resilience after reconnection

## Background

When a SMB client connection experiences a network disruption and reconnects
to the server, several critical parameters are renegotiated with the server.
However, in current code, the negotiated IO sizes (rsize/wsize) for existing
mount points are not updated to reflect the server's potentially changed
capabilities. This can lead to suboptimal performance or even failures.

## Problem Discovery

I encountered a critical issue in the Linux 5.10 kernel where CIFS would crash
due to rsize becoming zero, which led to a null pointer dereference in the
file read path. This could happen when either:

1. The server returned a MaxReadSize of 0 in a negotiation response, or
2. During reconnection when server->max_read might be reset to 0

While investigating, I noted that the MS-SMB2 specification (section 3.2.5.2
"Receiving an SMB2 NEGOTIATE Response") states:

  "The client SHOULD<175> disconnect the connection if the size, in bytes,
   received in MaxTransactSize, MaxReadSize, or MaxWriteSize is less than 65536."

However, current CIFS client implementations do not strictly enforce this
recommended minimum and may even operate with smaller values. Nevertheless,
it is evident that these values should not be set to zero.

While subsequent kernel versions have addressed the null pointer dereference
itself, the underlying issue of potentially zero or outdated IO sizes after
reconnection remains. After reviewing the code, I discovered that the
reconnection flow includes negotiation with the server but does not update
the IO parameters for existing mounted filesystems.

## Solution Approach

This patch series implements a comprehensive solution by:

1. Creating a bidirectional relationship between tree connections and
   superblocks to locate all mounts using a specific connection
2. Preserving original user-specified IO parameters separately from
   negotiated values
3. Adding safeguards against zero IO sizes
4. Implementing automatic renegotiation of IO parameters after reconnection

## Patch Breakdown

- Patch 1: Adds bidirectional mapping between tcon and superblocks, enabling
  efficient discovery of all filesystems using a specific tree connection
- Patch 2: Stores original user-specified IO parameters and adds protection
  against zero IO sizes by enforcing the MS-SMB2 specification minimum
- Patch 3: Implements automatic renegotiation of IO parameters after
  successful reconnection

These changes make the CIFS client more resilient to network disruptions
by ensuring all mounted filesystems maintain optimal and safe IO parameters
that reflect current server capabilities.

Wang Zhaolong (3):
  smb:client: smb: client: Add reverse mapping from tcon to superblocks
  smb: client: Store original IO parameters and prevent zero IO sizes
  smb: client: Update IO sizes after reconnection

 fs/smb/client/cifs_fs_sb.h |  1 +
 fs/smb/client/cifsglob.h   |  3 ++-
 fs/smb/client/connect.c    | 15 +++++++++++++++
 fs/smb/client/fs_context.c |  2 ++
 fs/smb/client/fs_context.h |  3 +++
 fs/smb/client/misc.c       |  2 ++
 fs/smb/client/smb1ops.c    |  6 +++---
 fs/smb/client/smb2ops.c    | 27 +++++++++++++++++++--------
 fs/smb/client/smb2pdu.c    | 24 ++++++++++++++++++++++--
 fs/smb/common/smb2pdu.h    |  3 +++
 10 files changed, 72 insertions(+), 14 deletions(-)

-- 
2.39.2

Powered by blists - more mailing lists