Comment 8 for bug 1492570

halfdog [2015-09-11 20:54 -0000]:
> The first patch will solve the issue only when the new crashdump is not
> only with e.g. O_CREAT|O_EXCL|O_NOFOLLOW, and then it would not be
> needed any more.

There are some parts missing here, but I think I understand what you
mean. With the second patch, the .crash file is now opened with
O_EXCL. However, that does not do anything to prevent self-inclusion:
If you create a

  /var/crash/vmcore.log -> linux_1234.crash

then even with O_EXCL the creation of /var/crash/linux_1234.crash will
succeed. It will just try to slurp in the contents of itself. This is
now prevented with the os.O_NOFOLLOW.

I don't see how a hardlink would work: You'd have to pre-create a file
/var/crash/linux_1234.crash in order to hardlink to it, and then
kernel_crashdump would fail to open it due to the O_EXCL in the second
patch.

> With current patch, a hardlink to the crash dump would still allow DOS.

With just the first one, yes. With the second one I believe no.

> This would make also sense in another way: the os.walk below is in that
> combination far more dangerous than the plain open:
>

Indeed, I missed that. This needs to be fixed in the second patch, to
create the .crash files there atomically too. Thanks!

> Apart from that, the loop misses joining in the root element into the
> path, so os.walk will essentially walk some objects, but the open calls
> afterwards will just open something completely different.

I believe that's indentional. We don't want to put the .crash files
into the per-timestamp subdirs, but right into /var/crash/.

I'll post an updated second patch shortly.
--
Martin Pitt | http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)