[PATCH 0/2] afs: Miscellaneous fixes

[PATCH 0/2] afs: Miscellaneous fixes

[David Howells]

Hi Christian,

Here are some fixes for afs, if you could take them?

 (1) Fix searching for the AFS fileserver record for an incoming callback
     in a mixed IPv4/IPv6 environment.

 (2) Fix the size of a buffer in afs_update_volume_status() to avoid
     overrunning it and use snprintf() as well.

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

Daniil Dulov (1):
  afs: Increase buffer size in afs_update_volume_status()

Marc Dionne (1):
  afs: Fix ignored callbacks over ipv4

 fs/afs/internal.h |  6 ++----
 fs/afs/main.c     |  3 +--
 fs/afs/server.c   | 14 +++++---------
 fs/afs/volume.c   |  4 ++--
 4 files changed, 10 insertions(+), 17 deletions(-)

[PATCH 1/2] afs: Fix ignored callbacks over ipv4

[David Howells]

From: Marc Dionne <marc.dionne@auristor.com>

When searching for a matching peer, all addresses need to be searched,
not just the ipv6 ones in the fs_addresses6 list.

Given that the lists no longer contain addresses, there is little
reason to splitting things between separate lists, so unify them
into a single list.

When processing an incoming callback from an ipv4 address, this would
lead to a failure to set call->server, resulting in the callback being
ignored and the client seeing stale contents.

Fixes: 72904d7b9bfb ("rxrpc, afs: Allow afs to pin rxrpc_peer objects")
Reported-by: Markus Suvanto <markus.suvanto@gmail.com>
Link: https://lists.infradead.org/pipermail/linux-afs/2024-February/008035.html
Signed-off-by: Marc Dionne <marc.dionne@auristor.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Link: https://lists.infradead.org/pipermail/linux-afs/2024-February/008037.html # v1
Link: https://lists.infradead.org/pipermail/linux-afs/2024-February/008066.html # v2
---
 fs/afs/internal.h |  6 ++----
 fs/afs/main.c     |  3 +--
 fs/afs/server.c   | 14 +++++---------
 3 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/fs/afs/internal.h b/fs/afs/internal.h
