Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200904195251.GA2139@voyager>
Date: Fri, 4 Sep 2020 21:52:51 +0200
From: Markus Wichmann <nullplan@....net>
To: musl@...ts.openwall.com
Subject: Bug in mmap_fixed()

Hi all,

now, the subject says "bug", but I don't think the conditions to trigger
this are even possible. But still, the code calls attention to it, so
here goes.

In ldso/dynlink.c there is a small function called mmap_fixed(). This
function contains this snippet:


|	ssize_t r;
|	if (lseek(fd, off, SEEK_SET) < 0) return MAP_FAILED;
|	for (q=p; n; q+=r, off+=r, n-=r) {
|		r = read(fd, q, n);
|		if (r < 0 && errno != EINTR) return MAP_FAILED;
|		if (!r) {
|			memset(q, 0, n);
|			break;
|		}
|	}

So when I read this, I immediately thought: What happens when the read()
call does fail due to EINTR? The code specifically excludes that error,
after all. The answer is that after EINTR, r is going to be -1, which
will not be corrected, so the iteration statements will actually back
off the target pointer and increase the remaining length. But since the
file position isn't also backed off, the results of that read will all
be shifted by one byte. If that happens the first time through the loop,
the code will also start overwriting one byte which it is not allowed to
touch (one byte in front of the buffer). I don't know, can this crash on
NOMMU systems? I am aware there were systems in the past lacking an MMU,
but having a memory protection unit. I just don't know if Linux runs on
any of them.

Because here's the crux of the issue: This code is unreachable on
anything but Super-H at the moment, since that is the only architecture
defining DL_NOMMU_SUPPORT. And it hinges on read() returning EINTR,
which, according to signal(7) is impossible: read() can only fail with
EINTR on devices where reading can block indefinitely, and those aren't
seekable. If someone did manage to push a file on the dynlinker where
this can happen, then the lseek() would fail already, and the rest of
the code would never run.

If I am right that the EINTR is impossible, it might be best to just
remove the exception for it.

Ciao,
Markus

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.