Bug 3853 - MIPS mmap can not access over 4G file
Summary: MIPS mmap can not access over 4G file
Status: RESOLVED FIXED
Alias: None
Product: uClibc
Classification: Unclassified
Component: Standard Compliance (show other bugs)
Version: <= 0.9.29.x
Hardware: Other Linux
: P3 major
Target Milestone: ---
Assignee: unassigned
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-06-14 04:43 UTC by highfly22
Modified: 2012-04-25 15:22 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:


Attachments
patch for mmap2 of mips (1.25 KB, patch)
2011-06-18 03:14 UTC, highfly22
Details
mantis 1303 bug info (763 bytes, text/plain)
2012-04-09 19:32 UTC, Mike Frysinger
Details
mmap64 fix (1.83 KB, patch)
2012-04-10 04:28 UTC, Mike Frysinger
Details

Note You need to log in before you can comment on or make changes to this bug.
Description highfly22 2011-06-14 04:43:46 UTC
mmap use 32bit offset to access files. So over 4G file will not be accessed.
in libc/sysdeps/linux/mips/mmap.c, there is a code to disable mmap2.
It seems uclibc mips porting is incompleted.

#if 0
/* For now, leave mmap using mmap1 since mmap2 seems
 * to have issues (i.e. it doesn't work 100% properly).
 */
#ifdef __NR_mmap2
# undef __NR_mmap
# define __NR_mmap __NR_mmap2
#endif
#endif
Comment 1 Bernhard Reutner-Fischer 2011-06-17 20:10:17 UTC
If you can confirm that mmap2 for mips on something like 2.6.39.1 works then we could enable it for recent versions.
Comment 2 highfly22 2011-06-18 03:14:15 UTC
Created attachment 3439 [details]
patch for mmap2 of mips

I used this patch to fix the problem. It seems a mmap64 common implementation truncate to 32bit in the NOT USE 64 OFFSET branch.
Comment 3 Leonid 2012-04-09 16:46:34 UTC
I apologize that this issue must be renamed - problems with mmap64 & offsets > 4Gb observed on any 32-bit platform(__USE_FILE_OFFSET64 not set) has sizeof(__u_long) == 4.

Bug introduced in commit 22a4424b0b "Fix arm mmap when using mmap2 syscall.  Fixes bug #1303" - wrong parentheses placement cause off64_t offset argument truncated to low 32-bit.

Trivial fix:

diff --git a/libc/sysdeps/linux/common/mmap64.c b/libc/sysdeps/linux/common/mmap64.c
--- a/libc/sysdeps/linux/common/mmap64.c
+++ b/libc/sysdeps/linux/common/mmap64.c
@@ -61,10 +61,10 @@ __ptr_t mmap64(__ptr_t addr, size_t len, int prot, int flags, int fd, __off64_t offset)
 
 #  ifdef __USE_FILE_OFFSET64
 	return __syscall_mmap2(addr, len, prot, flags,
-	                       fd, ((__u_quad_t) offset >> MMAP2_PAGE_SHIFT));
+	                       fd, (__u_quad_t)(offset >> MMAP2_PAGE_SHIFT));
 #  else
 	return __syscall_mmap2(addr, len, prot, flags,
-	                       fd, ((__u_long) offset >> MMAP2_PAGE_SHIFT));
+	                       fd, (__u_long)(offset >> MMAP2_PAGE_SHIFT));
 #  endif
 }
Comment 4 Mike Frysinger 2012-04-09 19:32:24 UTC
Created attachment 4244 [details]
mantis 1303 bug info

your proposed change will undo the previous bug fix.  here's the text of the old mantis bug that prompted 22a4424b0b.

pretty sure the right way to fix this would be to avoid ifdefs and use:
    return __syscall_mmap2(addr, len, prot, flags, fd,
                           ((uint64_t)offset >> MMAP2_PAGE_SHIFT));

this is because we know __off64_t is always going to be a 64bit signed value.
Comment 5 Khem Raj 2012-04-09 19:46:42 UTC
(In reply to comment #4)
> Created attachment 4244 [details]
> mantis 1303 bug info
> 
> your proposed change will undo the previous bug fix.  here's the text of the
> old mantis bug that prompted 22a4424b0b.
> 
> pretty sure the right way to fix this would be to avoid ifdefs and use:
>     return __syscall_mmap2(addr, len, prot, flags, fd,
>                            ((uint64_t)offset >> MMAP2_PAGE_SHIFT));
> 
> this is because we know __off64_t is always going to be a 64bit signed value.

yes that seems to be a better fix. Did you mean to use __off64_t instead of uint64_t
Comment 6 Mike Frysinger 2012-04-09 19:54:15 UTC
(In reply to comment #5)

no, because __off64_t is signed, and there doesn't appear to be an "unsigned off64_t" type, so uint64_t is as close as i could get :(
Comment 7 Mike Frysinger 2012-04-10 04:28:52 UTC
Created attachment 4250 [details]
mmap64 fix

please try the attached patch
Comment 8 Leonid 2012-04-10 15:34:06 UTC
Yes, it works on our MIPS32 platform.

Thank you very much, I had to take into account signedness of offset myself.
Comment 9 Mike Frysinger 2012-04-10 16:10:07 UTC
pushed then ... thanks for testing!
Comment 10 Bernhard Reutner-Fischer 2012-04-25 11:30:26 UTC
Does not work for me on ppc32.
Current master gives:

  LD libuClibc-0.9.34-git.so
libc/libc_so.a(mmap64.os): In function `mmap64':
/scratch/src/uClibc.push/libc/sysdeps/linux/common/mmap64.c:60: undefined reference to `__illegally_sized_syscall_arg6'
collect2: ld returned 1 exit status
Comment 11 Bernhard Reutner-Fischer 2012-04-25 11:35:24 UTC
Mike, can you please have a look?

Something like below may be appropriate, but why can't we just use (off_t)offset>>shift, again?


index 0086496..75b4ebc 100644
--- a/libc/sysdeps/linux/common/mmap64.c
+++ b/libc/sysdeps/linux/common/mmap64.c
@@ -57,7 +57,7 @@ void *mmap64(void *addr, size_t len, int prot, int flags, int fd, __off64_t offs
 	 * an unsigned 64-bit value before doing the shift.
 	 */
 	return (void*) INLINE_SYSCALL(mmap2, 6, addr, len, prot, flags, fd,
-	                                ((uint64_t)offset >> MMAP2_PAGE_SHIFT));
+	                                (off_t)((uint64_t)offset >> MMAP2_PAGE_SHIFT));
 }
 
 #endif
Comment 12 Mike Frysinger 2012-04-25 15:22:41 UTC
we can't cast the inside value to off_t because it is signed and that will reintroduce a previous bug.  you can't cast to off_t outside of the paren because that could be 64bits when LFS is enabled.

Khem mentioned this on the list already, so i'll follow up there.