Re: [PATCH v3] smb/client: fix state corruption in smb3_reconfigure multichannel path

[CharSyam]

Hi Rajasi,

Thanks a lot for the careful review — you're right, that read of
ses->chan_max was unprotected and races with chan_lock writers in
cifs_try_adding_channels() / cifs_chan_skip_or_disable().

v4 fixes this by folding the read into smb3_sync_ses_chan_max()
itself: the helper now returns the previous chan_max value, so the
read+write happens atomically under chan_lock. The caller no longer
reads ses->chan_max outside the lock. I also switched the helper's
parameter/return type to size_t to match struct cifs_ses::chan_max.

While I was at it, I also restored ctx->multichannel_specified and
ctx->max_channels_specified together
with multichannel / max_channels on the rollback path,
so the user-specified flags don't end up out of sync with
the restored values once smb3_fs_context_dup() copies the ctx back.
v4 just went out — would appreciate another look when you get a chance.

Thanks,
DaeMyung


2026년 4월 29일 (수) 오후 12:17, RAJASI MANDAL <rajasimandalos@xxxxxxxxx>님이 작성:
>
> Hi DaeMyung,
>
> Thanks for the v3. One minor suggestion
>
> > + old_chan_max = ses->chan_max;
> > + /* Synchronize ses->chan_max with the new mount context */
> > + smb3_sync_ses_chan_max(ses, ctx->max_channels);
>
> This reads ses->chan_max without holding chan_lock.
> smb3_sync_ses_chan_max() itself takes chan_lock for the write, and
> cifs_try_adding_channels() / cifs_chan_skip_or_disable() also access
> chan_max under chan_lock. So there is a potential data race between
> this unlocked read and a concurrent reconnect path writing chan_max.
>
> Other than that, this  patch looks good to me.
>
> Thanks,
> Rajasi