Hi Shyam,
On 12/18, Shyam Prasad N wrote:
Hi Steve/Paulo/David,
Please review the attached patch.
I noticed that DNS resolution did not always upcall to userspace when
the IP address changed. This addresses the fix for it.
I would even recommend CC:stable for this one.
(I'm pasting the patch here so I can comment inline)
From 604ab4c350c2552daa8e77f861a54032b49bc706 Mon Sep 17 00:00:00 2001
From: Shyam Prasad N <sprasad@xxxxxxxxxxxxx>
Date: Sat, 18 Dec 2021 17:28:10 +0000
Subject: [PATCH] cifs: invalidate dns resolver keys after use
We rely on dns resolver module to upcall to userspace
using request_key and get us the DNS mapping.
However, the invalidate arg for dns_query was set
to false, which meant that the key created during the
first call for a hostname would continue to be cached
till it expires. This expiration period depends on
how the dns_resolver is configured.
Ok.
Fixing this by setting invalidate=true during dns_query.
This means that the key will be cleaned up by dns_resolver
soon after it returns the data. This also means that
the dns_resolver subsystem will not cache the key for
an interval indicated by the DNS records TTL.
This is ok too, which is an approach that I did try before, but
didn't work (see below).
But this is
okay since we use the TTL value returned to schedule the
next lookup.
This is an incorrect assumption. keyutils' key.dns_resolve uses
getaddrinfo() for A/AAAA queries, which doesn't contain DNS TTL
information.
Meaning that the TTL returned to dns_resolve_server_name_to_ip() is
actually either key.dns_resolve.conf's default_ttl value, or the default
key_expiry value (5).
I have a patch ready (working, but still testing) for keyutils that implements
a "common" DNS interface, and the caller only specifies the query type,
which is then resolved via res_nquery(). This way, all returned records
have generic their metadata (TTL included), along with type-specific
metadata (e.g. AFSDB or SRV specifics) as well.
Another option/suggestion would be to:
1. decrease SMB_DNS_RESOLVE_INTERVAL_DEFAULT
2. and/or make it user-configurable via sysfs
3. call dns_query() with expiry=NULL and invalidate=true
So we'd use keyutils exclusively for kernel-userspace communication, and
handle the expiration checking/configuration on cifs side.
Cc: David Howells <dhowells@xxxxxxxxxx>
Signed-off-by: Shyam Prasad N <sprasad@xxxxxxxxxxxxx>
---
fs/cifs/dns_resolve.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/cifs/dns_resolve.c b/fs/cifs/dns_resolve.c
index 0458d28d71aa..8890af1537ef 100644
--- a/fs/cifs/dns_resolve.c
+++ b/fs/cifs/dns_resolve.c
@@ -66,7 +66,7 @@ dns_resolve_server_name_to_ip(const char *unc, char **ip_addr, time64_t *expiry)
/* Perform the upcall */
rc = dns_query(current->nsproxy->net_ns, NULL, hostname, len,
- NULL, ip_addr, expiry, false);
+ NULL, ip_addr, expiry, true);
if (rc < 0)
cifs_dbg(FYI, "%s: unable to resolve: %*.*s\n",
__func__, len, len, hostname);
Cheers,
Enzo
