Re: [XEN RFC for-4.14] Re: use of "stat -"

[Ian Jackson <ian.jackson@xxxxxxxxxx>]

Jason Andryuk writes ("Re: [XEN RFC for-4.14] Re: use of "stat -""):
> I was going to just write a patch to replace - with /dev/stdin and ask
> Jan to test it.  When I opened the script, this comment was staring at
> me:
>         # We can't just stat /dev/stdin or /proc/self/fd/$_lockfd or
>         # use bash's test -ef because those all go through what is
>         # actually a synthetic symlink in /proc and we aren't
>         # guaranteed that our stat(2) won't lose the race with an
>         # rm(1) between reading the synthetic link and traversing the
>         # file system to find the inum.
> 
> On my system:
> $ ls -l /dev/stdin
> lrwxrwxrwx 1 root root 15 Jun 24 21:13 /dev/stdin -> /proc/self/fd/0
> $ ls -l /proc/self/fd/0 0<lockfile
> lrwx------ 1 jason jason 64 Jun 24 21:26 /proc/self/fd/0 -> 
> /home/jason/lockfile
> 
> stat /dev/stdin will work around the lack of `stat -` support, but it
> doesn't address the race in the comment.  Is the comment valid?

Thanks, but:

The tests in my transcript show that the comment (which I wrote) is
false.  It shows that stat /dev/stdin works on deleted files, and
stats the right file even if the name has been rused.

>  How would we prove there is no race for /dev/stdin?

It is easy to create the "bad" situation by hand, without racing.

The transcript shows that the output from readlink(2) is a fiction and
that stat works to stat the actual open-file.

> I've mentioned it before, but maybe we should just use the Qubes
> patch.  It leaves the lockfile even when no-one is holding the lock,
> but it simplifies the code and sidesteps the issues we are discussing
> here.
> https://github.com/QubesOS/qubes-vmm-xen/blob/xen-4.13/patch-tools-hotplug-drop-perl-usage-in-locking-mechanism.patch

I don't like that because this locking code might be reused (or maybe
already is used) in contexts with a varying lockfile filename, leaving
many lockfiles.  And because having lockfiles lying about might
confuse sysadmins who are used to programs which use (the broken)
LOCK_EX-style locking paradigm.

So tl;dr: yes, we need that patch to replace - with /dev/stdin.

Thanks,
Ian.