From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from rcsinet10.oracle.com ([148.87.113.121]:65477 "EHLO rcsinet10.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757991Ab1FVPIk convert rfc822-to-8bit (ORCPT ); Wed, 22 Jun 2011 11:08:40 -0400 Subject: Re: [PATCH/RFC] mount: improve signal management when locking mtab. Content-Type: text/plain; charset=us-ascii From: Chuck Lever In-Reply-To: <20110622163852.71a2ae80@notabene.brown> Date: Wed, 22 Jun 2011 09:08:25 -0600 Cc: Steve Dickson , linux-nfs@vger.kernel.org Message-Id: References: <20110622163852.71a2ae80@notabene.brown> To: NeilBrown Sender: linux-nfs-owner@vger.kernel.org List-ID: MIME-Version: 1.0 Since we are going to adopt libmount for mount.nfs in the near future, would it be better to update libmount instead? On Jun 22, 2011, at 12: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 > --- > 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