* [PATCH 1/5] mountd: fix export upcall failure in use_ipaddr case.
2012-05-02 21:56 use_ipaddr fixes version surely-that's-enough J. Bruce Fields
@ 2012-05-02 21:56 ` J. Bruce Fields
2012-05-08 14:07 ` Steve Dickson
2012-05-02 21:56 ` [PATCH 2/5] mountd: parse ip address earlier J. Bruce Fields
` (3 subsequent siblings)
4 siblings, 1 reply; 8+ messages in thread
From: J. Bruce Fields @ 2012-05-02 21:56 UTC (permalink / raw)
To: steved; +Cc: linux-nfs, Jeff Layton, NeilBrown, J. Bruce Fields, Chuck Lever
From: "J. Bruce Fields" <bfields@redhat.com>
After 0509d3428f523 "mountd: Replace "struct hostent" with "struct
addinfo"", the export upcall fails in the use_ipaddr case.
I think we never noticed because a) the use_ipaddr case is rarer than
the !use_ipaddr case, and b) the nfsd_fh upcall does a preemptive export
downcall that renders the nfsd export call unnecessary in some cases.
Cc: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
utils/mountd/cache.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index ac9cdbd..cf07b56 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -1064,6 +1064,7 @@ static void nfsd_export(FILE *f)
goto out;
ai = client_resolve(tmp->ai_addr);
freeaddrinfo(tmp);
+ if (!ai)
goto out;
}
--
1.7.7.6
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 1/5] mountd: fix export upcall failure in use_ipaddr case.
2012-05-02 21:56 ` [PATCH 1/5] mountd: fix export upcall failure in use_ipaddr case J. Bruce Fields
@ 2012-05-08 14:07 ` Steve Dickson
0 siblings, 0 replies; 8+ messages in thread
From: Steve Dickson @ 2012-05-08 14:07 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: linux-nfs, Jeff Layton, NeilBrown, Chuck Lever
On 05/02/2012 05:56 PM, J. Bruce Fields wrote:
> From: "J. Bruce Fields" <bfields@redhat.com>
>
> After 0509d3428f523 "mountd: Replace "struct hostent" with "struct
> addinfo"", the export upcall fails in the use_ipaddr case.
>
> I think we never noticed because a) the use_ipaddr case is rarer than
> the !use_ipaddr case, and b) the nfsd_fh upcall does a preemptive export
> downcall that renders the nfsd export call unnecessary in some cases.
>
> Cc: Chuck Lever <chuck.lever@oracle.com>
> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
All 5 committed....
steved.
> ---
> utils/mountd/cache.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
> index ac9cdbd..cf07b56 100644
> --- a/utils/mountd/cache.c
> +++ b/utils/mountd/cache.c
> @@ -1064,6 +1064,7 @@ static void nfsd_export(FILE *f)
> goto out;
> ai = client_resolve(tmp->ai_addr);
> freeaddrinfo(tmp);
> + if (!ai)
> goto out;
> }
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/5] mountd: parse ip address earlier
2012-05-02 21:56 use_ipaddr fixes version surely-that's-enough J. Bruce Fields
2012-05-02 21:56 ` [PATCH 1/5] mountd: fix export upcall failure in use_ipaddr case J. Bruce Fields
@ 2012-05-02 21:56 ` J. Bruce Fields
2012-05-02 21:56 ` [PATCH 3/5] mountd: add trivial helpers for client-matching J. Bruce Fields
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: J. Bruce Fields @ 2012-05-02 21:56 UTC (permalink / raw)
To: steved; +Cc: linux-nfs, Jeff Layton, NeilBrown, J. Bruce Fields
From: "J. Bruce Fields" <bfields@redhat.com>
I don't see the point of waiting to the last minute to parse the ip
address. If the client name isn't a legal ip address then this will
fail fairly quickly, so there's not much of a performance penalty.
Also, note the previous code incorrectly assumed client_resolve would
always return non-NULL.
Also factor out some common code.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
utils/mountd/cache.c | 40 ++++++++++++++++++++++------------------
1 files changed, 22 insertions(+), 18 deletions(-)
diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index cf07b56..d2ac258 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -495,6 +495,19 @@ static bool match_fsid(struct parsed_fsid *parsed, nfs_export *exp, char *path)
return false;
}
+struct addrinfo *lookup_client_addr(char *dom)
+{
+ struct addrinfo *ret;
+ struct addrinfo *tmp;
+
+ tmp = host_pton(dom);
+ if (tmp == NULL)
+ return NULL;
+ ret = client_resolve(tmp->ai_addr);
+ freeaddrinfo(tmp);
+ return ret;
+}
+
static void nfsd_fh(FILE *f)
{
/* request are:
@@ -538,6 +551,12 @@ static void nfsd_fh(FILE *f)
auth_reload();
+ if (use_ipaddr) {
+ ai = lookup_client_addr(dom);
+ if (!ai)
+ goto out;
+ }
+
/* Now determine export point for this fsid/domain */
for (i=0 ; i < MCL_MAXTYPES; i++) {
nfs_export *next_exp;
@@ -578,18 +597,8 @@ static void nfsd_fh(FILE *f)
if (!match_fsid(&parsed, exp, path))
continue;
- if (use_ipaddr) {
- if (ai == NULL) {
- struct addrinfo *tmp;
- tmp = host_pton(dom);
- if (tmp == NULL)
- goto out;
- ai = client_resolve(tmp->ai_addr);
- freeaddrinfo(tmp);
- }
- if (!client_check(exp->m_client, ai))
- continue;
- }
+ if (use_ipaddr && !client_check(exp->m_client, ai))
+ continue;
if (!found || subexport(&exp->m_export, found)) {
found = &exp->m_export;
free(found_path);
@@ -1058,12 +1067,7 @@ static void nfsd_export(FILE *f)
auth_reload();
if (use_ipaddr) {
- struct addrinfo *tmp;
- tmp = host_pton(dom);
- if (tmp == NULL)
- goto out;
- ai = client_resolve(tmp->ai_addr);
- freeaddrinfo(tmp);
+ ai = lookup_client_addr(dom);
if (!ai)
goto out;
}
--
1.7.7.6
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 3/5] mountd: add trivial helpers for client-matching
2012-05-02 21:56 use_ipaddr fixes version surely-that's-enough J. Bruce Fields
2012-05-02 21:56 ` [PATCH 1/5] mountd: fix export upcall failure in use_ipaddr case J. Bruce Fields
2012-05-02 21:56 ` [PATCH 2/5] mountd: parse ip address earlier J. Bruce Fields
@ 2012-05-02 21:56 ` J. Bruce Fields
2012-05-02 21:56 ` [PATCH 4/5] mountd: prepend '$' to make use_ipaddr clients self-describing J. Bruce Fields
2012-05-02 21:56 ` [PATCH 5/5] mountd: handle allocation failures in auth_unix_ip upcall J. Bruce Fields
4 siblings, 0 replies; 8+ messages in thread
From: J. Bruce Fields @ 2012-05-02 21:56 UTC (permalink / raw)
To: steved; +Cc: linux-nfs, Jeff Layton, NeilBrown, J. Bruce Fields
From: "J. Bruce Fields" <bfields@redhat.com>
Pull out a tiny bit of common logic from three functions.
Possibly minor overkill, but simplifies the next patch.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
utils/mountd/auth.c | 21 ++++++++++++++++++---
utils/mountd/cache.c | 12 ++----------
utils/mountd/mountd.h | 4 ++++
3 files changed, 24 insertions(+), 13 deletions(-)
diff --git a/utils/mountd/auth.c b/utils/mountd/auth.c
index ccc849a..1ed9a4b 100644
--- a/utils/mountd/auth.c
+++ b/utils/mountd/auth.c
@@ -131,6 +131,23 @@ get_client_hostname(const struct sockaddr *caller, struct addrinfo *ai,
return strdup("DEFAULT");
}
+bool ipaddr_client_matches(nfs_export *exp, struct addrinfo *ai)
+{
+ return client_check(exp->m_client, ai);
+}
+
+bool namelist_client_matches(nfs_export *exp, char *dom)
+{
+ return client_member(dom, exp->m_client->m_hostname);
+}
+
+bool client_matches(nfs_export *exp, char *dom, struct addrinfo *ai)
+{
+ if (use_ipaddr)
+ return ipaddr_client_matches(exp, ai);
+ return namelist_client_matches(exp, dom);
+}
+
/* return static nfs_export with details filled in */
static nfs_export *
auth_authenticate_newcache(const struct sockaddr *caller,
@@ -155,9 +172,7 @@ auth_authenticate_newcache(const struct sockaddr *caller,
for (exp = exportlist[i].p_head; exp; exp = exp->m_next) {
if (strcmp(path, exp->m_export.e_path))
continue;
- if (!use_ipaddr && !client_member(my_client.m_hostname, exp->m_client->m_hostname))
- continue;
- if (use_ipaddr && !client_check(exp->m_client, ai))
+ if (!client_matches(exp, my_client.m_hostname, ai))
continue;
break;
}
diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index d2ac258..6e01b2f 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -587,7 +587,7 @@ static void nfsd_fh(FILE *f)
next_exp = exp->m_next;
}
- if (!use_ipaddr && !client_member(dom, exp->m_client->m_hostname))
+ if (!use_ipaddr && !namelist_client_matches(exp, dom))
continue;
if (exp->m_export.e_mountpoint &&
!is_mountpoint(exp->m_export.e_mountpoint[0]?
@@ -597,7 +597,7 @@ static void nfsd_fh(FILE *f)
if (!match_fsid(&parsed, exp, path))
continue;
- if (use_ipaddr && !client_check(exp->m_client, ai))
+ if (use_ipaddr && !ipaddr_client_matches(exp, ai))
continue;
if (!found || subexport(&exp->m_export, found)) {
found = &exp->m_export;
@@ -751,14 +751,6 @@ static int path_matches(nfs_export *exp, char *path)
}
static int
-client_matches(nfs_export *exp, char *dom, struct addrinfo *ai)
-{
- if (use_ipaddr)
- return client_check(exp->m_client, ai);
- return client_member(dom, exp->m_client->m_hostname);
-}
-
-static int
export_matches(nfs_export *exp, char *dom, char *path, struct addrinfo *ai)
{
return path_matches(exp, path) && client_matches(exp, dom, ai);
diff --git a/utils/mountd/mountd.h b/utils/mountd/mountd.h
index 4c184d2..c969a27 100644
--- a/utils/mountd/mountd.h
+++ b/utils/mountd/mountd.h
@@ -56,4 +56,8 @@ struct nfs_fh_len *
cache_get_filehandle(nfs_export *exp, int len, char *p);
int cache_export(nfs_export *exp, char *path);
+bool ipaddr_client_matches(nfs_export *exp, struct addrinfo *ai);
+bool namelist_client_matches(nfs_export *exp, char *dom);
+bool client_matches(nfs_export *exp, char *dom, struct addrinfo *ai);
+
#endif /* MOUNTD_H */
--
1.7.7.6
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 4/5] mountd: prepend '$' to make use_ipaddr clients self-describing
2012-05-02 21:56 use_ipaddr fixes version surely-that's-enough J. Bruce Fields
` (2 preceding siblings ...)
2012-05-02 21:56 ` [PATCH 3/5] mountd: add trivial helpers for client-matching J. Bruce Fields
@ 2012-05-02 21:56 ` J. Bruce Fields
2012-05-02 21:56 ` [PATCH 5/5] mountd: handle allocation failures in auth_unix_ip upcall J. Bruce Fields
4 siblings, 0 replies; 8+ messages in thread
From: J. Bruce Fields @ 2012-05-02 21:56 UTC (permalink / raw)
To: steved; +Cc: linux-nfs, Jeff Layton, NeilBrown, J. Bruce Fields
From: "J. Bruce Fields" <bfields@redhat.com>
Mountd is responsible for filling three interrelated kernel caches:
- auth_unix_ip maps an incoming ip addresses to a "domain".
- nfsd_fh maps (domain, filehandle-fragment) pairs to paths.
- nfsd_export maps (domain, path) pairs to export options.
Note that each export is assocated with a "client" string--the part
before the parentheses in an /etc/export line--which may be a domain
name, a netgroup, etc.
The "domain" string in the above three caches may be either:
- in the !use_ipaddr case, a comma-separated list of client
strings.
- in the use_ipaddr case, an ip address.
In the former case, mountd does the hard work of matching an ip address
to the clients when doing the auth_unix_ip mapping. In the latter case,
it delays that until the nfsd_fh or nfsd_export upcall.
We're currently depending on being able to flush the kernel caches
completely when switching between the use_ipaddr and !use_ipaddr cases.
However, the kernel's cache-flushing doesn't really provide reliable
guarantees on return; it's still possible we could see nfsd_fh or
nfsd_export upcalls with the old domain-type after flushing.
So, instead, make the two domain types self-describing by prepending a
"$" in the use_ipaddr case.
Reviewed-by: NeilBrown <neilb@suse.de>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
utils/mountd/auth.c | 14 +++++++++++---
utils/mountd/cache.c | 12 ++++++++----
utils/mountd/mountd.h | 5 +++++
3 files changed, 24 insertions(+), 7 deletions(-)
diff --git a/utils/mountd/auth.c b/utils/mountd/auth.c
index 1ed9a4b..15da54c 100644
--- a/utils/mountd/auth.c
+++ b/utils/mountd/auth.c
@@ -112,15 +112,23 @@ auth_reload()
return counter;
}
+static char *get_client_ipaddr_name(const struct sockaddr *caller)
+{
+ char buf[INET6_ADDRSTRLEN + 1];
+
+ buf[0] = '$';
+ host_ntop(caller, buf + 1, sizeof(buf) - 1);
+ return strdup(buf);
+}
+
static char *
get_client_hostname(const struct sockaddr *caller, struct addrinfo *ai,
enum auth_error *error)
{
- char buf[INET6_ADDRSTRLEN];
char *n;
if (use_ipaddr)
- return strdup(host_ntop(caller, buf, sizeof(buf)));
+ return get_client_ipaddr_name(caller);
n = client_compose(ai);
*error = unknown_host;
if (!n)
@@ -143,7 +151,7 @@ bool namelist_client_matches(nfs_export *exp, char *dom)
bool client_matches(nfs_export *exp, char *dom, struct addrinfo *ai)
{
- if (use_ipaddr)
+ if (is_ipaddr_client(dom))
return ipaddr_client_matches(exp, ai);
return namelist_client_matches(exp, dom);
}
diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index 6e01b2f..6710eca 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -500,6 +500,8 @@ struct addrinfo *lookup_client_addr(char *dom)
struct addrinfo *ret;
struct addrinfo *tmp;
+ dom++; /* skip initial "$" */
+
tmp = host_pton(dom);
if (tmp == NULL)
return NULL;
@@ -551,7 +553,7 @@ static void nfsd_fh(FILE *f)
auth_reload();
- if (use_ipaddr) {
+ if (is_ipaddr_client(dom)) {
ai = lookup_client_addr(dom);
if (!ai)
goto out;
@@ -587,7 +589,8 @@ static void nfsd_fh(FILE *f)
next_exp = exp->m_next;
}
- if (!use_ipaddr && !namelist_client_matches(exp, dom))
+ if (!is_ipaddr_client(dom)
+ && !namelist_client_matches(exp, dom))
continue;
if (exp->m_export.e_mountpoint &&
!is_mountpoint(exp->m_export.e_mountpoint[0]?
@@ -597,7 +600,8 @@ static void nfsd_fh(FILE *f)
if (!match_fsid(&parsed, exp, path))
continue;
- if (use_ipaddr && !ipaddr_client_matches(exp, ai))
+ if (is_ipaddr_client(dom)
+ && !ipaddr_client_matches(exp, ai))
continue;
if (!found || subexport(&exp->m_export, found)) {
found = &exp->m_export;
@@ -1058,7 +1062,7 @@ static void nfsd_export(FILE *f)
auth_reload();
- if (use_ipaddr) {
+ if (is_ipaddr_client(dom)) {
ai = lookup_client_addr(dom);
if (!ai)
goto out;
diff --git a/utils/mountd/mountd.h b/utils/mountd/mountd.h
index c969a27..6d358a7 100644
--- a/utils/mountd/mountd.h
+++ b/utils/mountd/mountd.h
@@ -60,4 +60,9 @@ bool ipaddr_client_matches(nfs_export *exp, struct addrinfo *ai);
bool namelist_client_matches(nfs_export *exp, char *dom);
bool client_matches(nfs_export *exp, char *dom, struct addrinfo *ai);
+static inline bool is_ipaddr_client(char *dom)
+{
+ return dom[0] == '$';
+}
+
#endif /* MOUNTD_H */
--
1.7.7.6
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 5/5] mountd: handle allocation failures in auth_unix_ip upcall
2012-05-02 21:56 use_ipaddr fixes version surely-that's-enough J. Bruce Fields
` (3 preceding siblings ...)
2012-05-02 21:56 ` [PATCH 4/5] mountd: prepend '$' to make use_ipaddr clients self-describing J. Bruce Fields
@ 2012-05-02 21:56 ` J. Bruce Fields
4 siblings, 0 replies; 8+ messages in thread
From: J. Bruce Fields @ 2012-05-02 21:56 UTC (permalink / raw)
To: steved; +Cc: linux-nfs, Jeff Layton, NeilBrown, J. Bruce Fields
From: "J. Bruce Fields" <bfields@redhat.com>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
utils/mountd/cache.c | 12 +++++++++---
1 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index 6710eca..8f14032 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -84,7 +84,6 @@ static void auth_unix_ip(FILE *f)
char ipaddr[INET6_ADDRSTRLEN];
char *client = NULL;
struct addrinfo *tmp = NULL;
- struct addrinfo *ai = NULL;
if (readline(fileno(f), &lbuf, &lbuflen) != 1)
return;
@@ -107,12 +106,16 @@ static void auth_unix_ip(FILE *f)
/* addr is a valid, interesting address, find the domain name... */
if (!use_ipaddr) {
+ struct addrinfo *ai = NULL;
+
ai = client_resolve(tmp->ai_addr);
+ if (ai == NULL)
+ goto out;
client = client_compose(ai);
freeaddrinfo(ai);
+ if (!client)
+ goto out;
}
- freeaddrinfo(tmp);
-
qword_print(f, "nfsd");
qword_print(f, ipaddr);
qword_printuint(f, time(0) + DEFAULT_TTL);
@@ -124,6 +127,9 @@ static void auth_unix_ip(FILE *f)
xlog(D_CALL, "auth_unix_ip: client %p '%s'", client, client?client: "DEFAULT");
free(client);
+out:
+ freeaddrinfo(tmp);
+
}
static void auth_unix_gid(FILE *f)
--
1.7.7.6
^ permalink raw reply related [flat|nested] 8+ messages in thread