2024-02-19 14:39:22

by David Howells

[permalink] [raw]
Subject: [PATCH 0/2] afs: Miscellaneous fixes

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(-)



2024-02-19 14:41:28

by David Howells

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

From: Marc Dionne <[email protected]>

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 <[email protected]>
Link: https://lists.infradead.org/pipermail/linux-afs/2024-February/008035.html
Signed-off-by: Marc Dionne <[email protected]>
Signed-off-by: David Howells <[email protected]>
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);



2024-02-19 14:41:28

by David Howells

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

From: Daniil Dulov <[email protected]>

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 <[email protected]>
Signed-off-by: David Howells <[email protected]>
Link: https://lore.kernel.org/r/[email protected]/ # v1
Link: https://lore.kernel.org/r/[email protected]/ # 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)) {


2024-02-19 16:54:51

by Matthew Wilcox

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

On Mon, Feb 19, 2024 at 02:39:03PM +0000, David Howells wrote:
> From: Daniil Dulov <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: David Howells <[email protected]>
> Link: https://lore.kernel.org/r/[email protected]/ # v1
> Link: https://lore.kernel.org/r/[email protected]/ # v2

Tag it for stable?

2024-02-19 20:35:28

by Matthew Wilcox

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

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


2024-02-20 09:05:49

by Christian Brauner

[permalink] [raw]
Subject: Re: [PATCH 0/2] afs: Miscellaneous fixes

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