Linux-NFS Archive on lore.kernel.org
 help / color / Atom feed
From: NeilBrown <neilb@suse.de>
To: Chuck Lever <chuck.lever@oracle.com>
Cc: Steve Dickson <SteveD@redhat.com>, linux-nfs@vger.kernel.org
Subject: Re: [PATCH/RFC] mount: improve signal management when locking mtab.
Date: Thu, 23 Jun 2011 09:20:52 +1000
Message-ID: <20110623092052.414d78a8@notabene.brown> (raw)
In-Reply-To: <B1677B7E-3A7E-418B-9CCA-0844510B59C8@oracle.com>

On Wed, 22 Jun 2011 09:08:25 -0600 Chuck Lever <chuck.lever@oracle.com> wrote:

> Since we are going to adopt libmount for mount.nfs in the near future, would it be better to update libmount instead?

How near?

Libmount appears to get signal handling right already (though I haven't
tested it).  It blocks all signals rather than catching some of them.

So switching to libmount would a perfectly reasonably response.  However if
that is more than a few weeks away I think I would rather see this fixed up
anyway...

Thanks,
NeilBrown


> 
> On Jun 22, 2011, at 12:38 AM, NeilBrown wrote:
> 
> > 
> > From: Neil Brown <neilb@suse.de>
> > Date: Wed, 22 Jun 2011 16:15:45 +1000
> > Subject: [PATCH] mount: improve signal management when locking mtab.
> > 
> > As mount.nfs can run setuid it must be careful about how the user can
> > interact with in.  In particular it needs to ensure it does not
> > respond badly to any signals that the user might be able to generate.
> > 
> > This is particularly an issue while updating /etc/mtab (when that is
> > not linked to /proc/mounts).  If the user can generate a signal which
> > kills mount.nfs while /etc/mtab is locked, then it will leave the file
> > locked, and could possibly corrupt mtab (particularly if 'ulimit 1'
> > was previously issued).
> > 
> > Currently lock_mtab does set some handlers for signals, but not
> > enough.  It arranges for every signal up to (but not including)
> > SIGCHLD to cause mount.nfs to unlock mdadm promptly exit ... even if
> > the default behaviour would be to ignore the signal.  SIGALRM is
> > handled specially, and signals after SIGCHLD are left with their
> > default behaviour.  This includes for example SIGXFSZ which can be
> > generated by the user running "ulimit 1".
> > 
> > So: change this so that some signals are left unchanged, SIGALRM is
> > handled as required, and all signals that the user can generate are
> > explicitly ignored.
> > 
> > The remainder still cause mount.nfs to print a message, unlock mtab, and exit.
> > 
> > Signed-off-by: NeilBrown <neilb@suse.de>
> > ---
> > utils/mount/fstab.c |   37 ++++++++++++++++++++++++++++++++-----
> > 1 files changed, 32 insertions(+), 5 deletions(-)
> > 
> > diff --git a/utils/mount/fstab.c b/utils/mount/fstab.c
> > index a742e64..1fc9efe 100644
> > --- a/utils/mount/fstab.c
> > +++ b/utils/mount/fstab.c
> > @@ -331,16 +331,43 @@ lock_mtab (void) {
> > 		int sig = 0;
> > 		struct sigaction sa;
> > 
> > -		sa.sa_handler = handler;
> > 		sa.sa_flags = 0;
> > 		sigfillset (&sa.sa_mask);
> > 
> > -		while (sigismember (&sa.sa_mask, ++sig) != -1
> > -		       && sig != SIGCHLD) {
> > -			if (sig == SIGALRM)
> > +		while (sigismember (&sa.sa_mask, ++sig) != -1) {
> > +			switch(sig) {
> > +			case SIGCHLD:
> > +			case SIGKILL:
> > +			case SIGCONT:
> > +			case SIGSTOP:
> > +				/* The cannot be caught, or should not,
> > +				 * so don't even try.
> > +				 */
> > +				continue;
> > +			case SIGALRM:
> > 				sa.sa_handler = setlkw_timeout;
> > -			else
> > +				break;
> > +			case SIGHUP:
> > +			case SIGINT:
> > +			case SIGQUIT:
> > +			case SIGWINCH:
> > +			case SIGTSTP:
> > +			case SIGTTIN:
> > +			case SIGTTOU:
> > +			case SIGPIPE:
> > +			case SIGXFSZ:
> > +			case SIGXCPU:
> > +				/* non-priv user can cause these to be
> > +				 * generated, so ignore them.
> > +				 */
> > +				sa.sa_handler = SIG_IGN;
> > +				break;
> > +			default:
> > +				/* The rest should not be possible, so just
> > +				 * print a message and unlock mtab.
> > +				 */
> > 				sa.sa_handler = handler;
> > +			}
> > 			sigaction (sig, &sa, (struct sigaction *) 0);
> > 		}
> > 		signals_have_been_setup = 1;
> > -- 
> > 1.7.3.4
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> --
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com
> 
> 


  reply index

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-06-22  6:38 NeilBrown
2011-06-22 15:08 ` Chuck Lever
2011-06-22 23:20   ` NeilBrown [this message]
2011-06-23 16:09     ` Chuck Lever
2011-06-27 17:03       ` Steve Dickson
2011-06-28  2:11         ` NeilBrown
2011-06-28 17:30           ` Steve Dickson
2011-06-29 14:27 ` Steve Dickson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110623092052.414d78a8@notabene.brown \
    --to=neilb@suse.de \
    --cc=SteveD@redhat.com \
    --cc=chuck.lever@oracle.com \
    --cc=linux-nfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-NFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-nfs/0 linux-nfs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-nfs linux-nfs/ https://lore.kernel.org/linux-nfs \
		linux-nfs@vger.kernel.org
	public-inbox-index linux-nfs

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-nfs


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git