Hi Markus, Marc,
Here's a set of fixes to improve the interaction of arbitrary lookups in
the AFS dynamic root that hit DNS lookup failures[1]:
(1) Always delete unused (particularly negative) dentries as soon as
possible so that they don't prevent future lookups from retrying.
(2) Fix the handling of new-style negative DNS lookups in ->lookup() to
make them return ENOENT so that userspace doesn't get confused when
stat succeeds but the following open on the looked up file then fails.
(3) Fix key handling so that DNS lookup results are reclaimed as soon as
they expire rather than sitting round either forever or for an
additional 5 mins beyond a set expiry time returning EKEYEXPIRED.
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
Link: https://bugzilla.kernel.org/show_bug.cgi?id=216637 [1]
Link: https://lore.kernel.org/r/[email protected] # v1
Link: https://lore.kernel.org/r/[email protected] # v2
Changes
=======
ver #3)
- Rebased to v6.7-rc5 which has an additional afs patch.
- Don't add to TIME64_MAX (ie. permanent) when checking expiry time.
ver #2)
- Fix signed-unsigned comparison when checking return val.
David Howells (3):
afs: Fix the dynamic root's d_delete to always delete unused dentries
afs: Fix dynamic root lookup DNS check
keys, dns: Allow key types (eg. DNS) to be reclaimed immediately on
expiry
fs/afs/dynroot.c | 31 +++++++++++++++++--------------
include/linux/key-type.h | 1 +
net/dns_resolver/dns_key.c | 10 +++++++++-
security/keys/gc.c | 31 +++++++++++++++++++++----------
security/keys/internal.h | 11 ++++++++++-
security/keys/key.c | 15 +++++----------
security/keys/proc.c | 2 +-
7 files changed, 64 insertions(+), 37 deletions(-)
In the afs dynamic root directory, the ->lookup() function does a DNS check
on the cell being asked for and if the DNS upcall reports an error it will
report an error back to userspace (typically ENOENT).
However, if a failed DNS upcall returns a new-style result, it will return
a valid result, with the status field set appropriately to indicate the
type of failure - and in that case, dns_query() doesn't return an error and
we let stat() complete with no error - which can cause confusion in
userspace as subsequent calls that trigger d_automount then fail with
ENOENT.
Fix this by checking the status result from a valid dns_query() and
returning an error if it indicates a failure.
Fixes: bbb4c4323a4d ("dns: Allow the dns resolver to retrieve a server set")
Reported-by: Markus Suvanto <[email protected]>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=216637
Signed-off-by: David Howells <[email protected]>
cc: Marc Dionne <[email protected]>
cc: [email protected]
---
Notes:
Changes
=======
ver #2)
- Fix signed-unsigned comparison when checking return val.
fs/afs/dynroot.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/fs/afs/dynroot.c b/fs/afs/dynroot.c
index 34474a061654..1f656005018e 100644
--- a/fs/afs/dynroot.c
+++ b/fs/afs/dynroot.c
@@ -114,6 +114,7 @@ static int afs_probe_cell_name(struct dentry *dentry)
struct afs_net *net = afs_d2net(dentry);
const char *name = dentry->d_name.name;
size_t len = dentry->d_name.len;
+ char *result = NULL;
int ret;
/* Names prefixed with a dot are R/W mounts. */
@@ -131,9 +132,22 @@ static int afs_probe_cell_name(struct dentry *dentry)
}
ret = dns_query(net->net, "afsdb", name, len, "srv=1",
- NULL, NULL, false);
- if (ret == -ENODATA || ret == -ENOKEY)
+ &result, NULL, false);
+ if (ret == -ENODATA || ret == -ENOKEY || ret == 0)
ret = -ENOENT;
+ if (ret > 0 && ret >= sizeof(struct dns_server_list_v1_header)) {
+ struct dns_server_list_v1_header *v1 = (void *)result;
+
+ if (v1->hdr.zero == 0 &&
+ v1->hdr.content == DNS_PAYLOAD_IS_SERVER_LIST &&
+ v1->hdr.version == 1 &&
+ (v1->status != DNS_LOOKUP_GOOD &&
+ v1->status != DNS_LOOKUP_GOOD_WITH_BAD))
+ return -ENOENT;
+
+ }
+
+ kfree(result);
return ret;
}
Fix the afs dynamic root's d_delete function to always delete unused
dentries rather than only deleting them if they're positive. With things
as they stand upstream, negative dentries stemming from failed DNS lookups
stick around preventing retries.
Fixes: 66c7e1d319a5 ("afs: Split the dynroot stuff out and give it its own ops tables")
Signed-off-by: David Howells <[email protected]>
cc: Marc Dionne <[email protected]>
cc: [email protected]
---
fs/afs/dynroot.c | 13 +------------
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/fs/afs/dynroot.c b/fs/afs/dynroot.c
index 1fa8cf23bd36..34474a061654 100644
--- a/fs/afs/dynroot.c
+++ b/fs/afs/dynroot.c
@@ -252,20 +252,9 @@ static int afs_dynroot_d_revalidate(struct dentry *dentry, unsigned int flags)
return 1;
}
-/*
- * Allow the VFS to enquire as to whether a dentry should be unhashed (mustn't
- * sleep)
- * - called from dput() when d_count is going to 0.
- * - return 1 to request dentry be unhashed, 0 otherwise
- */
-static int afs_dynroot_d_delete(const struct dentry *dentry)
-{
- return d_really_is_positive(dentry);
-}
-
const struct dentry_operations afs_dynroot_dentry_operations = {
.d_revalidate = afs_dynroot_d_revalidate,
- .d_delete = afs_dynroot_d_delete,
+ .d_delete = always_delete_dentry,
.d_release = afs_d_release,
.d_automount = afs_d_automount,
};
ti, 2023-12-12 kello 14:46 +0000, David Howells kirjoitti:
> Hi Markus, Marc,
>
> Here's a set of fixes to improve the interaction of arbitrary lookups in
> the AFS dynamic root that hit DNS lookup failures[1]:
>
> (1) Always delete unused (particularly negative) dentries as soon as
> possible so that they don't prevent future lookups from retrying.
>
> (2) Fix the handling of new-style negative DNS lookups in ->lookup() to
> make them return ENOENT so that userspace doesn't get confused when
> stat succeeds but the following open on the looked up file then fails.
>
> (3) Fix key handling so that DNS lookup results are reclaimed as soon as
> they expire rather than sitting round either forever or for an
> additional 5 mins beyond a set expiry time returning EKEYEXPIRED.
>
> 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
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=216637 [1]
> Link: https://lore.kernel.org/r/[email protected] # v1
> Link: https://lore.kernel.org/r/[email protected] # v2
>
> Changes
> =======
> ver #3)
> - Rebased to v6.7-rc5 which has an additional afs patch.
> - Don't add to TIME64_MAX (ie. permanent) when checking expiry time.
>
> ver #2)
> - Fix signed-unsigned comparison when checking return val.
>
> David Howells (3):
> afs: Fix the dynamic root's d_delete to always delete unused dentries
> afs: Fix dynamic root lookup DNS check
> keys, dns: Allow key types (eg. DNS) to be reclaimed immediately on
> expiry
>
> fs/afs/dynroot.c | 31 +++++++++++++++++--------------
> include/linux/key-type.h | 1 +
> net/dns_resolver/dns_key.c | 10 +++++++++-
> security/keys/gc.c | 31 +++++++++++++++++++++----------
> security/keys/internal.h | 11 ++++++++++-
> security/keys/key.c | 15 +++++----------
> security/keys/proc.c | 2 +-
> 7 files changed, 64 insertions(+), 37 deletions(-)
>
masu@t470 ~ % uname -r
6.7.0-rc5-gb946001d3bb1
This fixes my problem :) https://bugzilla.kernel.org/show_bug.cgi?id=216637
Tested-by: Markus Suvanto <[email protected]>
-Markus