NetBSD Problem Report #44093

From yamt@NetBSD.org  Mon Nov 15 15:18:23 2010
Return-Path: <yamt@NetBSD.org>
Received: by www.NetBSD.org (Postfix, from userid 1270)
	id 52A7563BA50; Mon, 15 Nov 2010 15:18:23 +0000 (UTC)
Message-Id: <20101115151823.52A7563BA50@www.NetBSD.org>
Date: Mon, 15 Nov 2010 15:18:23 +0000 (UTC)
From: yamt@NetBSD.org
Reply-To: yamt@NetBSD.org
To: gnats-bugs@NetBSD.org
Subject: puffs is too sensitive to signal
X-Send-Pr-Version: 3.95

>Number:         44093
>Category:       kern
>Synopsis:       puffs is too sensitive to signal
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Nov 15 15:20:01 +0000 2010
>Closed-Date:    Tue Nov 16 00:16:15 +0000 2010
>Last-Modified:  Fri Jul 15 23:45:01 +0000 2011
>Originator:     YAMAMOTO Takashi
>Release:        NetBSD-current
>Organization:

>Environment:

>Description:
	puffs is too sensitive to signal.
	filesystem operations should not be interrupted by arbitrary signals.
>How-To-Repeat:
	"git clone" to a puffs-based filesystem and see "write error".
>Fix:

