Bug 203852

Summary: emulators/qemu-user-static: mmap may return NULL instead of MAP_FAILED to indicate error
Product: Ports & Packages Reporter: Eugene Grosbein <ports>
Component: Individual Port(s)Assignee: Sean Bruno <sbruno>
Status: Closed FIXED    
Severity: Affects Some People CC: crees, emaste, mikael, ports, sbruno
Priority: --- Flags: bugzilla: maintainer-feedback? (nox)
Version: Latest   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
return -1 when len is zero none

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 freebsd_triage 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 freebsd_triage 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 freebsd_committer freebsd_triage 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 freebsd_committer freebsd_triage 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 freebsd_triage 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 freebsd_triage 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