INPUT_COMPAT_TEST

INPUT_COMPAT_TEST

[H. Peter Anvin]

Hello,

I'm trying to figure out what system calls can actually invoke code that
depends on INPUT_COMPAT_TEST, and in particular which of those changes
actually matter.

The input system and seccomp are the *only* arch-neutral subsystem in
Linux which appear to trigger on if we are in compat mode, and this is
causing some serious consternation in trying to freeze the proposed x32
(32-bit x86-64) ABI.

I would really like to understand if that dependency can be eliminated
or mitigated; as it currently sits we may need an entirely separate
system call table *just to support the input system*.

The problem is that I am personally not familiar enough with the input
subsystem to know what the dependencies are.  ioctls are not an issue;
there is already an entire infrastructure to handle compatibility ioctls
(and that infrastructure should be used!), but it looks like input also
does things like change the format(?!) of sysfs entries, all of which
makes me very concerned.

Any help in creating an inventory for this would be appreciated.

	-hpa

Re: INPUT_COMPAT_TEST

[Dmitry Torokhov]

Hi Peter,

On Fri, Jul 08, 2011 at 11:45:35AM -0700, H. Peter Anvin wrote:
> Hello,
> 
> I'm trying to figure out what system calls can actually invoke code that
> depends on INPUT_COMPAT_TEST, and in particular which of those changes
> actually matter.
> 
> The input system and seccomp are the *only* arch-neutral subsystem in
> Linux which appear to trigger on if we are in compat mode, and this is
> causing some serious consternation in trying to freeze the proposed x32
> (32-bit x86-64) ABI.
> 
> I would really like to understand if that dependency can be eliminated
> or mitigated; as it currently sits we may need an entirely separate
> system call table *just to support the input system*.
> 
> The problem is that I am personally not familiar enough with the input
> subsystem to know what the dependencies are.  ioctls are not an issue;
> there is already an entire infrastructure to handle compatibility ioctls
> (and that infrastructure should be used!),

One such place is evdev read/write - unfortunately "struct input_event"
was not created 32/64 bit safe so we need to mangle it when running 32
bit userspace with 64-bit kernels. See drivers/input/input-compat.c.

We also have similar issues with uinput API and uploading force-freedack
effects.

> but it looks like input also
> does things like change the format(?!) of sysfs entries, all of which
> makes me very concerned.

Another historical unfortunate decision. /proc/bus/input (and later
added sysfs entries) export bitmaps in "compressed" form so that
userspace can not figure out the size of the segment (32 or 64 bit) on
its own so we have to convert to userspace size for longs.

> 
> Any help in creating an inventory for this would be appreciated.
> 

I believe the above are the only non-ioctl compat issues in input core.

-- 
Dmitry

Re: INPUT_COMPAT_TEST

[H. Peter Anvin]

On 07/08/2011 01:46 PM, Dmitry Torokhov wrote:
> 
> One such place is evdev read/write - unfortunately "struct input_event"
> was not created 32/64 bit safe so we need to mangle it when running 32
> bit userspace with 64-bit kernels. See drivers/input/input-compat.c.
> 

So these are actually present on all the read/write path system calls.
That is truly awful.

Do you happen to know if there is any kind of passing around of file
descriptors, or if this could perhaps be tied to file descriptor state.

> We also have similar issues with uinput API and uploading force-freedack
> effects.

Those are ioctl, though, if I read the code right, or did I miss
something obvious?

>> but it looks like input also
>> does things like change the format(?!) of sysfs entries, all of which
>> makes me very concerned.
> 
> Another historical unfortunate decision. /proc/bus/input (and later
> added sysfs entries) export bitmaps in "compressed" form so that
> userspace can not figure out the size of the segment (32 or 64 bit) on
> its own so we have to convert to userspace size for longs.

"Compressed form"?  Could you give a concrete example?  They look like
they are emitted in text form.

Do you have a program that someone could run to see the differences
between compat and non-compat paths?

	-hpa

Re: INPUT_COMPAT_TEST

[Dmitry Torokhov]

On Fri, Jul 08, 2011 at 03:22:24PM -0700, H. Peter Anvin wrote:
> On 07/08/2011 01:46 PM, Dmitry Torokhov wrote:
> > 
> > One such place is evdev read/write - unfortunately "struct input_event"
> > was not created 32/64 bit safe so we need to mangle it when running 32
> > bit userspace with 64-bit kernels. See drivers/input/input-compat.c.
> > 
> 
> So these are actually present on all the read/write path system calls.
> That is truly awful.
> 
> Do you happen to know if there is any kind of passing around of file
> descriptors, or if this could perhaps be tied to file descriptor state.

