Re: [PATCH v2 0/6] perf: Introduce extended syscall error reporting

[Johannes Berg <johannes@sipsolutions.net>]

On Tue, 2015-08-25 at 12:07 +0200, Ingo Molnar wrote:
> Having a separate syscall has two (big!) appeals:
> 
>  - we wouldn't have to touch existing system calls at all.
> 
>  - extended error reporting would be available for any system call that opts to
>    use it. (The current scheme as submitted is only available to system calls
>    using the perf-style flexible attribute ABI.)

Yeah, I agree this is nice. However, more generally, I think we need to
actually think more about the module problem then since while syscalls
can't be implemented in modules (I think) they can still end up calling
into modules.

Of course a first iteration could be exactly like what Alexander
posted.

The other issue with this is namespacing - can all syscalls, and
everything that eventually gets called, really use a single error code
namespace with its 3k limit? On the one hand I'm thinking "3k strings
are so big ... we don't want more", but on the other hand all kinds of
drivers etc. might start getting annotations?

> > Considering the wifi case, or more generally any netlink based
> > protocol, the syscall (sendmsg) won't return an error, but a subsequent
> > recvmsg() (which also won't return an error) returns an error message
> > [in the sense of a protocol message, not a human readable message] to a
> > buffer provided by the application.
> > However, this message can be extended relatively easily to include the
> > string information, but the syscall/prctl wouldn't work since the
> > syscalls didn't actually fail.
> 
> Ok. So assuming we can make a 1:1 mapping between the 'extended error code' 
> integer space and the message:owner strings, it would be enough for netlink to 
> pass along the integer code itself, not the full strings?

Considering that this would likely have to be opt-in at the netlink
level (e.g. through a flag in the request message), perhaps. I'd say
it'd still be easier for the message to carry the intended error code
(e.g. -EINVAL) and the actual message in the ACK message [where
requested]. That way, applications that actually behave depending on
the error code can far more easily be extended.

> That would simplify things and make the scheme more robust from a security POV I 
> suspect.

You could also argue the other way around, in that being able to look
up (from userspace) arbitrary extended error IDs, even those that
haven't ever been used, could be an information leak of sorts.

> So my hope would be that we can represent this all with a single 'large' error 
> code integer space. That integer would be constant and translateable (as long as 
> the module is loaded).

Ok, I wasn't really what I was assuming. As I said above, on the one
hand I agree, but on the other I'm looking at the reality of a few
hundred (!) -EINVAL callsites in net/wireless/nl80211.c alone, so
having an overall 3k limit seems somewhat low.

> That way the error passing mechanism wouldn't have to be specifically module-aware 
> - during build we generate the integer space, with all possible modules 
> considered.
> 

That would be no improvement for me as I work heavily with (upstream)
modules that are compiled out-of-tree, so I'm not all inclined to spend
much time on it if that ends up being the solution ;)

There are, however, are other ways it seems: for example assigning each
module an offset and taking that into account if the code was built
modular. That needs a small allocator to put the module range somewhere
into the global range, of course:

#ifdef MODULE
#define mod_ext_err(errno, string)
	struct ext_err { ... } _my_err __section(errors);
	return mod_offset < 0 ?
		errno :
		mod_ext_err_offset + &_my_err - __start_errors;
#else
...
#endif

or something like that - that leaves the possibility of allocation
failures (e.g. due to error space fragmentation) which sets the offset
to -1. The only additional wrinkle would be that a lookup would have to
walk some kind of extents tree that says which module (table) to look
at.

johannes