From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com ([209.132.183.28]:47715 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756934Ab1F2O14 (ORCPT ); Wed, 29 Jun 2011 10:27:56 -0400 Message-ID: <4E0B3669.4070702@RedHat.com> Date: Wed, 29 Jun 2011 10:27:53 -0400 From: Steve Dickson To: NeilBrown CC: Linux NFS Mailing list Subject: Re: [PATCH/RFC] mount: improve signal management when locking mtab. References: <20110622163852.71a2ae80@notabene.brown> In-Reply-To: <20110622163852.71a2ae80@notabene.brown> Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-nfs-owner@vger.kernel.org List-ID: MIME-Version: 1.0 On 06/22/2011 02:38 AM, NeilBrown wrote: > > From: Neil Brown > 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 Committed... steved. > --- > 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;