Not sure.

> 
> > We also have similar issues with uinput API and uploading force-freedack
> > effects.
> 
> Those are ioctl, though, if I read the code right, or did I miss
> something obvious?

Ah, yes, indeed.

> 
> >> but it looks like input also
> >> does things like change the format(?!) of sysfs entries, all of which
> >> makes me very concerned.
> > 
> > Another historical unfortunate decision. /proc/bus/input (and later
> > added sysfs entries) export bitmaps in "compressed" form so that
> > userspace can not figure out the size of the segment (32 or 64 bit) on
> > its own so we have to convert to userspace size for longs.
> 
> "Compressed form"?  Could you give a concrete example?  They look like
> they are emitted in text form.

We drop leading zeroes so if you get "1 0 0 1ffff" you do not know
the bit position of the most significant '1' unless we keep segments of
known size. Unfortunately we started with 32 bit segments on 32 bit
kernels and 64 bit segments on 64 bit kernels so we coudl not simply say
that we always split on 32 bit boundary when we discovered compat
problem a few years later.

> 
> Do you have a program that someone could run to see the differences
> between compat and non-compat paths?

Hmm, cat for /proc/bus/input/devices and sysfs nodes and evtest would
either work or give garbage if compat code woudl not work.

-- 
Dmitry

Re: INPUT_COMPAT_TEST

[Thadeu Lima de Souza Cascardo]

