smb: client: fix use-after-free of iface in cifs_try_adding_channels()

cifs_try_adding_channels() iterates ses->iface_list with
list_for_each_entry_safe_from(), which captures the next entry
(niface) under iface_lock.  The loop body then drops iface_lock for
the whole duration of cifs_ses_add_channel().

A concurrent interface refresh (SMB3_request_interfaces() ->
parse_server_interfaces()) marks all ifaces inactive and removes and
frees any that are not re-advertised via list_del() + kref_put(),
where release_iface() is a bare kfree().  Since niface typically has
no channel holding a reference, the list reference is its last and it
can be freed inside the unlocked window.  On continue, the iterator
advance step then dereferences niface->iface_head.next, and the loop
body reads iface->rdma_capable/is_active, both on freed memory.

Fix this by never keeping an unreferenced list pointer across the
unlocked window.  Each channel attempt now re-scans the list from the
head under iface_lock, takes a kref on the selected candidate, and
passes only that referenced candidate to cifs_ses_add_channel().
weight_fulfilled still tracks selection progress, so restarting the
scan preserves the original weighted distribution and the
weight_fulfilled-before-kref_put ordering on the failure path.

Add a per-pass attempts cap so a flapping interface refresh cannot
keep the inner loop spinning within a single tries increment.

Fixes: aa45dadd34 ("cifs: change iface_list from array to sorted linked list")
Cc: stable@vger.kernel.org
Assisted-by: Qoder:Qwen3.8-Max
Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
Acked-by: Shyam Prasad N <sprasad@microsoft.com>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
This commit is contained in:
Joseph Qi 2026-09-01 09:04:13 +08:00 committed by Paulo Alcantara
parent e1253a82bb
commit d034e836ee

View File

@ -149,9 +149,9 @@ int cifs_try_adding_channels(struct cifs_ses *ses)
int old_chan_count, new_chan_count;
int left;
int rc = 0;
int tries = 0;
int tries = 0, attempts;
size_t iface_weight = 0, iface_min_speed = 0;
struct cifs_server_iface *iface = NULL, *niface = NULL;
struct cifs_server_iface *iface = NULL, *candidate = NULL;
struct cifs_server_iface *last_iface = NULL;
spin_lock(&ses->chan_lock);
@ -197,67 +197,89 @@ int cifs_try_adding_channels(struct cifs_ses *ses)
break;
}
if (!iface)
iface = list_first_entry(&ses->iface_list, struct cifs_server_iface,
iface_head);
last_iface = list_last_entry(&ses->iface_list, struct cifs_server_iface,
iface_head);
iface_min_speed = last_iface->speed;
spin_unlock(&ses->iface_lock);
list_for_each_entry_safe_from(iface, niface, &ses->iface_list,
iface_head) {
/* do not mix rdma and non-rdma interfaces */
if (iface->rdma_capable != ses->server->rdma)
continue;
attempts = 0;
while (left > 0) {
spin_lock(&ses->iface_lock);
/* skip ifaces that are unusable */
if (!iface->is_active ||
(is_ses_using_iface(ses, iface) &&
!iface->rss_capable))
continue;
/*
* iface_lock must be dropped while opening a channel,
* and a concurrent interface refresh may remove and
* free entries during that window, so no list entry
* may be kept across it without a reference. Scan
* the list from the beginning each time and only pass
* a referenced candidate to cifs_ses_add_channel();
* weight_fulfilled tracks the progress so that no
* iface is selected beyond its weight.
*/
candidate = NULL;
list_for_each_entry(iface, &ses->iface_list, iface_head) {
/* do not mix rdma and non-rdma interfaces */
if (iface->rdma_capable != ses->server->rdma)
continue;
/* check if we already allocated enough channels */
iface_weight = iface->speed / iface_min_speed;
/* skip ifaces that are unusable */
if (!iface->is_active ||
(is_ses_using_iface(ses, iface) &&
!iface->rss_capable))
continue;
if (iface->weight_fulfilled >= iface_weight)
continue;
/* check if we already allocated enough channels */
iface_weight = iface->speed / iface_min_speed;
/* take ref before unlock */
kref_get(&iface->refcount);
if (iface->weight_fulfilled >= iface_weight)
continue;
/* take ref before unlock */
kref_get(&iface->refcount);
candidate = iface;
break;
}
if (!candidate) {
/* no usable iface. reset weight_fulfilled and start over */
list_for_each_entry(iface, &ses->iface_list, iface_head)
iface->weight_fulfilled = 0;
spin_unlock(&ses->iface_lock);
break;
}
attempts++;
if (attempts > 3 * ses->chan_max) {
kref_put(&candidate->refcount, release_iface);
spin_unlock(&ses->iface_lock);
break;
}
spin_unlock(&ses->iface_lock);
rc = cifs_ses_add_channel(ses, iface);
rc = cifs_ses_add_channel(ses, candidate);
spin_lock(&ses->iface_lock);
if (rc) {
cifs_dbg(VFS, "failed to open extra channel on iface:%pIS rc=%d\n",
&iface->sockaddr,
&candidate->sockaddr,
rc);
/* failure to add chan should increase weight */
iface->weight_fulfilled++;
kref_put(&iface->refcount, release_iface);
candidate->weight_fulfilled++;
kref_put(&candidate->refcount, release_iface);
spin_unlock(&ses->iface_lock);
continue;
}
iface->num_channels++;
iface->weight_fulfilled++;
candidate->num_channels++;
candidate->weight_fulfilled++;
cifs_info("successfully opened new channel on iface:%pIS\n",
&iface->sockaddr);
&candidate->sockaddr);
spin_unlock(&ses->iface_lock);
left--;
new_chan_count++;
break;
}
/* reached end of list. reset weight_fulfilled and start over */
if (list_entry_is_head(iface, &ses->iface_list, iface_head)) {
list_for_each_entry(iface, &ses->iface_list, iface_head)
iface->weight_fulfilled = 0;
spin_unlock(&ses->iface_lock);
iface = NULL;
continue;
}
spin_unlock(&ses->iface_lock);
left--;
new_chan_count++;
}
return new_chan_count - old_chan_count;