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

[Jason Andryuk <jandryuk@gmail.com>]

On Wed, Jun 24, 2020 at 12:19 PM Ian Jackson <ian.jackson@citrix.com> wrote:
>
> Jan Beulich writes ("Re: use of "stat -""):
> > [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe.
> > On 14.05.2020 13:02, Ian Jackson wrote:
> > > I've read this thread.  Jan, I'm sorry that this causes you
> > > inconvenience.  I'm hoping it won't come down to a choice between
> > > supporting people who want to ship a dom0 without perl, and people who
> > > want a dom0 using more-than-a-decade-old coreutils.
> > >
> > > Jan, can you tell me what the output is of this on your ancient
> > > system:
> > >
> > >   $ rm -f t
> > >   $ >t
> > >   $ exec 3<t
> > >   $ stat -L -c '%F %i' /dev/stdin <&3
> > >   regular empty file 393549
> > >   $ rm t
> > >   $ stat -L -c '%F %i' /dev/stdin <&3
> > >   regular empty file 393549
> > >   $ strace -ou stat -L -c '%F %i' /dev/stdin <&3
> > >   $
> >
> > $ rm -f t
> > $ >t
> > $ exec 3<t
> > $ stat -L -c '%F %i' /dev/stdin <&3
> > regular empty file 3380369
> > $ rm t
> > $ stat -L -c '%F %i' /dev/stdin <&3
> > regular empty file 3380369
> > $ strace -ou stat -L -c '%F %i' /dev/stdin <&3
> > regular empty file 3380369
> >
> > > Also, the contents of the file "u" afterwards, please.
> >
> > Attached.
>
> Thanks.
>
> I think this means that `stat -' can be replaced by `stat /dev/stdin'.
>
> This script is only run on Linux where /dev/stdin has existed
> basically forever.  The strace output shows
>   stat("/dev/stdin", {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
> and the transcript shows that your stat(1) behaves as I hope.
>
> Jan, will you send a patch ?  It is best if someone else but me writes
> it and tests it because then I am a "clean" reviewer.
>
> Paul, supposing I review such a patch and say it is low risk, and we
> commit it by Friday, can it have a release-ack ?

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?  How
would we prove there is no race for /dev/stdin?  And as a refresher,
`stat -` does an fstat(0), so there is no symlink lookup.  Or is there
no race since `stat /proc/self/fd/0` isn't a symlink lookup but just
accessing the already open fd via the proc special file? i.e.
equivalent to fstat.

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

Regards,
Jason