Hi Marc,
Here are a set of miscellaneous small fixes to the afs filesystem
including:
(1) Fix the afs_server_list struct to be cleaned up with RCU.
(2) Fix afs to translate a no-data result from a DNS lookup into ENOENT,
not EDESTADDRREQ for consistency with OpenAFS.
(3) Fix afs to translate a negative DNS lookup result into ENOENT rather
than EDESTADDRREQ.
(4) Fix file locking on R/O volumes to operate in local mode as the server
doesn't handle exclusive locks on such files.
(5) Not a fix per se, but set SB_RDONLY on superblocks for RO and Backup
volumes so that the VFS can see that they're read only.
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 (5):
afs: Fix afs_server_list to be cleaned up with RCU
afs: Make error on cell lookup failure consistent with OpenAFS
afs: Return ENOENT if no cell DNS record can be found
afs: Fix file locking on R/O volumes to operate in local mode
afs: Mark a superblock for an R/O or Backup volume as SB_RDONLY
fs/afs/dynroot.c | 4 ++--
fs/afs/internal.h | 1 +
fs/afs/server_list.c | 2 +-
fs/afs/super.c | 4 ++++
fs/afs/vl_rotate.c | 10 ++++++++++
5 files changed, 18 insertions(+), 3 deletions(-)
Mark a superblock that is for for an R/O or Backup volume as SB_RDONLY when
mounting it.
Signed-off-by: David Howells <[email protected]>
cc: Marc Dionne <[email protected]>
cc: [email protected]
---
fs/afs/super.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/afs/super.c b/fs/afs/super.c
index e95fb4cb4fcd..a01a0fb2cdbb 100644
--- a/fs/afs/super.c
+++ b/fs/afs/super.c
@@ -407,8 +407,10 @@ static int afs_validate_fc(struct fs_context *fc)
return PTR_ERR(volume);
ctx->volume = volume;
- if (volume->type != AFSVL_RWVOL)
+ if (volume->type != AFSVL_RWVOL) {
ctx->flock_mode = afs_flock_mode_local;
+ fc->sb_flags |= SB_RDONLY;
+ }
}
return 0;
afs_server_list is accessed with the rcu_read_lock() held from
volume->servers, so it needs to be cleaned up correctly.
Fix this by using kfree_rcu() instead of kfree().
Fixes: 8a070a964877 ("afs: Detect cell aliases 1 - Cells with root volumes")
Signed-off-by: David Howells <[email protected]>
cc: Marc Dionne <[email protected]>
cc: [email protected]
---
fs/afs/internal.h | 1 +
fs/afs/server_list.c | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/afs/internal.h b/fs/afs/internal.h
index c9cef3782b4a..a812952be1c9 100644
--- a/fs/afs/internal.h
+++ b/fs/afs/internal.h
@@ -553,6 +553,7 @@ struct afs_server_entry {
};
struct afs_server_list {
+ struct rcu_head rcu;
afs_volid_t vids[AFS_MAXTYPES]; /* Volume IDs */
refcount_t usage;
unsigned char nr_servers;
diff --git a/fs/afs/server_list.c b/fs/afs/server_list.c
index ed9056703505..b59896b1de0a 100644
--- a/fs/afs/server_list.c
+++ b/fs/afs/server_list.c
@@ -17,7 +17,7 @@ void afs_put_serverlist(struct afs_net *net, struct afs_server_list *slist)
for (i = 0; i < slist->nr_servers; i++)
afs_unuse_server(net, slist->servers[i].server,
afs_server_trace_put_slist);
- kfree(slist);
+ kfree_rcu(slist, rcu);
}
}
Make AFS return error ENOENT if no cell SRV or AFSDB DNS record (or
cellservdb config file record) can be found rather than returning
EDESTADDRREQ.
Also add cell name lookup info to the cursor dump.
Fixes: d5c32c89b208 ("afs: Fix cell DNS lookup")
Reported-by: Markus Suvanto <[email protected]>
Link: https://bugzilla.kernel.org/show_bug.cgi?id=216637
Signed-off-by: David Howells <[email protected]>
cc: Marc Dionne <[email protected]>
cc: [email protected]
---
fs/afs/vl_rotate.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/fs/afs/vl_rotate.c b/fs/afs/vl_rotate.c
index 488e58490b16..eb415ce56360 100644
--- a/fs/afs/vl_rotate.c
+++ b/fs/afs/vl_rotate.c
@@ -58,6 +58,12 @@ static bool afs_start_vl_iteration(struct afs_vl_cursor *vc)
}
/* Status load is ordered after lookup counter load */
+ if (cell->dns_status == DNS_LOOKUP_GOT_NOT_FOUND) {
+ pr_warn("No record of cell %s\n", cell->name);
+ vc->error = -ENOENT;
+ return false;
+ }
+
if (cell->dns_source == DNS_RECORD_UNAVAILABLE) {
vc->error = -EDESTADDRREQ;
return false;
@@ -285,6 +291,7 @@ bool afs_select_vlserver(struct afs_vl_cursor *vc)
*/
static void afs_vl_dump_edestaddrreq(const struct afs_vl_cursor *vc)
{
+ struct afs_cell *cell = vc->cell;
static int count;
int i;
@@ -294,6 +301,9 @@ static void afs_vl_dump_edestaddrreq(const struct afs_vl_cursor *vc)
rcu_read_lock();
pr_notice("EDESTADDR occurred\n");
+ pr_notice("CELL: %s err=%d\n", cell->name, cell->error);
+ pr_notice("DNS: src=%u st=%u lc=%x\n",
+ cell->dns_source, cell->dns_status, cell->dns_lookup_count);
pr_notice("VC: ut=%lx ix=%u ni=%hu fl=%hx err=%hd\n",
vc->untried, vc->index, vc->nr_iterations, vc->flags, vc->error);
AFS doesn't really do locking on R/O volumes as fileservers don't maintain
state with each other and thus a lock on a R/O volume file on one
fileserver will not be be visible to someone looking at the same file on
another fileserver.
Further, the server may return an error if you try it.
Fix this by doing what other AFS clients do and handle filelocking on R/O
volume files entirely within the client and don't touch the server.
Fixes: 6c6c1d63c243 ("afs: Provide mount-time configurable byte-range file locking emulation")
Signed-off-by: David Howells <[email protected]>
cc: Marc Dionne <[email protected]>
cc: [email protected]
---
fs/afs/super.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/fs/afs/super.c b/fs/afs/super.c
index 95d713074dc8..e95fb4cb4fcd 100644
--- a/fs/afs/super.c
+++ b/fs/afs/super.c
@@ -407,6 +407,8 @@ static int afs_validate_fc(struct fs_context *fc)
return PTR_ERR(volume);
ctx->volume = volume;
+ if (volume->type != AFSVL_RWVOL)
+ ctx->flock_mode = afs_flock_mode_local;
}
return 0;
On Thu, Nov 16, 2023 at 11:53 AM David Howells <[email protected]> wrote:
>
> Mark a superblock that is for for an R/O or Backup volume as SB_RDONLY when
> mounting it.
>
> Signed-off-by: David Howells <[email protected]>
> cc: Marc Dionne <[email protected]>
> cc: [email protected]
> ---
> fs/afs/super.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/fs/afs/super.c b/fs/afs/super.c
> index e95fb4cb4fcd..a01a0fb2cdbb 100644
> --- a/fs/afs/super.c
> +++ b/fs/afs/super.c
> @@ -407,8 +407,10 @@ static int afs_validate_fc(struct fs_context *fc)
> return PTR_ERR(volume);
>
> ctx->volume = volume;
> - if (volume->type != AFSVL_RWVOL)
> + if (volume->type != AFSVL_RWVOL) {
> ctx->flock_mode = afs_flock_mode_local;
> + fc->sb_flags |= SB_RDONLY;
> + }
> }
>
> return 0;
Reviewed-by: Marc Dionne <[email protected]>
Marc
On Thu, Nov 16, 2023 at 11:53 AM David Howells <[email protected]> wrote:
>
> afs_server_list is accessed with the rcu_read_lock() held from
> volume->servers, so it needs to be cleaned up correctly.
>
> Fix this by using kfree_rcu() instead of kfree().
>
> Fixes: 8a070a964877 ("afs: Detect cell aliases 1 - Cells with root volumes")
> Signed-off-by: David Howells <[email protected]>
> cc: Marc Dionne <[email protected]>
> cc: [email protected]
> ---
> fs/afs/internal.h | 1 +
> fs/afs/server_list.c | 2 +-
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/afs/internal.h b/fs/afs/internal.h
> index c9cef3782b4a..a812952be1c9 100644
> --- a/fs/afs/internal.h
> +++ b/fs/afs/internal.h
> @@ -553,6 +553,7 @@ struct afs_server_entry {
> };
>
> struct afs_server_list {
> + struct rcu_head rcu;
> afs_volid_t vids[AFS_MAXTYPES]; /* Volume IDs */
> refcount_t usage;
> unsigned char nr_servers;
> diff --git a/fs/afs/server_list.c b/fs/afs/server_list.c
> index ed9056703505..b59896b1de0a 100644
> --- a/fs/afs/server_list.c
> +++ b/fs/afs/server_list.c
> @@ -17,7 +17,7 @@ void afs_put_serverlist(struct afs_net *net, struct afs_server_list *slist)
> for (i = 0; i < slist->nr_servers; i++)
> afs_unuse_server(net, slist->servers[i].server,
> afs_server_trace_put_slist);
> - kfree(slist);
> + kfree_rcu(slist, rcu);
> }
> }
Reviewed-by: Marc Dionne <[email protected]>
Marc
On 11/16/2023 10:53 AM, David Howells wrote:
> Hi Marc,
>
> Here are a set of miscellaneous small fixes to the afs filesystem
> including:
>
> (1) Fix the afs_server_list struct to be cleaned up with RCU.
>
> (2) Fix afs to translate a no-data result from a DNS lookup into ENOENT,
> not EDESTADDRREQ for consistency with OpenAFS.
>
> (3) Fix afs to translate a negative DNS lookup result into ENOENT rather
> than EDESTADDRREQ.
>
> (4) Fix file locking on R/O volumes to operate in local mode as the server
> doesn't handle exclusive locks on such files.
>
> (5) Not a fix per se, but set SB_RDONLY on superblocks for RO and Backup
> volumes so that the VFS can see that they're read only.
>
> 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 (5):
> afs: Fix afs_server_list to be cleaned up with RCU
> afs: Make error on cell lookup failure consistent with OpenAFS
> afs: Return ENOENT if no cell DNS record can be found
> afs: Fix file locking on R/O volumes to operate in local mode
> afs: Mark a superblock for an R/O or Backup volume as SB_RDONLY
>
> fs/afs/dynroot.c | 4 ++--
> fs/afs/internal.h | 1 +
> fs/afs/server_list.c | 2 +-
> fs/afs/super.c | 4 ++++
> fs/afs/vl_rotate.c | 10 ++++++++++
> 5 files changed, 18 insertions(+), 3 deletions(-)
Reviewed-by: Jeffrey Altman <[email protected]>
On Thu, Nov 16, 2023 at 11:53 AM David Howells <[email protected]> wrote:
>
> AFS doesn't really do locking on R/O volumes as fileservers don't maintain
> state with each other and thus a lock on a R/O volume file on one
> fileserver will not be be visible to someone looking at the same file on
> another fileserver.
>
> Further, the server may return an error if you try it.
>
> Fix this by doing what other AFS clients do and handle filelocking on R/O
> volume files entirely within the client and don't touch the server.
>
> Fixes: 6c6c1d63c243 ("afs: Provide mount-time configurable byte-range file locking emulation")
> Signed-off-by: David Howells <[email protected]>
> cc: Marc Dionne <[email protected]>
> cc: [email protected]
> ---
> fs/afs/super.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/fs/afs/super.c b/fs/afs/super.c
> index 95d713074dc8..e95fb4cb4fcd 100644
> --- a/fs/afs/super.c
> +++ b/fs/afs/super.c
> @@ -407,6 +407,8 @@ static int afs_validate_fc(struct fs_context *fc)
> return PTR_ERR(volume);
>
> ctx->volume = volume;
> + if (volume->type != AFSVL_RWVOL)
> + ctx->flock_mode = afs_flock_mode_local;
> }
>
> return 0;
Reviewed-by: Marc Dionne <[email protected]>
Marc