Possible bug in sys_mmap64

Denys Vlasenko dvlasenk at redhat.com
Fri Aug 19 14:23:51 UTC 2011


I noticed particularly ugly ifdef forest in sys_mmap64 and decided
to simplify it (see the commit below).

While looking at simplified version, I noticed that we seem to use
tcp->u_arg[i] while we have to use u_arg[i].

It definitely looks like a bug to me.

However, I failed to make test program which invokes mmap64
(need to try on x86-64), thus I hesitated to fix the bug.
I only added a comment about it.

Dmitry, can you take a look? Maybe I'm confused...
-- 
vda

commit 4286e4bfc127faa3d0443d1d876b955ce7449ab1
Author: Denys Vlasenko <dvlasenk at redhat.com>
Date:   Fri Aug 19 16:11:07 2011 +0200

    Untangle ifdef forest in sys_mmap64. No code changes
    
    After careful analysis, it looks like !LINUX and ALPHA
    pass all seven parameters in registers; and in all other cases
    parameters are on stack (pointed to by tcp->u_arg[0]).
    In light of this, reorganize ifdefs, making them simpler,
    without changing any logic.
    After this, it's apparent we use tcp->u_arg[4,5,6] and possibly
    [7] without checking that it's valid to do so.
    So far, just add a comment about this.
    
    * mem.c (sys_mmap64): Rewrite ifdefs in a much simpler way.
    Add comments about apparent bugs.
    
    Signed-off-by: Denys Vlasenko <dvlasenk at redhat.com>

diff --git a/mem.c b/mem.c
index f0ccfc7..de9b6bb 100644
--- a/mem.c
+++ b/mem.c
@@ -334,25 +334,15 @@ sys_mmap(struct tcb *tcp)
 int
 sys_mmap64(struct tcb *tcp)
 {
-#ifdef linux
-#ifdef ALPHA
-	long *u_arg = tcp->u_arg;
-#else /* !ALPHA */
-	long u_arg[7];
-#endif /* !ALPHA */
-#else /* !linux */
-	long *u_arg = tcp->u_arg;
-#endif /* !linux */
-
 	if (entering(tcp)) {
-#ifdef linux
-#ifndef ALPHA
+#if !defined(LINUX) || defined(ALPHA)
+		long *u_arg = tcp->u_arg;
+#else
+		long u_arg[7];
 		if (umoven(tcp, tcp->u_arg[0], sizeof u_arg,
 				(char *) u_arg) == -1)
 			return 0;
-#endif /* ALPHA */
-#endif /* linux */
-
+#endif
 		/* addr */
 		tprintf("%#lx, ", u_arg[0]);
 		/* len */
@@ -369,13 +359,16 @@ sys_mmap64(struct tcb *tcp)
 #endif
 		/* fd */
 		tprintf(", ");
+	/* BUG?! should be u_arg[4] (without tcp->)? */
 		printfd(tcp, tcp->u_arg[4]);
 		/* offset */
+	/* BUG?! on non-ALPHA linux, offset will be not in tcp->u_arg,
+	 * but in local u_arg, but printllval prints tcp->u_arg! */
 		printllval(tcp, ", %#llx", 5);
 	}
 	return RVAL_HEX;
 }
-#endif
+#endif /* _LFS64_LARGEFILE || HAVE_LONG_LONG_OFF_T */
 
 
 int





More information about the Strace-devel mailing list