[-- Attachment #1: Type: text/plain, Size: 2628 bytes --]

On Fri, Jul 08, 2011 at 03:37:09PM -0700, Dmitry Torokhov wrote:
> On Fri, Jul 08, 2011 at 03:22:24PM -0700, H. Peter Anvin wrote:
> > On 07/08/2011 01:46 PM, Dmitry Torokhov wrote:
> > > 
> > > One such place is evdev read/write - unfortunately "struct input_event"
> > > was not created 32/64 bit safe so we need to mangle it when running 32
> > > bit userspace with 64-bit kernels. See drivers/input/input-compat.c.
> > > 
> > 
> > So these are actually present on all the read/write path system calls.
> > That is truly awful.
> > 
> > Do you happen to know if there is any kind of passing around of file
> > descriptors, or if this could perhaps be tied to file descriptor state.
> 
> Not sure.
> 
> > 
> > > We also have similar issues with uinput API and uploading force-freedack
> > > effects.
> > 
> > Those are ioctl, though, if I read the code right, or did I miss
> > something obvious?
> 
> Ah, yes, indeed.
> 

uinput uses input_event_from_user in its write path. To inject events
into the kernel, one must write struct input_event into uinput. So, in
this case, it's not only ioctl that is affected.

Regards,
Cascardo.

> > 
> > >> but it looks like input also
> > >> does things like change the format(?!) of sysfs entries, all of which
> > >> makes me very concerned.
> > > 
> > > Another historical unfortunate decision. /proc/bus/input (and later
> > > added sysfs entries) export bitmaps in "compressed" form so that
> > > userspace can not figure out the size of the segment (32 or 64 bit) on
> > > its own so we have to convert to userspace size for longs.
> > 
> > "Compressed form"?  Could you give a concrete example?  They look like
> > they are emitted in text form.
> 
> We drop leading zeroes so if you get "1 0 0 1ffff" you do not know
> the bit position of the most significant '1' unless we keep segments of
> known size. Unfortunately we started with 32 bit segments on 32 bit
> kernels and 64 bit segments on 64 bit kernels so we coudl not simply say
> that we always split on 32 bit boundary when we discovered compat
> problem a few years later.
> 
> > 
> > Do you have a program that someone could run to see the differences
> > between compat and non-compat paths?
> 
> Hmm, cat for /proc/bus/input/devices and sysfs nodes and evtest would
> either work or give garbage if compat code woudl not work.
> 
> -- 
> Dmitry
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: INPUT_COMPAT_TEST

[H. Peter Anvin]

On 07/08/2011 03:37 PM, Dmitry Torokhov wrote:
>>
>>> We also have similar issues with uinput API and uploading force-freedack
>>> effects.
>>
>> Those are ioctl, though, if I read the code right, or did I miss
>> something obvious?
> 
> Ah, yes, indeed.
> 

So the point still holds... you're right now using INPUT_COMPAT_TEST for
those, but what you *should* use is whether or not you were entered via
the compat ioctl entry point.

>>
>>>> but it looks like input also
>>>> does things like change the format(?!) of sysfs entries, all of which
>>>> makes me very concerned.
>>>
>>> Another historical unfortunate decision. /proc/bus/input (and later
>>> added sysfs entries) export bitmaps in "compressed" form so that
>>> userspace can not figure out the size of the segment (32 or 64 bit) on
>>> its own so we have to convert to userspace size for longs.
>>
>> "Compressed form"?  Could you give a concrete example?  They look like
>> they are emitted in text form.
> 
> We drop leading zeroes so if you get "1 0 0 1ffff" you do not know
> the bit position of the most significant '1' unless we keep segments of
> known size. Unfortunately we started with 32 bit segments on 32 bit
> kernels and 64 bit segments on 64 bit kernels so we coudl not simply say
> that we always split on 32 bit boundary when we discovered compat
> problem a few years later.

Ah yes, it is the "binary output masquerading as text, so we end up with
something that is worse a mess than either" problem.

>>
>> Do you have a program that someone could run to see the differences
>> between compat and non-compat paths?
> 
> Hmm, cat for /proc/bus/input/devices and sysfs nodes and evtest would
> either work or give garbage if compat code woudl not work.
> 

I'm desperately trying to come up with a solution which doesn't require
us to replicate every single system call (which is what relying on
is_compat_task() does -- it remembers the entry point used) in order
support one single misdesigned subsystem.  Do you have any kind of ideas
for what we might be able to do?

	-hpa

Re: INPUT_COMPAT_TEST

[Dmitry Torokhov]

On Fri, Jul 08, 2011 at 04:18:11PM -0700, H. Peter Anvin wrote:
> On 07/08/2011 03:37 PM, Dmitry Torokhov wrote:
> >>
> >>> We also have similar issues with uinput API and uploading force-freedack
> >>> effects.
> >>
> >> Those are ioctl, though, if I read the code right, or did I miss
> >> something obvious?
> > 
> > Ah, yes, indeed.
> > 
> 
> So the point still holds... you're right now using INPUT_COMPAT_TEST for
> those, but what you *should* use is whether or not you were entered via
> the compat ioctl entry point.

Since we still do need (at least for now) INPUT_COMPAT_TEST in non-ioctl
paths it does not matter much.

> 
> >>
> >>>> but it looks like input also
> >>>> does things like change the format(?!) of sysfs entries, all of which
> >>>> makes me very concerned.
> >>>
> >>> Another historical unfortunate decision. /proc/bus/input (and later
> >>> added sysfs entries) export bitmaps in "compressed" form so that
> >>> userspace can not figure out the size of the segment (32 or 64 bit) on
> >>> its own so we have to convert to userspace size for longs.
> >>
> >> "Compressed form"?  Could you give a concrete example?  They look like
> >> they are emitted in text form.
> > 
> > We drop leading zeroes so if you get "1 0 0 1ffff" you do not know
> > the bit position of the most significant '1' unless we keep segments of
> > known size. Unfortunately we started with 32 bit segments on 32 bit
> > kernels and 64 bit segments on 64 bit kernels so we coudl not simply say
> > that we always split on 32 bit boundary when we discovered compat
> > problem a few years later.
> 
> Ah yes, it is the "binary output masquerading as text, so we end up with
> something that is worse a mess than either" problem.
> 
> >>
> >> Do you have a program that someone could run to see the differences
> >> between compat and non-compat paths?
> > 
> > Hmm, cat for /proc/bus/input/devices and sysfs nodes and evtest would
> > either work or give garbage if compat code woudl not work.
> > 
> 
> I'm desperately trying to come up with a solution which doesn't require
> us to replicate every single system call (which is what relying on
> is_compat_task() does -- it remembers the entry point used) in order
> support one single misdesigned subsystem.  Do you have any kind of ideas
> for what we might be able to do?

Input only need to do this compat stuff on read/write paths so maybe if
you add plumbing similar to compat_ioctl we could switch owver to it.

Thanks.

-- 
Dmitry

Re: INPUT_COMPAT_TEST

[H. Peter Anvin]

On 07/08/2011 05:35 PM, Dmitry Torokhov wrote:
> 
> Input only need to do this compat stuff on read/write paths so maybe if
> you add plumbing similar to compat_ioctl we could switch owver to it.
> 

The problem is that read/write ties into a large number of system calls,
and input is the *only* subsystem which needs it.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: INPUT_COMPAT_TEST

[Dmitry Torokhov]

On Sat, Jul 09, 2011 at 01:04:53AM -0700, H. Peter Anvin wrote:
> On 07/08/2011 05:35 PM, Dmitry Torokhov wrote:
> > 
> > Input only need to do this compat stuff on read/write paths so maybe if
> > you add plumbing similar to compat_ioctl we could switch owver to it.
> > 
> 
> The problem is that read/write ties into a large number of system calls,
> and input is the *only* subsystem which needs it.
> 

BTW, while listening to x32 resentation on LPC I realized that the need
for compat tests on read/write paths in input subsystem is due to use of
timeval in input_event structures. If x32 solved the time_t issue by
moving to 64 bit times then input read/write should simply use native
64 bit operations.

That still leaves sysfs and proc business of course...

Thanks.

-- 
Dmitry

Re: INPUT_COMPAT_TEST

[H. Peter Anvin]

On 09/07/2011 11:16 AM, Dmitry Torokhov wrote:
> On Sat, Jul 09, 2011 at 01:04:53AM -0700, H. Peter Anvin wrote:
>> On 07/08/2011 05:35 PM, Dmitry Torokhov wrote:
>>>
>>> Input only need to do this compat stuff on read/write paths so maybe if
>>> you add plumbing similar to compat_ioctl we could switch owver to it.
>>>
>>
>> The problem is that read/write ties into a large number of system calls,
>> and input is the *only* subsystem which needs it.
>>
> 
> BTW, while listening to x32 resentation on LPC I realized that the need
> for compat tests on read/write paths in input subsystem is due to use of
> timeval in input_event structures. If x32 solved the time_t issue by
> moving to 64 bit times then input read/write should simply use native
> 64 bit operations.
> 
> That still leaves sysfs and proc business of course...
> 

I thought there were pointers, (or longs) there too.

In fact, we might have had the worst of both worlds here...

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: INPUT_COMPAT_TEST

[Dmitry Torokhov]

On Wed, Sep 07, 2011 at 11:22:10AM -0700, H. Peter Anvin wrote:
> On 09/07/2011 11:16 AM, Dmitry Torokhov wrote:
> > On Sat, Jul 09, 2011 at 01:04:53AM -0700, H. Peter Anvin wrote:
> >> On 07/08/2011 05:35 PM, Dmitry Torokhov wrote:
> >>>
> >>> Input only need to do this compat stuff on read/write paths so maybe if
> >>> you add plumbing similar to compat_ioctl we could switch owver to it.
> >>>
> >>
> >> The problem is that read/write ties into a large number of system calls,
> >> and input is the *only* subsystem which needs it.
> >>
> > 
> > BTW, while listening to x32 resentation on LPC I realized that the need
> > for compat tests on read/write paths in input subsystem is due to use of
> > timeval in input_event structures. If x32 solved the time_t issue by
> > moving to 64 bit times then input read/write should simply use native
> > 64 bit operations.
> > 
> > That still leaves sysfs and proc business of course...
> > 
> 
> I thought there were pointers, (or longs) there too.
> 
> In fact, we might have had the worst of both worlds here...
> 

The pointers are in ioctl paths (upload of force-feedback effects with
custom waveforms - noone actually uses them in real life).

-- 
Dmitry

Re: INPUT_COMPAT_TEST

[H. Peter Anvin]

On 09/07/2011 11:37 AM, Dmitry Torokhov wrote:
>>
>> I thought there were pointers, (or longs) there too.
>>
>> In fact, we might have had the worst of both worlds here...
>
> The pointers are in ioctl paths (upload of force-feedback effects with
> custom waveforms - noone actually uses them in real life).
>

Ah... we might have lucked out then!

The proc/sysfs issue is still HUGE, however, because that affects all 
the read/write paths *indirectly*.

	-hpa

Re: INPUT_COMPAT_TEST

[Dmitry Torokhov]

On Wed, Sep 07, 2011 at 11:40:46AM -0700, H. Peter Anvin wrote:
> On 09/07/2011 11:37 AM, Dmitry Torokhov wrote:
> >>
> >>I thought there were pointers, (or longs) there too.
> >>
> >>In fact, we might have had the worst of both worlds here...
> >
> >The pointers are in ioctl paths (upload of force-feedback effects with
> >custom waveforms - noone actually uses them in real life).
> >
> 
> Ah... we might have lucked out then!
> 
> The proc/sysfs issue is still HUGE, however, because that affects
> all the read/write paths *indirectly*.
> 

Yeah, we are stuck with these unfortunately... Unless we want to say
that x32 applications will be presented with bitmaps encoded with 64-bit
segments instead of 32-bit ones. This would mostly matter to startup
scripts... X and the others use ioctls, not proc/sysfs.

-- 
Dmitry