[PATCH net] afs: Clear stale peer app data after address list changes

Qi Zhang <[email protected]>
Newsgroups gmane.linux.kernel,gmane.linux.kernel.stable
Message-ID <[email protected]>
From: Chengfeng Ye <[email protected]>

afs_fs_probe_fileserver() fetches the current endpoint state under
server->fs_lock, but leaves old_alist as NULL.  Consequently,
afs_set_peer_appdata() treats every address list replacement as initial
setup and only binds the new peers; it never unbinds peers removed from
the old list.

An address refresh can therefore proceed as follows.  CPU 0 replaces
server S's list and drops Pold without clearing Pold->app_data.  The
server destroyer then clears only S's current peers and lets S reach its
RCU callback.  After the callback frees S, CPU 1 handles a callback
through an RxRPC connection that still pins Pold, reads Pold->app_data,
and calls afs_use_server() on the freed object.

KASAN reported:

  BUG: KASAN: slab-use-after-free in afs_find_server+0x3c/0xa0
  Read of size 4 at addr ffff8881013e1af0 by task krxrpcio/7001/74
  Call Trace:
   afs_find_server+0x3c/0xa0
   afs_rx_new_call+0x15c/0x390
   rxrpc_new_incoming_call+0x97c/0x1730
   rxrpc_input_packet.constprop.0+0xd03/0xec0
   rxrpc_io_thread+0x967/0x1640
  Allocated by task 93:
   afs_lookup_server+0x1a7/0x14c0
   afs_alloc_server_list+0x43f/0xb60
   afs_create_volume+0x923/0x1490
   afs_get_tree+0x1c6/0x10a0
  Freed by task 0:
   kfree+0x131/0x3c0
   rcu_core+0x50a/0x1850
  Last potentially related work creation:
   __call_rcu_common.constprop.0+0x71/0xa10
   afs_put_server+0x213/0x2b0

Preserve old->addresses for the peer app-data update so that removed
peers are cleared before the endpoint state is replaced.  Also advance
both cursors when the old and new lists share a peer; activating the
old/new comparison without this would otherwise loop forever on the
shared entry.

Fixes: 40e8b52fe8c8 ("afs: Use the per-peer app data provided by rxrpc")
Cc: [email protected]
Signed-off-by: Chengfeng Ye <[email protected]>
Signed-off-by: Qi Zhang <[email protected]>
---
 fs/afs/addr_list.c | 5 ++++-
 fs/afs/fs_probe.c  | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/afs/addr_list.c b/fs/afs/addr_list.c
index 63bf096b721a..73195d76b481 100644
--- a/fs/afs/addr_list.c
+++ b/fs/afs/addr_list.c
@@ -394,8 +394,11 @@ void afs_set_peer_appdata(struct afs_server *server,
 		struct rxrpc_peer *pn = new_alist->addrs[n].peer;
 		struct rxrpc_peer *po = old_alist->addrs[o].peer;
 
-		if (pn == po)
+		if (pn == po) {
+			n++;
+			o++;
 			continue;
+		}
 		if (pn < po) {
 			rxrpc_kernel_set_peer_data(pn, data);
 			n++;
diff --git a/fs/afs/fs_probe.c b/fs/afs/fs_probe.c
index a91ad1938d07..8c62334dbfe7 100644
--- a/fs/afs/fs_probe.c
+++ b/fs/afs/fs_probe.c
@@ -258,6 +258,7 @@ int afs_fs_probe_fileserver(struct afs_net *net, struct afs_server *server,
 					lockdep_is_held(&server->fs_lock));
 	if (old) {
 		estate->responsive_set = old->responsive_set;
+		old_alist = old->addresses;
 		if (!new_alist)
 			new_alist = old->addresses;
 	}
-- 
2.43.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.