NFS: fix bug in legacy DNS resolver.
Subsystems (2)
PATCH 1/1 NFS: fix bug in legacy DNS resolver.
The DNS resolver's use of the sunrpc cache involves a 'ttl' number (relative) rather that a timeout (absolute). This confused me when I wrote
commit c5b29f885afe890f953f7f23424045cdad31d3e4
"sunrpc: use seconds since boot in expiry cache"
and I managed to break it. The effect is that any TTL is interpreted as 0, and nothing useful gets into the cache.
This patch removes the use of get_expiry() - which really expects an expiry time - and uses get_uint() instead, treating the int correctly as a ttl.
This fixes a regression that has been present since 2.6.37, causing certain NFS accesses in certain environments to incorrectly fail.
fs/nfs/dns_resolve.c +3 -2
@@ -217,7 +217,7 @@ static int nfs_dns_parse(struct cache_detail *cd, char *buf, int buflen) {char buf1[NFS_DNS_HOSTNAME_MAXLEN +1 ];struct nfs_dns_ent key, *item; -unsigned long ttl; +unsigned int ttl;ssize_t len;int ret = -EINVAL ;
@@ -240,7 +240,8 @@ static int nfs_dns_parse(struct cache_detail *cd, char *buf, int buflen) key.namelen = len;memset (&key.h,0 ,sizeof (key.h)); - ttl =get_expiry (&buf); +if (get_int (&buf, &ttl) <0 ) + goto out;if (ttl ==0 ) goto out; key.h.expiry_time = ttl +seconds_since_boot ();
Thanks, Neil!
Quoted 7 lines · NeilBrown
The DNS resolver's use of the sunrpc cache involves a 'ttl' number (relative) rather that a timeout (absolute). This confused me when I wrote
commitc5b29f885afe890f953f7f23424045cdad31d3e4
"sunrpc: use seconds since boot in expiry cache"
Quoted 6 lines
and I managed to break it. The effect is that any TTL is interpreted as 0, and nothing useful gets into the cache.
This patch removes the use of
get_expiry()- which really expects an expiry time - and usesget_uint()instead, treating the int correctly as a ttl.
Nit: The description says "get_uint()" but the patch uses "get_int()." Should we change the patch?
Quoted 2 lines
This fixes a regression that has been present since 2.6.37, causing certain NFS accesses in certain environments to incorrectly fail.
In particular, when the legacy DNS resolver is in use, this regression prevents the NFS client from following any NFSv4 referral which contains a DNS hostname.
Quoted 28 lines
Reported-by Chuck Lever <[email protected]>Tested-by Chuck Lever <[email protected]>Signed-off-by NeilBrown <[email protected]>fs/nfs/dns_resolve.c +3
-2@@ -217,7 +217,7 @@ static int nfs_dns_parse(struct cache_detail *cd, char *buf, int buflen) {char buf1[NFS_DNS_HOSTNAME_MAXLEN +1 ];struct nfs_dns_ent key, *item; -unsigned long ttl; +unsigned int ttl;ssize_t len;int ret = -EINVAL ;@@ -240,7 +240,8 @@ static int nfs_dns_parse(struct cache_detail *cd, char *buf, int buflen) key.namelen = len;memset (&key.h,0 ,sizeof (key.h)); - ttl =get_expiry (&buf); +if (get_int (&buf, &ttl) <0 ) + goto out;if (ttl ==0 ) goto out; key.h.expiry_time = ttl +seconds_since_boot ();
Signature · 2 lines
Chuck Lever chuck[dot]lever[at]oracle[dot]com
NeilBrown
Tested-by Chuck LeverQuoted 11 lines · On Wed, 31 Oct 2012 11:09:27 -0400 Chuck Lever <[email protected]> wrote:
Thanks, Neil!
Quoted 7 lines · NeilBrown
The DNS resolver's use of the sunrpc cache involves a 'ttl' number (relative) rather that a timeout (absolute). This confused me when I wrote
commitc5b29f885afe890f953f7f23424045cdad31d3e4
"sunrpc: use seconds since boot in expiry cache"
Quoted 9 lines
Quoted 6 lines
and I managed to break it. The effect is that any TTL is interpreted as 0, and nothing useful gets into the cache.
This patch removes the use of
get_expiry()- which really expects an expiry time - and usesget_uint()instead, treating the int correctly as a ttl.Nit: The description says "
get_uint()" but the patch uses "get_int()." Should we change the patch?
Yes, change the patch. I remember looking at that issue and deciding that get_uint() was the right thing to use as 'ttl' is already unsigned. And I wrote the description based on the memory. But it seems I didn't change the code. Weird.
Quoted 4 lines
Quoted 2 lines
This fixes a regression that has been present since 2.6.37, causing certain NFS accesses in certain environments to incorrectly fail.
In particular, when the legacy DNS resolver is in use, this regression prevents the NFS client from following any NFSv4 referral which contains a DNS hostname.
Thanks. Updated patch below.
NeilBrown
From e0b382c5b880017853f6a4cc0eda472cf6fef0e4 Mon Sep 17 00:00:00 2001
From: NeilBrown <[email protected]>
Date: Wed, 31 Oct 2012 12:10:48 +1100
Subject: [PATCH] NFS: fix bug in legacy DNS resolver.
The DNS resolver's use of the sunrpc cache involves a 'ttl' number (relative) rather that a timeout (absolute). This confused me when I wrote
commit c5b29f885afe890f953f7f23424045cdad31d3e4
"sunrpc: use seconds since boot in expiry cache"
and I managed to break it. The effect is that any TTL is interpreted as 0, and nothing useful gets into the cache.
This patch removes the use of get_expiry() - which really expects an expiry time - and uses get_uint() instead, treating the int correctly as a ttl.
This fixes a regression that has been present since 2.6.37, causing certain NFS accesses in certain environments to incorrectly fail.
In particular, when the legacy DNS resolver is in use, this regression prevents the NFS client from following any NFSv4 referral which contains a DNS hostname.
fs/nfs/dns_resolve.c +3 -2
@@ -217,7 +217,7 @@ static int nfs_dns_parse(struct cache_detail *cd, char *buf, int buflen) {char buf1[NFS_DNS_HOSTNAME_MAXLEN +1 ];struct nfs_dns_ent key, *item; -unsigned long ttl; +unsigned int ttl;ssize_t len;int ret = -EINVAL ;
@@ -240,7 +240,8 @@ static int nfs_dns_parse(struct cache_detail *cd, char *buf, int buflen) key.namelen = len;memset (&key.h,0 ,sizeof (key.h)); - ttl =get_expiry (&buf); +if (get_uint (&buf, &ttl) <0 ) + goto out;if (ttl ==0 ) goto out; key.h.expiry_time = ttl +seconds_since_boot ();