Index: puffs_msgif.c
===================================================================
RCS file: /cvsroot/src/sys/fs/puffs/puffs_msgif.c,v
retrieving revision 1.83
diff -u -p -r1.83 puffs_msgif.c
--- puffs_msgif.c	12 Nov 2010 17:46:09 -0000	1.83
+++ puffs_msgif.c	15 Nov 2010 15:13:20 -0000
@@ -378,17 +378,30 @@ puffs_msg_enqueue(struct puffs_mount *pm
 	if (__predict_false((park->park_flags & PARKFLAG_WANTREPLY)
 	   && (park->park_flags & PARKFLAG_CALL) == 0
 	   && (l->l_flag & LW_PENDSIG) != 0 && sigispending(l, 0))) {
-		park->park_flags |= PARKFLAG_HASERROR;
-		preq->preq_rv = EINTR;
-		if (PUFFSOP_OPCLASS(preq->preq_opclass) == PUFFSOP_VN
-		    && (preq->preq_optype == PUFFS_VN_INACTIVE
-		     || preq->preq_optype == PUFFS_VN_RECLAIM)) {
-			park->park_preq->preq_opclass |= PUFFSOPFLAG_FAF;
-			park->park_flags &= ~PARKFLAG_WANTREPLY;
-			DPRINTF(("puffs_msg_enqueue: converted to FAF %p\n",
-			    park));
-		} else {
-			return;
+		sigset_t ss;
+
+		/*
+		 * see the comment about signals in puffs_msg_wait.
+		 */
+		sigpending1(l, &ss);
+		if (sigismember(&ss, SIGINT) ||
+		    sigismember(&ss, SIGTERM) ||
+		    sigismember(&ss, SIGKILL) ||
+		    sigismember(&ss, SIGHUP) ||
+		    sigismember(&ss, SIGQUIT)) {
+			park->park_flags |= PARKFLAG_HASERROR;
+			preq->preq_rv = EINTR;
+			if (PUFFSOP_OPCLASS(preq->preq_opclass) == PUFFSOP_VN
+			    && (preq->preq_optype == PUFFS_VN_INACTIVE
+			     || preq->preq_optype == PUFFS_VN_RECLAIM)) {
+				park->park_preq->preq_opclass |=
+				    PUFFSOPFLAG_FAF;
+				park->park_flags &= ~PARKFLAG_WANTREPLY;
+				DPRINTF(("puffs_msg_enqueue: "
+				    "converted to FAF %p\n", park));
+			} else {
+				return;
+			}
 		}
 	}

@@ -426,10 +439,30 @@ puffs_msg_enqueue(struct puffs_mount *pm
 int
 puffs_msg_wait(struct puffs_mount *pmp, struct puffs_msgpark *park)
 {
+	lwp_t *l = curlwp;
+	proc_t *p = l->l_proc;
 	struct puffs_req *preq = park->park_preq; /* XXX: hmmm */
+	sigset_t ss;
+	sigset_t oss;
 	int error = 0;
 	int rv;

+	/*
+	 * block unimportant signals.
+	 *
+	 * The set of "important" signals here was chosen to be same as
+	 * nfs interruptible mount.
+	 */
+	sigfillset(&ss);
+	sigdelset(&ss, SIGINT);
+	sigdelset(&ss, SIGTERM);
+	sigdelset(&ss, SIGKILL);
+	sigdelset(&ss, SIGHUP);
+	sigdelset(&ss, SIGQUIT);
+	mutex_enter(p->p_lock);
+	sigprocmask1(l, SIG_BLOCK, &ss, &oss);
+	mutex_exit(p->p_lock);
+
 	mutex_enter(&pmp->pmp_lock);
 	puffs_mp_reference(pmp);
 	mutex_exit(&pmp->pmp_lock);
@@ -503,6 +536,10 @@ puffs_msg_wait(struct puffs_mount *pmp, 
 	puffs_mp_release(pmp);
 	mutex_exit(&pmp->pmp_lock);

+	mutex_enter(p->p_lock);
+	sigprocmask1(l, SIG_SETMASK, &oss, NULL);
+	mutex_exit(p->p_lock);
+
 	return rv;
 }


>Release-Note:

>Audit-Trail:
From: "Antti Kantee" <pooka@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/44093 CVS commit: src/sys/fs/puffs
Date: Mon, 15 Nov 2010 20:31:41 +0000

 Module Name:	src
 Committed By:	pooka
 Date:		Mon Nov 15 20:31:41 UTC 2010

 Modified Files:
 	src/sys/fs/puffs: puffs_msgif.c

 Log Message:
 Apply patch from PR kern/44093 by yamt:

 Interrupt server wait only on certain signals (same set at nfs -i)
 instead of all signals.  According to the PR this helps with
 "git clone" run on a puffs file system.


 To generate a diff of this commit:
 cvs rdiff -u -r1.83 -r1.84 src/sys/fs/puffs/puffs_msgif.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Antti Kantee" <pooka@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/44093 CVS commit: src/tests/fs/puffs
Date: Mon, 15 Nov 2010 20:39:01 +0000

 Module Name:	src
 Committed By:	pooka
 Date:		Mon Nov 15 20:39:00 UTC 2010

 Modified Files:
 	src/tests/fs/puffs: t_basic.c

 Log Message:
 Add test case for PR kern/44093 (fixed already, but tested to fail
 without fix applied).


 To generate a diff of this commit:
 cvs rdiff -u -r1.8 -r1.9 src/tests/fs/puffs/t_basic.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->closed
State-Changed-By: pooka@NetBSD.org
State-Changed-When: Tue, 16 Nov 2010 02:16:15 +0200
State-Changed-Why:
fix applied, thanks


From: "Manuel Bouyer" <bouyer@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/44093 CVS commit: [netbsd-5] src/sys/fs/puffs
Date: Thu, 19 May 2011 21:00:16 +0000

 Module Name:	src
 Committed By:	bouyer
 Date:		Thu May 19 21:00:16 UTC 2011

 Modified Files:
 	src/sys/fs/puffs [netbsd-5]: puffs_msgif.c

 Log Message:
 Pull up following revision(s) (requested by manu in ticket #1604):
 	sys/fs/puffs/puffs_msgif.c: revision 1.84 via patch
 Apply patch from PR kern/44093 by yamt:
 Interrupt server wait only on certain signals (same set at nfs -i)
 instead of all signals.  According to the PR this helps with
 "git clone" run on a puffs file system.


 To generate a diff of this commit:
 cvs rdiff -u -r1.72.4.1 -r1.72.4.2 src/sys/fs/puffs/puffs_msgif.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Jeff Rizzo" <riz@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/44093 CVS commit: [netbsd-5] src/sys
Date: Fri, 15 Jul 2011 23:41:14 +0000

 Module Name:	src
 Committed By:	riz
 Date:		Fri Jul 15 23:41:13 UTC 2011

 Modified Files:
 	src/sys/fs/puffs [netbsd-5]: puffs_msgif.c
 	src/sys/rump/librump/rumpkern [netbsd-5]: Makefile.rumpkern emul.c
 Added Files:
 	src/sys/rump/librump/rumpkern/opt [netbsd-5]: opt_compat_netbsd32.h

 Log Message:
 Pull up following revision(s) (requested by manu in ticket #1604):
 	sys/fs/puffs/puffs_msgif.c: revision 1.84
 Apply patch from PR kern/44093 by yamt:
 Interrupt server wait only on certain signals (same set at nfs -i)
 instead of all signals.  According to the PR this helps with
 "git clone" run on a puffs file system.


 To generate a diff of this commit:
 cvs rdiff -u -r1.72.4.3 -r1.72.4.4 src/sys/fs/puffs/puffs_msgif.c
 cvs rdiff -u -r1.17 -r1.17.4.1 \
     src/sys/rump/librump/rumpkern/Makefile.rumpkern
 cvs rdiff -u -r1.53 -r1.53.4.1 src/sys/rump/librump/rumpkern/emul.c
 cvs rdiff -u -r0 -r1.1.2.1 \
     src/sys/rump/librump/rumpkern/opt/opt_compat_netbsd32.h

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

>Unformatted:

Home	PR Database Search