[{"id":3677934,"web_url":"http://patchwork.ozlabs.org/comment/3677934/","msgid":"<CAEY6_V1+dzW3OD5zqXhsWyXwrDTrg5tAMGZ1AJ7_GAuRE+aevA@mail.gmail.com>","list_archive_url":null,"date":"2026-04-16T04:56:31","subject":"Re: [PATCH] smb/client: fix chan_max race and rollback in\n smb3_reconfigure","submitter":{"id":91740,"url":"http://patchwork.ozlabs.org/api/people/91740/","name":"RAJASI MANDAL","email":"rajasimandalos@gmail.com"},"content":"Hi DaeMyung,\n\nI reviewed your patch. The two core fixes (moving\nsmb3_sync_ses_chan_max after the SCALE_CHANNELS check, and rolling\nback chan_max on failure) are correct. However the error handling\npaths corrupt the mount context leading to false future remount\nfailures.\n\nIssue 1: return rc after STEAL_STRING nullifies UNC/source/username\n\nEarlier in smb3_reconfigure(), STEAL_STRING moves UNC, source, and\nusername from cifs_sb->ctx into ctx, setting cifs_sb->ctx->UNC = NULL.\nThe new if (rc < 0) return rc at the end of the multichannel block\nexits before smb3_fs_context_dup() can copy them back.\n\nHow to repro:\nmount -t cifs //server/share /mnt -o multichannel,max_channels=2,...\nmount -o remount,multichannel,max_channels=4 /mnt  # fails (server error)\ngrep /mnt /proc/mounts   # shows \"none\" instead of //server/share\nmount -o remount,multichannel,max_channels=2 /mnt  # fails: \"bad UNC (none)\"\n\n\nIssue 2: return -EINVAL on loser path has the same STEAL_STRING corruption\n\nWhen CIFS_SES_FLAG_SCALE_CHANNELS is already set (concurrent remount),\nthe return -EINVAL exits after STEAL_STRING but before\nsmb3_fs_context_dup(), same permanent NULL UNC corruption.\n\nHow to repro: Tested with a module parameter that forces the\nSCALE_CHANNELS flag check to take the loser path. After the failed\nremount, /proc/mounts shows \"none\" and all future remounts fail.\n\nSuggested fix:\n\n--- a/fs/smb/client/fs_context.c\n+++ b/fs/smb/client/fs_context.c\n@@ -NNNN,7 +NNNN,7 @@ static int smb3_reconfigure(struct fs_context *fc)\n     char *new_password = NULL, *new_password2 = NULL;\n     bool need_recon = false;\n- int rc;\n+ int rc, mchan_rc = 0;\n\n     if (ses->expired_pwd)\n@@ -NNNN,8 +NNNN,9 @@ static int smb3_reconfigure(struct fs_context *fc)\n         if (ses->flags & CIFS_SES_FLAG_SCALE_CHANNELS) {\n             spin_unlock(&ses->ses_lock);\n             mutex_unlock(&ses->session_mutex);\n- return -EINVAL;\n+ mchan_rc = -EINVAL;\n+ goto out;\n         }\n         ses->flags |= CIFS_SES_FLAG_SCALE_CHANNELS;\n@@ -NNNN,13 +NNNN,14 @@ static int smb3_reconfigure(struct fs_context *fc)\n         ses->flags &= ~CIFS_SES_FLAG_SCALE_CHANNELS;\n         spin_unlock(&ses->ses_lock);\n\n- if (rc < 0)\n- return rc;\n+ if (rc < 0)\n+ mchan_rc = rc;\n     } else {\n         mutex_unlock(&ses->session_mutex);\n     }\n\n+out:\n     STEAL_STRING(cifs_sb, ctx, domainname);\n     STEAL_STRING(cifs_sb, ctx, nodename);\n     STEAL_STRING(cifs_sb, ctx, iocharset);\n@@ -NNNN,9 +NNNN,22 @@ static int smb3_reconfigure(struct fs_context *fc)\n     ctx->rsize = rsize ? CIFS_ALIGN_RSIZE(fc, rsize) : cifs_sb->ctx->rsize;\n     ctx->wsize = wsize ? CIFS_ALIGN_WSIZE(fc, wsize) : cifs_sb->ctx->wsize;\n\n+ /*\n+ * On multichannel failure, restore the old multichannel settings in ctx\n+ * so smb3_fs_context_dup does not desync cifs_sb->ctx from ses->chan_max.\n+ */\n+ if (mchan_rc) {\n+ ctx->multichannel = cifs_sb->ctx->multichannel;\n+ ctx->max_channels = cifs_sb->ctx->max_channels;\n+ }\n+\n     smb3_cleanup_fs_context_contents(cifs_sb->ctx);\n     rc = smb3_fs_context_dup(cifs_sb->ctx, ctx);\n     smb3_update_mnt_flags(cifs_sb);\n+\n+ if (mchan_rc)\n+ return mchan_rc;\n+\n #ifdef CONFIG_CIFS_DFS_UPCALL\n     if (!rc)\n         rc = dfs_cache_remount_fs(cifs_sb);\n\nOn Wed, Apr 15, 2026 at 7:28 PM DaeMyung Kang <charsyam@gmail.com> wrote:\n>\n> smb3_reconfigure() has two bugs when handling multichannel remount:\n>\n> 1) smb3_sync_ses_chan_max() is called before acquiring\n>    CIFS_SES_FLAG_SCALE_CHANNELS. If a concurrent operation (e.g.\n>    smb2_reconnect) holds the flag, the current thread takes the\n>    loser path and returns -EINVAL, but ses->chan_max has already\n>    been updated to the new value. This leaves chan_max inconsistent\n>    with the actual channel state.\n>\n> 2) When smb3_update_ses_channels() fails, chan_max is not rolled\n>    back to its previous value and the error is not propagated to\n>    the caller. The mount command returns success even though the\n>    channel configuration was not applied, and repeated failures\n>    cause chan_max to drift further from reality.\n>\n> Fix both by moving smb3_sync_ses_chan_max() after the\n> CIFS_SES_FLAG_SCALE_CHANNELS acquisition so the loser path cannot\n> corrupt chan_max, and by restoring chan_max from old_chan_max on\n> update failure while still holding the scaling flag to prevent a\n> concurrent reconfigure from observing the intermediate state.\n> Propagate the error to the caller so userspace is informed.\n>\n> Note: smb3_reconfigure() is not fully transactional — credentials\n> are committed before the multichannel block, so any early return\n> (including the existing CIFS_SES_FLAG_SCALE_CHANNELS loser path)\n> can leave ses->password updated while cifs_sb->ctx is not yet\n> synced. Making the entire function atomic is a larger refactor\n> and is left for a separate patch.\n>\n> Tested with a QEMU VM (ksmbd + cifs) using module-parameter-based\n> fault injection:\n>  - Forced smb3_update_ses_channels() failure via module param and\n>    verified chan_max is preserved at the old value after remount\n>    failure (fault-injection test for rollback).\n>  - Pre-set CIFS_SES_FLAG_SCALE_CHANNELS before entering the\n>    scaling path and verified the loser path returns -EINVAL\n>    without corrupting chan_max (loser-path invariant test).\n>  - Repeated 19 forced-failure remounts with varying max_channels\n>    (range 2-8) and confirmed no chan_max drift (stress test).\n> All three scenarios pass with the patch and fail without it.\n>\n> Fixes: ef529f655a2c (\"cifs: client: allow changing multichannel mount options on remount\")\n> Signed-off-by: DaeMyung Kang <charsyam@gmail.com>\n> ---\n>  fs/smb/client/fs_context.c | 19 ++++++++++++++++---\n>  1 file changed, 16 insertions(+), 3 deletions(-)\n>\n> diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c\n> index b9544eb0381b..99e62c2dbf50 100644\n> --- a/fs/smb/client/fs_context.c\n> +++ b/fs/smb/client/fs_context.c\n> @@ -1085,6 +1085,7 @@ static int smb3_reconfigure(struct fs_context *fc)\n>         struct dentry *root = fc->root;\n>         struct cifs_sb_info *cifs_sb = CIFS_SB(root->d_sb);\n>         struct cifs_ses *ses = cifs_sb_master_tcon(cifs_sb)->ses;\n> +       unsigned int old_chan_max;\n>         unsigned int rsize = ctx->rsize, wsize = ctx->wsize;\n>         char *new_password = NULL, *new_password2 = NULL;\n>         bool need_recon = false;\n> @@ -1170,9 +1171,6 @@ static int smb3_reconfigure(struct fs_context *fc)\n>         if ((ctx->multichannel != cifs_sb->ctx->multichannel) ||\n>             (ctx->max_channels != cifs_sb->ctx->max_channels)) {\n>\n> -               /* Synchronize ses->chan_max with the new mount context */\n> -               smb3_sync_ses_chan_max(ses, ctx->max_channels);\n> -               /* Now update the session's channels to match the new configuration */\n>                 /* Prevent concurrent scaling operations */\n>                 spin_lock(&ses->ses_lock);\n>                 if (ses->flags & CIFS_SES_FLAG_SCALE_CHANNELS) {\n> @@ -1183,16 +1181,31 @@ static int smb3_reconfigure(struct fs_context *fc)\n>                 ses->flags |= CIFS_SES_FLAG_SCALE_CHANNELS;\n>                 spin_unlock(&ses->ses_lock);\n>\n> +               old_chan_max = ses->chan_max;\n> +               /* Synchronize ses->chan_max with the new mount context */\n> +               smb3_sync_ses_chan_max(ses, ctx->max_channels);\n> +\n>                 mutex_unlock(&ses->session_mutex);\n>\n>                 rc = smb3_update_ses_channels(ses, ses->server,\n>                                                false /* from_reconnect */,\n>                                                false /* disable_mchan */);\n>\n> +               /*\n> +                * On failure, restore chan_max while still holding\n> +                * CIFS_SES_FLAG_SCALE_CHANNELS so a concurrent reconfigure\n> +                * cannot observe or race with the rollback.\n> +                */\n> +               if (rc < 0)\n> +                       smb3_sync_ses_chan_max(ses, old_chan_max);\n> +\n>                 /* Clear scaling flag after operation */\n>                 spin_lock(&ses->ses_lock);\n>                 ses->flags &= ~CIFS_SES_FLAG_SCALE_CHANNELS;\n>                 spin_unlock(&ses->ses_lock);\n> +\n> +               if (rc < 0)\n> +                       return rc;\n>         } else {\n>                 mutex_unlock(&ses->session_mutex);\n>         }\n> --\n> 2.43.0\n>\n>","headers":{"Return-Path":"\n <linux-cifs+bounces-10847-incoming=patchwork.ozlabs.org@vger.kernel.org>","X-Original-To":["incoming@patchwork.ozlabs.org","linux-cifs@vger.kernel.org"],"Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256\n header.s=20251104 header.b=h+jOfjck;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2600:3c04:e001:36c::12fc:5321; helo=tor.lore.kernel.org;\n envelope-from=linux-cifs+bounces-10847-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n\tdkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com\n header.b=\"h+jOfjck\"","smtp.subspace.kernel.org;\n arc=pass smtp.client-ip=209.85.208.49","smtp.subspace.kernel.org;\n dmarc=pass (p=none dis=none) header.from=gmail.com","smtp.subspace.kernel.org;\n spf=pass smtp.mailfrom=gmail.com"],"Received":["from tor.lore.kernel.org (tor.lore.kernel.org\n [IPv6:2600:3c04:e001:36c::12fc:5321])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519 server-signature ECDSA (secp384r1) server-digest SHA384)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4fx5Mp5nldz1yCv\n\tfor <incoming@patchwork.ozlabs.org>; Thu, 16 Apr 2026 14:56:54 +1000 (AEST)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby tor.lore.kernel.org (Postfix) with ESMTP id 75D0230901C7\n\tfor <incoming@patchwork.ozlabs.org>; Thu, 16 Apr 2026 04:56:47 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id 2040433C19E;\n\tThu, 16 Apr 2026 04:56:46 +0000 (UTC)","from mail-ed1-f49.google.com (mail-ed1-f49.google.com\n [209.85.208.49])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id 8328F33C183\n\tfor <linux-cifs@vger.kernel.org>; Thu, 16 Apr 2026 04:56:44 +0000 (UTC)","by mail-ed1-f49.google.com with SMTP id\n 4fb4d7f45d1cf-6725295a113so2220459a12.3\n        for <linux-cifs@vger.kernel.org>;\n Wed, 15 Apr 2026 21:56:44 -0700 (PDT)"],"ARC-Seal":["i=2; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1776315406; cv=pass;\n b=dUwJbU/HCVIEN2ibWdsFGGiFQmZTZPPnHUoN3XEOr2HhMbI7ETK/iwW+5cGyoC+VD/pQ8d2LS5oYqGNAy5xdH85POgjLnQezsJi3x5FP2kRKoafKhmaKA7TFEC23eDAUEiAZnOHhGRQCFPRvI4250bBOrumk+m9+PTYE9VrYY9c=","i=1; a=rsa-sha256; t=1776315403; cv=none;\n        d=google.com; s=arc-20240605;\n        b=Mr0D4IgZTbFSeZH2zZuJd+yiitGLWZgiqRfCdZAZR3qTk+KwP8OMJh81ldW2ejUqjf\n         eHbvdPRrk0po8y/B+42Yd7tVyl8W4kz42NV9832FmEBD11XDZlVVCpqjCcctgYlmDv+M\n         ZW1yw7Ke1LcCCaKQUMph9kCLzPIq08mAOmFTBdGeeFqJtI2jZLmJSZiWYM0FPOxzcDPn\n         iIutTsZZl9nGSHUOsAKutLj8GNhWbzmexqpe06VxY1ag1aUK1qkVaXo9fxgJwtW3FQXk\n         +UKjQr71Y4CYwHel0hbCl2tXHZm+33i7pfRntssLC6LJKRKD6P5apIEiyYm7WMDCgL4S\n         YUkA=="],"ARC-Message-Signature":["i=2; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1776315406; c=relaxed/simple;\n\tbh=Tdo0wkU/+7H7lOFsHJdcy03PvNSQKaMKnAQISrpO0kM=;\n\th=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject:\n\t To:Cc:Content-Type;\n b=gy6uCE+tcQNv8BP0Zmo+sokgWjZ4W3lFOs7dxTj9CSVH8jjpgQyFDpiuwy8fosbxm7rZ0FJzAXp8ywQ0QUTdR+udzPiqqVW8EWDA9igJXZillaQEJa1+LQiLcrIBVP4lpTpcsYCfbA+8dMh3S0DLVDJhbxTdrXHDGFZfssVfT38=","i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com;\n s=arc-20240605;\n        h=content-transfer-encoding:cc:to:subject:message-id:date:from\n         :in-reply-to:references:mime-version:dkim-signature;\n        bh=V4wXnE/acyxugJcIAU1vSEb4rGdOI7xIqig6cNf2g2I=;\n        fh=HSAIwjWR6sx7No3gg0dPfGs8VwPoQfE+gYVmIkCFamc=;\n        b=Qfhu0saoTzQdHJ7qQcGEh6DlRiECh/YMMp0HfAt3WAW2WgTwPLOdXuHmNBWUMJMOlg\n         M6zRbj2mW4Fvyy7jO/2Xzw5o7+HhvqvSViZdvinpX4wRIYq/fi44vh7WlNiGSrJSG2FH\n         z7c7uBgYR1nJMBGZ1yHK0Unw5q9+NpIiV6zFqvyhUQR8XBQ/H1v0VsxSzYHq7pjssmFT\n         B3k8/lT4AiGhE+3anl6Zypta6DjMuL/o3Y3RPhk3XvUggXp0EtbSB9UpNVP6dAVbYKIv\n         trxSZuqK8D6emzquOoypgKUwql3SkHqrH85o0rOUmcwgENmA34JmeUI4I2Ma4bL9Ryx5\n         0AaA==;\n        darn=vger.kernel.org"],"ARC-Authentication-Results":["i=2; smtp.subspace.kernel.org;\n dmarc=pass (p=none dis=none) header.from=gmail.com;\n spf=pass smtp.mailfrom=gmail.com;\n dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com\n header.b=h+jOfjck; arc=pass smtp.client-ip=209.85.208.49","i=1; mx.google.com; arc=none"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n        d=gmail.com; s=20251104; t=1776315403; x=1776920203;\n darn=vger.kernel.org;\n        h=content-transfer-encoding:cc:to:subject:message-id:date:from\n         :in-reply-to:references:mime-version:from:to:cc:subject:date\n         :message-id:reply-to;\n        bh=V4wXnE/acyxugJcIAU1vSEb4rGdOI7xIqig6cNf2g2I=;\n        b=h+jOfjck0cZneRphifffDP7UMCI+G/QhRdtJz2xOCro/4pYegXry2NLbfnmn2qMMEr\n         81MKV7RKYxFc06mYdvrAVSbMkBCKOfn66r4T57c/r5XasQ0faoif650JKM8g2hlFG0jj\n         5hWAAeQnZJIEPOmHBHlC4UfKEdji3OEfTO8Y2I6FtD92S8uLhkNul3IH/iF3G8GZf/sa\n         wvrILsmLjYlU3n0WJUWettGz1UJtvxmkRfXqHxQCAOdE0T2b2UWbcmS3tUCjzNQ7tGmL\n         85VFsZbiDG7/Of9SNhvdK7mZHWhMppYPEIZ8bMCVFgHWs/Ulm5RpdghGxse5DZNhytJ4\n         5gjQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n        d=1e100.net; s=20251104; t=1776315403; x=1776920203;\n        h=content-transfer-encoding:cc:to:subject:message-id:date:from\n         :in-reply-to:references:mime-version:x-gm-gg:x-gm-message-state:from\n         :to:cc:subject:date:message-id:reply-to;\n        bh=V4wXnE/acyxugJcIAU1vSEb4rGdOI7xIqig6cNf2g2I=;\n        b=aGqHHCoT0QOcLZo9hk0Qt6ArLmcGH1aTcS7bOqmXwCq0JmI2vMbjBvtI+ScqaaSvKg\n         Is+qW964K1uduaIAvNEEO1EvngVg2oaRlbZ2QcEZDnVW464T0SonHuzJMQ2P1VrstFWw\n         HeOFtL1Xo/YlXcZAEP4EsAal5GGS0GmDoXBWP0FxXHmZeqdKnXh7t2mxkPHawEnZrnuN\n         K4USnK6ObXPYFS1E8sUwq5X/eV06MZ+GfG2l2CRPF2iYO1fhDpyqBMM/Up3iuVhMfFhd\n         GTm7/8UGP9ZYUnGh9ZspXBAiIg2KWofWOhBD+QzODfgHBfXXsI+BaQjoCq20Irz0uZpR\n         LlIA==","X-Forwarded-Encrypted":"i=1;\n AFNElJ8KXVTZ0kvVAa+tiV3B/fH0Jy0AxICKgBQqyy0v7WxCJQTNAB7SemaWWF6VsxTsKalsY4VGx9l+RnMj@vger.kernel.org","X-Gm-Message-State":"AOJu0Yy8mPVRCpwb/tZOYnb28CSWoVxqufjE3w8XrJKyggOwfRW9F9hV\n\tdN/ozRRsQwtVxtyhlyZ5A6gJy77CiZSruKu4o4mu+8Mxonu5KU0ZY2URjh2021O618AJ5vxIU9K\n\t8GsRFTSh1zsDm4C0k4GltgwDx+HIXnyc=","X-Gm-Gg":"AeBDiet+cQjT9H/IuOJdHkd3mQ0YCd/y69yvtMzLgi1QFq+kB8SmelzPka/pOlD2yxD\n\tsOHVSw/+tL8ny045kFqMofYM7qoHTASyFzAbOZ4mBw3Dtx96KlhHaHI2wequOO0sAoHmaVJalRd\n\tLnHkpeQ+t/w7vkpOa2rz2aa/wvcZPzOycIKISe9QGcDht9fCLfHo9G6XX8oxbBYWkwDuPZthRgm\n\tluENnTqkrXwg0KjR2H/NoMbbjWPI03phpOrD4DYL5nQr8IHyvcUZL5TAUtCao5wne26YqVdgnnU\n\tv93E+tL8docCOO6S","X-Received":"by 2002:a05:6402:3885:b0:671:a228:a403 with SMTP id\n 4fb4d7f45d1cf-671a228a846mr6716558a12.17.1776315402579; Wed, 15 Apr 2026\n 21:56:42 -0700 (PDT)","Precedence":"bulk","X-Mailing-List":"linux-cifs@vger.kernel.org","List-Id":"<linux-cifs.vger.kernel.org>","List-Subscribe":"<mailto:linux-cifs+subscribe@vger.kernel.org>","List-Unsubscribe":"<mailto:linux-cifs+unsubscribe@vger.kernel.org>","MIME-Version":"1.0","References":"<20260416022811.2692096-1-charsyam@gmail.com>","In-Reply-To":"<20260416022811.2692096-1-charsyam@gmail.com>","From":"RAJASI MANDAL <rajasimandalos@gmail.com>","Date":"Wed, 15 Apr 2026 21:56:31 -0700","X-Gm-Features":"AQROBzA3-nKaWio2xT-lfLsB6EBPDfbK-2TtLLZRRHZL1VMPWtw3tmYPMWJSTNM","Message-ID":"\n <CAEY6_V1+dzW3OD5zqXhsWyXwrDTrg5tAMGZ1AJ7_GAuRE+aevA@mail.gmail.com>","Subject":"Re: [PATCH] smb/client: fix chan_max race and rollback in\n smb3_reconfigure","To":"DaeMyung Kang <charsyam@gmail.com>","Cc":"sfrench@samba.org, pc@manguebit.org, ronniesahlberg@gmail.com,\n\tsprasad@microsoft.com, tom@talpey.com, bharathsm@microsoft.com,\n\trajasimandal@microsoft.com, linux-cifs@vger.kernel.org,\n\tsamba-technical@lists.samba.org, linux-kernel@vger.kernel.org","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable"}}]