[PATCH] Fix syscall32 to supply 6th arg to syscal

[PATCH] Fix syscall32 to supply 6th arg to syscal

[Ildar Muslukhov]

This patch fixes missing 6th param for 32bit syscalls in i386 and x86_64 
archs.

Note: I have to remove static from syscall32, because gcc messes up with 
inline asm becuase of that.

Signed-off-by: Ildar Muslukhov <ildarm@google.com>

---
 include/syscall.h |  1 +
 syscall.c         | 44 +++++++++++++++++++++++++++++++++++---------
 2 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/include/syscall.h b/include/syscall.h
index 9fc9885..faaeafc 100644
--- a/include/syscall.h
+++ b/include/syscall.h
@@ -134,6 +134,7 @@ void count_syscalls_enabled(void);
 void display_enabled_syscalls(void);
 void disable_non_net_syscalls(void);
 void init_syscalls(void);
+long syscall32(int num_args, unsigned int call, unsigned long a1, unsigned long a2, unsigned long a3, unsigned long a4, unsigned long a5, unsigned long a6);
 
 #define for_each_32bit_syscall(i) \
 	for (i = 0; i < max_nr_32bit_syscalls; i++)
diff --git a/syscall.c b/syscall.c
index 866ac71..63f25a6 100644
--- a/syscall.c
+++ b/syscall.c
@@ -33,22 +33,48 @@
 	return (type) (res); \
 } while (0)
 
