From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from rcsinet10.oracle.com ([148.87.113.121]:22287 "EHLO rcsinet10.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758276Ab1FWQJs convert rfc822-to-8bit (ORCPT ); Thu, 23 Jun 2011 12:09:48 -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: <20110623092052.414d78a8@notabene.brown> Date: Thu, 23 Jun 2011 10:09:33 -0600 Cc: Linux NFS Mailing List Message-Id: <4D5CF8CE-FDFE-491D-99A4-D35853DDEA61@oracle.com> References: <20110622163852.71a2ae80@notabene.brown> <20110623092052.414d78a8@notabene.brown> To: NeilBrown , Steve Dickson Sender: linux-nfs-owner@vger.kernel.org List-ID: MIME-Version: 1.0 On Jun 22, 2011, at 5:20 PM, NeilBrown wrote: > On Wed, 22 Jun 2011 09:08:25 -0600 Chuck Lever 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... We have a libmount-based mount.nfs already in the nfs-utils tree, IIRC. I don't think we yet have a generic plan for switching to installing that one by default. Steve? > Thanks, > NeilBrown > > >> >> 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 >> >> > -- Chuck Lever chuck[dot]lever[at]oracle[dot]com