afs: Miscellaneous fixes

Hi Christian,

Can you pick up these miscellaneous AFS fixes, please? Three were found by Sashiko[1][2] and one was a posted patch.

(1) Fix missing kunmap in afs_dir_search_bucket().

(2) Fix double-unmap of directory block.

(3) Fix an incorrect free in afs_lookup_server().

(4) Clear stale peer app data after address list changes.

The patches can be found here:

https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=afs-fixes

Thanks,
David

[1] https://sashiko.dev/#/patchset/20260716103030.3065561-1-dhowells%40redhat.com
[2] https://sashiko.dev/#/patchset/20260729160108.2031453-1-dhowells%40redhat.com

Chengfeng Ye (1):
afs: Clear stale peer app data after address list changes

David Howells (3):
afs: Fix missing kunmap in afs_dir_search_bucket()
afs: Fix double-unmap of directory block
afs: Fix incorrect free in candidate cleanup in afs_lookup_server()

 fs/afs/addr_list.c  |  5 ++++-
 fs/afs/dir_edit.c   |  9 ++-------
 fs/afs/dir_search.c | 11 ++---------
 fs/afs/fs_probe.c   |  1 +
 fs/afs/internal.h   |  8 ++++++++
 fs/afs/server.c     |  1 -
 6 files changed, 17 insertions(+), 18 deletions(-)

Fix afs_dir_search_bucket() to kunmap the block it's using in the "bad:" path.

---
 fs/afs/dir_search.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