-static long syscall32(int num_args, unsigned int call,
+long syscall32(int num_args, unsigned int call,
 	unsigned long a1, unsigned long a2, unsigned long a3,
-	unsigned long a4, unsigned long a5, __unused__ unsigned long a6)
+	unsigned long a4, unsigned long a5, unsigned long a6)
 {
 #if defined(__i386__) || defined (__x86_64__)
-	if (num_args < 6) {
+
+	if (num_args <= 6) {
 		long __res;
-		__asm__ volatile ("int $0x80"
+#if defined( __i386__)
+		__asm__ volatile (
+			"pushl %%ebp\n\t"
+			"movl %7, %%ebp\n\t"
+			"int $0x80\n\t"
+			"popl %%ebp\n\t"
+			: "=a" (__res)
+			: "0" (call),"b" ((long)(a1)),"c" ((long)(a2)),"d" ((long)(a3)), "S" ((long)(a4)),"D" ((long)(a5)), "g" ((long)(a6))
+			: "%ebp" /* mark EBP reg as dirty */
+			);
+#elif defined(__x86_64__)
+		__asm__ volatile (
+			"pushq %%rbp\n\t"
+			"movq %7, %%rbp\n\t"
+			"int $0x80\n\t"
+			"popq %%rbp\n\t"
 			: "=a" (__res)
-			: "0" (call),"b" ((long)(a1)),"c" ((long)(a2)),
-			"d" ((long)(a3)), "S" ((long)(a4)),
-			"D" ((long)(a5)));
+			: "0" (call),"b" ((long)(a1)),"c" ((long)(a2)),"d" ((long)(a3)), "S" ((long)(a4)),"D" ((long)(a5)), "g" ((long)(a6))
+			: "%rbp" /* mark EBP reg as dirty */
+			);
+#else
+	//To shut up gcc on unused args. This code should never be reached.
+	__res = 0;
+	UNUSED(num_args);
+	UNUSED(call);
+	UNUSED(a1);
+	UNUSED(a2);
+	UNUSED(a3);
+	UNUSED(a4);
+	UNUSED(a5);
+	UNUSED(a6);
+#endif
 		__syscall_return(long,__res);
-		return __res;
 	}
-/* TODO: 6 arg 32bit x86 syscall goes here.*/
 #else
 
 // TODO: 32-bit syscall entry for non-x86 archs goes here.
-- 
1.8.4

Re: [PATCH] Fix syscall32 to supply 6th arg to syscal

[Dave Jones]

On Thu, Sep 19, 2013 at 02:24:24PM -0700, Ildar Muslukhov wrote:
 > 
 > This patch fixes missing 6th param for 32bit syscalls in i386 and x86_64 
 > archs.
 > 
 > Note: I have to remove static from syscall32, because gcc messes up with 
 > inline asm becuase of that.
 > 
 > Signed-off-by: Ildar Muslukhov <ildarm@google.com>
 
thanks, applied and pushed out.
That might have been one of the oldest TODO's in the source code :-)

	Dave

Re: [PATCH] Fix syscall32 to supply 6th arg to syscal

[Dave Jones]

On Thu, Sep 19, 2013 at 02:24:24PM -0700, Ildar Muslukhov wrote:
 > 
 > This patch fixes missing 6th param for 32bit syscalls in i386 and x86_64 
 > archs.
 > 
 > Note: I have to remove static from syscall32, because gcc messes up with 
 > inline asm becuase of that.

Something isn't quite right with this.
I was looking at a log file and noticed that in a lot of cases, we end up
doing a 32bit syscall, and then the child respawns.  I suspected it
was segfaulting, and running with -D confirms it.

A whole bunch of core dumps appears, looking like this..

Core was generated by `../trinity -q -D'.
Program terminated with signal 11, Segmentation fault.
#0  0x0000000000403aa5 in syscall32 (a6=<optimized out>, a5=5, a4=9709, a3=29, a2=4, a1=<optimized out>, call=279, num_args=5) at syscall.c:76
76			__syscall_return(long,__res);

This is running on x86-64, I haven't tested actually running on a 32-bit machine,
but I suspect it's a problem there too.

	Dave

Re: [PATCH] Fix syscall32 to supply 6th arg to syscal

[Ildar Muslukhov]

Yes, it gave me problems in the beginning, but after removing static
from the function and taking care of ebp it worked. Will test it on my
machine in a moment. BTW which kernel version are you running against?



On Wed, Oct 2, 2013 at 8:12 AM, Dave Jones <davej@redhat.com> wrote:
> On Thu, Sep 19, 2013 at 02:24:24PM -0700, Ildar Muslukhov wrote:
>  >
>  > This patch fixes missing 6th param for 32bit syscalls in i386 and x86_64
>  > archs.
>  >
>  > Note: I have to remove static from syscall32, because gcc messes up with
>  > inline asm becuase of that.
>
> Something isn't quite right with this.
> I was looking at a log file and noticed that in a lot of cases, we end up
> doing a 32bit syscall, and then the child respawns.  I suspected it
> was segfaulting, and running with -D confirms it.
>
> A whole bunch of core dumps appears, looking like this..
>
> Core was generated by `../trinity -q -D'.
> Program terminated with signal 11, Segmentation fault.
> #0  0x0000000000403aa5 in syscall32 (a6=<optimized out>, a5=5, a4=9709, a3=29, a2=4, a1=<optimized out>, call=279, num_args=5) at syscall.c:76
> 76                      __syscall_return(long,__res);
>
> This is running on x86-64, I haven't tested actually running on a 32-bit machine,
> but I suspect it's a problem there too.
>
>         Dave
>

Re: [PATCH] Fix syscall32 to supply 6th arg to syscal

[Dave Jones]

On Wed, Oct 02, 2013 at 09:14:43AM -0700, Ildar Muslukhov wrote:

 > On Wed, Oct 2, 2013 at 8:12 AM, Dave Jones <davej@redhat.com> wrote:
 > > On Thu, Sep 19, 2013 at 02:24:24PM -0700, Ildar Muslukhov wrote:
 > >  >
 > >  > This patch fixes missing 6th param for 32bit syscalls in i386 and x86_64
 > >  > archs.
 > >
 > > Something isn't quite right with this.
 > > I was looking at a log file and noticed that in a lot of cases, we end up
 > > doing a 32bit syscall, and then the child respawns.  I suspected it
 > > was segfaulting, and running with -D confirms it.
 > >
 > > A whole bunch of core dumps appears, looking like this..
 > >
 > > Core was generated by `../trinity -q -D'.
 > > Program terminated with signal 11, Segmentation fault.
 > > #0  0x0000000000403aa5 in syscall32 (a6=<optimized out>, a5=5, a4=9709, a3=29, a2=4, a1=<optimized out>, call=279, num_args=5) at syscall.c:76
 > > 76                      __syscall_return(long,__res);
 > >
 > > This is running on x86-64, I haven't tested actually running on a 32-bit machine,
 > > but I suspect it's a problem there too.
 >
 > Yes, it gave me problems in the beginning, but after removing static
 > from the function and taking care of ebp it worked. Will test it on my
 > machine in a moment. BTW which kernel version are you running against?

Happens with 3.11, and Linus' git. Seems to not matter.

	Dave

 

Re: [PATCH] Fix syscall32 to supply 6th arg to syscal

[Ildar Muslukhov]

I finally was able to reproduce that bug. In order to validate the
assumption that the "syscall32 fix" patch introduced that bug I pulled
the latest trinity source and removed the patch at question from the
source (gladly patch was small). And I am still getting segfault.
Thus, assumption that this patch introduced that bug looks less
probable now. Furthermore, if you run trinity with a single 32 bit
function (-c perf_event_open,32 for example) it runs just fine.

Several observations from my runs:
1) Not all 32 bit calls end up in segfault.
2) All segfaults are calls to 32 bit syscall.

Still digging into what causes this, will keep you posted.

On Wed, Oct 2, 2013 at 9:43 AM, Dave Jones <davej@redhat.com> wrote:
> On Wed, Oct 02, 2013 at 09:14:43AM -0700, Ildar Muslukhov wrote:
>
>  > On Wed, Oct 2, 2013 at 8:12 AM, Dave Jones <davej@redhat.com> wrote:
>  > > On Thu, Sep 19, 2013 at 02:24:24PM -0700, Ildar Muslukhov wrote:
>  > >  >
>  > >  > This patch fixes missing 6th param for 32bit syscalls in i386 and x86_64
>  > >  > archs.
>  > >
>  > > Something isn't quite right with this.
>  > > I was looking at a log file and noticed that in a lot of cases, we end up
>  > > doing a 32bit syscall, and then the child respawns.  I suspected it
>  > > was segfaulting, and running with -D confirms it.
>  > >
>  > > A whole bunch of core dumps appears, looking like this..
>  > >
>  > > Core was generated by `../trinity -q -D'.
>  > > Program terminated with signal 11, Segmentation fault.
>  > > #0  0x0000000000403aa5 in syscall32 (a6=<optimized out>, a5=5, a4=9709, a3=29, a2=4, a1=<optimized out>, call=279, num_args=5) at syscall.c:76
>  > > 76                      __syscall_return(long,__res);
>  > >
>  > > This is running on x86-64, I haven't tested actually running on a 32-bit machine,
>  > > but I suspect it's a problem there too.
>  >
>  > Yes, it gave me problems in the beginning, but after removing static
>  > from the function and taking care of ebp it worked. Will test it on my
>  > machine in a moment. BTW which kernel version are you running against?
>
> Happens with 3.11, and Linus' git. Seems to not matter.
>
>         Dave
>
>

Re: [PATCH] Fix syscall32 to supply 6th arg to syscal

[Dave Jones]

On Wed, Oct 02, 2013 at 11:12:21AM -0700, Ildar Muslukhov wrote:
 > I finally was able to reproduce that bug. In order to validate the
 > assumption that the "syscall32 fix" patch introduced that bug I pulled
 > the latest trinity source and removed the patch at question from the
 > source (gladly patch was small). And I am still getting segfault.
 > Thus, assumption that this patch introduced that bug looks less
 > probable now. Furthermore, if you run trinity with a single 32 bit
 > function (-c perf_event_open,32 for example) it runs just fine.
 > 
 > Several observations from my runs:
 > 1) Not all 32 bit calls end up in segfault.
 > 2) All segfaults are calls to 32 bit syscall.
 > 
 > Still digging into what causes this, will keep you posted.

