The following reply was made to PR kern/175674; it has been noted by GNATS.
From: David Xu <davi...@freebsd.org>
To: Jilles Tjoelker <jil...@stack.nl>
Cc: Giorgos Keramidas <keram...@freebsd.org>, Jukka Ukkonen <j...@iki.fi>,
freebsd-gnats-sub...@freebsd.org
Subject: Re: kern/175674: sem_open() should use O_EXLOCK with open() instead
of a separate flock() call
Date: Mon, 04 Feb 2013 10:08:54 +0800
On 2013/02/04 04:20, Jilles Tjoelker wrote:
> On Sun, Feb 03, 2013 at 06:25:25AM +0100, Giorgos Keramidas wrote:
>> On 2013-01-29 18:03, Jukka Ukkonen <j...@iki.fi> wrote:
>>>> Number: 175674
>>>> Category: kern
>>>> Synopsis: sem_open() should use O_EXLOCK with open() instead of a
>>>> separate flock() call
>>
>>>> Environment:
>>> FreeBSD sleipnir 9.1-STABLE FreeBSD 9.1-STABLE #2 r246056M: Tue Jan 29
>>> 07:33:01 EET 2013 root@sleipnir:/usr/obj/usr/src/sys/Sleipnir amd64
>>>> Description:
>>> sem_open() is calling flock() to set a lock on a newly created file
>>> descriptor.
>>> That is pointless. The open() call a few lines before the flock() could,
>>> and
>>> in my opinion should, be done with the O_EXLOCK flag set.
>
>> It's also a bit safer to obtain the exclusive lock atomically before
>> open() returns. Waiting for open() to complete and then calling flock()
>> has a race condition.
>
>> Jilles and David, do you think this patch looks ok for libc?
>
>>> Patch attached with submission follows:
>>>
>>> --- lib/libc/gen/sem_new.c.flock 2012-11-09 18:50:05.000000000 +0200
>>> +++ lib/libc/gen/sem_new.c 2012-11-09 18:44:59.000000000 +0200
>>> @@ -198,11 +198,13 @@
>>> goto error;
>>> }
>>>
>>> - fd = _open(path, flags|O_RDWR|O_CLOEXEC, mode);
>>> + fd = _open(path, flags|O_RDWR|O_CLOEXEC|O_EXLOCK, mode);
>>> if (fd == -1)
>>> goto error;
>>> +#if 0
>>> if (flock(fd, LOCK_EX) == -1)
>>> goto error;
>>> +#endif
>>> if (_fstat(fd, &sb)) {
>>> flock(fd, LOCK_UN);
>>> goto error;
>
> For a reason unknown to me, open(2) does not restart but always returns
> [EINTR] when a signal is caught. This is not POSIX-compliant. On the
> other hand, flock(2) is not broken in this way. So this change breaks
> sem_open(3) in the unlikely case that a signal with SA_RESTART arrives
> while it is waiting for the lock.
>
> The best way to fix this is in kern_openat() in the kernel but this
> might cause compatibility issues.
>
> The #if 0 is silly; we have version control to restore old code if need
> be.
>
Note that EINTR is allowed to be returned for sem_open().
http://pubs.opengroup.org/onlinepubs/009604499/functions/sem_open.html
Regards,
David Xu
_______________________________________________
freebsd-bugs@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-bugs
To unsubscribe, send any mail to "freebsd-bugs-unsubscr...@freebsd.org"
