Re: [musl] Bug in mmap_fixed()

[Markus Wichmann <nullplan@gmx.net>]

[-- Attachment #1: Type: text/plain, Size: 610 bytes --]

On Fri, Sep 04, 2020 at 11:41:54PM -0400, Rich Felker wrote:
> When I saw your report, I thought this code all ran with signals
> blocked, and actually had to check to see that this isn't the case.

In that case, making an exception for EINTR would be even weirder.

> The code hsould be fixed, and EINTR handling should probably be left
> in-place, just without the wrong pointer-advance logic.
>

See attached. Untested, obviously, since I lack a Super-H processor and
an NFS server, and even then the test case would be quite fiddly, but I
see nothing obviously wrong with it.

Ciao,
Markus

[-- Attachment #2: 0001-Fix-oversight-in-mmap_fixed.patch --]
[-- Type: text/x-diff, Size: 1097 bytes --]

From 3f1ab59a5db1f5c5d943c37981179621d44619d2 Mon Sep 17 00:00:00 2001
From: Markus Wichmann <nullplan@gmx.net>
Date: Sat, 5 Sep 2020 08:35:57 +0200
Subject: [PATCH] Fix oversight in mmap_fixed().

If the read() call in this function ever did return EINTR (which there
is an explicit exception for), then the pointers would be backed off by
one, resulting in the file contents being loaded in shifted by one byte.
And if that happens in the first run through the loop, one byte in front
of the destination buffer would be overwritten, which is invalid.
---
 ldso/dynlink.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ldso/dynlink.c b/ldso/dynlink.c
index f7474743..51c4c004 100644
--- a/ldso/dynlink.c
+++ b/ldso/dynlink.c
@@ -576,7 +576,8 @@ static void *mmap_fixed(void *p, size_t n, int prot, int flags, int fd, off_t of
 	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) {
+		else if (r < 0) r = 0;
+		else if (!r) {
 			memset(q, 0, n);
 			break;
 		}
--
2.17.1