This may have been why I had this..

// FIXME: I forgot why this got disabled. Revisit.
//                     if (rand() % 100 < 10)
//                             shm->do32bit[childno] = TRUE;

in choose_syscall_table.

Probably an old bug that I've never found time to dig into.

	Dave

Re: [PATCH] Fix syscall32 to supply 6th arg to syscal

[Ildar Muslukhov]

That's what I though as well. However, just encountered segfaults on
x64 read syscall. Although this one might be legit.

On Wed, Oct 2, 2013 at 11:15 AM, Dave Jones <davej@redhat.com> wrote:
> On Wed, Oct 02, 2013 at 11:12:21AM -0700, Ildar Muslukhov wrote:
>  > I finally was able to reproduce that bug. In order to validate the
>  > assumption that the "syscall32 fix" patch introduced that bug I pulled
>  > the latest trinity source and removed the patch at question from the
>  > source (gladly patch was small). And I am still getting segfault.
>  > Thus, assumption that this patch introduced that bug looks less
>  > probable now. Furthermore, if you run trinity with a single 32 bit
>  > function (-c perf_event_open,32 for example) it runs just fine.
>  >
>  > Several observations from my runs:
>  > 1) Not all 32 bit calls end up in segfault.
>  > 2) All segfaults are calls to 32 bit syscall.
>  >
>  > Still digging into what causes this, will keep you posted.
>
> This may have been why I had this..
>
> // FIXME: I forgot why this got disabled. Revisit.
> //                     if (rand() % 100 < 10)
> //                             shm->do32bit[childno] = TRUE;
>
> in choose_syscall_table.
>
> Probably an old bug that I've never found time to dig into.
>
>         Dave
>

