[patch 1/2] add init() function to struct syscall

[patch 1/2] add init() function to struct syscall

[Vince Weaver]

While working on the perf_event_open() sysfs init problem, I was wondering
if it might be easier if we added the possibility of an init() routine
to each syscall structure.  That way trinity can support doing setup
before fuzzing begins.

Below is a quick patch implementing this, it seems to work but I still 
don't have the best grasp of trinity internals.

Signed-off-by: Vince Weaver <vincent.weaver@maine.edu>

diff --git a/include/syscall.h b/include/syscall.h
index ccbe436..a58a506 100644
--- a/include/syscall.h
+++ b/include/syscall.h
@@ -32,6 +32,7 @@ struct arglist {
 struct syscall {
 	void (*sanitise)(int childno);
 	void (*post)(int);
+	int (*init)(void);
 
 	unsigned int number;
 	const char name[80];
@@ -132,6 +133,7 @@ void deactivate_disabled_syscalls(void);
 void count_syscalls_enabled(void);
 void display_enabled_syscalls(void);
 void disable_non_net_syscalls(void);
+void init_syscalls(void);
 
 #define for_each_32bit_syscall(i) \
 	for (i = 0; i < max_nr_32bit_syscalls; i++)
diff --git a/tables.c b/tables.c
index 7be1ae9..d12b541 100644
--- a/tables.c
+++ b/tables.c
@@ -106,6 +106,35 @@ void count_syscalls_enabled(void)
 	}
 }
 
+void init_syscalls(void)
+{
+	unsigned int i;
+
+	if (biarch == TRUE) {
+		for_each_64bit_syscall(i) {
+			if (syscalls_64bit[i].entry->flags & ACTIVE)
+				if (syscalls_64bit[i].entry->init)
+					syscalls_64bit[i].entry->init();
+		}
+
+		for_each_32bit_syscall(i) {
+			if (syscalls_32bit[i].entry->flags & ACTIVE)
+				if (syscalls_32bit[i].entry->init)
+					syscalls_32bit[i].entry->init();
+		}
+
+	} else {
+
+		/* non-biarch */
+		for_each_syscall(i) {
+			if (syscalls[i].entry->flags & ACTIVE)
+				if (syscalls[i].entry->init)
+					syscalls[i].entry->init();
+		}
+	}
+}
+
+
 bool no_syscalls_enabled(void)
 {
 	unsigned int i;
diff --git a/trinity.c b/trinity.c
index 4b536c9..93e7819 100644
--- a/trinity.c
+++ b/trinity.c
@@ -183,6 +183,8 @@ int main(int argc, char* argv[])
 		goto out;
 	}
 
+	init_syscalls();
+
 	if (show_ioctl_list == TRUE) {
 		dump_ioctls();
 		goto out;

[patch 2/2] make perf_event_open() use init routine

[Vince Weaver]

This makes the perf_event_open() syscall use the new init functionality.

Signed-off-by: Vince Weaver <vincent.weaver@maine.edu>

diff --git a/syscalls/perf_event_open.c b/syscalls/perf_event_open.c
index 909b807..ad3e903 100644
--- a/syscalls/perf_event_open.c
+++ b/syscalls/perf_event_open.c
@@ -672,7 +671,6 @@ static long long random_event_config(__u32 *event_type, __u64 *config1)
 		break;
 
 	case PERF_TYPE_READ_FROM_SYSFS:
-		if (pmus==NULL) init_pmus();
 		config = random_sysfs_config(event_type,config1);
 		break;
 
@@ -1091,5 +1089,6 @@ struct syscall syscall_perf_event_open = {
 		},
 	},
 	.sanitise = sanitise_perf_event_open,
+	.init = init_pmus,
 	.flags = NEED_ALARM,
 };

Re: [patch 1/2] add init() function to struct syscall

[Dave Jones]