index 104411c0692f..4977ad81fa82 100644
@@ -173,12 +173,11 @@ int afs_dir_search_bucket(struct afs_dir_iter *iter, const struct qstr *name,
 
 	ret = -ENOENT;
 found:
+bad:
 	if (iter->block) {
 		kunmap_local(iter->block);
 		iter->block = NULL;
 	}
-
-bad:
 	if (ret == -ESTALE)
 		afs_invalidate_dir(iter->dvnode, afs_dir_invalid_iter_stale);
 	_leave(" = %d", ret);

Fix afs_edit_dir_remove() to use a cleanup function to unmap the block pointed to by afs_dir_iter::block if it's left pointing to something rather than manually kunmapping the blocks. Manually kunmapping without clearing iter.blocks can result in a double-kunmap if afs_dir_find_block() is called twice in a row (which would be the case if the block being modified is not first in the hash chain).

---
 fs/afs/dir_edit.c   |  9 ++-------
 fs/afs/dir_search.c | 10 ++--------
 fs/afs/internal.h   |  8 ++++++++
 3 files changed, 12 insertions(+), 15 deletions(-)

index 3ead36a07048..c31303059444 100644
@@ -442,7 +442,7 @@ void afs_edit_dir_remove(struct afs_vnode *vnode,
 	/* Check and clear the entry. */
 	de = &block->dirents[slot];
 	if (de->u.valid != 1)
-		goto error_unmap;
+		goto error;
 
 	trace_afs_edit_dir(vnode, why, afs_edit_dir_delete, b, slot,
 			   ntohl(de->u.vnode), ntohl(de->u.unique),

@@ -458,7 +458,6 @@ void afs_edit_dir_remove(struct afs_vnode *vnode,
 	/* Clear the constituent entries. */
 	next = de->u.hash_next;
 	memset(de, 0, sizeof(*de) * iter.nr_slots);
-	kunmap_local(block);
 
 	/* Adjust the hash chain: if iter->prev_entry is 0, the hashtable head
 	 * index is previous; otherwise it's slot number of the previous entry.

@@ -485,7 +484,6 @@ void afs_edit_dir_remove(struct afs_vnode *vnode,
 		pde = &pblock->dirents[ps];
 		prev_next = pde->u.hash_next;
 		if (prev_next != htons(entry)) {
-			kunmap_local(pblock);
 			pr_warn("%llx:%llx:%x: not prev in chain b=%x p=%x,%x e=%x %*s",
 				vnode->fid.vid, vnode->fid.vnode, vnode->fid.unique,
 				iter.bucket, iter.prev_entry, prev_next, entry,

@@ -493,7 +491,6 @@ void afs_edit_dir_remove(struct afs_vnode *vnode,
 			goto error;
 		}
 		pde->u.hash_next = next;
-		kunmap_local(pblock);
 	}
 
 	netfs_single_mark_inode_dirty(&vnode->netfs.inode);

@@ -503,18 +500,16 @@ void afs_edit_dir_remove(struct afs_vnode *vnode,
 	_debug("Remove %s from %u[%u]", name->name, b, slot);
 
 out_unmap:
+	afs_dir_end_iter(&iter);
 	kunmap_local(meta);
 	_leave("");
 	return;
 
 already_invalidated:
-	kunmap_local(block);
 	trace_afs_edit_dir(vnode, why, afs_edit_dir_delete_inval,
 			   0, 0, 0, 0, name->name);
 	goto out_unmap;
 
-error_unmap:
-	kunmap_local(block);
 error:
 	trace_afs_edit_dir(vnode, why, afs_edit_dir_delete_error,
 			   0, 0, 0, 0, name->name);

index 4977ad81fa82..11ebdfffcb1d 100644
@@ -75,10 +75,7 @@ union afs_xdr_dir_block *afs_dir_find_block(struct afs_dir_iter *iter, size_t bl
 
 	_enter("%zx,%d", block, slot);
 
-	if (iter->block) {
-		kunmap_local(iter->block);
-		iter->block = NULL;
-	}
+	afs_dir_end_iter(iter);
 
 	if (dvnode->directory_size < blend)
 		goto fail;

@@ -174,10 +171,7 @@ int afs_dir_search_bucket(struct afs_dir_iter *iter, const struct qstr *name,
 	ret = -ENOENT;
 found:
 bad:
-	if (iter->block) {
-		kunmap_local(iter->block);
-		iter->block = NULL;
-	}
+	afs_dir_end_iter(iter);
 	if (ret == -ESTALE)
 		afs_invalidate_dir(iter->dvnode, afs_dir_invalid_iter_stale);
 	_leave(" = %d", ret);

index 290873bac89b..330654ed16ec 100644
@@ -1133,6 +1133,14 @@ int afs_dir_search_bucket(struct afs_dir_iter *iter, const struct qstr *name,
 int afs_dir_search(struct afs_vnode *dvnode, const struct qstr *name,
 		   struct afs_fid *_fid, afs_dataversion_t *_dir_version);
 
+static inline void afs_dir_end_iter(struct afs_dir_iter *iter)
+{
+	if (iter->block) {
+		kunmap_local(iter->block);
+		iter->block = NULL;
+	}
+}
+
 /*
  * dir_silly.c
  */

Fix afs_lookup_server() to not free an existing server's endpoint state when cleaning up a candidate server. The candidate record doesn't have an endpoint state yet at this point, so the free for that can just be removed.

---
 fs/afs/server.c | 1 -
 1 file changed, 1 deletion(-)

index 0fe162ea2a36..189138bd6d71 100644
@@ -242,7 +242,6 @@ struct afs_server *afs_lookup_server(struct afs_cell *cell, struct key *key,
 out:
 	afs_put_addrlist(alist, afs_alist_trace_put_server_create);
 	if (candidate) {
-		kfree(rcu_access_pointer(server->endpoint_state));
 		kfree(candidate);
 		afs_dec_servers_outstanding(cell->net);
 	}

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.

---
 fs/afs/addr_list.c | 5 ++++-
 fs/afs/fs_probe.c  | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

index 63bf096b721a..73195d76b481 100644
@@ -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++;

index a91ad1938d07..8c62334dbfe7 100644
@@ -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;
 	}

Applied to the vfs.fixes branch of the vfs/vfs.git tree. Patches in the vfs.fixes branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase, trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.fixes

[1/4] afs: Fix missing kunmap in afs_dir_search_bucket()

      https://git.kernel.org/vfs/vfs/c/950ae84b5cc9
[2/4] afs: Fix double-unmap of directory block
      https://git.kernel.org/vfs/vfs/c/e3cfd3eb7d5b
[3/4] afs: Fix incorrect free in candidate cleanup in afs_lookup_server()
      https://git.kernel.org/vfs/vfs/c/044d596094af
[4/4] afs: Clear stale peer app data after address list changes
      https://git.kernel.org/vfs/vfs/c/ba0623fc19a4