linux-kernel - Re: [NFS] [PATCH 005 of 9] knfsd: Be more selective in which sockets lockd listens on.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20060726191714.GE31172@fieldses.org>
Date:	Wed, 26 Jul 2006 15:17:14 -0400
From:	"J. Bruce Fields" <bfields@...ldses.org>
To:	NeilBrown <neilb@...e.de>
Cc:	Andrew Morton <akpm@...l.org>, nfs@...ts.sourceforge.net,
	linux-kernel@...r.kernel.org
Subject: Re: [NFS] [PATCH 005 of 9] knfsd: Be more selective in which sockets lockd listens on.

On Tue, Jul 25, 2006 at 11:54:47AM +1000, NeilBrown wrote:
> @@ -112,6 +114,7 @@ lockd(struct svc_rqst *rqstp)
>  	 * Let our maker know we're running.
>  	 */
>  	nlmsvc_pid = current->pid;
> +	nlmsvc_serv = serv;

Nitpick: any reason not to just get rid of the local variable "serv"
after that?

> @@ -224,8 +259,10 @@ lockd_up(void)
>  	/*
>  	 * Check whether we're already up and running.
>  	 */
> -	if (nlmsvc_pid)
> +	if (nlmsvc_pid) {
> +		error = make_socks(nlmsvc_serv, proto);
>  		goto out;

...

> +	if ((error = make_socks(serv, proto)) < 0) {
>  		if (warned++ == 0) 
>  			printk(KERN_WARNING
>  				"lockd_up: makesock failed, error=%d\n", error);

The warning is printk'ed a little inconsistently.  (If we care, maybe it
should just go inside make_socks?)

By the way, why don't most callers use the error returned from
lockd_up()?

> diff .prev/fs/nfsd/nfssvc.c ./fs/nfsd/nfssvc.c
> --- .prev/fs/nfsd/nfssvc.c	2006-07-24 15:14:31.000000000 +1000
> +++ ./fs/nfsd/nfssvc.c	2006-07-24 15:15:04.000000000 +1000
> @@ -134,6 +134,9 @@ static int killsig = 0; /* signal that w
>  static void nfsd_last_thread(struct svc_serv *serv)
>  {
>  	/* When last nfsd thread exits we need to do some clean-up */
> +	struct svc_sock *svsk;
> +	list_for_each_entry(svsk, &serv->sv_permsocks, sk_list)
> +		lockd_down();

So I guess it's a minor point, but: we take the trouble to only open tcp
or udp sockets as necessary, but then won't close them down till all the
mounts and nfsd's go away at which point we close them all down.

Would it be that bad just to always listen on both?

--b.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/