[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



On Thu, Jun 25, 2020 at 3:05 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> On 25.06.2020 04:37, Jason Andryuk wrote:
> > On Wed, Jun 24, 2020 at 12:19 PM Ian Jackson <ian.jackson@xxxxxxxxxx> 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.
>
> I was about to, when I saw this reply from Jason.
>
> >> 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.
>
> Looking at vfs_statx() in the kernel, I can't see any provisions to
> get at the data without traversing the specified path.

Ian, you wrote the comment originally.  Would you please clarify the
scenario where there is a race?  Only the lock holder is allowed to rm
the lockfile, so how is there a race between rm and stat?

Regards,
Jason



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.