But that's quite unlikely to happen. And I have no fix to propose.
If arbitrary other tasks are running, the only way to be absolutely
certain you're not calling unlink() in a directory is to never call
unlink().
SUS describes a safe remove(), but Solaris's implementation contains the
same race:
http://src.opensolaris.org/source/xref/pef/phase_I/usr/src/lib/libc/port/gen/rename.c
so I think this patch is the best that can be done.
Best regards,
Scott
--
Scott Lamb <http://www.slamb.org/>
From: Thomas Glanzmann <hidden> Date: 2016-06-15 22:43:21
Hello,
If arbitrary other tasks are running, the only way to be absolutely
certain you're not calling unlink() in a directory is to never call
unlink().
there is one way to do it safe but it is so ugly that it is
unacceptable: don't call unlink as a privileged user (eg. root). So I
hope that one of the patches make it into git soon. I like the second
patch better because it does less system calls. Not that it matters.
For my co-workers I already build a git version with the patch in so
that they can continue to work as root. Don't even think about asking.
Thomas
so I think this patch is the best that can be done.
is there a reason why we call unlink and not remove?
Exactly because we only want to remove _files_.
If it's already a directory, we don't need to do anything at all (we just
want to go to the next path component).
So what git wants is the modern "unlink()" behaviour that will return
EPERM (oe EISDIR) for a directory.
Not doing that in this day and age is *insane*. That whole "unlink/link"
on directories is original UNIX, but it's original UNIX from several
decades ago. It got fixed long long ago, and mkdir/rmdir have existed as
system calls since at least SVR3. Nobody does the insane "unlink()" any
more.
Except in Solaris, it would appear.
Linus
is there a reason why we call unlink and not remove?
quoted
Exactly because we only want to remove _files_.
of course. That is the whole point. Call unlink for files, rmdir for
directories.
No, but we don't *want* the "rmdir for directories" part!
That's the whole point.
Calling "remove()" would be *wrong*. We want the *sane* "unlink()"
behaviour, where it only removes files, and returns an error for
directories.
Linus
From: Thomas Glanzmann <hidden> Date: 2016-06-15 22:43:21
Hello,
No, but we don't *want* the "rmdir for directories" part!
that is what I meant. We call unlink because we want unlink to _fail_ on
directories while it deletes file. I forgot about the original
discussion but Johannes refreshed my memory. If a file in our history
becomes a directory we want to get it out of our way. And we want to do
that by call unlink.
Thomas
From: Scott Lamb <hidden> Date: 2016-06-15 22:43:21
Linus Torvalds wrote:
No, but we don't *want* the "rmdir for directories" part!
That's the whole point.
Calling "remove()" would be *wrong*. We want the *sane* "unlink()"
behaviour, where it only removes files, and returns an error for
directories.
Of course, but when used immediately after stat() says the path does not
refer to a directory, I would prefer SUS remove() (rmdir() for
directories) to Solaris unlink() (break_filesystem() on directories).
But Solaris remove() is broken, too, so it's a moot point. The
post-patch behavior is good enough - as you said, it won't happen during
reasonable usage and the problem's not unique to git.
Best regards,
Scott
--
Scott Lamb <http://www.slamb.org/>
But Solaris remove() is broken, too, so it's a moot point.
In fact, with the Solaris behaviour for unlink(), you *cannot* have a
non-broken "remove()".
So the right fix is always to fix "unlink()" instead.
There really aren't any downsides (since no program can rely on it
_anyway_, unless we're talking about some magic "early bootup" time
scripts that depend on only running as root, and only ever running on UFS
- but those kinds of scripts could be trivially fixed and are obviously
under Sun control anyway)
Linus
From: Scott Lamb <hidden> Date: 2016-06-15 22:43:21
Linus Torvalds wrote:
On Mon, 16 Jul 2007, Scott Lamb wrote:
quoted
But Solaris remove() is broken, too, so it's a moot point.
In fact, with the Solaris behaviour for unlink(), you *cannot* have a
non-broken "remove()".
I'd hoped to see that they made a new syscall to properly implement the
new behavior. But they didn't. It reminds me of glibc's pselect().
Best regards,
Scott
--
Scott Lamb <http://www.slamb.org/>