Bug 203852 - emulators/qemu-user-static: mmap may return NULL instead of MAP_FAILED to indicate error
Summary: emulators/qemu-user-static: mmap may return NULL instead of MAP_FAILED to ind...
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: Sean Bruno
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-10-18 07:36 UTC by Eugene Grosbein
Modified: 2016-01-04 18:27 UTC (History)
5 users (show)

See Also:
bugzilla: maintainer-feedback? (nox)


Attachments
return -1 when len is zero (1014 bytes, patch)
2015-12-29 15:08 UTC, mikael.urankar
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eugene Grosbein 2015-10-18 07:36:33 UTC
While using qemu-mips-static to build packages for MIPS32 platform, I have found the following problem:

1) mmap(2) system call returns NULL (0) instead of MAP_FAILED (-1) when len==0.
2) install(1) system utility tries to use mmap() to read source file if it resides on UFS of CD9660 file system and its size is less than 8 megabytes
3) install checks for MAP_FAILED mmap result but not for NULL. Then is passes NULL pointer to write(2) system call and it fails with EFAULT, breaking install(1).
4) print/texinfo is needed to build gmake using HEAD world and texinfo calls install(1) for zero length file. So, install fails and package build fails.

How-to-Repeat: use UFS and command "touch file && install file newfile" using qemu-mips-static
Comment 1 Eugene Grosbein 2015-10-18 07:58:09 UTC
Another note: problem does not crop up using nullfs-mounted file systems. And it does not exist for FreeBSD running real MIPS32 hardware.
Comment 2 Sean Bruno freebsd_committer 2015-12-14 02:03:03 UTC
Eugene:

Hey, I forgot about this one.  Did you look into my github branch (that's where this package comes from) to send a pull request?

https://github.com/seanbruno/qemu-bsd-user/tree/bsd-user
Comment 3 Eugene Grosbein 2015-12-14 05:47:09 UTC
(In reply to Sean Bruno from comment #2)

Not sure I understand you quite right. I have no solution for pull request.
Comment 4 Sean Bruno freebsd_committer 2015-12-14 17:23:16 UTC
(In reply to eugen from comment #3)
ah, ok.  I thought you knew what code was broken.  I'll add this to my "TODO" list for 2016.
Comment 5 mikael.urankar 2015-12-29 15:08:17 UTC
Created attachment 164821 [details]
return -1 when len is zero

Can you try the attached patch?
We should probably do the same for target_mprotect [1], ie return -1?

[1] https://github.com/seanbruno/qemu-bsd-user/blob/master/bsd-user/mmap.c#L99
Comment 6 Eugene Grosbein 2015-12-29 18:39:29 UTC
(In reply to mikael.urankar from comment #5)

I've just put this patch to /usr/ports/emulators/qemu-sbruno/files/patch-bsd-user_mmap.c and verified that it is applied cleanly while building emulators/qemu-user-static port.

However, running mips32 world with updated /usr/local/bin/qemu-mips-static binary does not fix the problem and this still fails:

# chroot $dir /usr/local/bin/qemu-mips-static /bin/sh
# touch file && install file newfile
install: newfile: Bad address
Comment 7 mikael.urankar 2015-12-29 18:46:02 UTC
(In reply to eugen from comment #6)
Have you updated qemu in the jail?, ie something like cp -f
/usr/ports/emulators/qemu-user-static/work/stage/usr/local/bin/qemu-arm-static
/data/jails/11armv6/usr/local/bin/qemu-arm-static
Comment 8 Eugene Grosbein 2015-12-29 18:46:28 UTC
(In reply to mikael.urankar from comment #5)

Nevermind, I've just realized that I forgot to update qemu-mips-static binary located inside chroot directory and was running old one. With new one, the problem disappeared, so the patch does work.

As for mprotect, I am not sure. For mmap(2), manual page clearly states what len==0 case should return MAP_FAILED (1) and errno==EINVAL. For mprotect, this error is should be returned only if "The virtual address range specified by the addr and len arguments is not valid."
Comment 9 Chris Rees freebsd_committer 2015-12-30 11:00:21 UTC
Works for the same issue on armv6 for me too.  Thanks a lot!

Chris
Comment 10 commit-hook freebsd_committer 2016-01-04 18:26:51 UTC
A commit references this bug:

Author: sbruno
Date: Mon Jan  4 18:26:26 UTC 2016
New revision: 405260
URL: https://svnweb.freebsd.org/changeset/ports/405260

Log:
  Bump to 2.5.50.g20160103, catchup with latest bug fixes.

  Fix dereference of wrong pointer arg in ipc/semctl syscall.
  Fix return from mmap() to setEINVAL and MAP_FAILED when len is 0

  PR:		203852 200613
  Submitted by:	mikael.urankar@gmail.com

Changes:
  head/emulators/qemu-sbruno/Makefile
  head/emulators/qemu-sbruno/distinfo