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) 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
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 | 8 +++++++-
security/keys/key.c | 15 +++++----------
security/keys/proc.c | 2 +-
7 files changed, 61 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]
---
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..4089d77a7a4d 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 >= 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,
};
This is the related bug: https://bugzilla.kernel.org/show_bug.cgi?id=216637
ma, 2023-12-11 kello 16:34 +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) 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
>
I tested this patches
6.7.0-rc4-gdfbc00cb940b
It seems that not existing directory will remove my valid rxprc key.
Reproduce:
1) kinit ....
2) aklog....
3) keyctl show
Session Keyring
347100937 --alswrv 1001 65534 keyring: _uid_ses.1001
1062692655 --alswrv 1001 65534 \_ keyring: _uid.1001
698363997 --als-rv 1001 100 \_ rxrpc: [email protected]
klist
Ticket cache: KEYRING:persistent:1001:1001
Default principal: .....
...
4) ls /afs/notfound
5) keyctl show
Session Keyring
709308533 --alswrv 1001 65534 keyring: _uid_ses.1001
385820479 --alswrv 1001 65534 \_ keyring: _uid.1001
klist
klist: Credentials cache keyring 'persistent:1001:1001' not found
-Markus
[email protected] wrote:
> Reproduce:
> 1) kinit ....
> 2) aklog....
> 3) keyctl show
> Session Keyring
> 347100937 --alswrv 1001 65534 keyring: _uid_ses.1001
> 1062692655 --alswrv 1001 65534 \_ keyring: _uid.1001
> 698363997 --als-rv 1001 100 \_ rxrpc: [email protected]
>
> klist
> Ticket cache: KEYRING:persistent:1001:1001
> Default principal: .....
Can you "grep rxrpc /proc/keys" at this point?
> 4) ls /afs/notfound
> 5) keyctl show
> Session Keyring
> 709308533 --alswrv 1001 65534 keyring: _uid_ses.1001
> 385820479 --alswrv 1001 65534 \_ keyring: _uid.1001
>
> klist
> klist: Credentials cache keyring 'persistent:1001:1001' not found
David
ti, 2023-12-12 kello 09:03 +0000, David Howells kirjoitti:
> [email protected] wrote:
>
> > Reproduce:
> > 1) kinit ....
> > 2) aklog....
> > 3) keyctl show
> > Session Keyring
> > 347100937 --alswrv 1001 65534 keyring: _uid_ses.1001
> > 1062692655 --alswrv 1001 65534 \_ keyring: _uid.1001
> > 698363997 --als-rv 1001 100 \_ rxrpc: [email protected]
> >
> > klist
> > Ticket cache: KEYRING:persistent:1001:1001
> > Default principal: .....
>
> Can you "grep rxrpc /proc/keys" at this point?
>
different cell though...
masu@t470 ~ % grep rxrpc /proc/keys
23e16cda I--Q--- 1 3d 3b010000 1001 100 rxrpc [email protected]: ka
[email protected] wrote:
> > Can you "grep rxrpc /proc/keys" at this point?
> >
> different cell though...
>
> masu@t470 ~ % grep rxrpc /proc/keys
> 23e16cda I--Q--- 1 3d 3b010000 1001 100 rxrpc [email protected]: ka
Okay, I see the persistent keyring disappear, but I don't see a key linked
into my session keyring vanish.
David
> > masu@t470 ~ % grep rxrpc /proc/keys
> > 23e16cda I--Q--- 1 3d 3b010000 1001 100 rxrpc [email protected]: ka
>
> Okay, I see the persistent keyring disappear, but I don't see a key linked
> into my session keyring vanish.
Full log of my commands...
masu@t470 ~ % klist
klist: Credentials cache keyring 'persistent:1001:1001' not found
masu@t470 ~ % keyctl show
Session Keyring
388545754 --alswrv 1001 65534 keyring: _uid_ses.1001
946177719 --alswrv 1001 65534 \_ keyring: _uid.1001
masu@t470 ~ % grep rxrpc /proc/keys
masu@t470 ~ %
masu@t470 ~ %
masu@t470 ~ %
masu@t470 ~ % kinit [email protected]
Password for [email protected]:
masu@t470 ~ % aklog-kafs-kdf movesole.com MOVESOLE.COM
masu@t470 ~ %
masu@t470 ~ %
masu@t470 ~ % grep rxrpc /proc/keys
2600d2d5 I--Q--- 1 3d 3b010000 1001 100 rxrpc [email protected]: ka
masu@t470 ~ % klist
Ticket cache: KEYRING:persistent:1001:1001
Default principal: [email protected]
Valid starting Expires Service principal
12.12.2023 11.52.47 16.12.2023 11.52.40 afs/[email protected]
renew until 26.12.2023 11.52.40
12.12.2023 11.52.43 16.12.2023 11.52.40 krbtgt/[email protected]
renew until 26.12.2023 11.52.40
masu@t470 ~ % keyctl show
Session Keyring
388545754 --alswrv 1001 65534 keyring: _uid_ses.1001
946177719 --alswrv 1001 65534 \_ keyring: _uid.1001
637588181 --als-rv 1001 100 \_ rxrpc: [email protected]
masu@t470 ~ %
masu@t470 ~ %
masu@t470 ~ %
masu@t470 ~ %
masu@t470 ~ % ls /afs/notfound
ls: tiedostoa '/afs/notfound' ei voi käsitellä: Tiedostoa tai hakemistoa ei ole
masu@t470 ~ %
masu@t470 ~ %
masu@t470 ~ %
masu@t470 ~ % klist
klist: Credentials cache keyring 'persistent:1001:1001' not found
masu@t470 ~ % grep rxrpc /proc/keys
masu@t470 ~ % keyctl show
Session Keyring
1025218481 --alswrv 1001 65534 keyring: _uid_ses.1001
322736164 --alswrv 1001 65534 \_ keyring: _uid.1001