mailing list of musl libc
 help / color / mirror / code / Atom feed
From: Markus Wichmann <nullplan@gmx.net>
To: musl@lists.openwall.com
Subject: [musl] Bug in mmap_fixed()
Date: Fri, 4 Sep 2020 21:52:51 +0200	[thread overview]
Message-ID: <20200904195251.GA2139@voyager> (raw)

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

             reply	other threads:[~2020-09-04 19:53 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-04 19:52 Markus Wichmann [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200904195251.GA2139@voyager \
    --to=nullplan@gmx.net \
    --cc=musl@lists.openwall.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).