Signed-off-by: Jose Castillo <[email protected]>
---
utils/mountd/cache.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index 978698d..e1027f3 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -80,7 +80,7 @@ static void auth_unix_ip(FILE *f)
*/
char *cp;
char class[20];
- char ipaddr[INET6_ADDRSTRLEN];
+ char ipaddr[INET6_ADDRSTRLEN + 1];
char *client = NULL;
struct addrinfo *tmp = NULL;
if (readline(fileno(f), &lbuf, &lbuflen) != 1)
@@ -94,7 +94,7 @@ static void auth_unix_ip(FILE *f)
strcmp(class, "nfsd") != 0)
return;
- if (qword_get(&cp, ipaddr, sizeof(ipaddr)) <= 0)
+ if (qword_get(&cp, ipaddr, sizeof(ipaddr) - 1) <= 0)
return;
tmp = host_pton(ipaddr);
@@ -116,9 +116,11 @@ static void auth_unix_ip(FILE *f)
qword_print(f, "nfsd");
qword_print(f, ipaddr);
qword_printtimefrom(f, DEFAULT_TTL);
- if (use_ipaddr)
+ if (use_ipaddr) {
+ memmove(ipaddr + 1, ipaddr, strlen(ipaddr) + 1);
+ ipaddr[0] = '$';
qword_print(f, ipaddr);
- else if (client)
+ } else if (client)
qword_print(f, *client?client:"DEFAULT");
qword_eol(f);
xlog(D_CALL, "auth_unix_ip: client %p '%s'", client, client?client: "DEFAULT");
--
1.7.11.7
On 04/11/2013 01:27 PM, J. Bruce Fields wrote:
> On Wed, Apr 10, 2013 at 12:11:20PM +0100, Jose Castillo wrote:
>> On 04/09/2013 08:11 PM, J. Bruce Fields wrote:
>>> Could you explain a little more?
>>>
>>> I assume this is something I forgot to do as part of
>>> c2544b77566690ebec32a2d47c9249548b1a0941 "mountd: prepend '$'
>>> to make use_ipaddr clients self-describing" but I haven't
>>> thought about that in a while....
>>>
>>
>> Hello,
>>
>> Yes, sorry, a bit more of background: We found this problem
>> because NFS clients to a RHEL6 NFS server were experiencing
>> periods of ESTALE errors after being mounted and initially
>> working successfully. Tests were run which snapshotted the
>> nfs/sunrpc caches before and after the issue, and it was found
>> that the '$' character at the beginning of the ID strings, used
>> when in use_ipaddr mode, was getting lost:
>>
>> GOOD, while mount was working: #class IP domain #
>> expiry=1362416801 refcnt=2 flags=1 nfsd 1.2.3.4 $1.2.3.4
>>
>> BAD, after mount started returning ESTALE: #class IP domain #
>> expiry=1362418641 refcnt=2 flags=1 nfsd 1.2.3.4 1.2.3.4
>>
>> This would then cause the export checks to fail by passing
>> '1.2.3.4' instead of '$1.2.3.4' up to rpc.mountd.
>
> Oh, I see--and this stuff probably never worked for v4 either since
> it can't depend on mountd to fill the cache at all....
>
> Thanks!
>
> Acked-by: J. Bruce Fields <[email protected]>
>
> (Could you just add the extra details to the commit message and
> resend to steved?
>
Done
> Also, is there a RHEL bug for this?)
>
Yes, this one:
https://bugzilla.redhat.com/show_bug.cgi?id=920293
Bug 920293 - files on nfs server's exports start returning ESTALE with
a large number of exports and netgroups
Thank you!
> --b.
>
>>
>> The problem appears to be in the auth_unix_ip() function when
>> renewing the auth.unix.ip cache entry. It would fail to add the
>> '$' character back to the beginning of the string used for the
>> domain string, breaking the use_ipaddr mode.
>>
>> The issue is reliably repeatable with large numbers of exports
>> and netgroups to cause rpc.mountd to enter use_ipaddr mode.
>>
>> Steps to reproduce:
>>
>> 1. Export several hundred exports each with its own unique
>> netgroup 2. Mount from a client (NFS3). check
>> /proc/net/rpc/auth.unix.ip/content to ensure the '$' character
>> is there from use_ipaddr mode. 3. Wait long enough for the entry
>> to expire (1 hour was used), and try and access a file from the
>> client. It returned ESTALE and the
>> /proc/net/rpc/auth.unix.ip/content was now missing the '$'
>> character.
>>
>> Actual results: Mount returns ESTALE on the client after being
>> mounted for an extended period of time.
>>
>> Expected results: File access should succeed on the mount.
>>
>> The patch was tested in a reproduction environment, and the
>> issue could no longer be reproduced.
>>
>>> --b.
>>>
>>> On Tue, Apr 09, 2013 at 04:54:59PM +0100, Jose Castillo wrote:
>>>> Signed-off-by: Jose Castillo <[email protected]> ---
>>>> utils/mountd/cache.c | 10 ++++++---- 1 file changed, 6
>>>> insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
>>>> index 978698d..e1027f3 100644 --- a/utils/mountd/cache.c +++
>>>> b/utils/mountd/cache.c @@ -80,7 +80,7 @@ static void
>>>> auth_unix_ip(FILE *f) */ char *cp; char class[20]; - char
>>>> ipaddr[INET6_ADDRSTRLEN]; + char ipaddr[INET6_ADDRSTRLEN +
>>>> 1]; char *client = NULL; struct addrinfo *tmp = NULL; if
>>>> (readline(fileno(f), &lbuf, &lbuflen) != 1) @@ -94,7 +94,7
>>>> @@ static void auth_unix_ip(FILE *f) strcmp(class, "nfsd") !=
>>>> 0) return;
>>>>
>>>> - if (qword_get(&cp, ipaddr, sizeof(ipaddr)) <= 0) + if
>>>> (qword_get(&cp, ipaddr, sizeof(ipaddr) - 1) <= 0) return;
>>>>
>>>> tmp = host_pton(ipaddr); @@ -116,9 +116,11 @@ static void
>>>> auth_unix_ip(FILE *f) qword_print(f, "nfsd"); qword_print(f,
>>>> ipaddr); qword_printtimefrom(f, DEFAULT_TTL); - if
>>>> (use_ipaddr) + if (use_ipaddr) { + memmove(ipaddr + 1,
>>>> ipaddr, strlen(ipaddr) + 1); + ipaddr[0] = '$';
>>>> qword_print(f, ipaddr); - else if (client) + } else if
>>>> (client) qword_print(f, *client?client:"DEFAULT");
>>>> qword_eol(f); xlog(D_CALL, "auth_unix_ip: client %p '%s'",
>>>> client, client?client: "DEFAULT"); -- 1.7.11.7
>>>>
>>>> -- To unsubscribe from this list: send the line "unsubscribe
>>>> linux-nfs" in the body of a message to
>>>> [email protected] More majordomo info at
>>>> http://vger.kernel.org/majordomo-info.html
>>
>>
On 09/04/13 11:54, Jose Castillo wrote:
> Signed-off-by: Jose Castillo <[email protected]>
Committed...
steved.
> ---
> utils/mountd/cache.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
> index 978698d..e1027f3 100644
> --- a/utils/mountd/cache.c
> +++ b/utils/mountd/cache.c
> @@ -80,7 +80,7 @@ static void auth_unix_ip(FILE *f)
> */
> char *cp;
> char class[20];
> - char ipaddr[INET6_ADDRSTRLEN];
> + char ipaddr[INET6_ADDRSTRLEN + 1];
> char *client = NULL;
> struct addrinfo *tmp = NULL;
> if (readline(fileno(f), &lbuf, &lbuflen) != 1)
> @@ -94,7 +94,7 @@ static void auth_unix_ip(FILE *f)
> strcmp(class, "nfsd") != 0)
> return;
>
> - if (qword_get(&cp, ipaddr, sizeof(ipaddr)) <= 0)
> + if (qword_get(&cp, ipaddr, sizeof(ipaddr) - 1) <= 0)
> return;
>
> tmp = host_pton(ipaddr);
> @@ -116,9 +116,11 @@ static void auth_unix_ip(FILE *f)
> qword_print(f, "nfsd");
> qword_print(f, ipaddr);
> qword_printtimefrom(f, DEFAULT_TTL);
> - if (use_ipaddr)
> + if (use_ipaddr) {
> + memmove(ipaddr + 1, ipaddr, strlen(ipaddr) + 1);
> + ipaddr[0] = '$';
> qword_print(f, ipaddr);
> - else if (client)
> + } else if (client)
> qword_print(f, *client?client:"DEFAULT");
> qword_eol(f);
> xlog(D_CALL, "auth_unix_ip: client %p '%s'", client, client?client: "DEFAULT");
>
On Wed, Apr 10, 2013 at 12:11:20PM +0100, Jose Castillo wrote:
> On 04/09/2013 08:11 PM, J. Bruce Fields wrote:
> > Could you explain a little more?
> >
> > I assume this is something I forgot to do as part of
> > c2544b77566690ebec32a2d47c9249548b1a0941 "mountd: prepend '$' to
> > make use_ipaddr clients self-describing" but I haven't thought
> > about that in a while....
> >
>
> Hello,
>
> Yes, sorry, a bit more of background: We found this problem because
> NFS clients to a RHEL6 NFS server were experiencing periods of ESTALE
> errors after being mounted and initially working successfully.
> Tests were run which snapshotted the nfs/sunrpc caches before and
> after the issue, and it was found that the '$' character at the
> beginning of the ID strings, used when in use_ipaddr mode, was getting
> lost:
>
> GOOD, while mount was working:
> #class IP domain
> # expiry=1362416801 refcnt=2 flags=1
> nfsd 1.2.3.4 $1.2.3.4
>
> BAD, after mount started returning ESTALE:
> #class IP domain
> # expiry=1362418641 refcnt=2 flags=1
> nfsd 1.2.3.4 1.2.3.4
>
> This would then cause the export checks to fail by passing '1.2.3.4'
> instead of '$1.2.3.4' up to rpc.mountd.
Oh, I see--and this stuff probably never worked for v4 either since it
can't depend on mountd to fill the cache at all....
Thanks!
Acked-by: J. Bruce Fields <[email protected]>
(Could you just add the extra details to the commit message and resend
to steved?
Also, is there a RHEL bug for this?)
--b.
>
> The problem appears to be in the auth_unix_ip() function when renewing
> the auth.unix.ip cache entry. It would fail to add the '$' character
> back to the beginning of the string used for the domain string,
> breaking the use_ipaddr mode.
>
> The issue is reliably repeatable with large numbers of exports and
> netgroups to cause rpc.mountd to enter use_ipaddr mode.
>
> Steps to reproduce:
>
> 1. Export several hundred exports each with its own unique netgroup
> 2. Mount from a client (NFS3). check
> /proc/net/rpc/auth.unix.ip/content to ensure the '$' character is
> there from use_ipaddr mode.
> 3. Wait long enough for the entry to expire (1 hour was used), and try
> and access a file from the client. It returned ESTALE and the
> /proc/net/rpc/auth.unix.ip/content was now missing the '$' character.
>
> Actual results:
> Mount returns ESTALE on the client after being mounted for an extended
> period of time.
>
> Expected results:
> File access should succeed on the mount.
>
> The patch was tested in a reproduction environment, and the issue
> could no longer be reproduced.
>
> > --b.
> >
> > On Tue, Apr 09, 2013 at 04:54:59PM +0100, Jose Castillo wrote:
> >> Signed-off-by: Jose Castillo <[email protected]> ---
> >> utils/mountd/cache.c | 10 ++++++---- 1 file changed, 6
> >> insertions(+), 4 deletions(-)
> >>
> >> diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c index
> >> 978698d..e1027f3 100644 --- a/utils/mountd/cache.c +++
> >> b/utils/mountd/cache.c @@ -80,7 +80,7 @@ static void
> >> auth_unix_ip(FILE *f) */ char *cp; char class[20]; - char
> >> ipaddr[INET6_ADDRSTRLEN]; + char ipaddr[INET6_ADDRSTRLEN + 1];
> >> char *client = NULL; struct addrinfo *tmp = NULL; if
> >> (readline(fileno(f), &lbuf, &lbuflen) != 1) @@ -94,7 +94,7 @@
> >> static void auth_unix_ip(FILE *f) strcmp(class, "nfsd") != 0)
> >> return;
> >>
> >> - if (qword_get(&cp, ipaddr, sizeof(ipaddr)) <= 0) + if
> >> (qword_get(&cp, ipaddr, sizeof(ipaddr) - 1) <= 0) return;
> >>
> >> tmp = host_pton(ipaddr); @@ -116,9 +116,11 @@ static void
> >> auth_unix_ip(FILE *f) qword_print(f, "nfsd"); qword_print(f,
> >> ipaddr); qword_printtimefrom(f, DEFAULT_TTL); - if (use_ipaddr) +
> >> if (use_ipaddr) { + memmove(ipaddr + 1, ipaddr, strlen(ipaddr) +
> >> 1); + ipaddr[0] = '$'; qword_print(f, ipaddr); - else if
> >> (client) + } else if (client) qword_print(f,
> >> *client?client:"DEFAULT"); qword_eol(f); xlog(D_CALL,
> >> "auth_unix_ip: client %p '%s'", client, client?client:
> >> "DEFAULT"); -- 1.7.11.7
> >>
> >> -- To unsubscribe from this list: send the line "unsubscribe
> >> linux-nfs" in the body of a message to [email protected]
> >> More majordomo info at
> >> http://vger.kernel.org/majordomo-info.html
>
>
On 04/09/2013 08:11 PM, J. Bruce Fields wrote:
> Could you explain a little more?
>
> I assume this is something I forgot to do as part of
> c2544b77566690ebec32a2d47c9249548b1a0941 "mountd: prepend '$' to
> make use_ipaddr clients self-describing" but I haven't thought
> about that in a while....
>
Hello,
Yes, sorry, a bit more of background: We found this problem because
NFS clients to a RHEL6 NFS server were experiencing periods of ESTALE
errors after being mounted and initially working successfully.
Tests were run which snapshotted the nfs/sunrpc caches before and
after the issue, and it was found that the '$' character at the
beginning of the ID strings, used when in use_ipaddr mode, was getting
lost:
GOOD, while mount was working:
#class IP domain
# expiry=1362416801 refcnt=2 flags=1
nfsd 1.2.3.4 $1.2.3.4
BAD, after mount started returning ESTALE:
#class IP domain
# expiry=1362418641 refcnt=2 flags=1
nfsd 1.2.3.4 1.2.3.4
This would then cause the export checks to fail by passing '1.2.3.4'
instead of '$1.2.3.4' up to rpc.mountd.
The problem appears to be in the auth_unix_ip() function when renewing
the auth.unix.ip cache entry. It would fail to add the '$' character
back to the beginning of the string used for the domain string,
breaking the use_ipaddr mode.
The issue is reliably repeatable with large numbers of exports and
netgroups to cause rpc.mountd to enter use_ipaddr mode.
Steps to reproduce:
1. Export several hundred exports each with its own unique netgroup
2. Mount from a client (NFS3). check
/proc/net/rpc/auth.unix.ip/content to ensure the '$' character is
there from use_ipaddr mode.
3. Wait long enough for the entry to expire (1 hour was used), and try
and access a file from the client. It returned ESTALE and the
/proc/net/rpc/auth.unix.ip/content was now missing the '$' character.
Actual results:
Mount returns ESTALE on the client after being mounted for an extended
period of time.
Expected results:
File access should succeed on the mount.
The patch was tested in a reproduction environment, and the issue
could no longer be reproduced.
> --b.
>
> On Tue, Apr 09, 2013 at 04:54:59PM +0100, Jose Castillo wrote:
>> Signed-off-by: Jose Castillo <[email protected]> ---
>> utils/mountd/cache.c | 10 ++++++---- 1 file changed, 6
>> insertions(+), 4 deletions(-)
>>
>> diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c index
>> 978698d..e1027f3 100644 --- a/utils/mountd/cache.c +++
>> b/utils/mountd/cache.c @@ -80,7 +80,7 @@ static void
>> auth_unix_ip(FILE *f) */ char *cp; char class[20]; - char
>> ipaddr[INET6_ADDRSTRLEN]; + char ipaddr[INET6_ADDRSTRLEN + 1];
>> char *client = NULL; struct addrinfo *tmp = NULL; if
>> (readline(fileno(f), &lbuf, &lbuflen) != 1) @@ -94,7 +94,7 @@
>> static void auth_unix_ip(FILE *f) strcmp(class, "nfsd") != 0)
>> return;
>>
>> - if (qword_get(&cp, ipaddr, sizeof(ipaddr)) <= 0) + if
>> (qword_get(&cp, ipaddr, sizeof(ipaddr) - 1) <= 0) return;
>>
>> tmp = host_pton(ipaddr); @@ -116,9 +116,11 @@ static void
>> auth_unix_ip(FILE *f) qword_print(f, "nfsd"); qword_print(f,
>> ipaddr); qword_printtimefrom(f, DEFAULT_TTL); - if (use_ipaddr) +
>> if (use_ipaddr) { + memmove(ipaddr + 1, ipaddr, strlen(ipaddr) +
>> 1); + ipaddr[0] = '$'; qword_print(f, ipaddr); - else if
>> (client) + } else if (client) qword_print(f,
>> *client?client:"DEFAULT"); qword_eol(f); xlog(D_CALL,
>> "auth_unix_ip: client %p '%s'", client, client?client:
>> "DEFAULT"); -- 1.7.11.7
>>
>> -- To unsubscribe from this list: send the line "unsubscribe
>> linux-nfs" in the body of a message to [email protected]
>> More majordomo info at
>> http://vger.kernel.org/majordomo-info.html
Could you explain a little more?
I assume this is something I forgot to do as part of
c2544b77566690ebec32a2d47c9249548b1a0941 "mountd: prepend '$' to make
use_ipaddr clients self-describing" but I haven't thought about that in
a while....
--b.
On Tue, Apr 09, 2013 at 04:54:59PM +0100, Jose Castillo wrote:
> Signed-off-by: Jose Castillo <[email protected]>
> ---
> utils/mountd/cache.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
> index 978698d..e1027f3 100644
> --- a/utils/mountd/cache.c
> +++ b/utils/mountd/cache.c
> @@ -80,7 +80,7 @@ static void auth_unix_ip(FILE *f)
> */
> char *cp;
> char class[20];
> - char ipaddr[INET6_ADDRSTRLEN];
> + char ipaddr[INET6_ADDRSTRLEN + 1];
> char *client = NULL;
> struct addrinfo *tmp = NULL;
> if (readline(fileno(f), &lbuf, &lbuflen) != 1)
> @@ -94,7 +94,7 @@ static void auth_unix_ip(FILE *f)
> strcmp(class, "nfsd") != 0)
> return;
>
> - if (qword_get(&cp, ipaddr, sizeof(ipaddr)) <= 0)
> + if (qword_get(&cp, ipaddr, sizeof(ipaddr) - 1) <= 0)
> return;
>
> tmp = host_pton(ipaddr);
> @@ -116,9 +116,11 @@ static void auth_unix_ip(FILE *f)
> qword_print(f, "nfsd");
> qword_print(f, ipaddr);
> qword_printtimefrom(f, DEFAULT_TTL);
> - if (use_ipaddr)
> + if (use_ipaddr) {
> + memmove(ipaddr + 1, ipaddr, strlen(ipaddr) + 1);
> + ipaddr[0] = '$';
> qword_print(f, ipaddr);
> - else if (client)
> + } else if (client)
> qword_print(f, *client?client:"DEFAULT");
> qword_eol(f);
> xlog(D_CALL, "auth_unix_ip: client %p '%s'", client, client?client: "DEFAULT");
> --
> 1.7.11.7
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html