On Wed, Jun 04, 2008 at 11:03:13AM -0400, Jeff Layton wrote:
> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> index 5ac00c4..d601a77 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
...
> @@ -566,14 +574,13 @@ static ssize_t write_versions(struct file *file, char *buf, size_t size)
> return len;
> }
>
> -static ssize_t write_ports(struct file *file, char *buf, size_t size)
> +static ssize_t __write_ports(struct file *file, char *buf, size_t size)
> {
> if (size == 0) {
> int len = 0;
> - lock_kernel();
> +
> if (nfsd_serv)
> len = svc_xprt_names(nfsd_serv, buf, 0);
> - unlock_kernel();
svc_xprt_names() has to be prepared to accept NULL as a first parameter
(since we've got nothing here any longer to guarantee that nfsd_serv
won't change after we've checked it). And, indeed, it does check for
that (with its local copy, which won't change. So that's OK. But then
could we just ditch this redundant check here? It's confusing.
Oops, but: what happens if something like this races with svc_destroy,
so svc_xprt_names() is passed a pointer to freed memory?
--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
