From owner-svn-src-head@FreeBSD.ORG Tue Jul 31 19:07:59 2012 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id A41D1106566B; Tue, 31 Jul 2012 19:07:59 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from bigwig.baldwin.cx (bigknife-pt.tunnel.tserv9.chi1.ipv6.he.net [IPv6:2001:470:1f10:75::2]) by mx1.freebsd.org (Postfix) with ESMTP id 793368FC14; Tue, 31 Jul 2012 19:07:59 +0000 (UTC) Received: from jhbbsd.localnet (unknown [209.249.190.124]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id CC140B926; Tue, 31 Jul 2012 15:07:58 -0400 (EDT) From: John Baldwin To: src-committers@freebsd.org Date: Tue, 31 Jul 2012 15:07:57 -0400 User-Agent: KMail/1.13.5 (FreeBSD/8.2-CBSD-20110714-p17; KDE/4.5.5; amd64; ; ) References: <201207311825.q6VIP113056983@svn.freebsd.org> In-Reply-To: <201207311825.q6VIP113056983@svn.freebsd.org> MIME-Version: 1.0 Content-Type: Text/Plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <201207311507.57986.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Tue, 31 Jul 2012 15:07:58 -0400 (EDT) Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org Subject: Re: svn commit: r238952 - head/sys/kern X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 31 Jul 2012 19:07:59 -0000 On Tuesday, July 31, 2012 2:25:01 pm John Baldwin wrote: > Author: jhb > Date: Tue Jul 31 18:25:00 2012 > New Revision: 238952 > URL: http://svn.freebsd.org/changeset/base/238952 > > Log: > Reorder the managament of advisory locks on open files so that the advisory > lock is obtained before the write count is increased during open() and the > lock is released after the write count is decreased during close(). > > The first change closes a race where an open() that will block with O_SHLOCK > or O_EXLOCK can increase the write count while it waits. If the process > holding the current lock on the file then tries to call exec() on the file > it has locked, it can fail with ETXTBUSY even though the advisory lock is > preventing other threads from succesfully completeing a writable open(). > > The second change closes a race where a read-only open() with O_SHLOCK or > O_EXLOCK may return successfully while the write count is non-zero due to > another descriptor that had the advisory lock and was blocking the open() > still being in the process of closing. If the process that completed the > open() then attempts to call exec() on the file it locked, it can fail with > ETXTBUSY even though the other process that held a write lock has closed > the file and released the lock. > > Reviewed by: kib > MFC after: 1 month Oops, should have included: Tested by: pho I have a regression test (of sorts) for this. If you run it on an unpatched system it should start emitting errors within a few seconds. #include #include #include #include #include #include #include #include #include static void usage(void) { fprintf(stderr, "Usage: flock_close_race [args]\n"); exit(1); } static void child(const char *binary) { int fd; /* Exit as soon as our parent exits. */ while (getppid() != 1) { fd = open(binary, O_RDWR | O_EXLOCK); if (fd < 0) { /* * This may get ETXTBSY since exit() will * close its open fd's (thus releasing the * lock), before it releases the vmspace (and * mapping of the binary). */ if (errno == ETXTBSY) continue; err(1, "can't open %s", binary); } close(fd); } exit(0); } static void exec_child(char **av) { int fd; fd = open(av[0], O_RDONLY | O_SHLOCK); execv(av[0], av); err(127, "execv"); } int main(int ac, char **av) { struct stat sb; pid_t pid; if (ac < 2) usage(); if (stat(av[1], &sb) != 0) err(1, "stat(%s)", av[1]); if (!S_ISREG(sb.st_mode)) errx(1, "%s not an executable", av[1]); pid = fork(); if (pid < 0) err(1, "fork"); if (pid == 0) child(av[1]); for (;;) { pid = fork(); if (pid < 0) err(1, "fork"); if (pid == 0) exec_child(av + 1); wait(NULL); } return (0); } -- John Baldwin