index 9c03fcf7ffaa..6ce5a612937c 100644
--- a/fs/afs/internal.h
+++ b/fs/afs/internal.h
@@ -321,8 +321,7 @@ struct afs_net {
 	struct list_head	fs_probe_slow;	/* List of afs_server to probe at 5m intervals */
 	struct hlist_head	fs_proc;	/* procfs servers list */
 
-	struct hlist_head	fs_addresses4;	/* afs_server (by lowest IPv4 addr) */
-	struct hlist_head	fs_addresses6;	/* afs_server (by lowest IPv6 addr) */
+	struct hlist_head	fs_addresses;	/* afs_server (by lowest IPv6 addr) */
 	seqlock_t		fs_addr_lock;	/* For fs_addresses[46] */
 
 	struct work_struct	fs_manager;
@@ -561,8 +560,7 @@ struct afs_server {
 	struct afs_server __rcu	*uuid_next;	/* Next server with same UUID */
 	struct afs_server	*uuid_prev;	/* Previous server with same UUID */
 	struct list_head	probe_link;	/* Link in net->fs_probe_list */
-	struct hlist_node	addr4_link;	/* Link in net->fs_addresses4 */
-	struct hlist_node	addr6_link;	/* Link in net->fs_addresses6 */
+	struct hlist_node	addr_link;	/* Link in net->fs_addresses6 */
 	struct hlist_node	proc_link;	/* Link in net->fs_proc */
 	struct list_head	volumes;	/* RCU list of afs_server_entry objects */
 	struct afs_server	*gc_next;	/* Next server in manager's list */
diff --git a/fs/afs/main.c b/fs/afs/main.c
index 1b3bd21c168a..a14f6013e316 100644
--- a/fs/afs/main.c
+++ b/fs/afs/main.c
@@ -90,8 +90,7 @@ static int __net_init afs_net_init(struct net *net_ns)
 	INIT_LIST_HEAD(&net->fs_probe_slow);
 	INIT_HLIST_HEAD(&net->fs_proc);
 
-	INIT_HLIST_HEAD(&net->fs_addresses4);
-	INIT_HLIST_HEAD(&net->fs_addresses6);
+	INIT_HLIST_HEAD(&net->fs_addresses);
 	seqlock_init(&net->fs_addr_lock);
 
 	INIT_WORK(&net->fs_manager, afs_manage_servers);
diff --git a/fs/afs/server.c b/fs/afs/server.c
index e169121f603e..038f9d0ae3af 100644
--- a/fs/afs/server.c
+++ b/fs/afs/server.c
@@ -38,7 +38,7 @@ struct afs_server *afs_find_server(struct afs_net *net, const struct rxrpc_peer
 		seq++; /* 2 on the 1st/lockless path, otherwise odd */
 		read_seqbegin_or_lock(&net->fs_addr_lock, &seq);
 
-		hlist_for_each_entry_rcu(server, &net->fs_addresses6, addr6_link) {
+		hlist_for_each_entry_rcu(server, &net->fs_addresses, addr_link) {
 			estate = rcu_dereference(server->endpoint_state);
 			alist = estate->addresses;
 			for (i = 0; i < alist->nr_addrs; i++)
@@ -177,10 +177,8 @@ static struct afs_server *afs_install_server(struct afs_cell *cell,
 	 * bit, but anything we might want to do gets messy and memory
 	 * intensive.
 	 */
-	if (alist->nr_ipv4 > 0)
-		hlist_add_head_rcu(&server->addr4_link, &net->fs_addresses4);
-	if (alist->nr_addrs > alist->nr_ipv4)
-		hlist_add_head_rcu(&server->addr6_link, &net->fs_addresses6);
+	if (alist->nr_addrs > 0)
+		hlist_add_head_rcu(&server->addr_link, &net->fs_addresses);
 
 	write_sequnlock(&net->fs_addr_lock);
 
@@ -511,10 +509,8 @@ static void afs_gc_servers(struct afs_net *net, struct afs_server *gc_list)
 
 			list_del(&server->probe_link);
 			hlist_del_rcu(&server->proc_link);
-			if (!hlist_unhashed(&server->addr4_link))
-				hlist_del_rcu(&server->addr4_link);
-			if (!hlist_unhashed(&server->addr6_link))
-				hlist_del_rcu(&server->addr6_link);
+			if (!hlist_unhashed(&server->addr_link))
+				hlist_del_rcu(&server->addr_link);
 		}
 		write_sequnlock(&net->fs_lock);
 

Re: [PATCH 1/2] afs: Fix ignored callbacks over ipv4

[Matthew Wilcox]

On Mon, Feb 19, 2024 at 02:39:02PM +0000, David Howells wrote:
> +++ b/fs/afs/internal.h
> @@ -321,8 +321,7 @@ struct afs_net {
>  	struct list_head	fs_probe_slow;	/* List of afs_server to probe at 5m intervals */
>  	struct hlist_head	fs_proc;	/* procfs servers list */
>  
> -	struct hlist_head	fs_addresses4;	/* afs_server (by lowest IPv4 addr) */
> -	struct hlist_head	fs_addresses6;	/* afs_server (by lowest IPv6 addr) */
> +	struct hlist_head	fs_addresses;	/* afs_server (by lowest IPv6 addr) */

Comment is out of date ...

> @@ -561,8 +560,7 @@ struct afs_server {
>  	struct afs_server __rcu	*uuid_next;	/* Next server with same UUID */
>  	struct afs_server	*uuid_prev;	/* Previous server with same UUID */
>  	struct list_head	probe_link;	/* Link in net->fs_probe_list */
> -	struct hlist_node	addr4_link;	/* Link in net->fs_addresses4 */
> -	struct hlist_node	addr6_link;	/* Link in net->fs_addresses6 */
> +	struct hlist_node	addr_link;	/* Link in net->fs_addresses6 */

Ditto

[PATCH 2/2] afs: Increase buffer size in afs_update_volume_status()

[David Howells]

From: Daniil Dulov <d.dulov@aladdin.ru>

The max length of volume->vid value is 20 characters.
So increase idbuf[] size up to 24 to avoid overflow.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

[DH: Actually, it's 20 + NUL, so increase it to 24 and use snprintf()]

Fixes: d2ddc776a458 ("afs: Overhaul volume and server record caching and fileserver rotation")
Signed-off-by: Daniil Dulov <d.dulov@aladdin.ru>
Signed-off-by: David Howells <dhowells@redhat.com>
Link: https://lore.kernel.org/r/20240211150442.3416-1-d.dulov@aladdin.ru/ # v1
Link: https://lore.kernel.org/r/20240212083347.10742-1-d.dulov@aladdin.ru/ # v2
---
 fs/afs/volume.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/afs/volume.c b/fs/afs/volume.c
index 020ecd45e476..af3a3f57c1b3 100644
--- a/fs/afs/volume.c
+++ b/fs/afs/volume.c
@@ -353,7 +353,7 @@ static int afs_update_volume_status(struct afs_volume *volume, struct key *key)
 {
 	struct afs_server_list *new, *old, *discard;
 	struct afs_vldb_entry *vldb;
-	char idbuf[16];
+	char idbuf[24];
 	int ret, idsz;
 
 	_enter("");
@@ -361,7 +361,7 @@ static int afs_update_volume_status(struct afs_volume *volume, struct key *key)
 	/* We look up an ID by passing it as a decimal string in the
 	 * operation's name parameter.
 	 */
-	idsz = sprintf(idbuf, "%llu", volume->vid);
+	idsz = snprintf(idbuf, sizeof(idbuf), "%llu", volume->vid);
 
 	vldb = afs_vl_lookup_vldb(volume->cell, key, idbuf, idsz);
 	if (IS_ERR(vldb)) {

Re: [PATCH 2/2] afs: Increase buffer size in afs_update_volume_status()

[Matthew Wilcox]

On Mon, Feb 19, 2024 at 02:39:03PM +0000, David Howells wrote:
> From: Daniil Dulov <d.dulov@aladdin.ru>
> 
> The max length of volume->vid value is 20 characters.
> So increase idbuf[] size up to 24 to avoid overflow.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> [DH: Actually, it's 20 + NUL, so increase it to 24 and use snprintf()]
> 
> Fixes: d2ddc776a458 ("afs: Overhaul volume and server record caching and fileserver rotation")
> Signed-off-by: Daniil Dulov <d.dulov@aladdin.ru>
> Signed-off-by: David Howells <dhowells@redhat.com>
> Link: https://lore.kernel.org/r/20240211150442.3416-1-d.dulov@aladdin.ru/ # v1
> Link: https://lore.kernel.org/r/20240212083347.10742-1-d.dulov@aladdin.ru/ # v2

Tag it for stable?

Re: [PATCH 0/2] afs: Miscellaneous fixes

[Christian Brauner]

On Mon, 19 Feb 2024 14:39:01 +0000, David Howells wrote:
> Here are some fixes for afs, if you could take them?
> 
>  (1) Fix searching for the AFS fileserver record for an incoming callback
>      in a mixed IPv4/IPv6 environment.
> 
>  (2) Fix the size of a buffer in afs_update_volume_status() to avoid
>      overrunning it and use snprintf() as well.
> 
> [...]

vfs.fixes means that these things will go in this week. Let me know if
this is not what you intended! :)

---

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/2] afs: Fix ignored callbacks over ipv4
      https://git.kernel.org/vfs/vfs/c/bfacaf71a148
[2/2] afs: Increase buffer size in afs_update_volume_status()
      https://git.kernel.org/vfs/vfs/c/6ea38e2aeb72

[PATCH 0/2] afs: Miscellaneous fixes

[David Howells]

Hi Marc,

Here are some fixes for afs, if you could look them over?

 (1) Fix the caching of preferred address of a fileserver.  By doing that, we
     stick with whatever address we get a response back from first rather then
     obeying any preferences set.

 (2) Fix an occasional FetchStatus-after-RemoveDir.  The FetchStatus then
     fails with VNOVNODE (equivalent to -ENOENT) that confuses parts of the
     driver that aren't expecting that.

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

David Howells (2):
  afs: Don't cache preferred address
  afs: Fix occasional rmdir-then-VNOVNODE with generic/011

 fs/afs/rotate.c     | 21 ++++-----------------
 fs/afs/validation.c | 16 +++++++++-------
 2 files changed, 13 insertions(+), 24 deletions(-)

Re: [PATCH 0/2] afs: Miscellaneous fixes

[Marc Dionne]

On Wed, Mar 13, 2024 at 5:15 AM David Howells <dhowells@redhat.com> wrote:
>
> Hi Marc,
>
> Here are some fixes for afs, if you could look them over?
>
>  (1) Fix the caching of preferred address of a fileserver.  By doing that, we
>      stick with whatever address we get a response back from first rather then
>      obeying any preferences set.
>
>  (2) Fix an occasional FetchStatus-after-RemoveDir.  The FetchStatus then
>      fails with VNOVNODE (equivalent to -ENOENT) that confuses parts of the
>      driver that aren't expecting that.
>
> 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
>
> David Howells (2):
>   afs: Don't cache preferred address
>   afs: Fix occasional rmdir-then-VNOVNODE with generic/011
>
>  fs/afs/rotate.c     | 21 ++++-----------------
>  fs/afs/validation.c | 16 +++++++++-------
>  2 files changed, 13 insertions(+), 24 deletions(-)

Reviewed-by: Marc Dionne <marc.dionne@auristor.com>

Marc

Re: [PATCH 0/2] afs: Miscellaneous fixes

[Christian Brauner]

On Wed, 13 Mar 2024 08:15:01 +0000, David Howells wrote:
> Here are some fixes for afs, if you could look them over?
> 
>  (1) Fix the caching of preferred address of a fileserver.  By doing that, we
>      stick with whatever address we get a response back from first rather then
>      obeying any preferences set.
> 
>  (2) Fix an occasional FetchStatus-after-RemoveDir.  The FetchStatus then
>      fails with VNOVNODE (equivalent to -ENOENT) that confuses parts of the
>      driver that aren't expecting that.
> 
> [...]

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/2] afs: Don't cache preferred address
      https://git.kernel.org/vfs/vfs/c/b719dcc8b02d
[2/2] afs: Fix occasional rmdir-then-VNOVNODE with generic/011
      https://git.kernel.org/vfs/vfs/c/4eed8f8549f4