mailing list of musl libc
 help / color / mirror / code / Atom feed
* [musl] Bug in mmap_fixed()
@ 2020-09-04 19:52 Markus Wichmann
  2020-09-05  3:41 ` Rich Felker
  2020-09-05  8:39 ` Stefan O'Rear
  0 siblings, 2 replies; 7+ messages in thread
From: Markus Wichmann @ 2020-09-04 19:52 UTC (permalink / raw)
  To: musl

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-09-16 14:20 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-04 19:52 [musl] Bug in mmap_fixed() Markus Wichmann
2020-09-05  3:41 ` Rich Felker
2020-09-05  6:44   ` Markus Wichmann
2020-09-16  5:15     ` Rob Landley
2020-09-16 12:04       ` Rich Felker
2020-09-16 14:20       ` Markus Wichmann
2020-09-05  8:39 ` Stefan O'Rear

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).