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)
halfdog [2015-09-11 20:54 -0000]: O_EXCL| O_NOFOLLOW, and then it would not be
> The first patch will solve the issue only when the new crashdump is not
> only with e.g. O_CREAT|
> 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 linux_1234. crash in order to hardlink to it, and then
/var/crash/
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. www.piware. de
--
Martin Pitt | http://
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)