On Wed, Jun 19, 2013 at 10:09:09PM -0400, Vince Weaver wrote:
 > 
 > While working on the perf_event_open() sysfs init problem, I was wondering
 > if it might be easier if we added the possibility of an init() routine
 > to each syscall structure.  That way trinity can support doing setup
 > before fuzzing begins.
 > 
 > Below is a quick patch implementing this, it seems to work but I still 
 > don't have the best grasp of trinity internals.

Looks ok to me. Good idea actually. I'll queue these up.

	Dave

Re: [patch 1/2] add init() function to struct syscall

[Dave Jones]

On Wed, Jun 19, 2013 at 10:09:09PM -0400, Vince Weaver wrote:
 > 
 > While working on the perf_event_open() sysfs init problem, I was wondering
 > if it might be easier if we added the possibility of an init() routine
 > to each syscall structure.  That way trinity can support doing setup
 > before fuzzing begins.
 > 
 > Below is a quick patch implementing this, it seems to work but I still 
 > don't have the best grasp of trinity internals.
 > 
 > Signed-off-by: Vince Weaver <vincent.weaver@maine.edu>
 > 
 > diff --git a/include/syscall.h b/include/syscall.h
 > index ccbe436..a58a506 100644
 > --- a/include/syscall.h
 > +++ b/include/syscall.h
 > @@ -32,6 +32,7 @@ struct arglist {
 >  struct syscall {
 >  	void (*sanitise)(int childno);
 >  	void (*post)(int);
 > +	int (*init)(void);
 >  
 >  	unsigned int number;
 >  	const char name[80];

Nothing wrong with this patch, but it's highlighted a bug that's been in trinity
for a while.  Changes to the syscall struct should cause everything in syscalls/*
to be rebuilt. But somehow the dependency magic in the Makefile doesn't pick it up.

Anyone with better make-fu than me want to take a stab at that ?

	Dave

Re: [patch 1/2] add init() function to struct syscall

[Michael Ellerman]

On Wed, 2013-06-19 at 22:40 -0400, Dave Jones wrote:
> On Wed, Jun 19, 2013 at 10:09:09PM -0400, Vince Weaver wrote:
>  > 
>  > While working on the perf_event_open() sysfs init problem, I was wondering
>  > if it might be easier if we added the possibility of an init() routine
>  > to each syscall structure.  That way trinity can support doing setup
>  > before fuzzing begins.
>  > 
>  > Below is a quick patch implementing this, it seems to work but I still 
>  > don't have the best grasp of trinity internals.
>  > 
>  > Signed-off-by: Vince Weaver <vincent.weaver@maine.edu>
>  > 
>  > diff --git a/include/syscall.h b/include/syscall.h
>  > index ccbe436..a58a506 100644
>  > --- a/include/syscall.h
>  > +++ b/include/syscall.h
>  > @@ -32,6 +32,7 @@ struct arglist {
>  >  struct syscall {
>  >  	void (*sanitise)(int childno);
>  >  	void (*post)(int);
>  > +	int (*init)(void);
>  >  
>  >  	unsigned int number;
>  >  	const char name[80];
> 
> Nothing wrong with this patch, but it's highlighted a bug that's been in trinity
> for a while.  Changes to the syscall struct should cause everything in syscalls/*
> to be rebuilt. But somehow the dependency magic in the Makefile doesn't pick it up.
> 
> Anyone with better make-fu than me want to take a stab at that ?

I'm not a make expert, but I think it's because it's looking for .deps/syscalls/foo.d
whereas we generate .deps/foo.d

This patch works for me, touching include/syscall.h rebuilds a few
things and then everything under syscalls.

I guess the other approach would be to flatten the directory structure
where we include the .d files.

cheers


diff --git a/Makefile b/Makefile
index f5a35b6..e64d87d 100644
--- a/Makefile
+++ b/Makefile
@@ -63,14 +63,11 @@ trinity: test $(OBJS) $(HEADERS)
        $(QUIET_CC)$(CC) $(CFLAGS) -o trinity $(OBJS)
        @mkdir -p tmp
 
-df = $(DEPDIR)/$(*F)
+df = $(DEPDIR)/$(*D)/$(*F)
 
-# FIXME:
-# Dependancy information for .c files in subdirs seems to be broken.
-# Example: touch include/sanitise.h should cause syscalls/*.c to be rebuilt.
-#
 %.o : %.c
        $(QUIET_CC)$(CC) $(CFLAGS) -o $@ -c $<
+       @mkdir -p $(DEPDIR)/$(*D)
        @gcc -MM $(CFLAGS) $*.c > $(df).d
        @mv -f $(df).d $(df).d.tmp
        @sed -e 's|.*:|$*.o:|' <$(df).d.tmp > $(df).d
@@ -83,7 +80,7 @@ clean:
        @rm -f core.*
        @rm -f trinity
        @rm -f tags
-       @rm -f $(DEPDIR)/*.d
+       @rm -rf $(DEPDIR)/*
 
 devel:
        @perl -p -i -e 's/^#CFLAGS \+\= -Werror/CFLAGS += -Werror/' Makefile

Re: [patch 1/2] add init() function to struct syscall

[Dave Jones]

On Thu, Jun 20, 2013 at 02:15:15PM +1000, Michael Ellerman wrote:
 > On Wed, 2013-06-19 at 22:40 -0400, Dave Jones wrote:
 > > Nothing wrong with this patch, but it's highlighted a bug that's been in trinity
 > > for a while.  Changes to the syscall struct should cause everything in syscalls/*
 > > to be rebuilt. But somehow the dependency magic in the Makefile doesn't pick it up.
 > > 
 > > Anyone with better make-fu than me want to take a stab at that ?
 > 
 > I'm not a make expert, but I think it's because it's looking for .deps/syscalls/foo.d
 > whereas we generate .deps/foo.d

Bah, I knew it would be something that simple that I was overlooking.

 > This patch works for me, touching include/syscall.h rebuilds a few
 > things and then everything under syscalls.

Looks good to me.

 > I guess the other approach would be to flatten the directory structure
 > where we include the .d files.
 
That sounds like it would be a mess if we're unfortunate enough to have the same
name file in 2 dirs.  Your patch seems to do the right thing, so I'll
just apply that and move on.

thanks

	Dave

Re: [patch 1/2] add init() function to struct syscall

[Michael Ellerman]

On Thu, 2013-06-20 at 00:48 -0400, Dave Jones wrote:
> On Thu, Jun 20, 2013 at 02:15:15PM +1000, Michael Ellerman wrote:
>  > On Wed, 2013-06-19 at 22:40 -0400, Dave Jones wrote:
>  > > Nothing wrong with this patch, but it's highlighted a bug that's been in trinity
>  > > for a while.  Changes to the syscall struct should cause everything in syscalls/*
>  > > to be rebuilt. But somehow the dependency magic in the Makefile doesn't pick it up.
>  > > 
>  > > Anyone with better make-fu than me want to take a stab at that ?
>  > 
>  > I'm not a make expert, but I think it's because it's looking for .deps/syscalls/foo.d
>  > whereas we generate .deps/foo.d
> 
> Bah, I knew it would be something that simple that I was overlooking.

Ain't it always :)

>  > This patch works for me, touching include/syscall.h rebuilds a few
>  > things and then everything under syscalls.
> 
> Looks good to me.
> 
>  > I guess the other approach would be to flatten the directory structure
>  > where we include the .d files.
>  
> That sounds like it would be a mess if we're unfortunate enough to have the same
> name file in 2 dirs.  Your patch seems to do the right thing, so I'll
> just apply that and move on.

Yeah that's why I went with that approach.

Turns out we already had dupes:

$ find . -name random.c -o -name socket.c -o -name watchdog.c
./watchdog.c
./ioctls/watchdog.c
./ioctls/socket.c
./syscalls/socket.c
./ioctls/random.c
./random.c


cheers