Experiment on how to improve our temporary file handing.

Project Infrastructure LLVM Dev List Archives
1

Currently a power failure or other hard crash can cause lld leave a temporary
file around. The same is true for other llvm tools.

As an example, put a breakpoint in Writer.cpp:236 ("writeBuildId()") and
restart the run a few times. You will get

t.tmp43a735a t.tmp4deeabb t.tmp9bacdd3 t.tmpe4115c4 t.tmpeb01fff

The same would happen if there was a fatal error between the
FileOutputBuffer creation and commit. I don't think that is a code path
where that is possible right now, but it would be an easy thing to miss
in a code review.

I was hopping the OS could help us manage the temporary file so that
there was no way to accidentally leave it behind.

On linux there is O_TMPFILE, which allows us to create a file with no
name in the file system. A name can be given with linkat. Unfortunately
we can't use

linkat(fd, "", AT_FDCWD, "destination", AT_EMPTY_PATH)

Without special permissions and have instead to depend on proc:

linkat(AT_FDCWD, "/proc/self/fd/<num>", AT_FDCWD, "destination",
       AT_SYMLINK_FOLLOW)

Another annoyance is that linkat will not replace the destination and
renameat2 doesn't support AT_EMPTY_PATH. The result is that we have to
use unlink+linkat and loop.

On windows there is FILE_FLAG_DELETE_ON_CLOSE, but there seems to be no
way to cancel it. If a file is created with it I can rename it, but it
is still deleted in the end.

I couldn't find any support for this on FreeBSD.

This suggest that we cannot just have a createUniqueEntity that returs
just an FD. We will need a class that stores a temporary name too for
the systems where we cannot give a filename to a FD.

I have attached the patch I got so far, but given the existing
restrictions I would say it is not worth it.

It will try to just make it more obvious that lld's FileOutputBuffer is
deleted on all code paths.

t.diff (7.54 KB)

2

I agree, having a nameless FD would be the most elegant way to solve this problem on Unix, but when I investigated it, I came to the same conclusion that you did: it can’t be done portably. Nameless files are just too weird, so I think we’re stuck with RemoveFileOnSignal. =/

For Windows, I was able to use SetFileInformationByHandle to get the OS to clean up files for us on exit. I attached my proof-of-concept program that does this.

writedelclose.cpp (3.48 KB)

3

Reid Kleckner <rnk@google.com> writes:

  // OK, we want to keep this file.
  disposition.DeleteFile = FALSE;
  if (!SetFileInformationByHandle(h, FileDispositionInfo, &disposition,
                                  sizeof(disposition)))
    error("SetFileInformationByHandle");

Nice!

That seems to be the part I was missing. That motivates me to finish the
implementation. Having perfect output file handling on windows is going
to be useful for us and might hopefully put some pressure on posix
vendors to make it possible :slight_smile:

Cheers,
Rafael

4

Date: Thu, 09 Nov 2017 13:13:52 -0800
From: Rafael Avila de Espindola via llvm-dev <llvm-dev@lists.llvm.org>

Currently a power failure or other hard crash can cause lld leave a temporary
file around. The same is true for other llvm tools.

As an example, put a breakpoint in Writer.cpp:236 ("writeBuildId()") and
restart the run a few times. You will get

t.tmp43a735a t.tmp4deeabb t.tmp9bacdd3 t.tmpe4115c4 t.tmpeb01fff

The same would happen if there was a fatal error between the
FileOutputBuffer creation and commit. I don't think that is a code path
where that is possible right now, but it would be an easy thing to miss
in a code review.

I was hopping the OS could help us manage the temporary file so that
there was no way to accidentally leave it behind.

On linux there is O_TMPFILE, which allows us to create a file with no
name in the file system.

For the LLVM use case creatinmg the file and immediately unlinking it
would probably be good enough. Howver...

A name can be given with linkat. Unfortunately we can't use

linkat(fd, "", AT_FDCWD, "destination", AT_EMPTY_PATH)

Without special permissions and have instead to depend on proc:

linkat(AT_FDCWD, "/proc/self/fd/<num>", AT_FDCWD, "destination",
       AT_SYMLINK_FOLLOW)

Another annoyance is that linkat will not replace the destination and
renameat2 doesn't support AT_EMPTY_PATH. The result is that we have to
use unlink+linkat and loop.

I'd strongly recommend avoiding such unportable cleverness.

I'm actually surprised Linux allows this as there are some serious
security implications as this allows programs to create an entry in
the filesystem for file descriptors passed over a socket.

5

In a very large class of cases the correct thing is to cleanup on next run. Not sure clang is in that class by any means, just tossing this out there…

6

Mark Kettenis <mark.kettenis@xs4all.nl> writes:

The same would happen if there was a fatal error between the
FileOutputBuffer creation and commit. I don't think that is a code path
where that is possible right now, but it would be an easy thing to miss
in a code review.

