[PATCH 1/2] Add an IGNORE_ENOSYS flag and use it

[PATCH 1/2] Add an IGNORE_ENOSYS flag and use it

[Michael Ellerman]

Some syscalls return ENOSYS depending on their arguments. We don't want
to stop calling them just because we hit one of those cases. Add a flag
to specify this behaviour so we don't have to keep special-casing those
calls in mkcall().
---
 include/syscall.h   |  1 +
 syscall.c           | 23 ++++++-----------------
 syscalls/futex.c    |  2 +-
 syscalls/ioctl.c    |  2 +-
 syscalls/sendfile.c |  4 ++--
 5 files changed, 11 insertions(+), 21 deletions(-)

diff --git a/include/syscall.h b/include/syscall.h
index e42cd85..bc19273 100644
--- a/include/syscall.h
+++ b/include/syscall.h
@@ -121,4 +121,5 @@ struct syscalltable {
 #define TO_BE_DEACTIVATED	(1<<4)
 #define NEED_ALARM		(1<<5)
 #define EXTRA_FORK		(1<<6)
+#define IGNORE_ENOSYS		(1<<7)
 
diff --git a/syscall.c b/syscall.c
index bca2aea..50e64ac 100644
--- a/syscall.c
+++ b/syscall.c
@@ -176,29 +176,18 @@ bool mkcall(int childno)
 	if (dopause == TRUE)
 		sleep(1);
 
-	/* If the syscall doesn't exist don't bother calling it next time. */
-	if ((ret == -1UL) && (errno_saved == ENOSYS)) {
-
-		/* Futex is awesome, it ENOSYS's depending on arguments. Sigh. */
-		if (call == (unsigned int) search_syscall_table(syscalls, max_nr_syscalls, "futex"))
-			goto skip_enosys;
-
-		/* Unknown ioctls also ENOSYS. */
-		if (call == (unsigned int) search_syscall_table(syscalls, max_nr_syscalls, "ioctl"))
-			goto skip_enosys;
-
-		/* sendfile() may ENOSYS depending on args. */
-		if (call == (unsigned int) search_syscall_table(syscalls, max_nr_syscalls, "sendfile"))
-			goto skip_enosys;
-
+	/*
+	 * If the syscall doesn't exist don't bother calling it next time.
+	 * Some syscalls return ENOSYS depending on their arguments, we mark
+	 * those as IGNORE_ENOSYS and keep calling them.
+	 */
+	if ((ret == -1UL) && (errno_saved == ENOSYS) && !(entry->flags & IGNORE_ENOSYS)) {
 		output(1, "%s (%d) returned ENOSYS, marking as inactive.\n",
 			entry->name, call + SYSCALL_OFFSET);
 
 		deactivate_syscall(call, syscallrec->do32bit);
 	}
 
-skip_enosys:
-
 	if (entry->post)
 	    entry->post(childno, syscallrec);
 
diff --git a/syscalls/futex.c b/syscalls/futex.c
index 5e194ab..4d18f30 100644
--- a/syscalls/futex.c
+++ b/syscalls/futex.c
@@ -33,5 +33,5 @@ struct syscallentry syscall_futex = {
 	.arg5type = ARG_ADDRESS,
 	.arg6name = "val3",
 	.rettype = RET_FD,		// FIXME: Needs to mutate depending on 'op' value
-	.flags = NEED_ALARM,
+	.flags = NEED_ALARM | IGNORE_ENOSYS,
 };
diff --git a/syscalls/ioctl.c b/syscalls/ioctl.c
index 012f055..db3b485 100644
--- a/syscalls/ioctl.c
+++ b/syscalls/ioctl.c
@@ -70,5 +70,5 @@ struct syscallentry syscall_ioctl = {
 	.arg3name = "arg",
 	.arg3type = ARG_RANDPAGE,
 	.sanitise = sanitise_ioctl,
-	.flags = NEED_ALARM,
+	.flags = NEED_ALARM | IGNORE_ENOSYS,
 };
diff --git a/syscalls/sendfile.c b/syscalls/sendfile.c
index 05e49e9..3e74841 100644
--- a/syscalls/sendfile.c
+++ b/syscalls/sendfile.c
@@ -14,7 +14,7 @@ struct syscallentry syscall_sendfile = {
 	.arg3type = ARG_ADDRESS,
 	.arg4name = "count",
 	.arg4type = ARG_LEN,
-	.flags = NEED_ALARM,
+	.flags = NEED_ALARM | IGNORE_ENOSYS,
 };
 
 /*
@@ -32,5 +32,5 @@ struct syscallentry syscall_sendfile64 = {
 	.arg3type = ARG_ADDRESS,
 	.arg4name = "count",
 	.arg4type = ARG_LEN,
-	.flags = NEED_ALARM,
+	.flags = NEED_ALARM | IGNORE_ENOSYS,
 };
-- 
1.9.1

[PATCH 2/2] Mark perf_event_open() with IGNORE_ENOSYS

[Michael Ellerman]

At the very least specifying PERF_SAMPLE_STACK_USER will return ENOSYS
on arches that don't implement it.
---
 syscalls/perf_event_open.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/syscalls/perf_event_open.c b/syscalls/perf_event_open.c
index d6c404c..0fe64ce 100644
--- a/syscalls/perf_event_open.c
+++ b/syscalls/perf_event_open.c
@@ -1316,5 +1316,5 @@ struct syscallentry syscall_perf_event_open = {
 	},
 	.sanitise = sanitise_perf_event_open,
 	.init = init_pmus,
-	.flags = NEED_ALARM,
+	.flags = NEED_ALARM | IGNORE_ENOSYS,
 };
-- 
1.9.1

Re: [PATCH 1/2] Add an IGNORE_ENOSYS flag and use it

[Dave Jones]

On Mon, May 26, 2014 at 10:32:01PM +1000, Michael Ellerman wrote:
 > Some syscalls return ENOSYS depending on their arguments. We don't want
 > to stop calling them just because we hit one of those cases. Add a flag
 > to specify this behaviour so we don't have to keep special-casing those
 > calls in mkcall().

I was hopeful this list wouldn't grow, but that doesn't seem to be
the case.  Begrudgingly, I applied this.  It's going to be a lot
cleaner to maintain if people keep doing this.

thanks,

	Dave

Re: [PATCH 1/2] Add an IGNORE_ENOSYS flag and use it

[Michael Ellerman]

On Mon, 2014-05-26 at 20:41 -0400, Dave Jones wrote:
> On Mon, May 26, 2014 at 10:32:01PM +1000, Michael Ellerman wrote:
>  > Some syscalls return ENOSYS depending on their arguments. We don't want
>  > to stop calling them just because we hit one of those cases. Add a flag
>  > to specify this behaviour so we don't have to keep special-casing those
>  > calls in mkcall().
> 
> I was hopeful this list wouldn't grow, but that doesn't seem to be
> the case.  Begrudgingly, I applied this.  It's going to be a lot
> cleaner to maintain if people keep doing this.

Yeah it's annoying for sure, maybe perf will be the last one, but at least
there's a clean way to handle it if not.

cheers

Re: [PATCH 1/2] Add an IGNORE_ENOSYS flag and use it

[Vince Weaver]

On Tue, 27 May 2014, Michael Ellerman wrote:

> On Mon, 2014-05-26 at 20:41 -0400, Dave Jones wrote:
> > On Mon, May 26, 2014 at 10:32:01PM +1000, Michael Ellerman wrote:
> >  > Some syscalls return ENOSYS depending on their arguments. We don't want
> >  > to stop calling them just because we hit one of those cases. Add a flag
> >  > to specify this behaviour so we don't have to keep special-casing those
> >  > calls in mkcall().
> > 
> > I was hopeful this list wouldn't grow, but that doesn't seem to be
> > the case.  Begrudgingly, I applied this.  It's going to be a lot
> > cleaner to maintain if people keep doing this.
> 
> Yeah it's annoying for sure, maybe perf will be the last one, but at least
> there's a clean way to handle it if not.

As the author of the man page that you probably got the perf ENOSYS info 
from, I have to put out there that perf_event_open() has really 
inconsistent and confusing error return values, and they vary among the 
various architectures.

I'd hope other syscalls do better, but it's really easy to slip in weird 
return values in unexpected places.  I somewhat try to follow the 
perf_event ABI and watch for stuff like this (if only to keep the manpage 
up to date) but I missed the ENOSYS commit at the time (especially since
it's not possible to trigger on x86 where I do most of my testing).

Vince

Re: [PATCH 1/2] Add an IGNORE_ENOSYS flag and use it

[Michael Ellerman]

On Tue, 2014-05-27 at 01:02 -0400, Vince Weaver wrote:
> On Tue, 27 May 2014, Michael Ellerman wrote:
> 
> > On Mon, 2014-05-26 at 20:41 -0400, Dave Jones wrote:
> > > On Mon, May 26, 2014 at 10:32:01PM +1000, Michael Ellerman wrote:
> > >  > Some syscalls return ENOSYS depending on their arguments. We don't want
> > >  > to stop calling them just because we hit one of those cases. Add a flag
> > >  > to specify this behaviour so we don't have to keep special-casing those
> > >  > calls in mkcall().
> > > 
> > > I was hopeful this list wouldn't grow, but that doesn't seem to be
> > > the case.  Begrudgingly, I applied this.  It's going to be a lot
> > > cleaner to maintain if people keep doing this.
> > 
> > Yeah it's annoying for sure, maybe perf will be the last one, but at least
> > there's a clean way to handle it if not.
> 
> As the author of the man page that you probably got the perf ENOSYS info 
> from, I have to put out there that perf_event_open() has really 
> inconsistent and confusing error return values, and they vary among the 
> various architectures.

Actually I didn't read the man page, but thanks for reminding me that it
exists. And yes I am aware of your dislike of the perf interface.

> I'd hope other syscalls do better, but it's really easy to slip in weird 
> return values in unexpected places.  I somewhat try to follow the 
> perf_event ABI and watch for stuff like this (if only to keep the manpage 
> up to date) but I missed the ENOSYS commit at the time (especially since
> it's not possible to trigger on x86 where I do most of my testing).

Yeah it's our fault in a way, in that we didn't test that code on powerpc prior
to it going in. But with the pace things move sometimes that happens. It's
annoying but not the end of the world.

cheers

Re: [PATCH 1/2] Add an IGNORE_ENOSYS flag and use it

[Vince Weaver]

On Wed, 28 May 2014, Michael Ellerman wrote:

> On Tue, 2014-05-27 at 01:02 -0400, Vince Weaver wrote:
> > On Tue, 27 May 2014, Michael Ellerman wrote:
> > 
> > > On Mon, 2014-05-26 at 20:41 -0400, Dave Jones wrote:
> > > > On Mon, May 26, 2014 at 10:32:01PM +1000, Michael Ellerman wrote:
> > > >  > Some syscalls return ENOSYS depending on their arguments. We don't want
> > > >  > to stop calling them just because we hit one of those cases. Add a flag
> > > >  > to specify this behaviour so we don't have to keep special-casing those
> > > >  > calls in mkcall().
> > > > 
> > > > I was hopeful this list wouldn't grow, but that doesn't seem to be
> > > > the case.  Begrudgingly, I applied this.  It's going to be a lot
> > > > cleaner to maintain if people keep doing this.
> > > 
> > > Yeah it's annoying for sure, maybe perf will be the last one, but at least
> > > there's a clean way to handle it if not.
> > 
> > As the author of the man page that you probably got the perf ENOSYS info 
> > from, I have to put out there that perf_event_open() has really 
> > inconsistent and confusing error return values, and they vary among the 
> > various architectures.
> 
> Actually I didn't read the man page, but thanks for reminding me that it
> exists. And yes I am aware of your dislike of the perf interface.

I had just recently contributed an improved "ERRORS" section to the 
perf_event_open manpage that tries to be complete, but I'm sure I missed 
some corner cases.  It does document the odd choice of returning ENOSYS in 
the PERF_SAMPLE_STACK_USER case.

It's a shame there isn't some sort of automated tool that could spit out 
all possible system call return values, it would make it a lot easier to 
keep things consistent.

Vince