Re: [PATCH] Fix syscall32 to supply 6th arg to syscal

[Dave Jones]

On Wed, Oct 02, 2013 at 01:44:44PM -0700, Ildar Muslukhov wrote:
 > That's what I though as well. However, just encountered segfaults on
 > x64 read syscall. Although this one might be legit.

in case you didn't realise, there's a few cases where we expect to segfault.

for eg, if we mprotect page_rand to PROT_NONE and then something tries to read it.
There's a whole bunch of similar gotchas. I've special cased the sanitise routines
of some syscalls to avoid this sort of thing, but there's a few cases where
a trinity child will corrupt itself. It then dies, and respawns, so I haven't really
worried too much about it.

	Dave

Re: [PATCH] Fix syscall32 to supply 6th arg to syscal

[Ildar Muslukhov]

Got it. I wonder, what are the settings you running the trinity
yourself? Are you fuzzing all calls at the same time, or you limit
your self to a set or one function at a time?

Re: [PATCH] Fix syscall32 to supply 6th arg to syscal

[Dave Jones]

On Wed, Oct 02, 2013 at 01:53:15PM -0700, Ildar Muslukhov wrote:
 > Got it. I wonder, what are the settings you running the trinity
 > yourself? Are you fuzzing all calls at the same time, or you limit
 > your self to a set or one function at a time?

depends. if I'm not looking for anything specific, I'll just fire off
scripts/test-multi.sh   Once I've triggered something, I might narrow
it down with more specific parameters (see scripts/find.sh) for eg.

Also sometimes I'll add something like -x perf_event_open or the like
to the scripts to avoid hitting certain bugs until they're fixed.

Sometimes I'll remove the '-l off' if I want to try and pick through the logs
after hitting a bug.


	Dave

Re: [PATCH] Fix syscall32 to supply 6th arg to syscal

[Ildar Muslukhov]

OK, I've looked through logs of several runs and seems to me that
32bit faults look legit, they just doing something bad or hang the
process and thus getting killed by the os or by watchdog. At the end
of the day, if someone does not want to have such noise on x86_64 arch
use "-a 64" argument to switch off all 32bit calls.

On Wed, Oct 2, 2013 at 1:57 PM, Dave Jones <davej@redhat.com> wrote:
> On Wed, Oct 02, 2013 at 01:53:15PM -0700, Ildar Muslukhov wrote:
>  > Got it. I wonder, what are the settings you running the trinity
>  > yourself? Are you fuzzing all calls at the same time, or you limit
>  > your self to a set or one function at a time?
>
> depends. if I'm not looking for anything specific, I'll just fire off
> scripts/test-multi.sh   Once I've triggered something, I might narrow
> it down with more specific parameters (see scripts/find.sh) for eg.
>
> Also sometimes I'll add something like -x perf_event_open or the like
> to the scripts to avoid hitting certain bugs until they're fixed.
>
> Sometimes I'll remove the '-l off' if I want to try and pick through the logs
> after hitting a bug.
>
>
>         Dave
>