I was hopping the OS could help us manage the temporary file so that
there was no way to accidentally leave it behind.

On linux there is O_TMPFILE, which allows us to create a file with no
name in the file system.

For the LLVM use case creatinmg the file and immediately unlinking it
would probably be good enough. Howver...

One is not normally allowed to create a link once the count goes down to
zero. O_TMPFILE is a special case.

Cheers,
Rafael

7

Mark Kettenis <mark.kettenis@xs4all.nl> writes:

I'm actually surprised Linux allows this as there are some serious
security implications as this allows programs to create an entry in
the filesystem for file descriptors passed over a socket.

BTW, would you mind expanding on what is the security problem of that?

Thanks,
Rafael

8

NTFS journals writes and keeps a fairly large buffer so it’d have to be a very well timed, fast power loss. Software should do what it could to alleviate this (especially in long runtime cases) but a cheap UPS or some form of hardware battery backed hardware RAID is a better solution if the data is that important, IMO.

As an aside, Windows 10 with enough RAM can run for at least 3 minutes with the system drive cable pulled. I did it accidentally once while hot-swapping another drive that snagged it. No data corruption, NTFS journals in memory finished the write when I noticed my clumsy and and plugged it back in. Just anecdotal but funny to me.

9

Currently a power failure or other hard crash can cause lld leave a
temporary
file around. The same is true for other llvm tools.

As an example, put a breakpoint in Writer.cpp:236 ("writeBuildId()") and
restart the run a few times. You will get

t.tmp43a735a t.tmp4deeabb t.tmp9bacdd3 t.tmpe4115c4 t.tmpeb01fff

The same would happen if there was a fatal error between the
FileOutputBuffer creation and commit. I don't think that is a code path
where that is possible right now, but it would be an easy thing to miss
in a code review.

I was hopping the OS could help us manage the temporary file so that
there was no way to accidentally leave it behind.

On linux there is O_TMPFILE, which allows us to create a file with no
name in the file system. A name can be given with linkat. Unfortunately
we can't use

linkat(fd, "", AT_FDCWD, "destination", AT_EMPTY_PATH)

Without special permissions and have instead to depend on proc:

linkat(AT_FDCWD, "/proc/self/fd/<num>", AT_FDCWD, "destination",
       AT_SYMLINK_FOLLOW)

I often execute lld in a chroot environment in which /proc is not mounted,
and I expect lld would work just fine in that environment. So the presence
of /proc should be considered optional even on Linux.

Another annoyance is that linkat will not replace the destination and

renameat2 doesn't support AT_EMPTY_PATH. The result is that we have to
use unlink+linkat and loop.

I think you can avoid unlink+linkat. You can instead give a temporary file
name to an unnamed file and then rename the temporary file the destination
file. There's a small chance between linkat and rename, but that race
condition is probably better than unlink+linkat.

On windows there is FILE_FLAG_DELETE_ON_CLOSE, but there seems to be no

10

Without special permissions and have instead to depend on proc:

linkat(AT_FDCWD, "/proc/self/fd/<num>", AT_FDCWD, "destination",
       AT_SYMLINK_FOLLOW)

I often execute lld in a chroot environment in which /proc is not mounted,
and I expect lld would work just fine in that environment. So the presence
of /proc should be considered optional even on Linux.

I agree. We would have to check if proc is present before using O_TMPFILE.

Another annoyance is that linkat will not replace the destination and

renameat2 doesn't support AT_EMPTY_PATH. The result is that we have to
use unlink+linkat and loop.

I think you can avoid unlink+linkat. You can instead give a temporary file
name to an unnamed file and then rename the temporary file the destination
file. There's a small chance between linkat and rename, but that race
condition is probably better than unlink+linkat.

True. I feels silly that we still need to create a temporary name with
O_TMPFILE, but that is a possibility.

In any case. I will focus on the windows case first as it seems to be
the more generally available API.

Cheers,
Rafael

11

