From a4846750f090702e2fb848ac4fe5827bcef34060 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 4 Dec 2008 14:20:23 -0500 Subject: NSM: Use C99 structure initializer to initialize nsm_args Clean up: Use a C99 structure initializer instead of open-coding the initialization of nsm_args. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/lockd/mon.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'fs/lockd/mon.c') diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c index ffd3461f75e..6f6ff410341 100644 --- a/fs/lockd/mon.c +++ b/fs/lockd/mon.c @@ -37,7 +37,13 @@ nsm_mon_unmon(struct nsm_handle *nsm, u32 proc, struct nsm_res *res) { struct rpc_clnt *clnt; int status; - struct nsm_args args; + struct nsm_args args = { + .addr = nsm_addr_in(nsm)->sin_addr.s_addr, + .prog = NLM_PROGRAM, + .vers = 3, + .proc = NLMPROC_NSM_NOTIFY, + .mon_name = nsm->sm_name, + }; struct rpc_message msg = { .rpc_argp = &args, .rpc_resp = res, @@ -49,12 +55,6 @@ nsm_mon_unmon(struct nsm_handle *nsm, u32 proc, struct nsm_res *res) goto out; } - memset(&args, 0, sizeof(args)); - args.mon_name = nsm->sm_name; - args.addr = nsm_addr_in(nsm)->sin_addr.s_addr; - args.prog = NLM_PROGRAM; - args.vers = 3; - args.proc = NLMPROC_NSM_NOTIFY; memset(res, 0, sizeof(*res)); msg.rpc_proc = &clnt->cl_procinfo[proc]; -- cgit v1.2.3 From 5acf43155d1bcc412d892c73f64044f9a826cde6 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 4 Dec 2008 14:20:31 -0500 Subject: NSM: convert printk(KERN_DEBUG) to a dprintk() Clean up: make the printk(KERN_DEBUG) in nsm_mon_unmon() a dprintk, and add another dprintk to note if creating an RPC client for the upcall failed. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/lockd/mon.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'fs/lockd/mon.c') diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c index 6f6ff410341..497dfea02e8 100644 --- a/fs/lockd/mon.c +++ b/fs/lockd/mon.c @@ -52,6 +52,8 @@ nsm_mon_unmon(struct nsm_handle *nsm, u32 proc, struct nsm_res *res) clnt = nsm_create(); if (IS_ERR(clnt)) { status = PTR_ERR(clnt); + dprintk("lockd: failed to create NSM upcall transport, " + "status=%d\n", status); goto out; } @@ -60,8 +62,8 @@ nsm_mon_unmon(struct nsm_handle *nsm, u32 proc, struct nsm_res *res) msg.rpc_proc = &clnt->cl_procinfo[proc]; status = rpc_call_sync(clnt, &msg, 0); if (status < 0) - printk(KERN_DEBUG "nsm_mon_unmon: rpc failed, status=%d\n", - status); + dprintk("lockd: NSM upcall RPC failed, status=%d\n", + status); else status = 0; rpc_shutdown_client(clnt); -- cgit v1.2.3 From 29ed1407ed81086b778ebf12145b048ac3f7e10e Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 4 Dec 2008 14:20:46 -0500 Subject: NSM: Support IPv6 version of mon_name The "mon_name" argument of the NSMPROC_MON and NSMPROC_UNMON upcalls is a string that contains the hostname or IP address of the remote peer to be notified when this host has rebooted. The sm-notify command uses this identifier to contact the peer when we reboot, so it must be either a well-qualified DNS hostname or a presentation format IP address string. When the "nsm_use_hostnames" sysctl is set to zero, the kernel's NSM provides a presentation format IP address in the "mon_name" argument. Otherwise, the "caller_name" argument from NLM requests is used, which is usually just the DNS hostname of the peer. To support IPv6 addresses for the mon_name argument, we use the nsm_handle's address eye-catcher, which already contains an appropriate presentation format address string. Using the eye-catcher string obviates the need to use a large buffer on the stack to form the presentation address string for the upcall. This patch also addresses a subtle bug. An NSMPROC_MON request and the subsequent NSMPROC_UNMON request for the same peer are required to use the same value for the "mon_name" argument. Otherwise, rpc.statd's NSMPROC_UNMON processing cannot locate the database entry for that peer and remove it. If the setting of nsm_use_hostnames is changed between the time the kernel sends an NSMPROC_MON request and the time it sends the NSMPROC_UNMON request for the same peer, the "mon_name" argument for these two requests may not be the same. This is because the value of "mon_name" is currently chosen at the moment the call is made based on the setting of nsm_use_hostnames To ensure both requests pass identical contents in the "mon_name" argument, we now select which string to use for the argument in the nsm_monitor() function. A pointer to this string is saved in the nsm_handle so it can be used for a subsequent NSMPROC_UNMON upcall. NB: There are other potential problems, such as how nlm_host_rebooted() might behave if nsm_use_hostnames were changed while hosts are still being monitored. This patch does not attempt to address those problems. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/lockd/mon.c | 27 ++++++++------------------- 1 file changed, 8 insertions(+), 19 deletions(-) (limited to 'fs/lockd/mon.c') diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c index 497dfea02e8..a606fbbf804 100644 --- a/fs/lockd/mon.c +++ b/fs/lockd/mon.c @@ -18,8 +18,6 @@ #define NLMDBG_FACILITY NLMDBG_MONITOR -#define XDR_ADDRBUF_LEN (20) - static struct rpc_clnt * nsm_create(void); static struct rpc_program nsm_program; @@ -42,7 +40,7 @@ nsm_mon_unmon(struct nsm_handle *nsm, u32 proc, struct nsm_res *res) .prog = NLM_PROGRAM, .vers = 3, .proc = NLMPROC_NSM_NOTIFY, - .mon_name = nsm->sm_name, + .mon_name = nsm->sm_mon_name, }; struct rpc_message msg = { .rpc_argp = &args, @@ -87,6 +85,12 @@ nsm_monitor(struct nlm_host *host) if (nsm->sm_monitored) return 0; + /* + * Choose whether to record the caller_name or IP address of + * this peer in the local rpc.statd's database. + */ + nsm->sm_mon_name = nsm_use_hostnames ? nsm->sm_name : nsm->sm_addrbuf; + status = nsm_mon_unmon(nsm, SM_MON, &res); if (status < 0 || res.status != 0) @@ -167,25 +171,10 @@ static __be32 *xdr_encode_nsm_string(__be32 *p, char *string) /* * "mon_name" specifies the host to be monitored. - * - * Linux uses a text version of the IP address of the remote - * host as the host identifier (the "mon_name" argument). - * - * Linux statd always looks up the canonical hostname first for - * whatever remote hostname it receives, so this works alright. */ static __be32 *xdr_encode_mon_name(__be32 *p, struct nsm_args *argp) { - char buffer[XDR_ADDRBUF_LEN + 1]; - char *name = argp->mon_name; - - if (!nsm_use_hostnames) { - snprintf(buffer, XDR_ADDRBUF_LEN, - "%pI4", &argp->addr); - name = buffer; - } - - return xdr_encode_nsm_string(p, name); + return xdr_encode_nsm_string(p, argp->mon_name); } /* -- cgit v1.2.3 From 9fee49024ed19d849413df4ab6ec1a1a60aaae94 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 4 Dec 2008 14:20:53 -0500 Subject: NSM: Use sm_name instead of h_name in nsm_monitor() and nsm_unmonitor() Clean up: Use the sm_name field for reporting the hostname in nsm_monitor() and nsm_unmonitor(), just as the other functions in fs/lockd/mon.c do. The h_name field is just a copy of the sm_name pointer. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/lockd/mon.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'fs/lockd/mon.c') diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c index a606fbbf804..697bdcdd20c 100644 --- a/fs/lockd/mon.c +++ b/fs/lockd/mon.c @@ -79,7 +79,7 @@ nsm_monitor(struct nlm_host *host) struct nsm_res res; int status; - dprintk("lockd: nsm_monitor(%s)\n", host->h_name); + dprintk("lockd: nsm_monitor(%s)\n", nsm->sm_name); BUG_ON(nsm == NULL); if (nsm->sm_monitored) @@ -94,7 +94,7 @@ nsm_monitor(struct nlm_host *host) status = nsm_mon_unmon(nsm, SM_MON, &res); if (status < 0 || res.status != 0) - printk(KERN_NOTICE "lockd: cannot monitor %s\n", host->h_name); + printk(KERN_NOTICE "lockd: cannot monitor %s\n", nsm->sm_name); else nsm->sm_monitored = 1; return status; @@ -116,12 +116,12 @@ nsm_unmonitor(struct nlm_host *host) if (atomic_read(&nsm->sm_count) == 1 && nsm->sm_monitored && !nsm->sm_sticky) { - dprintk("lockd: nsm_unmonitor(%s)\n", host->h_name); + dprintk("lockd: nsm_unmonitor(%s)\n", nsm->sm_name); status = nsm_mon_unmon(nsm, SM_UNMON, &res); if (status < 0) printk(KERN_NOTICE "lockd: cannot unmonitor %s\n", - host->h_name); + nsm->sm_name); else nsm->sm_monitored = 0; } -- cgit v1.2.3 From 5bc74bef7c9b652f0f2aa9c5a8d5ac86881aba79 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 4 Dec 2008 14:21:08 -0500 Subject: NSM: Remove BUG_ON() in nsm_monitor() Clean up: Remove the BUG_ON() invocation in nsm_monitor(). It's not likely that nsm_monitor() is ever called with a NULL host pointer, and the code will die anyway if host is NULL. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/lockd/mon.c | 1 - 1 file changed, 1 deletion(-) (limited to 'fs/lockd/mon.c') diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c index 697bdcdd20c..bb5fc1bb37f 100644 --- a/fs/lockd/mon.c +++ b/fs/lockd/mon.c @@ -80,7 +80,6 @@ nsm_monitor(struct nlm_host *host) int status; dprintk("lockd: nsm_monitor(%s)\n", nsm->sm_name); - BUG_ON(nsm == NULL); if (nsm->sm_monitored) return 0; -- cgit v1.2.3 From 5d254b119823658cc318f88589c6c426b3d0a153 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 4 Dec 2008 14:21:15 -0500 Subject: NSM: Make sure to return an error if the SM_MON call result is not zero The nsm_monitor() function reports an error and does not set sm_monitored if the SM_MON upcall reply has a non-zero result code, but nsm_monitor() does not return an error to its caller in this case. Since sm_monitored is not set, the upcall is retried when the next NLM request invokes nsm_monitor(). However, that may not come for a while. In the meantime, at least one NLM request will potentially proceed without the peer being monitored properly. Have nsm_monitor() return an error if the result code is non-zero. This will cause all NLM requests to fail immediately if the upcall completed successfully but rpc.statd returned an error. This may be inconvenient in some cases (for example if rpc.statd cannot complete a proper DNS reverse lookup of the hostname), but will make the reboot monitoring service more robust by forcing such issues to be corrected by an admin. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/lockd/mon.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'fs/lockd/mon.c') diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c index bb5fc1bb37f..07e16b81498 100644 --- a/fs/lockd/mon.c +++ b/fs/lockd/mon.c @@ -91,8 +91,9 @@ nsm_monitor(struct nlm_host *host) nsm->sm_mon_name = nsm_use_hostnames ? nsm->sm_name : nsm->sm_addrbuf; status = nsm_mon_unmon(nsm, SM_MON, &res); - - if (status < 0 || res.status != 0) + if (res.status != 0) + status = -EIO; + if (status < 0) printk(KERN_NOTICE "lockd: cannot monitor %s\n", nsm->sm_name); else nsm->sm_monitored = 1; -- cgit v1.2.3 From 1e49323c4ab044d05bbc68cf13cadcbd4372468c Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 4 Dec 2008 14:21:24 -0500 Subject: NLM: Move the public declaration of nsm_monitor() to lockd.h Clean up. Make the nlm_host argument "const," and move the public declaration to lockd.h with other NSM public function (nsm_release, eg) and global variable declarations. Add a documenting comment. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/lockd/mon.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) (limited to 'fs/lockd/mon.c') diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c index 07e16b81498..aaaa08e7ae7 100644 --- a/fs/lockd/mon.c +++ b/fs/lockd/mon.c @@ -69,11 +69,18 @@ nsm_mon_unmon(struct nsm_handle *nsm, u32 proc, struct nsm_res *res) return status; } -/* - * Set up monitoring of a remote host +/** + * nsm_monitor - Notify a peer in case we reboot + * @host: pointer to nlm_host of peer to notify + * + * If this peer is not already monitored, this function sends an + * upcall to the local rpc.statd to record the name/address of + * the peer to notify in case we reboot. + * + * Returns zero if the peer is monitored by the local rpc.statd; + * otherwise a negative errno value is returned. */ -int -nsm_monitor(struct nlm_host *host) +int nsm_monitor(const struct nlm_host *host) { struct nsm_handle *nsm = host->h_nsmhandle; struct nsm_res res; -- cgit v1.2.3 From c8c23c423dec49cb439697d3dc714e1500ff1610 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 4 Dec 2008 14:21:31 -0500 Subject: NSM: Release nsmhandle in nlm_destroy_host The nsm_handle's reference count is bumped in nlm_lookup_host(). It should be decremented in nlm_destroy_host() to make it easier to see the balance of these two operations. Move the nsm_release() call to fs/lockd/host.c. The h_nsmhandle pointer is set in nlm_lookup_host(), and never cleared. The nlm_destroy_host() function is never called for the same nlm_host twice, so h_nsmhandle won't ever be NULL when nsm_unmonitor() is called. All references to the nlm_host are gone before it is freed. We can skip making h_nsmhandle NULL just before the nlm_host is deallocated. It's also likely we can remove the h_nsmhandle NULL check in nlmsvc_is_client() as well, but we can do that later when rearchitect- ing the nlm_host cache. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/lockd/mon.c | 5 ----- 1 file changed, 5 deletions(-) (limited to 'fs/lockd/mon.c') diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c index aaaa08e7ae7..15fab22db02 100644 --- a/fs/lockd/mon.c +++ b/fs/lockd/mon.c @@ -117,10 +117,6 @@ nsm_unmonitor(struct nlm_host *host) struct nsm_res res; int status = 0; - if (nsm == NULL) - return 0; - host->h_nsmhandle = NULL; - if (atomic_read(&nsm->sm_count) == 1 && nsm->sm_monitored && !nsm->sm_sticky) { dprintk("lockd: nsm_unmonitor(%s)\n", nsm->sm_name); @@ -132,7 +128,6 @@ nsm_unmonitor(struct nlm_host *host) else nsm->sm_monitored = 0; } - nsm_release(nsm); return status; } -- cgit v1.2.3 From 356c3eb466fd1a12afd6448d90fba3922836e5f1 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 4 Dec 2008 14:21:38 -0500 Subject: NLM: Move the public declaration of nsm_unmonitor() to lockd.h Clean up. Make the nlm_host argument "const," and move the public declaration to lockd.h. Add a documenting comment. Bruce observed that nsm_unmonitor()'s only caller doesn't care about its return code, so make nsm_unmonitor() return void. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/lockd/mon.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) (limited to 'fs/lockd/mon.c') diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c index 15fab22db02..d61cdc61cb5 100644 --- a/fs/lockd/mon.c +++ b/fs/lockd/mon.c @@ -107,15 +107,19 @@ int nsm_monitor(const struct nlm_host *host) return status; } -/* - * Cease to monitor remote host +/** + * nsm_unmonitor - Unregister peer notification + * @host: pointer to nlm_host of peer to stop monitoring + * + * If this peer is monitored, this function sends an upcall to + * tell the local rpc.statd not to send this peer a notification + * when we reboot. */ -int -nsm_unmonitor(struct nlm_host *host) +void nsm_unmonitor(const struct nlm_host *host) { struct nsm_handle *nsm = host->h_nsmhandle; struct nsm_res res; - int status = 0; + int status; if (atomic_read(&nsm->sm_count) == 1 && nsm->sm_monitored && !nsm->sm_sticky) { @@ -128,7 +132,6 @@ nsm_unmonitor(struct nlm_host *host) else nsm->sm_monitored = 0; } - return status; } /* -- cgit v1.2.3 From 0c7aef4569f8680951b7dee01dddffb9d2f809ff Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 4 Dec 2008 14:21:46 -0500 Subject: NSM: Check result of SM_UNMON upcall Make sure any error returned by rpc.statd during an SM_UNMON call is reported rather than ignored completely. There isn't much to do with such an error, but we should log it in any case. Similar to a recent change to nsm_monitor(). Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/lockd/mon.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'fs/lockd/mon.c') diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c index d61cdc61cb5..3bb71e1b1e1 100644 --- a/fs/lockd/mon.c +++ b/fs/lockd/mon.c @@ -126,6 +126,8 @@ void nsm_unmonitor(const struct nlm_host *host) dprintk("lockd: nsm_unmonitor(%s)\n", nsm->sm_name); status = nsm_mon_unmon(nsm, SM_UNMON, &res); + if (res.status != 0) + status = -EIO; if (status < 0) printk(KERN_NOTICE "lockd: cannot unmonitor %s\n", nsm->sm_name); -- cgit v1.2.3 From 9c1bfd037f7ff8badaecb47418f109148d88bf45 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Fri, 5 Dec 2008 19:01:59 -0500 Subject: NSM: Move NSM-related XDR data structures to lockd's xdr.h Clean up: NSM's XDR data structures are used only in fs/lockd/mon.c, so move them there. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/lockd/mon.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'fs/lockd/mon.c') diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c index 3bb71e1b1e1..81308832e99 100644 --- a/fs/lockd/mon.c +++ b/fs/lockd/mon.c @@ -18,6 +18,20 @@ #define NLMDBG_FACILITY NLMDBG_MONITOR +struct nsm_args { + __be32 addr; /* remote address */ + u32 prog; /* RPC callback info */ + u32 vers; + u32 proc; + + char *mon_name; +}; + +struct nsm_res { + u32 status; + u32 state; +}; + static struct rpc_clnt * nsm_create(void); static struct rpc_program nsm_program; -- cgit v1.2.3 From 36e8e668d3e6a61848a8921ddeb663b417299fa5 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Fri, 5 Dec 2008 19:02:07 -0500 Subject: NSM: Move NSM program and procedure numbers to fs/lockd/mon.c Clean up: Move the RPC program and procedure numbers for NSM into the one source file that needs them: fs/lockd/mon.c. And, as with NLM, NFS, and rpcbind calls, use NSMPROC_FOO instead of SM_FOO for NSM procedure numbers. Finally, make a couple of comments more precise: what is referred to here as SM_NOTIFY is really the NLM (lockd) NLMPROC_SM_NOTIFY downcall, not NSMPROC_NOTIFY. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/lockd/mon.c | 42 +++++++++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 15 deletions(-) (limited to 'fs/lockd/mon.c') diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c index 81308832e99..0fc9836db4e 100644 --- a/fs/lockd/mon.c +++ b/fs/lockd/mon.c @@ -17,6 +17,18 @@ #define NLMDBG_FACILITY NLMDBG_MONITOR +#define NSM_PROGRAM 100024 +#define NSM_VERSION 1 + +enum { + NSMPROC_NULL, + NSMPROC_STAT, + NSMPROC_MON, + NSMPROC_UNMON, + NSMPROC_UNMON_ALL, + NSMPROC_SIMU_CRASH, + NSMPROC_NOTIFY, +}; struct nsm_args { __be32 addr; /* remote address */ @@ -42,7 +54,7 @@ static struct rpc_program nsm_program; int nsm_local_state; /* - * Common procedure for SM_MON/SM_UNMON calls + * Common procedure for NSMPROC_MON/NSMPROC_UNMON calls */ static int nsm_mon_unmon(struct nsm_handle *nsm, u32 proc, struct nsm_res *res) @@ -111,7 +123,7 @@ int nsm_monitor(const struct nlm_host *host) */ nsm->sm_mon_name = nsm_use_hostnames ? nsm->sm_name : nsm->sm_addrbuf; - status = nsm_mon_unmon(nsm, SM_MON, &res); + status = nsm_mon_unmon(nsm, NSMPROC_MON, &res); if (res.status != 0) status = -EIO; if (status < 0) @@ -139,7 +151,7 @@ void nsm_unmonitor(const struct nlm_host *host) && nsm->sm_monitored && !nsm->sm_sticky) { dprintk("lockd: nsm_unmonitor(%s)\n", nsm->sm_name); - status = nsm_mon_unmon(nsm, SM_UNMON, &res); + status = nsm_mon_unmon(nsm, NSMPROC_UNMON, &res); if (res.status != 0) status = -EIO; if (status < 0) @@ -167,7 +179,7 @@ nsm_create(void) .addrsize = sizeof(sin), .servername = "localhost", .program = &nsm_program, - .version = SM_VERSION, + .version = NSM_VERSION, .authflavor = RPC_AUTH_NULL, }; @@ -201,7 +213,7 @@ static __be32 *xdr_encode_mon_name(__be32 *p, struct nsm_args *argp) /* * The "my_id" argument specifies the hostname and RPC procedure * to be called when the status manager receives notification - * (via the SM_NOTIFY call) that the state of host "mon_name" + * (via the NLMPROC_SM_NOTIFY call) that the state of host "mon_name" * has changed. */ static __be32 *xdr_encode_my_id(__be32 *p, struct nsm_args *argp) @@ -219,7 +231,7 @@ static __be32 *xdr_encode_my_id(__be32 *p, struct nsm_args *argp) /* * The "mon_id" argument specifies the non-private arguments - * of an SM_MON or SM_UNMON call. + * of an NSMPROC_MON or NSMPROC_UNMON call. */ static __be32 *xdr_encode_mon_id(__be32 *p, struct nsm_args *argp) { @@ -232,8 +244,8 @@ static __be32 *xdr_encode_mon_id(__be32 *p, struct nsm_args *argp) /* * The "priv" argument may contain private information required - * by the SM_MON call. This information will be supplied in the - * SM_NOTIFY call. + * by the NSMPROC_MON call. This information will be supplied in the + * NLMPROC_SM_NOTIFY call. * * Linux provides the raw IP address of the monitored host, * left in network byte order. @@ -300,22 +312,22 @@ xdr_decode_stat(struct rpc_rqst *rqstp, __be32 *p, struct nsm_res *resp) #define SM_unmonres_sz 1 static struct rpc_procinfo nsm_procedures[] = { -[SM_MON] = { - .p_proc = SM_MON, +[NSMPROC_MON] = { + .p_proc = NSMPROC_MON, .p_encode = (kxdrproc_t) xdr_encode_mon, .p_decode = (kxdrproc_t) xdr_decode_stat_res, .p_arglen = SM_mon_sz, .p_replen = SM_monres_sz, - .p_statidx = SM_MON, + .p_statidx = NSMPROC_MON, .p_name = "MONITOR", }, -[SM_UNMON] = { - .p_proc = SM_UNMON, +[NSMPROC_UNMON] = { + .p_proc = NSMPROC_UNMON, .p_encode = (kxdrproc_t) xdr_encode_unmon, .p_decode = (kxdrproc_t) xdr_decode_stat, .p_arglen = SM_mon_id_sz, .p_replen = SM_unmonres_sz, - .p_statidx = SM_UNMON, + .p_statidx = NSMPROC_UNMON, .p_name = "UNMONITOR", }, }; @@ -334,7 +346,7 @@ static struct rpc_stat nsm_stats; static struct rpc_program nsm_program = { .name = "statd", - .number = SM_PROGRAM, + .number = NSM_PROGRAM, .nrvers = ARRAY_SIZE(nsm_version), .version = nsm_version, .stats = &nsm_stats -- cgit v1.2.3 From 03eb1dcbb799304b58730f4dba65812f49fb305e Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Fri, 5 Dec 2008 19:02:15 -0500 Subject: NSM: move to xdr_stream-based XDR encoders and decoders Introduce xdr_stream-based XDR encoder and decoder functions, which are more careful about preventing RPC buffer overflows. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/lockd/mon.c | 130 ++++++++++++++++++++++++++++++++++----------------------- 1 file changed, 78 insertions(+), 52 deletions(-) (limited to 'fs/lockd/mon.c') diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c index 0fc9836db4e..81e1cc14246 100644 --- a/fs/lockd/mon.c +++ b/fs/lockd/mon.c @@ -193,21 +193,26 @@ nsm_create(void) * Status Monitor wire protocol. */ -static __be32 *xdr_encode_nsm_string(__be32 *p, char *string) +static int encode_nsm_string(struct xdr_stream *xdr, const char *string) { - size_t len = strlen(string); - - if (len > SM_MAXSTRLEN) - len = SM_MAXSTRLEN; - return xdr_encode_opaque(p, string, len); + const u32 len = strlen(string); + __be32 *p; + + if (unlikely(len > SM_MAXSTRLEN)) + return -EIO; + p = xdr_reserve_space(xdr, sizeof(u32) + len); + if (unlikely(p == NULL)) + return -EIO; + xdr_encode_opaque(p, string, len); + return 0; } /* * "mon_name" specifies the host to be monitored. */ -static __be32 *xdr_encode_mon_name(__be32 *p, struct nsm_args *argp) +static int encode_mon_name(struct xdr_stream *xdr, const struct nsm_args *argp) { - return xdr_encode_nsm_string(p, argp->mon_name); + return encode_nsm_string(xdr, argp->mon_name); } /* @@ -216,30 +221,35 @@ static __be32 *xdr_encode_mon_name(__be32 *p, struct nsm_args *argp) * (via the NLMPROC_SM_NOTIFY call) that the state of host "mon_name" * has changed. */ -static __be32 *xdr_encode_my_id(__be32 *p, struct nsm_args *argp) +static int encode_my_id(struct xdr_stream *xdr, const struct nsm_args *argp) { - p = xdr_encode_nsm_string(p, utsname()->nodename); - if (!p) - return ERR_PTR(-EIO); - + int status; + __be32 *p; + + status = encode_nsm_string(xdr, utsname()->nodename); + if (unlikely(status != 0)) + return status; + p = xdr_reserve_space(xdr, 3 * sizeof(u32)); + if (unlikely(p == NULL)) + return -EIO; *p++ = htonl(argp->prog); *p++ = htonl(argp->vers); *p++ = htonl(argp->proc); - - return p; + return 0; } /* * The "mon_id" argument specifies the non-private arguments * of an NSMPROC_MON or NSMPROC_UNMON call. */ -static __be32 *xdr_encode_mon_id(__be32 *p, struct nsm_args *argp) +static int encode_mon_id(struct xdr_stream *xdr, const struct nsm_args *argp) { - p = xdr_encode_mon_name(p, argp); - if (!p) - return ERR_PTR(-EIO); + int status; - return xdr_encode_my_id(p, argp); + status = encode_mon_name(xdr, argp); + if (unlikely(status != 0)) + return status; + return encode_my_id(xdr, argp); } /* @@ -250,55 +260,71 @@ static __be32 *xdr_encode_mon_id(__be32 *p, struct nsm_args *argp) * Linux provides the raw IP address of the monitored host, * left in network byte order. */ -static __be32 *xdr_encode_priv(__be32 *p, struct nsm_args *argp) +static int encode_priv(struct xdr_stream *xdr, const struct nsm_args *argp) { + __be32 *p; + + p = xdr_reserve_space(xdr, SM_PRIV_SIZE); + if (unlikely(p == NULL)) + return -EIO; *p++ = argp->addr; *p++ = 0; *p++ = 0; *p++ = 0; - - return p; + return 0; } -static int -xdr_encode_mon(struct rpc_rqst *rqstp, __be32 *p, struct nsm_args *argp) +static int xdr_enc_mon(struct rpc_rqst *req, __be32 *p, + const struct nsm_args *argp) { - p = xdr_encode_mon_id(p, argp); - if (IS_ERR(p)) - return PTR_ERR(p); - - p = xdr_encode_priv(p, argp); - if (IS_ERR(p)) - return PTR_ERR(p); + struct xdr_stream xdr; + int status; - rqstp->rq_slen = xdr_adjust_iovec(rqstp->rq_svec, p); - return 0; + xdr_init_encode(&xdr, &req->rq_snd_buf, p); + status = encode_mon_id(&xdr, argp); + if (unlikely(status)) + return status; + return encode_priv(&xdr, argp); } -static int -xdr_encode_unmon(struct rpc_rqst *rqstp, __be32 *p, struct nsm_args *argp) +static int xdr_enc_unmon(struct rpc_rqst *req, __be32 *p, + const struct nsm_args *argp) { - p = xdr_encode_mon_id(p, argp); - if (IS_ERR(p)) - return PTR_ERR(p); - rqstp->rq_slen = xdr_adjust_iovec(rqstp->rq_svec, p); - return 0; + struct xdr_stream xdr; + + xdr_init_encode(&xdr, &req->rq_snd_buf, p); + return encode_mon_id(&xdr, argp); } -static int -xdr_decode_stat_res(struct rpc_rqst *rqstp, __be32 *p, struct nsm_res *resp) +static int xdr_dec_stat_res(struct rpc_rqst *rqstp, __be32 *p, + struct nsm_res *resp) { + struct xdr_stream xdr; + + xdr_init_decode(&xdr, &rqstp->rq_rcv_buf, p); + p = xdr_inline_decode(&xdr, 2 * sizeof(u32)); + if (unlikely(p == NULL)) + return -EIO; resp->status = ntohl(*p++); - resp->state = ntohl(*p++); - dprintk("nsm: xdr_decode_stat_res status %d state %d\n", + resp->state = ntohl(*p); + + dprintk("lockd: xdr_dec_stat_res status %d state %d\n", resp->status, resp->state); return 0; } -static int -xdr_decode_stat(struct rpc_rqst *rqstp, __be32 *p, struct nsm_res *resp) +static int xdr_dec_stat(struct rpc_rqst *rqstp, __be32 *p, + struct nsm_res *resp) { - resp->state = ntohl(*p++); + struct xdr_stream xdr; + + xdr_init_decode(&xdr, &rqstp->rq_rcv_buf, p); + p = xdr_inline_decode(&xdr, sizeof(u32)); + if (unlikely(p == NULL)) + return -EIO; + resp->state = ntohl(*p); + + dprintk("lockd: xdr_dec_stat state %d\n", resp->state); return 0; } @@ -314,8 +340,8 @@ xdr_decode_stat(struct rpc_rqst *rqstp, __be32 *p, struct nsm_res *resp) static struct rpc_procinfo nsm_procedures[] = { [NSMPROC_MON] = { .p_proc = NSMPROC_MON, - .p_encode = (kxdrproc_t) xdr_encode_mon, - .p_decode = (kxdrproc_t) xdr_decode_stat_res, + .p_encode = (kxdrproc_t)xdr_enc_mon, + .p_decode = (kxdrproc_t)xdr_dec_stat_res, .p_arglen = SM_mon_sz, .p_replen = SM_monres_sz, .p_statidx = NSMPROC_MON, @@ -323,8 +349,8 @@ static struct rpc_procinfo nsm_procedures[] = { }, [NSMPROC_UNMON] = { .p_proc = NSMPROC_UNMON, - .p_encode = (kxdrproc_t) xdr_encode_unmon, - .p_decode = (kxdrproc_t) xdr_decode_stat, + .p_encode = (kxdrproc_t)xdr_enc_unmon, + .p_decode = (kxdrproc_t)xdr_dec_stat, .p_arglen = SM_mon_id_sz, .p_replen = SM_unmonres_sz, .p_statidx = NSMPROC_UNMON, -- cgit v1.2.3 From 67c6d107a689243979a2b5f15244b5261634a924 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Fri, 5 Dec 2008 19:02:45 -0500 Subject: NSM: Move nsm_find() to fs/lockd/mon.c The nsm_find() function sets up fresh nsm_handle entries. This is where we will store the "priv" cookie used to lookup nsm_handles during reboot recovery. The cookie will be constructed when nsm_find() creates a new nsm_handle. As much as possible, I would like to keep everything that handles a "priv" cookie in fs/lockd/mon.c so that all the smarts are in one source file. That organization should make it pretty simple to see how all this works. To me, it makes more sense than the current arrangement to keep nsm_find() with nsm_monitor() and nsm_unmonitor(). So, start reorganizing by moving nsm_find() into fs/lockd/mon.c. The nsm_release() function comes along too, since it shares the nsm_lock global variable. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/lockd/mon.c | 133 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 133 insertions(+) (limited to 'fs/lockd/mon.c') diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c index 81e1cc14246..8e68e799293 100644 --- a/fs/lockd/mon.c +++ b/fs/lockd/mon.c @@ -47,12 +47,51 @@ struct nsm_res { static struct rpc_clnt * nsm_create(void); static struct rpc_program nsm_program; +static LIST_HEAD(nsm_handles); +static DEFINE_SPINLOCK(nsm_lock); /* * Local NSM state */ int nsm_local_state; +static void nsm_display_ipv4_address(const struct sockaddr *sap, char *buf, + const size_t len) +{ + const struct sockaddr_in *sin = (struct sockaddr_in *)sap; + snprintf(buf, len, "%pI4", &sin->sin_addr.s_addr); +} + +static void nsm_display_ipv6_address(const struct sockaddr *sap, char *buf, + const size_t len) +{ + const struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)sap; + + if (ipv6_addr_v4mapped(&sin6->sin6_addr)) + snprintf(buf, len, "%pI4", &sin6->sin6_addr.s6_addr32[3]); + else if (sin6->sin6_scope_id != 0) + snprintf(buf, len, "%pI6%%%u", &sin6->sin6_addr, + sin6->sin6_scope_id); + else + snprintf(buf, len, "%pI6", &sin6->sin6_addr); +} + +static void nsm_display_address(const struct sockaddr *sap, + char *buf, const size_t len) +{ + switch (sap->sa_family) { + case AF_INET: + nsm_display_ipv4_address(sap, buf, len); + break; + case AF_INET6: + nsm_display_ipv6_address(sap, buf, len); + break; + default: + snprintf(buf, len, "unsupported address family"); + break; + } +} + /* * Common procedure for NSMPROC_MON/NSMPROC_UNMON calls */ @@ -162,6 +201,100 @@ void nsm_unmonitor(const struct nlm_host *host) } } +/** + * nsm_find - Find or create a cached nsm_handle + * @sap: pointer to socket address of handle to find + * @salen: length of socket address + * @hostname: pointer to C string containing hostname to find + * @hostname_len: length of C string + * @create: one means create new handle if not found in cache + * + * Behavior is modulated by the global nsm_use_hostnames variable + * and by the @create argument. + * + * Returns a cached nsm_handle after bumping its ref count, or if + * @create is set, returns a fresh nsm_handle if a handle that + * matches @sap and/or @hostname cannot be found in the handle cache. + * Returns NULL if an error occurs. + */ +struct nsm_handle *nsm_find(const struct sockaddr *sap, const size_t salen, + const char *hostname, const size_t hostname_len, + const int create) +{ + struct nsm_handle *nsm = NULL; + struct nsm_handle *pos; + + if (!sap) + return NULL; + + if (hostname && memchr(hostname, '/', hostname_len) != NULL) { + if (printk_ratelimit()) { + printk(KERN_WARNING "Invalid hostname \"%.*s\" " + "in NFS lock request\n", + (int)hostname_len, hostname); + } + return NULL; + } + +retry: + spin_lock(&nsm_lock); + list_for_each_entry(pos, &nsm_handles, sm_link) { + + if (hostname && nsm_use_hostnames) { + if (strlen(pos->sm_name) != hostname_len + || memcmp(pos->sm_name, hostname, hostname_len)) + continue; + } else if (!nlm_cmp_addr(nsm_addr(pos), sap)) + continue; + atomic_inc(&pos->sm_count); + kfree(nsm); + nsm = pos; + goto found; + } + if (nsm) { + list_add(&nsm->sm_link, &nsm_handles); + goto found; + } + spin_unlock(&nsm_lock); + + if (!create) + return NULL; + + nsm = kzalloc(sizeof(*nsm) + hostname_len + 1, GFP_KERNEL); + if (nsm == NULL) + return NULL; + + memcpy(nsm_addr(nsm), sap, salen); + nsm->sm_addrlen = salen; + nsm->sm_name = (char *) (nsm + 1); + memcpy(nsm->sm_name, hostname, hostname_len); + nsm->sm_name[hostname_len] = '\0'; + nsm_display_address((struct sockaddr *)&nsm->sm_addr, + nsm->sm_addrbuf, sizeof(nsm->sm_addrbuf)); + atomic_set(&nsm->sm_count, 1); + goto retry; + +found: + spin_unlock(&nsm_lock); + return nsm; +} + +/** + * nsm_release - Release an NSM handle + * @nsm: pointer to handle to be released + * + */ +void nsm_release(struct nsm_handle *nsm) +{ + if (!nsm) + return; + if (atomic_dec_and_lock(&nsm->sm_count, &nsm_lock)) { + list_del(&nsm->sm_link); + spin_unlock(&nsm_lock); + kfree(nsm); + } +} + /* * Create NSM client for the local host */ -- cgit v1.2.3 From 5cf1c4b19db99d21d44c2ab457cfd44eb86b4439 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Fri, 5 Dec 2008 19:02:53 -0500 Subject: NSM: Add dprintk() calls in nsm_find and nsm_release Introduce some dprintk() calls in fs/lockd/mon.c that are enabled by the NLMDBG_MONITOR flag. These report when we find, create, and release nsm_handles. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/lockd/mon.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'fs/lockd/mon.c') diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c index 8e68e799293..38255455563 100644 --- a/fs/lockd/mon.c +++ b/fs/lockd/mon.c @@ -249,10 +249,15 @@ retry: atomic_inc(&pos->sm_count); kfree(nsm); nsm = pos; + dprintk("lockd: found nsm_handle for %s (%s), cnt %d\n", + pos->sm_name, pos->sm_addrbuf, + atomic_read(&pos->sm_count)); goto found; } if (nsm) { list_add(&nsm->sm_link, &nsm_handles); + dprintk("lockd: created nsm_handle for %s (%s)\n", + nsm->sm_name, nsm->sm_addrbuf); goto found; } spin_unlock(&nsm_lock); @@ -291,6 +296,8 @@ void nsm_release(struct nsm_handle *nsm) if (atomic_dec_and_lock(&nsm->sm_count, &nsm_lock)) { list_del(&nsm->sm_link); spin_unlock(&nsm_lock); + dprintk("lockd: destroyed nsm_handle for %s (%s)\n", + nsm->sm_name, nsm->sm_addrbuf); kfree(nsm); } } -- cgit v1.2.3 From bc1cc6c4e476b60df48227165990c87a22db6bb7 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Fri, 5 Dec 2008 19:03:01 -0500 Subject: NSM: Remove NULL pointer check from nsm_find() The nsm_find() function should never be called with a NULL IP address pointer. If it is, that's a bug. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/lockd/mon.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'fs/lockd/mon.c') diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c index 38255455563..0a066a13478 100644 --- a/fs/lockd/mon.c +++ b/fs/lockd/mon.c @@ -224,9 +224,6 @@ struct nsm_handle *nsm_find(const struct sockaddr *sap, const size_t salen, struct nsm_handle *nsm = NULL; struct nsm_handle *pos; - if (!sap) - return NULL; - if (hostname && memchr(hostname, '/', hostname_len) != NULL) { if (printk_ratelimit()) { printk(KERN_WARNING "Invalid hostname \"%.*s\" " -- cgit v1.2.3 From 05f3a9af58180d24a9decedd71d4587935782d70 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Fri, 5 Dec 2008 19:03:09 -0500 Subject: NSM: Remove !nsm check from nsm_release() The nsm_release() function should never be called with a NULL handle point. If it is, that's a bug. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/lockd/mon.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'fs/lockd/mon.c') diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c index 0a066a13478..0792900b628 100644 --- a/fs/lockd/mon.c +++ b/fs/lockd/mon.c @@ -288,8 +288,6 @@ found: */ void nsm_release(struct nsm_handle *nsm) { - if (!nsm) - return; if (atomic_dec_and_lock(&nsm->sm_count, &nsm_lock)) { list_del(&nsm->sm_link); spin_unlock(&nsm_lock); -- cgit v1.2.3 From 7e44d3bea21fbb9494930d1cd35ca92a9a4a3279 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Fri, 5 Dec 2008 19:03:16 -0500 Subject: NSM: Generate NSMPROC_MON's "priv" argument when nsm_handle is created Introduce a new data type, used by both the in-kernel NLM and NSM implementations, that is used to manage the opaque "priv" argument for the NSMPROC_MON and NLMPROC_SM_NOTIFY calls. Construct the "priv" cookie when the nsm_handle is created. The nsm_init_private() function may look a little strange, but it is roughly equivalent to how the XDR encoder formed the "priv" argument. It's going to go away soon. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/lockd/mon.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'fs/lockd/mon.c') diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c index 0792900b628..c8d18cd22b8 100644 --- a/fs/lockd/mon.c +++ b/fs/lockd/mon.c @@ -201,6 +201,21 @@ void nsm_unmonitor(const struct nlm_host *host) } } +/* + * Construct a unique cookie to match this nsm_handle to this monitored + * host. It is passed to the local rpc.statd via NSMPROC_MON, and + * returned via NLMPROC_SM_NOTIFY, in the "priv" field of these + * requests. + * + * Linux provides the raw IP address of the monitored host, + * left in network byte order. + */ +static void nsm_init_private(struct nsm_handle *nsm) +{ + __be32 *p = (__be32 *)&nsm->sm_priv.data; + *p = nsm_addr_in(nsm)->sin_addr.s_addr; +} + /** * nsm_find - Find or create a cached nsm_handle * @sap: pointer to socket address of handle to find @@ -271,6 +286,7 @@ retry: nsm->sm_name = (char *) (nsm + 1); memcpy(nsm->sm_name, hostname, hostname_len); nsm->sm_name[hostname_len] = '\0'; + nsm_init_private(nsm); nsm_display_address((struct sockaddr *)&nsm->sm_addr, nsm->sm_addrbuf, sizeof(nsm->sm_addrbuf)); atomic_set(&nsm->sm_count, 1); -- cgit v1.2.3 From cab2d3c99165abbba2943f1b269003b17fd3b1cb Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Fri, 5 Dec 2008 19:03:24 -0500 Subject: NSM: Encode the new "priv" cookie for NSMPROC_MON requests Pass the new "priv" cookie to NSMPROC_MON's XDR encoder, instead of creating the "priv" argument in the encoder at call time. This patch should not cause a behavioral change: the contents of the cookie remain the same for the time being. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/lockd/mon.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) (limited to 'fs/lockd/mon.c') diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c index c8d18cd22b8..4424b0a5a51 100644 --- a/fs/lockd/mon.c +++ b/fs/lockd/mon.c @@ -31,7 +31,7 @@ enum { }; struct nsm_args { - __be32 addr; /* remote address */ + struct nsm_private *priv; u32 prog; /* RPC callback info */ u32 vers; u32 proc; @@ -101,7 +101,7 @@ nsm_mon_unmon(struct nsm_handle *nsm, u32 proc, struct nsm_res *res) struct rpc_clnt *clnt; int status; struct nsm_args args = { - .addr = nsm_addr_in(nsm)->sin_addr.s_addr, + .priv = &nsm->sm_priv, .prog = NLM_PROGRAM, .vers = 3, .proc = NLMPROC_NSM_NOTIFY, @@ -407,9 +407,6 @@ static int encode_mon_id(struct xdr_stream *xdr, const struct nsm_args *argp) * The "priv" argument may contain private information required * by the NSMPROC_MON call. This information will be supplied in the * NLMPROC_SM_NOTIFY call. - * - * Linux provides the raw IP address of the monitored host, - * left in network byte order. */ static int encode_priv(struct xdr_stream *xdr, const struct nsm_args *argp) { @@ -418,10 +415,7 @@ static int encode_priv(struct xdr_stream *xdr, const struct nsm_args *argp) p = xdr_reserve_space(xdr, SM_PRIV_SIZE); if (unlikely(p == NULL)) return -EIO; - *p++ = argp->addr; - *p++ = 0; - *p++ = 0; - *p++ = 0; + xdr_encode_opaque_fixed(p, argp->priv->data, SM_PRIV_SIZE); return 0; } -- cgit v1.2.3 From 3420a8c4359a189f7d854ed7075d151257415447 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Fri, 5 Dec 2008 19:03:46 -0500 Subject: NSM: Add nsm_lookup() function Introduce a new API to fs/lockd/mon.c that allows nlm_host_rebooted() to lookup up nsm_handles via the contents of an nlm_reboot struct. The new function is equivalent to calling nsm_find() with @create set to zero, but it takes a struct nlm_reboot instead of separate arguments. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/lockd/mon.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) (limited to 'fs/lockd/mon.c') diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c index 4424b0a5a51..e46903995c9 100644 --- a/fs/lockd/mon.c +++ b/fs/lockd/mon.c @@ -201,6 +201,29 @@ void nsm_unmonitor(const struct nlm_host *host) } } +static struct nsm_handle *nsm_lookup_hostname(const char *hostname, + const size_t len) +{ + struct nsm_handle *nsm; + + list_for_each_entry(nsm, &nsm_handles, sm_link) + if (strlen(nsm->sm_name) == len && + memcmp(nsm->sm_name, hostname, len) == 0) + return nsm; + return NULL; +} + +static struct nsm_handle *nsm_lookup_priv(const struct nsm_private *priv) +{ + struct nsm_handle *nsm; + + list_for_each_entry(nsm, &nsm_handles, sm_link) + if (memcmp(nsm->sm_priv.data, priv->data, + sizeof(priv->data)) == 0) + return nsm; + return NULL; +} + /* * Construct a unique cookie to match this nsm_handle to this monitored * host. It is passed to the local rpc.statd via NSMPROC_MON, and @@ -297,6 +320,47 @@ found: return nsm; } +/** + * nsm_reboot_lookup - match NLMPROC_SM_NOTIFY arguments to an nsm_handle + * @info: pointer to NLMPROC_SM_NOTIFY arguments + * + * Returns a matching nsm_handle if found in the nsm cache; the returned + * nsm_handle's reference count is bumped and sm_monitored is cleared. + * Otherwise returns NULL if some error occurred. + */ +struct nsm_handle *nsm_reboot_lookup(const struct nlm_reboot *info) +{ + struct nsm_handle *cached; + + spin_lock(&nsm_lock); + + if (nsm_use_hostnames && info->mon != NULL) + cached = nsm_lookup_hostname(info->mon, info->len); + else + cached = nsm_lookup_priv(&info->priv); + + if (unlikely(cached == NULL)) { + spin_unlock(&nsm_lock); + dprintk("lockd: never saw rebooted peer '%.*s' before\n", + info->len, info->mon); + return cached; + } + + atomic_inc(&cached->sm_count); + spin_unlock(&nsm_lock); + + /* + * During subsequent lock activity, force a fresh + * notification to be set up for this host. + */ + cached->sm_monitored = 0; + + dprintk("lockd: host %s (%s) rebooted, cnt %d\n", + cached->sm_name, cached->sm_addrbuf, + atomic_read(&cached->sm_count)); + return cached; +} + /** * nsm_release - Release an NSM handle * @nsm: pointer to handle to be released -- cgit v1.2.3 From 92fd91b998a5216a6d6606704e71d541a180216c Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Fri, 5 Dec 2008 19:04:01 -0500 Subject: NLM: Remove "create" argument from nsm_find() Clean up: nsm_find() now has only one caller, and that caller unconditionally sets the @create argument. Thus the @create argument is no longer needed. Since nsm_find() now has a more specific purpose, pick a more appropriate name for it. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/lockd/mon.c | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) (limited to 'fs/lockd/mon.c') diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c index e46903995c9..74070221604 100644 --- a/fs/lockd/mon.c +++ b/fs/lockd/mon.c @@ -240,24 +240,22 @@ static void nsm_init_private(struct nsm_handle *nsm) } /** - * nsm_find - Find or create a cached nsm_handle + * nsm_get_handle - Find or create a cached nsm_handle * @sap: pointer to socket address of handle to find * @salen: length of socket address * @hostname: pointer to C string containing hostname to find * @hostname_len: length of C string - * @create: one means create new handle if not found in cache * - * Behavior is modulated by the global nsm_use_hostnames variable - * and by the @create argument. + * Behavior is modulated by the global nsm_use_hostnames variable. * - * Returns a cached nsm_handle after bumping its ref count, or if - * @create is set, returns a fresh nsm_handle if a handle that - * matches @sap and/or @hostname cannot be found in the handle cache. - * Returns NULL if an error occurs. + * Returns a cached nsm_handle after bumping its ref count, or + * returns a fresh nsm_handle if a handle that matches @sap and/or + * @hostname cannot be found in the handle cache. Returns NULL if + * an error occurs. */ -struct nsm_handle *nsm_find(const struct sockaddr *sap, const size_t salen, - const char *hostname, const size_t hostname_len, - const int create) +struct nsm_handle *nsm_get_handle(const struct sockaddr *sap, + const size_t salen, const char *hostname, + const size_t hostname_len) { struct nsm_handle *nsm = NULL; struct nsm_handle *pos; @@ -297,9 +295,6 @@ retry: } spin_unlock(&nsm_lock); - if (!create) - return NULL; - nsm = kzalloc(sizeof(*nsm) + hostname_len + 1, GFP_KERNEL); if (nsm == NULL) return NULL; -- cgit v1.2.3 From b39b897c259fc1fd1998505f2b1d4ec1f115bce1 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 11 Dec 2008 17:55:52 -0500 Subject: NSM: Refactor nsm_handle creation into a helper function Clean up. Refactor the creation of nsm_handles into a helper. Fields are initialized in increasing address order to make efficient use of CPU caches. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/lockd/mon.c | 38 ++++++++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 12 deletions(-) (limited to 'fs/lockd/mon.c') diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c index 74070221604..315ca07715c 100644 --- a/fs/lockd/mon.c +++ b/fs/lockd/mon.c @@ -239,6 +239,30 @@ static void nsm_init_private(struct nsm_handle *nsm) *p = nsm_addr_in(nsm)->sin_addr.s_addr; } +static struct nsm_handle *nsm_create_handle(const struct sockaddr *sap, + const size_t salen, + const char *hostname, + const size_t hostname_len) +{ + struct nsm_handle *new; + + new = kzalloc(sizeof(*new) + hostname_len + 1, GFP_KERNEL); + if (unlikely(new == NULL)) + return NULL; + + atomic_set(&new->sm_count, 1); + new->sm_name = (char *)(new + 1); + memcpy(nsm_addr(new), sap, salen); + new->sm_addrlen = salen; + nsm_init_private(new); + nsm_display_address((const struct sockaddr *)&new->sm_addr, + new->sm_addrbuf, sizeof(new->sm_addrbuf)); + memcpy(new->sm_name, hostname, hostname_len); + new->sm_name[hostname_len] = '\0'; + + return new; +} + /** * nsm_get_handle - Find or create a cached nsm_handle * @sap: pointer to socket address of handle to find @@ -295,19 +319,9 @@ retry: } spin_unlock(&nsm_lock); - nsm = kzalloc(sizeof(*nsm) + hostname_len + 1, GFP_KERNEL); - if (nsm == NULL) + nsm = nsm_create_handle(sap, salen, hostname, hostname_len); + if (unlikely(nsm == NULL)) return NULL; - - memcpy(nsm_addr(nsm), sap, salen); - nsm->sm_addrlen = salen; - nsm->sm_name = (char *) (nsm + 1); - memcpy(nsm->sm_name, hostname, hostname_len); - nsm->sm_name[hostname_len] = '\0'; - nsm_init_private(nsm); - nsm_display_address((struct sockaddr *)&nsm->sm_addr, - nsm->sm_addrbuf, sizeof(nsm->sm_addrbuf)); - atomic_set(&nsm->sm_count, 1); goto retry; found: -- cgit v1.2.3 From 77a3ef33e2de6fc8aabd7cb1700bfef81757c28a Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 11 Dec 2008 17:55:59 -0500 Subject: NSM: More clean up of nsm_get_handle() Clean up: refactor nsm_get_handle() so it is organized the same way that nsm_reboot_lookup() is. There is an additional micro-optimization here. This change moves the "hostname & nsm_use_hostnames" test out of the list_for_each_entry() clause in nsm_get_handle(), since it is loop-invariant. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/lockd/mon.c | 62 +++++++++++++++++++++++++++++++++------------------------- 1 file changed, 35 insertions(+), 27 deletions(-) (limited to 'fs/lockd/mon.c') diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c index 315ca07715c..99aec744474 100644 --- a/fs/lockd/mon.c +++ b/fs/lockd/mon.c @@ -213,6 +213,16 @@ static struct nsm_handle *nsm_lookup_hostname(const char *hostname, return NULL; } +static struct nsm_handle *nsm_lookup_addr(const struct sockaddr *sap) +{ + struct nsm_handle *nsm; + + list_for_each_entry(nsm, &nsm_handles, sm_link) + if (nlm_cmp_addr(nsm_addr(nsm), sap)) + return nsm; + return NULL; +} + static struct nsm_handle *nsm_lookup_priv(const struct nsm_private *priv) { struct nsm_handle *nsm; @@ -281,8 +291,7 @@ struct nsm_handle *nsm_get_handle(const struct sockaddr *sap, const size_t salen, const char *hostname, const size_t hostname_len) { - struct nsm_handle *nsm = NULL; - struct nsm_handle *pos; + struct nsm_handle *cached, *new = NULL; if (hostname && memchr(hostname, '/', hostname_len) != NULL) { if (printk_ratelimit()) { @@ -295,38 +304,37 @@ struct nsm_handle *nsm_get_handle(const struct sockaddr *sap, retry: spin_lock(&nsm_lock); - list_for_each_entry(pos, &nsm_handles, sm_link) { - - if (hostname && nsm_use_hostnames) { - if (strlen(pos->sm_name) != hostname_len - || memcmp(pos->sm_name, hostname, hostname_len)) - continue; - } else if (!nlm_cmp_addr(nsm_addr(pos), sap)) - continue; - atomic_inc(&pos->sm_count); - kfree(nsm); - nsm = pos; - dprintk("lockd: found nsm_handle for %s (%s), cnt %d\n", - pos->sm_name, pos->sm_addrbuf, - atomic_read(&pos->sm_count)); - goto found; + + if (nsm_use_hostnames && hostname != NULL) + cached = nsm_lookup_hostname(hostname, hostname_len); + else + cached = nsm_lookup_addr(sap); + + if (cached != NULL) { + atomic_inc(&cached->sm_count); + spin_unlock(&nsm_lock); + kfree(new); + dprintk("lockd: found nsm_handle for %s (%s), " + "cnt %d\n", cached->sm_name, + cached->sm_addrbuf, + atomic_read(&cached->sm_count)); + return cached; } - if (nsm) { - list_add(&nsm->sm_link, &nsm_handles); + + if (new != NULL) { + list_add(&new->sm_link, &nsm_handles); + spin_unlock(&nsm_lock); dprintk("lockd: created nsm_handle for %s (%s)\n", - nsm->sm_name, nsm->sm_addrbuf); - goto found; + new->sm_name, new->sm_addrbuf); + return new; } + spin_unlock(&nsm_lock); - nsm = nsm_create_handle(sap, salen, hostname, hostname_len); - if (unlikely(nsm == NULL)) + new = nsm_create_handle(sap, salen, hostname, hostname_len); + if (unlikely(new == NULL)) return NULL; goto retry; - -found: - spin_unlock(&nsm_lock); - return nsm; } /** -- cgit v1.2.3 From 94da7663db26530a8377f7219f8be8bd4d4822c2 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 11 Dec 2008 17:56:07 -0500 Subject: NSM: Replace IP address as our nlm_reboot lookup key NLM provides file locking services for NFS files. Part of this service includes a second protocol, known as NSM, which is a reboot notification service. NLM uses this service to determine when to reclaim locks or enter a grace period after a client or server reboots. The NLM service (implemented by lockd in the Linux kernel) contacts the local NSM service (implemented by rpc.statd in Linux user space) via NSM protocol upcalls to register a callback when a particular remote peer reboots. To match the callback to the correct remote peer, the NLM service constructs a cookie that it passes in the request. The NSM service passes that cookie back to the NLM service when it is notified that the given remote peer has indeed rebooted. Currently on Linux, the cookie is the raw 32-bit IPv4 address of the remote peer. To support IPv6 addresses, which are larger, we could use all 16 bytes of the cookie to represent a full IPv6 address, although we still can't represent an IPv6 address with a scope ID in just 16 bytes. Instead, to avoid the need for future changes to support additional address types, we'll use a manufactured value for the cookie, and use that to find the corresponding nsm_handle struct in the kernel during the NLMPROC_SM_NOTIFY callback. This should provide complete support in the kernel's NSM implementation for IPv6 hosts, while remaining backwards compatible with older rpc.statd implementations. Note we also deal with another case where nsm_use_hostnames can change while there are outstanding notifications, possibly resulting in the loss of reboot notifications. After this patch, the priv cookie is always used to lookup rebooted hosts in the kernel. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/lockd/mon.c | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) (limited to 'fs/lockd/mon.c') diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c index 99aec744474..8ae4c02d7df 100644 --- a/fs/lockd/mon.c +++ b/fs/lockd/mon.c @@ -9,6 +9,8 @@ #include #include #include +#include + #include #include #include @@ -240,13 +242,25 @@ static struct nsm_handle *nsm_lookup_priv(const struct nsm_private *priv) * returned via NLMPROC_SM_NOTIFY, in the "priv" field of these * requests. * - * Linux provides the raw IP address of the monitored host, - * left in network byte order. + * The NSM protocol requires that these cookies be unique while the + * system is running. We prefer a stronger requirement of making them + * unique across reboots. If user space bugs cause a stale cookie to + * be sent to the kernel, it could cause the wrong host to lose its + * lock state if cookies were not unique across reboots. + * + * The cookies are exposed only to local user space via loopback. They + * do not appear on the physical network. If we want greater security + * for some reason, nsm_init_private() could perform a one-way hash to + * obscure the contents of the cookie. */ static void nsm_init_private(struct nsm_handle *nsm) { - __be32 *p = (__be32 *)&nsm->sm_priv.data; - *p = nsm_addr_in(nsm)->sin_addr.s_addr; + u64 *p = (u64 *)&nsm->sm_priv.data; + struct timespec ts; + + ktime_get_ts(&ts); + *p++ = timespec_to_ns(&ts); + *p = (unsigned long)nsm; } static struct nsm_handle *nsm_create_handle(const struct sockaddr *sap, @@ -351,11 +365,7 @@ struct nsm_handle *nsm_reboot_lookup(const struct nlm_reboot *info) spin_lock(&nsm_lock); - if (nsm_use_hostnames && info->mon != NULL) - cached = nsm_lookup_hostname(info->mon, info->len); - else - cached = nsm_lookup_priv(&info->priv); - + cached = nsm_lookup_priv(&info->priv); if (unlikely(cached == NULL)) { spin_unlock(&nsm_lock); dprintk("lockd: never saw rebooted peer '%.*s' before\n", -- cgit v1.2.3 From e6765b83977f07983c7a10e6bbb19d6c7bbfc3a4 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 11 Dec 2008 17:56:14 -0500 Subject: NSM: Remove include/linux/lockd/sm_inter.h Clean up: The include/linux/lockd/sm_inter.h header is nearly empty now. Remove it. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/lockd/mon.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'fs/lockd/mon.c') diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c index 8ae4c02d7df..dfa9d80efcb 100644 --- a/fs/lockd/mon.c +++ b/fs/lockd/mon.c @@ -15,8 +15,6 @@ #include #include #include -#include - #define NLMDBG_FACILITY NLMDBG_MONITOR #define NSM_PROGRAM 100024 -- cgit v1.2.3 From 8529bc51d30b8f001734b29b21a51b579c260f5b Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 11 Dec 2008 17:56:22 -0500 Subject: NSM: Move nsm_addr() to fs/lockd/mon.c Clean up: nsm_addr_in() is no longer used, and nsm_addr() is used only in fs/lockd/mon.c, so move it there. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/lockd/mon.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'fs/lockd/mon.c') diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c index dfa9d80efcb..43be31c4a2d 100644 --- a/fs/lockd/mon.c +++ b/fs/lockd/mon.c @@ -55,6 +55,11 @@ static DEFINE_SPINLOCK(nsm_lock); */ int nsm_local_state; +static inline struct sockaddr *nsm_addr(const struct nsm_handle *nsm) +{ + return (struct sockaddr *)&nsm->sm_addr; +} + static void nsm_display_ipv4_address(const struct sockaddr *sap, char *buf, const size_t len) { -- cgit v1.2.3 From b7ba597fb964dfa44284904b3b3d74d44b8e1c42 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 11 Dec 2008 17:56:29 -0500 Subject: NSM: Move nsm_use_hostnames to mon.c Clean up. Treat the nsm_use_hostnames global variable like nsm_local_state. Note that the default value of nsm_use_hostnames is still zero. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/lockd/mon.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'fs/lockd/mon.c') diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c index 43be31c4a2d..fafa0ea7193 100644 --- a/fs/lockd/mon.c +++ b/fs/lockd/mon.c @@ -53,7 +53,8 @@ static DEFINE_SPINLOCK(nsm_lock); /* * Local NSM state */ -int nsm_local_state; +int __read_mostly nsm_local_state; +int __read_mostly nsm_use_hostnames; static inline struct sockaddr *nsm_addr(const struct nsm_handle *nsm) { -- cgit v1.2.3 From 49b5699b3fc22b363534c509c1b7dba06bc677bf Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 11 Dec 2008 17:56:37 -0500 Subject: NSM: Move nsm_create() Clean up: one last thing... relocate nsm_create() to eliminate the forward declaration and group it near the only function that actually uses it. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/lockd/mon.c | 51 ++++++++++++++++++++------------------------------- 1 file changed, 20 insertions(+), 31 deletions(-) (limited to 'fs/lockd/mon.c') diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c index fafa0ea7193..5e2c4d5ac82 100644 --- a/fs/lockd/mon.c +++ b/fs/lockd/mon.c @@ -44,8 +44,6 @@ struct nsm_res { u32 state; }; -static struct rpc_clnt * nsm_create(void); - static struct rpc_program nsm_program; static LIST_HEAD(nsm_handles); static DEFINE_SPINLOCK(nsm_lock); @@ -98,11 +96,26 @@ static void nsm_display_address(const struct sockaddr *sap, } } -/* - * Common procedure for NSMPROC_MON/NSMPROC_UNMON calls - */ -static int -nsm_mon_unmon(struct nsm_handle *nsm, u32 proc, struct nsm_res *res) +static struct rpc_clnt *nsm_create(void) +{ + struct sockaddr_in sin = { + .sin_family = AF_INET, + .sin_addr.s_addr = htonl(INADDR_LOOPBACK), + }; + struct rpc_create_args args = { + .protocol = XPRT_TRANSPORT_UDP, + .address = (struct sockaddr *)&sin, + .addrsize = sizeof(sin), + .servername = "rpc.statd", + .program = &nsm_program, + .version = NSM_VERSION, + .authflavor = RPC_AUTH_NULL, + }; + + return rpc_create(&args); +} + +static int nsm_mon_unmon(struct nsm_handle *nsm, u32 proc, struct nsm_res *res) { struct rpc_clnt *clnt; int status; @@ -408,30 +421,6 @@ void nsm_release(struct nsm_handle *nsm) } } -/* - * Create NSM client for the local host - */ -static struct rpc_clnt * -nsm_create(void) -{ - struct sockaddr_in sin = { - .sin_family = AF_INET, - .sin_addr.s_addr = htonl(INADDR_LOOPBACK), - .sin_port = 0, - }; - struct rpc_create_args args = { - .protocol = XPRT_TRANSPORT_UDP, - .address = (struct sockaddr *)&sin, - .addrsize = sizeof(sin), - .servername = "localhost", - .program = &nsm_program, - .version = NSM_VERSION, - .authflavor = RPC_AUTH_NULL, - }; - - return rpc_create(&args); -} - /* * XDR functions for NSM. * -- cgit v1.2.3