AFAIK FreeBSD supports some variant of /proc that should map Linux
(although the mapping isn't 1:1).
Does it lack support for this? Thanks for looking into this, BTW!

12

Davide Italiano <davide.italiano@gmail.com> writes:

I couldn't find any support for this on FreeBSD.

AFAIK FreeBSD supports some variant of /proc that should map Linux
(although the mapping isn't 1:1).
Does it lack support for this? Thanks for looking into this, BTW!

O_TMPFILE is the main thing that seems to be missing.

Cheers,
Rafael

13

It doesn't seem unreasonable to ask FreeBSD to implement this (at
least for ufs/zfs), maybe Ed/Kostik have thoughts about it.

Thanks,

14

Can you please explain, how exactly O_TMPFILE functionality you asking
for relates to the /proc ? Do you want to re-link O_TMPFILE-opened
inodes into named files ?

15

Konstantin Belousov <kostikbel@gmail.com> writes:

> Davide Italiano <davide.italiano@gmail.com> writes:
>
>>> I couldn't find any support for this on FreeBSD.
>>>
>>
>> AFAIK FreeBSD supports some variant of /proc that should map Linux
>> (although the mapping isn't 1:1).
>> Does it lack support for this? Thanks for looking into this, BTW!
>
> O_TMPFILE is the main thing that seems to be missing.
>

It doesn't seem unreasonable to ask FreeBSD to implement this (at
least for ufs/zfs), maybe Ed/Kostik have thoughts about it.

Can you please explain, how exactly O_TMPFILE functionality you asking
for relates to the /proc ? Do you want to re-link O_TMPFILE-opened
inodes into named files ?

The relation to proc seems to be a linux peculiarity.

What we want in the end is to implement "-o foo" in lld with the
following guarantees:

1 foo is only created with the final content.
2 We don't leave a temporary file behind on a power failure.
3 Less critical but nice: if foo already exists, it is replaced
  atomically.

Using mkstemp + rename we get 1 and 3.

On linux we can use O_TMPFILE, but the only way for a non privileged
process to then give a name to that fd is:

linkat(AT_FDCWD, "/proc/self/fd/<num>", AT_FDCWD, "destination",
       AT_SYMLINK_FOLLOW)

which has two problem: It requires proc to be mounted and it is not
atomic if the destination already exists.

A system with O_TMPFILE which allows some form of renameat to
atomically give that fd a name would be perfect.

Cheers,
Rafael

16

Konstantin Belousov <kostikbel@gmail.com> writes:

>> > Davide Italiano <davide.italiano@gmail.com> writes:
>> >
>> >>> I couldn't find any support for this on FreeBSD.
>> >>>
>> >>
>> >> AFAIK FreeBSD supports some variant of /proc that should map Linux
>> >> (although the mapping isn't 1:1).
>> >> Does it lack support for this? Thanks for looking into this, BTW!
>> >
>> > O_TMPFILE is the main thing that seems to be missing.
>> >
>>
>> It doesn't seem unreasonable to ask FreeBSD to implement this (at
>> least for ufs/zfs), maybe Ed/Kostik have thoughts about it.
>
> Can you please explain, how exactly O_TMPFILE functionality you asking
> for relates to the /proc ? Do you want to re-link O_TMPFILE-opened
> inodes into named files ?

The relation to proc seems to be a linux peculiarity.

What we want in the end is to implement "-o foo" in lld with the
following guarantees:

1 foo is only created with the final content.
2 We don't leave a temporary file behind on a power failure.
3 Less critical but nice: if foo already exists, it is replaced
  atomically.

Using mkstemp + rename we get 1 and 3.

On linux we can use O_TMPFILE, but the only way for a non privileged
process to then give a name to that fd is:

linkat(AT_FDCWD, "/proc/self/fd/<num>", AT_FDCWD, "destination",
       AT_SYMLINK_FOLLOW)

which has two problem: It requires proc to be mounted and it is not
atomic if the destination already exists.

This is quite different request from O_TMPFILE. The tmpfile can be
implemented in a day or two for UFS. The ability to enter the inode
referenced by a file descriptor, into the filesystem namespace at a
given location, was proposed several times and rejected, I believe. The
cause is that it potentially allows to gain additional access rights.

Imagine that a process obtained a file descriptor only opened for read,
e.g. by passing over unix domain socket. If it is possible to link
its inode, you can re-open it limited by access permissions on the
inode. Inode may have rw rights, but its current containing directory
disallowing the walk.

This is especially important for capabilities-based sandbox environments,
like capsicum.

A system with O_TMPFILE which allows some form of renameat to
atomically give that fd a name would be perfect.

I believe that #2 is not that critical.

17

Konstantin Belousov <kostikbel@gmail.com> writes:

This is quite different request from O_TMPFILE. The tmpfile can be
implemented in a day or two for UFS. The ability to enter the inode
referenced by a file descriptor, into the filesystem namespace at a
given location, was proposed several times and rejected, I believe. The
cause is that it potentially allows to gain additional access rights.

Imagine that a process obtained a file descriptor only opened for read,
e.g. by passing over unix domain socket. If it is possible to link
its inode, you can re-open it limited by access permissions on the
inode. Inode may have rw rights, but its current containing directory
disallowing the walk.

Thanks. I had seen security concerns mentioned before, but never an
actual example.

This is especially important for capabilities-based sandbox environments,
like capsicum.

Yes. I guess a "link" capability would be needed and for backwards
compatibility open would normally not return an fd with it.

A system with O_TMPFILE which allows some form of renameat to
atomically give that fd a name would be perfect.

I believe that #2 is not that critical.

Not critical, just very convenient.

A power failure is just the extreme case. The more common issue is
making sure that every code path removes or renames the temp file,
including signals terminating the program.

Having a temp file without a name or one that is deleted on close (like
on windows) would make the solution trivial.

Cheers,
Rafael