Re: linux-next: Tree for July 30

Re: linux-next: Tree for July 30

[Andrew Morton]

On Wed, 30 Jul 2008 17:06:35 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> I have created today's linux-next tree at
> git://git.kernel.org/pub/scm/linux/kernel/git/sfr/linux-next.git

The X server broke on my FC8 t61p thinkpad.  Mainline is OK.

Various information is at http://userweb.kernel.org/~akpm/mo/

I'm suspecting the input layer - my synaptics device seems to have
disappeared?  See http://userweb.kernel.org/~akpm/mo/Xorg-log-diff.txt

Re: linux-next: Tree for July 30

[Dmitry Torokhov]

On Wed, Jul 30, 2008 at 11:10:29PM -0700, Andrew Morton wrote:
> On Wed, 30 Jul 2008 17:06:35 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> 
> > I have created today's linux-next tree at
> > git://git.kernel.org/pub/scm/linux/kernel/git/sfr/linux-next.git
> 
> The X server broke on my FC8 t61p thinkpad.  Mainline is OK.
> 
> Various information is at http://userweb.kernel.org/~akpm/mo/
> 
> I'm suspecting the input layer - my synaptics device seems to have
> disappeared?  See http://userweb.kernel.org/~akpm/mo/Xorg-log-diff.txt
> 

I think this patch should help with Synaptics:

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Subject: [PATCH] Fix crash on kernels with extended keymap space

The len argument of EVIOCGBIT(ev,len) is the size of the receiving
buffer in bytes, not maximim number of bits to retrieve.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
 eventcomm.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/eventcomm.c b/eventcomm.c
index e3257cd..2d0a347 100644
--- a/eventcomm.c
+++ b/eventcomm.c
@@ -89,7 +89,7 @@ event_query_is_touchpad(int fd)
 
     /* Check for ABS_X, ABS_Y, ABS_PRESSURE and BTN_TOOL_FINGER */
 
-    SYSCALL(ret = ioctl(fd, EVIOCGBIT(0, EV_MAX), evbits));
+    SYSCALL(ret = ioctl(fd, EVIOCGBIT(0, sizeof(evbits)), evbits));
     if (ret < 0)
 	return FALSE;
     if (!TEST_BIT(EV_SYN, evbits) ||
@@ -97,7 +97,7 @@ event_query_is_touchpad(int fd)
 	!TEST_BIT(EV_KEY, evbits))
 	return FALSE;
 
-    SYSCALL(ret = ioctl(fd, EVIOCGBIT(EV_ABS, KEY_MAX), evbits));
+    SYSCALL(ret = ioctl(fd, EVIOCGBIT(EV_ABS, sizeof(evbits)), evbits));
     if (ret < 0)
 	return FALSE;
     if (!TEST_BIT(ABS_X, evbits) ||
@@ -105,7 +105,7 @@ event_query_is_touchpad(int fd)
 	!TEST_BIT(ABS_PRESSURE, evbits))
 	return FALSE;
 
-    SYSCALL(ret = ioctl(fd, EVIOCGBIT(EV_KEY, KEY_MAX), evbits));
+    SYSCALL(ret = ioctl(fd, EVIOCGBIT(EV_KEY, sizeof(evbits)), evbits));
     if (ret < 0)
 	return FALSE;
     if (!TEST_BIT(BTN_TOOL_FINGER, evbits))
-- 
1.5.5.1

Re: linux-next: Tree for July 30

[Bartlomiej Zolnierkiewicz]

On Thu, Jul 31, 2008 at 4:07 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Wed, Jul 30, 2008 at 11:10:29PM -0700, Andrew Morton wrote:
>> On Wed, 30 Jul 2008 17:06:35 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>>
>> > I have created today's linux-next tree at
>> > git://git.kernel.org/pub/scm/linux/kernel/git/sfr/linux-next.git
>>
>> The X server broke on my FC8 t61p thinkpad.  Mainline is OK.
>>
>> Various information is at http://userweb.kernel.org/~akpm/mo/
>>
>> I'm suspecting the input layer - my synaptics device seems to have
>> disappeared?  See http://userweb.kernel.org/~akpm/mo/Xorg-log-diff.txt
>>
>
> I think this patch should help with Synaptics:

Which unfortunately doesn't help all people running with older synaptics
user-space after commit 0571c5d20aca71c735222132b02aebddf593045c
("Input: expand keycode space").

Can't this be solved without breaking Xorg on newer kernels running
older synaptics?

> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Subject: [PATCH] Fix crash on kernels with extended keymap space
>
> The len argument of EVIOCGBIT(ev,len) is the size of the receiving
> buffer in bytes, not maximim number of bits to retrieve.
>
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> ---
>  eventcomm.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/eventcomm.c b/eventcomm.c
> index e3257cd..2d0a347 100644
> --- a/eventcomm.c
> +++ b/eventcomm.c
> @@ -89,7 +89,7 @@ event_query_is_touchpad(int fd)
>
>     /* Check for ABS_X, ABS_Y, ABS_PRESSURE and BTN_TOOL_FINGER */
>
> -    SYSCALL(ret = ioctl(fd, EVIOCGBIT(0, EV_MAX), evbits));
> +    SYSCALL(ret = ioctl(fd, EVIOCGBIT(0, sizeof(evbits)), evbits));
>     if (ret < 0)
>        return FALSE;
>     if (!TEST_BIT(EV_SYN, evbits) ||
> @@ -97,7 +97,7 @@ event_query_is_touchpad(int fd)
>        !TEST_BIT(EV_KEY, evbits))
>        return FALSE;
>
> -    SYSCALL(ret = ioctl(fd, EVIOCGBIT(EV_ABS, KEY_MAX), evbits));
> +    SYSCALL(ret = ioctl(fd, EVIOCGBIT(EV_ABS, sizeof(evbits)), evbits));
>     if (ret < 0)
>        return FALSE;
>     if (!TEST_BIT(ABS_X, evbits) ||
> @@ -105,7 +105,7 @@ event_query_is_touchpad(int fd)
>        !TEST_BIT(ABS_PRESSURE, evbits))
>        return FALSE;
>
> -    SYSCALL(ret = ioctl(fd, EVIOCGBIT(EV_KEY, KEY_MAX), evbits));
> +    SYSCALL(ret = ioctl(fd, EVIOCGBIT(EV_KEY, sizeof(evbits)), evbits));
>     if (ret < 0)
>        return FALSE;
>     if (!TEST_BIT(BTN_TOOL_FINGER, evbits))
> --
> 1.5.5.1

Re: linux-next: Tree for July 30

[Dmitry Torokhov]

On Thu, Jul 31, 2008 at 05:36:16PM +0200, Bartlomiej Zolnierkiewicz wrote:
> On Thu, Jul 31, 2008 at 4:07 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > On Wed, Jul 30, 2008 at 11:10:29PM -0700, Andrew Morton wrote:
> >> On Wed, 30 Jul 2008 17:06:35 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >>
> >> > I have created today's linux-next tree at
> >> > git://git.kernel.org/pub/scm/linux/kernel/git/sfr/linux-next.git
> >>
> >> The X server broke on my FC8 t61p thinkpad.  Mainline is OK.
> >>
> >> Various information is at http://userweb.kernel.org/~akpm/mo/
> >>
> >> I'm suspecting the input layer - my synaptics device seems to have
> >> disappeared?  See http://userweb.kernel.org/~akpm/mo/Xorg-log-diff.txt
> >>
> >
> > I think this patch should help with Synaptics:
> 
> Which unfortunately doesn't help all people running with older synaptics
> user-space after commit 0571c5d20aca71c735222132b02aebddf593045c
> ("Input: expand keycode space").
> 
> Can't this be solved without breaking Xorg on newer kernels running
> older synaptics?
> 

No. The X driver is broken. It tells kernel to use buffer bugger than
allocated and gets its stack smashed. Tslib has also soma funkiness
in the ioctl handling as well... *shrug*

We have a couple months to get distros updated...

-- 
Dmitry

Re: linux-next: Tree for July 30

[Andrew Morton]

On Thu, 31 Jul 2008 11:56:48 -0400 Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:

> On Thu, Jul 31, 2008 at 05:36:16PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > On Thu, Jul 31, 2008 at 4:07 PM, Dmitry Torokhov
> > <dmitry.torokhov@gmail.com> wrote:
> > > On Wed, Jul 30, 2008 at 11:10:29PM -0700, Andrew Morton wrote:
> > >> On Wed, 30 Jul 2008 17:06:35 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > >>
> > >> > I have created today's linux-next tree at
> > >> > git://git.kernel.org/pub/scm/linux/kernel/git/sfr/linux-next.git
> > >>
> > >> The X server broke on my FC8 t61p thinkpad.  Mainline is OK.
> > >>
> > >> Various information is at http://userweb.kernel.org/~akpm/mo/
> > >>
> > >> I'm suspecting the input layer - my synaptics device seems to have
> > >> disappeared?  See http://userweb.kernel.org/~akpm/mo/Xorg-log-diff.txt
> > >>
> > >
> > > I think this patch should help with Synaptics:
> > 
> > Which unfortunately doesn't help all people running with older synaptics
> > user-space after commit 0571c5d20aca71c735222132b02aebddf593045c
> > ("Input: expand keycode space").
> > 
> > Can't this be solved without breaking Xorg on newer kernels running
> > older synaptics?
> > 
> 
> No. The X driver is broken. It tells kernel to use buffer bugger than
> allocated and gets its stack smashed. Tslib has also soma funkiness
> in the ioctl handling as well... *shrug*
> 
> We have a couple months to get distros updated...
> 

aaarrrrgggggghhh.  I don't think this is practical.  This means that
(for example) FC5 machines (of which I happen to have one) are dead. 
And lots of other older-distro-based systems.

Is there some userspace workaround which doesn't require an X server
update?

Surely it must be possible to make the kernel contiue to support these
servers?

Re: linux-next: Tree for July 30

[Dmitry Torokhov]

On Thu, Jul 31, 2008 at 10:44:37AM -0700, Andrew Morton wrote:
> On Thu, 31 Jul 2008 11:56:48 -0400 Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> 
> > On Thu, Jul 31, 2008 at 05:36:16PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > > On Thu, Jul 31, 2008 at 4:07 PM, Dmitry Torokhov
> > > <dmitry.torokhov@gmail.com> wrote:
> > > > On Wed, Jul 30, 2008 at 11:10:29PM -0700, Andrew Morton wrote:
> > > >> On Wed, 30 Jul 2008 17:06:35 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > > >>
> > > >> > I have created today's linux-next tree at
> > > >> > git://git.kernel.org/pub/scm/linux/kernel/git/sfr/linux-next.git
> > > >>
> > > >> The X server broke on my FC8 t61p thinkpad.  Mainline is OK.
> > > >>
> > > >> Various information is at http://userweb.kernel.org/~akpm/mo/
> > > >>
> > > >> I'm suspecting the input layer - my synaptics device seems to have
> > > >> disappeared?  See http://userweb.kernel.org/~akpm/mo/Xorg-log-diff.txt
> > > >>
> > > >
> > > > I think this patch should help with Synaptics:
> > > 
> > > Which unfortunately doesn't help all people running with older synaptics
> > > user-space after commit 0571c5d20aca71c735222132b02aebddf593045c
> > > ("Input: expand keycode space").
> > > 
> > > Can't this be solved without breaking Xorg on newer kernels running
> > > older synaptics?
> > > 
> > 
> > No. The X driver is broken. It tells kernel to use buffer bugger than
> > allocated and gets its stack smashed. Tslib has also soma funkiness
> > in the ioctl handling as well... *shrug*
> > 
> > We have a couple months to get distros updated...
> > 
> 
> aaarrrrgggggghhh.  I don't think this is practical.  This means that
> (for example) FC5 machines (of which I happen to have one) are dead. 
> And lots of other older-distro-based systems.
> 
> Is there some userspace workaround which doesn't require an X server
> update?
> 
> Surely it must be possible to make the kernel contiue to support these
> servers?
> 

Andrew,

It is not like we broke ABI here. The progam (synaptics driver) had a
grave bug. Older kernels happened to paper over the bug because they
did not fill the whole buffer that was advertised as available. Now
that we have more data to report the bug bit us. What do you want me
to do?

Synaptics driver is a small package and takes 2 minutes to recompile.
You don't have to update entire X server with it (in fact I don't think
it is even part of X distribution because it is GPL).

-- 
Dmitry

Re: linux-next: Tree for July 30

[Andrew Morton]

On Thu, 31 Jul 2008 14:17:31 -0400 Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:

> On Thu, Jul 31, 2008 at 10:44:37AM -0700, Andrew Morton wrote:
> > On Thu, 31 Jul 2008 11:56:48 -0400 Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > 
> > > On Thu, Jul 31, 2008 at 05:36:16PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > > > On Thu, Jul 31, 2008 at 4:07 PM, Dmitry Torokhov
> > > > <dmitry.torokhov@gmail.com> wrote:
> > > > > On Wed, Jul 30, 2008 at 11:10:29PM -0700, Andrew Morton wrote:
> > > > >> On Wed, 30 Jul 2008 17:06:35 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > > > >>
> > > > >> > I have created today's linux-next tree at
> > > > >> > git://git.kernel.org/pub/scm/linux/kernel/git/sfr/linux-next.git
> > > > >>
> > > > >> The X server broke on my FC8 t61p thinkpad.  Mainline is OK.
> > > > >>
> > > > >> Various information is at http://userweb.kernel.org/~akpm/mo/
> > > > >>
> > > > >> I'm suspecting the input layer - my synaptics device seems to have
> > > > >> disappeared?  See http://userweb.kernel.org/~akpm/mo/Xorg-log-diff.txt
> > > > >>
> > > > >
> > > > > I think this patch should help with Synaptics:
> > > > 
> > > > Which unfortunately doesn't help all people running with older synaptics
> > > > user-space after commit 0571c5d20aca71c735222132b02aebddf593045c
> > > > ("Input: expand keycode space").
> > > > 
> > > > Can't this be solved without breaking Xorg on newer kernels running
> > > > older synaptics?
> > > > 
> > > 
> > > No. The X driver is broken. It tells kernel to use buffer bugger than
> > > allocated and gets its stack smashed. Tslib has also soma funkiness
> > > in the ioctl handling as well... *shrug*
> > > 
> > > We have a couple months to get distros updated...
> > > 
> > 
> > aaarrrrgggggghhh.  I don't think this is practical.  This means that
> > (for example) FC5 machines (of which I happen to have one) are dead. 
> > And lots of other older-distro-based systems.
> > 
> > Is there some userspace workaround which doesn't require an X server
> > update?
> > 
> > Surely it must be possible to make the kernel contiue to support these
> > servers?
> > 
> 
> Andrew,
> 
> It is not like we broke ABI here. The progam (synaptics driver) had a
> grave bug. Older kernels happened to paper over the bug because they
> did not fill the whole buffer that was advertised as available. Now
> that we have more data to report the bug bit us. What do you want me
> to do?

Paper over the bug again.  When it happens, spit out a loud printk.

> Synaptics driver is a small package and takes 2 minutes to recompile.
> You don't have to update entire X server with it (in fact I don't think
> it is even part of X distribution because it is GPL).

What proportion of the X servers out there did we just break?

Was the crash I saw due to this?

Where would I (Aunt Tillie running FC5) go to find out how to fix my
machine up again? 

Re: linux-next: Tree for July 30

[Dmitry Torokhov]

On Thu, Jul 31, 2008 at 11:26:54AM -0700, Andrew Morton wrote:
> On Thu, 31 Jul 2008 14:17:31 -0400 Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> 
> > On Thu, Jul 31, 2008 at 10:44:37AM -0700, Andrew Morton wrote:
> > > On Thu, 31 Jul 2008 11:56:48 -0400 Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > > 
> > > > On Thu, Jul 31, 2008 at 05:36:16PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > > > > On Thu, Jul 31, 2008 at 4:07 PM, Dmitry Torokhov
> > > > > <dmitry.torokhov@gmail.com> wrote:
> > > > > > On Wed, Jul 30, 2008 at 11:10:29PM -0700, Andrew Morton wrote:
> > > > > >> On Wed, 30 Jul 2008 17:06:35 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > > > > >>
> > > > > >> > I have created today's linux-next tree at
> > > > > >> > git://git.kernel.org/pub/scm/linux/kernel/git/sfr/linux-next.git
> > > > > >>
> > > > > >> The X server broke on my FC8 t61p thinkpad.  Mainline is OK.
> > > > > >>
> > > > > >> Various information is at http://userweb.kernel.org/~akpm/mo/
> > > > > >>
> > > > > >> I'm suspecting the input layer - my synaptics device seems to have
> > > > > >> disappeared?  See http://userweb.kernel.org/~akpm/mo/Xorg-log-diff.txt
> > > > > >>
> > > > > >
> > > > > > I think this patch should help with Synaptics:
> > > > > 
> > > > > Which unfortunately doesn't help all people running with older synaptics
> > > > > user-space after commit 0571c5d20aca71c735222132b02aebddf593045c
> > > > > ("Input: expand keycode space").
> > > > > 
> > > > > Can't this be solved without breaking Xorg on newer kernels running
> > > > > older synaptics?
> > > > > 
> > > > 
> > > > No. The X driver is broken. It tells kernel to use buffer bugger than
> > > > allocated and gets its stack smashed. Tslib has also soma funkiness
> > > > in the ioctl handling as well... *shrug*
> > > > 
> > > > We have a couple months to get distros updated...
> > > > 
> > > 
> > > aaarrrrgggggghhh.  I don't think this is practical.  This means that
> > > (for example) FC5 machines (of which I happen to have one) are dead. 
> > > And lots of other older-distro-based systems.
> > > 
> > > Is there some userspace workaround which doesn't require an X server
> > > update?
> > > 
> > > Surely it must be possible to make the kernel contiue to support these
> > > servers?
> > > 
> > 
> > Andrew,
> > 
> > It is not like we broke ABI here. The progam (synaptics driver) had a
> > grave bug. Older kernels happened to paper over the bug because they
> > did not fill the whole buffer that was advertised as available. Now
> > that we have more data to report the bug bit us. What do you want me
> > to do?
> 
> Paper over the bug again.  When it happens, spit out a loud printk.

For that we we need to be sure that the size of the buffer passed to
us is incorrect. I.e. if we decide that 512 is a magic bad number and
decide to limit the output then legit programs supplying 512 byte
buffers they will not get the whole thing.

> 
> > Synaptics driver is a small package and takes 2 minutes to recompile.
> > You don't have to update entire X server with it (in fact I don't think
> > it is even part of X distribution because it is GPL).
> 
> What proportion of the X servers out there did we just break?
> 
> Was the crash I saw due to this?
> 
> Where would I (Aunt Tillie running FC5) go to find out how to fix my
> machine up again? 

What is Aunt Tillie doing compiling her own kernels on FC5? You
OTOH managed to get an answer fairly quickly ;)

-- 
Dmitry

Re: linux-next: Tree for July 30

[Rafael J. Wysocki]

On Thursday, 31 of July 2008, Dmitry Torokhov wrote:
> On Thu, Jul 31, 2008 at 10:44:37AM -0700, Andrew Morton wrote:
> > On Thu, 31 Jul 2008 11:56:48 -0400 Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > 
> > > On Thu, Jul 31, 2008 at 05:36:16PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > > > On Thu, Jul 31, 2008 at 4:07 PM, Dmitry Torokhov
> > > > <dmitry.torokhov@gmail.com> wrote:
> > > > > On Wed, Jul 30, 2008 at 11:10:29PM -0700, Andrew Morton wrote:
> > > > >> On Wed, 30 Jul 2008 17:06:35 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > > > >>
> > > > >> > I have created today's linux-next tree at
> > > > >> > git://git.kernel.org/pub/scm/linux/kernel/git/sfr/linux-next.git
> > > > >>
> > > > >> The X server broke on my FC8 t61p thinkpad.  Mainline is OK.
> > > > >>
> > > > >> Various information is at http://userweb.kernel.org/~akpm/mo/
> > > > >>
> > > > >> I'm suspecting the input layer - my synaptics device seems to have
> > > > >> disappeared?  See http://userweb.kernel.org/~akpm/mo/Xorg-log-diff.txt
> > > > >>
> > > > >
> > > > > I think this patch should help with Synaptics:
> > > > 
> > > > Which unfortunately doesn't help all people running with older synaptics
> > > > user-space after commit 0571c5d20aca71c735222132b02aebddf593045c
> > > > ("Input: expand keycode space").
> > > > 
> > > > Can't this be solved without breaking Xorg on newer kernels running
> > > > older synaptics?
> > > > 
> > > 
> > > No. The X driver is broken. It tells kernel to use buffer bugger than
> > > allocated and gets its stack smashed. Tslib has also soma funkiness
> > > in the ioctl handling as well... *shrug*
> > > 
> > > We have a couple months to get distros updated...
> > > 
> > 
> > aaarrrrgggggghhh.  I don't think this is practical.  This means that
> > (for example) FC5 machines (of which I happen to have one) are dead. 
> > And lots of other older-distro-based systems.
> > 
> > Is there some userspace workaround which doesn't require an X server
> > update?
> > 
> > Surely it must be possible to make the kernel contiue to support these
> > servers?
> > 
> 
> Andrew,
> 
> It is not like we broke ABI here. The progam (synaptics driver) had a
> grave bug. Older kernels happened to paper over the bug because they
> did not fill the whole buffer that was advertised as available. Now
> that we have more data to report the bug bit us. What do you want me
> to do?
> 
> Synaptics driver is a small package and takes 2 minutes to recompile.
> You don't have to update entire X server with it (in fact I don't think
> it is even part of X distribution because it is GPL).

Well, we're not supposed to break user space that we used to work with, even
if it is known to be buggy.  Many people use the older user space on their
test systems which are not practical to upgrade.

IOW, if the change responsible for this makes it to the mainline kernel, it
will be considered as a regression.

Thanks,
Rafael

Re: linux-next: Tree for July 30

[Dmitry Torokhov]

On Thu, Jul 31, 2008 at 08:48:56PM +0200, Rafael J. Wysocki wrote:
> On Thursday, 31 of July 2008, Dmitry Torokhov wrote:
> > On Thu, Jul 31, 2008 at 10:44:37AM -0700, Andrew Morton wrote:
> > > On Thu, 31 Jul 2008 11:56:48 -0400 Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > > 
> > > > On Thu, Jul 31, 2008 at 05:36:16PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > > > > On Thu, Jul 31, 2008 at 4:07 PM, Dmitry Torokhov
> > > > > <dmitry.torokhov@gmail.com> wrote:
> > > > > > On Wed, Jul 30, 2008 at 11:10:29PM -0700, Andrew Morton wrote:
> > > > > >> On Wed, 30 Jul 2008 17:06:35 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > > > > >>
> > > > > >> > I have created today's linux-next tree at
> > > > > >> > git://git.kernel.org/pub/scm/linux/kernel/git/sfr/linux-next.git
> > > > > >>
> > > > > >> The X server broke on my FC8 t61p thinkpad.  Mainline is OK.
> > > > > >>
> > > > > >> Various information is at http://userweb.kernel.org/~akpm/mo/
> > > > > >>
> > > > > >> I'm suspecting the input layer - my synaptics device seems to have
> > > > > >> disappeared?  See http://userweb.kernel.org/~akpm/mo/Xorg-log-diff.txt
> > > > > >>
> > > > > >
> > > > > > I think this patch should help with Synaptics:
> > > > > 
> > > > > Which unfortunately doesn't help all people running with older synaptics
> > > > > user-space after commit 0571c5d20aca71c735222132b02aebddf593045c
> > > > > ("Input: expand keycode space").
> > > > > 
> > > > > Can't this be solved without breaking Xorg on newer kernels running
> > > > > older synaptics?
> > > > > 
> > > > 
> > > > No. The X driver is broken. It tells kernel to use buffer bugger than
> > > > allocated and gets its stack smashed. Tslib has also soma funkiness
> > > > in the ioctl handling as well... *shrug*
> > > > 
> > > > We have a couple months to get distros updated...
> > > > 
> > > 
> > > aaarrrrgggggghhh.  I don't think this is practical.  This means that
> > > (for example) FC5 machines (of which I happen to have one) are dead. 
> > > And lots of other older-distro-based systems.
> > > 
> > > Is there some userspace workaround which doesn't require an X server
> > > update?
> > > 
> > > Surely it must be possible to make the kernel contiue to support these
> > > servers?
> > > 
> > 
> > Andrew,
> > 
> > It is not like we broke ABI here. The progam (synaptics driver) had a
> > grave bug. Older kernels happened to paper over the bug because they
> > did not fill the whole buffer that was advertised as available. Now
> > that we have more data to report the bug bit us. What do you want me
> > to do?
> > 
> > Synaptics driver is a small package and takes 2 minutes to recompile.
> > You don't have to update entire X server with it (in fact I don't think
> > it is even part of X distribution because it is GPL).
> 
> Well, we're not supposed to break user space that we used to work with, even
> if it is known to be buggy.

No, I am sorry. We are not supposed to break userspace ABI, but that
is it. Can you vouch that 2.6.25 did not break a single userspace
program out there?

>  Many people use the older user space on their
> test systems which are not practical to upgrade.
> 

I don't understand this - it is expected that everyone jumps and
upgrades their kernels with ease but updating broken userspace
bits is super-hard... Plus, in this case the fixed driver will
happily work with old kernels.

> IOW, if the change responsible for this makes it to the mainline kernel, it
> will be considered as a regression.
> 

Like I said, I don't agree.

-- 
Dmitry

Re: linux-next: Tree for July 30

[Andrew Morton]

On Thu, 31 Jul 2008 14:34:41 -0400
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:

> > > > > No. The X driver is broken. It tells kernel to use buffer bugger than
> > > > > allocated and gets its stack smashed. Tslib has also soma funkiness
> > > > > in the ioctl handling as well... *shrug*
> > > > > 
> > > > > We have a couple months to get distros updated...
> > > > > 
> > > > 
> > > > aaarrrrgggggghhh.  I don't think this is practical.  This means that
> > > > (for example) FC5 machines (of which I happen to have one) are dead. 
> > > > And lots of other older-distro-based systems.
> > > > 
> > > > Is there some userspace workaround which doesn't require an X server
> > > > update?
> > > > 
> > > > Surely it must be possible to make the kernel contiue to support these
> > > > servers?
> > > > 
> > > 
> > > Andrew,
> > > 
> > > It is not like we broke ABI here. The progam (synaptics driver) had a
> > > grave bug. Older kernels happened to paper over the bug because they
> > > did not fill the whole buffer that was advertised as available. Now
> > > that we have more data to report the bug bit us. What do you want me
> > > to do?
> > 
> > Paper over the bug again.  When it happens, spit out a loud printk.
> 
> For that we we need to be sure that the size of the buffer passed to
> us is incorrect. I.e. if we decide that 512 is a magic bad number and
> decide to limit the output then legit programs supplying 512 byte
> buffers they will not get the whole thing.

uh.  I'm still scrabbling to understand all this.  Where is the
information about this kept?  Which commit caused this problem to
occur?

It _sounds_ like userspace is passing in a buffer and is also passing
in an incorrect (too large) `size' parameter, yes?

This is an ioctl interface?  Could we leave the old ioctl unchanged and
introduce the offending changes into a new ioctl number?

> > 
> > > Synaptics driver is a small package and takes 2 minutes to recompile.
> > > You don't have to update entire X server with it (in fact I don't think
> > > it is even part of X distribution because it is GPL).
> > 
> > What proportion of the X servers out there did we just break?
> > 
> > Was the crash I saw due to this?
> > 
> > Where would I (Aunt Tillie running FC5) go to find out how to fix my
> > machine up again? 
> 
> What is Aunt Tillie doing compiling her own kernels on FC5? You
> OTOH managed to get an answer fairly quickly ;)

I'll ask again: where do our users go to find out how to make their X
server work again?  If the answer is "nowhere" then can we please at 
least write up a simple step-by-step repair procedure, as we'll surely
be needing it a lot.

Re: linux-next: Tree for July 30

[Dmitry Torokhov]

On Thu, Jul 31, 2008 at 11:55:38AM -0700, Andrew Morton wrote:
> On Thu, 31 Jul 2008 14:34:41 -0400
> Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> 
> > > > > > No. The X driver is broken. It tells kernel to use buffer bugger than
> > > > > > allocated and gets its stack smashed. Tslib has also soma funkiness
> > > > > > in the ioctl handling as well... *shrug*
> > > > > > 
> > > > > > We have a couple months to get distros updated...
> > > > > > 
> > > > > 
> > > > > aaarrrrgggggghhh.  I don't think this is practical.  This means that
> > > > > (for example) FC5 machines (of which I happen to have one) are dead. 
> > > > > And lots of other older-distro-based systems.
> > > > > 
> > > > > Is there some userspace workaround which doesn't require an X server
> > > > > update?
> > > > > 
> > > > > Surely it must be possible to make the kernel contiue to support these
> > > > > servers?
> > > > > 
> > > > 
> > > > Andrew,
> > > > 
> > > > It is not like we broke ABI here. The progam (synaptics driver) had a
> > > > grave bug. Older kernels happened to paper over the bug because they
> > > > did not fill the whole buffer that was advertised as available. Now
> > > > that we have more data to report the bug bit us. What do you want me
> > > > to do?
> > > 
> > > Paper over the bug again.  When it happens, spit out a loud printk.
> > 
> > For that we we need to be sure that the size of the buffer passed to
> > us is incorrect. I.e. if we decide that 512 is a magic bad number and
> > decide to limit the output then legit programs supplying 512 byte
> > buffers they will not get the whole thing.
> 
> uh.  I'm still scrabbling to understand all this.  Where is the
> information about this kept?  Which commit caused this problem to
> occur?
> 

It is commit "Input: expand keycode space" (git log
include/linux/input.h)

> It _sounds_ like userspace is passing in a buffer and is also passing
> in an incorrect (too large) `size' parameter, yes?
> 

Yes.

> This is an ioctl interface?

Yes.

>  Could we leave the old ioctl unchanged and
> introduce the offending changes into a new ioctl number?
> 

Then you will punish other users of this ioctl (the ones that
use it correctly). X proper, HAL, DirectFB, etc, etc. They will have
to implement the new interface just because of other programs having
an issue.

> > > 
> > > > Synaptics driver is a small package and takes 2 minutes to recompile.
> > > > You don't have to update entire X server with it (in fact I don't think
> > > > it is even part of X distribution because it is GPL).
> > > 
> > > What proportion of the X servers out there did we just break?
> > > 
> > > Was the crash I saw due to this?
> > > 
> > > Where would I (Aunt Tillie running FC5) go to find out how to fix my
> > > machine up again? 
> > 
> > What is Aunt Tillie doing compiling her own kernels on FC5? You
> > OTOH managed to get an answer fairly quickly ;)
> 
> I'll ask again: where do our users go to find out how to make their X
> server work again?  If the answer is "nowhere" then can we please at 
> least write up a simple step-by-step repair procedure, as we'll surely
> be needing it a lot.
> 

Changes? We could also get Peter to add some verbage onto his web-page
that hosts the driver. We are looking at .28 release for this code so
we have some time.

-- 
Dmitry

Re: linux-next: Tree for July 30

[Linus Torvalds]

On Thu, 31 Jul 2008, Dmitry Torokhov wrote:
> > 
> > Well, we're not supposed to break user space that we used to work with, even
> > if it is known to be buggy.
> 
> No, I am sorry. We are not supposed to break userspace ABI, but that
> is it. Can you vouch that 2.6.25 did not break a single userspace
> program out there?

Dmitry - irrelevant. If we know of breakage, then that is a FACT, and it's 
a regression, and it needs to be fixed.

Trying to say "there might be _other_ breakage that we don't even know of" 
does not change the situation ONE LITTLE BIT!

Don't you see how stupid that approach is? You're basically trying to make 
excuses for known breakage by saying that there might be _other_ breakage 
that we don't know about? Why the _hell_ do you think that is an excuse at 
all?

> >  Many people use the older user space on their
> > test systems which are not practical to upgrade.
> 
> I don't understand this - it is expected that everyone jumps and
> upgrades their kernels with ease but updating broken userspace
> bits is super-hard...

You're missing the point.

People are supposed to be able to upgrade things _independently_. It's not 
about "you're supposed to be able to upgrade the kernel, but not upgrade 
user space". It's about "you shouldn't evemn have to _worry_ about it.

> > IOW, if the change responsible for this makes it to the mainline kernel, it
> > will be considered as a regression.
> 
> Like I said, I don't agree.

Sorry, but you're simply wrong.

If somebody has the commit that broke user space, that commit will be 
_reverted_ unless it's fixed. It's that simple. The rules are: we don't 
knowingly break user space. 

			Linus

Re: linux-next: Tree for July 30

[Andrew Morton]

On Thu, 31 Jul 2008 14:54:52 -0400
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:

> > IOW, if the change responsible for this makes it to the mainline kernel, it
> > will be considered as a regression.
> > 
> 
> Like I said, I don't agree.

It really really doesn't matter what the causes are or which piece of
code is at fault or anything else like that.

What _does_ matter is that people's stuff will break.  Apparently lots
of people's.  That's a problem.  A _practical_ problem.  Can we
pleeeeeeze be practical and find some way of preventing it?

I assume this is the commit?


commit 03bac96fae0efdb25e2059e5accbe4f3ee6328dd
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date:   Mon Jun 23 10:47:34 2008 -0400

    Input: expand keycode space
    
    Expand the number of potential key codes from 512 to 768 since people
    are coming up with more and more keys.
    
    Signed-off-by: Dmitry Torokhov <dtor@mail.ru>

diff --git a/include/linux/input.h b/include/linux/input.h
index a5802c9..7fae1de 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -579,7 +579,7 @@ struct input_absinfo {
 
 /* We avoid low common keys in module aliases so they don't get huge. */
 #define KEY_MIN_INTERESTING	KEY_MUTE
-#define KEY_MAX			0x1ff
+#define KEY_MAX			0x2ff
 #define KEY_CNT			(KEY_MAX+1)
 
 /*
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index c4db582..0dddfa4 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -274,7 +274,7 @@ struct pcmcia_device_id {
 /* Input */
 #define INPUT_DEVICE_ID_EV_MAX		0x1f
 #define INPUT_DEVICE_ID_KEY_MIN_INTERESTING	0x71
-#define INPUT_DEVICE_ID_KEY_MAX		0x1ff
+#define INPUT_DEVICE_ID_KEY_MAX		0x2ff
 #define INPUT_DEVICE_ID_REL_MAX		0x0f
 #define INPUT_DEVICE_ID_ABS_MAX		0x3f
 #define INPUT_DEVICE_ID_MSC_MAX		0x07


I'm unable to work out how necessary this change is?


Please: what proportion of the existing X installations do you expect will
be affected by this?

Re: linux-next: Tree for July 30

[Hugh Dickins]

On Thu, 31 Jul 2008, Andrew Morton wrote:
> On Thu, 31 Jul 2008 14:34:41 -0400
> Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > > 
> > > Where would I (Aunt Tillie running FC5) go to find out how to fix my
> > > machine up again? 
> > 
> > What is Aunt Tillie doing compiling her own kernels on FC5? You
> > OTOH managed to get an answer fairly quickly ;)
> 
> I'll ask again: where do our users go to find out how to make their X
> server work again?  If the answer is "nowhere" then can we please at 
> least write up a simple step-by-step repair procedure, as we'll surely
> be needing it a lot.

Thanks for pursuing this, Andrew: here's a kernel patch deduced
from this thread, which has indeed got me moving again.  Perhaps
an optional patch for hotfixes while the right answer is decided.


If you want to test 2.6.27-rc1-mm1, but your Synaptics pad is making X
crash immediately, and like Aunt Tillie and me you're more comfortable
patching your kernel than messing around in your userspace,
then reverting back from 768 to 512 keys should help.

Signed-off-by: Hugh Dickins <hugh@veritas.com>
---

 include/linux/input.h           |    2 +-
 include/linux/mod_devicetable.h |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

--- 2.6.27-rc1-mm1/include/linux/input.h	2008-07-31 09:30:09.000000000 +0100
+++ linux/include/linux/input.h	2008-07-31 19:39:20.000000000 +0100
@@ -592,7 +592,7 @@ struct input_absinfo {
 
 /* We avoid low common keys in module aliases so they don't get huge. */
 #define KEY_MIN_INTERESTING	KEY_MUTE
-#define KEY_MAX			0x2ff
+#define KEY_MAX			0x1ff
 #define KEY_CNT			(KEY_MAX+1)
 
 /*
--- 2.6.27-rc1-mm1/include/linux/mod_devicetable.h	2008-07-31 09:30:09.000000000 +0100
+++ linux/include/linux/mod_devicetable.h	2008-07-31 19:43:19.000000000 +0100
@@ -274,7 +274,7 @@ struct pcmcia_device_id {
 /* Input */
 #define INPUT_DEVICE_ID_EV_MAX		0x1f
 #define INPUT_DEVICE_ID_KEY_MIN_INTERESTING	0x71
-#define INPUT_DEVICE_ID_KEY_MAX		0x2ff
+#define INPUT_DEVICE_ID_KEY_MAX		0x1ff
 #define INPUT_DEVICE_ID_REL_MAX		0x0f
 #define INPUT_DEVICE_ID_ABS_MAX		0x3f
 #define INPUT_DEVICE_ID_MSC_MAX		0x07

Re: linux-next: Tree for July 30

[Dmitry Torokhov]

On Thu, Jul 31, 2008 at 12:10:40PM -0700, Linus Torvalds wrote:
> 
> 
> On Thu, 31 Jul 2008, Dmitry Torokhov wrote:
> > > 
> > > Well, we're not supposed to break user space that we used to work with, even
> > > if it is known to be buggy.
> > 
> > No, I am sorry. We are not supposed to break userspace ABI, but that
> > is it. Can you vouch that 2.6.25 did not break a single userspace
> > program out there?
> 
> Dmitry - irrelevant. If we know of breakage, then that is a FACT, and it's 
> a regression, and it needs to be fixed.

Does it have to be papered over in the kernel though?

> 
> Trying to say "there might be _other_ breakage that we don't even know of" 
> does not change the situation ONE LITTLE BIT!
> 
> Don't you see how stupid that approach is? You're basically trying to make 
> excuses for known breakage by saying that there might be _other_ breakage 
> that we don't know about? Why the _hell_ do you think that is an excuse at 
> all?

We can only guarantee one thing - ABI. And that is kept intact. But I
literally have no idea if a kernel breaks a random program out there
that happens to have a bug.

> 
> > >  Many people use the older user space on their
> > > test systems which are not practical to upgrade.
> > 
> > I don't understand this - it is expected that everyone jumps and
> > upgrades their kernels with ease but updating broken userspace
> > bits is super-hard...
> 
> You're missing the point.
> 
> People are supposed to be able to upgrade things _independently_. It's not 
> about "you're supposed to be able to upgrade the kernel, but not upgrade 
> user space". It's about "you shouldn't evemn have to _worry_ about it.
> 
> > > IOW, if the change responsible for this makes it to the mainline kernel, it
> > > will be considered as a regression.
> > 
> > Like I said, I don't agree.
> 
> Sorry, but you're simply wrong.
> 
> If somebody has the commit that broke user space, that commit will be 
> _reverted_ unless it's fixed. It's that simple. The rules are: we don't 
> knowingly break user space. 
>

We have 3 options now:

1. Never change KEY_MAX and dont add any new key definitions.
2. Introduce a new ioctl and have all wel-behaving programs rewritten
   to support it.
3. Fix userspace driver (patch is available).

Gioventhe fact that I wanted that change to go when .28 opens and it
will really hit users in 6+ months I'd still like to have 3.
 
-- 
Dmitry

Re: linux-next: Tree for July 30

[Dmitry Torokhov]

On Thu, Jul 31, 2008 at 03:24:02PM -0400, Dmitry Torokhov wrote:
> On Thu, Jul 31, 2008 at 12:10:40PM -0700, Linus Torvalds wrote:
> > 
> > Sorry, but you're simply wrong.
> > 
> > If somebody has the commit that broke user space, that commit will be 
> > _reverted_ unless it's fixed. It's that simple. The rules are: we don't 
> > knowingly break user space. 
> >
> 
> We have 3 options now:
> 
> 1. Never change KEY_MAX and dont add any new key definitions.
> 2. Introduce a new ioctl and have all wel-behaving programs rewritten
>    to support it.
> 3. Fix userspace driver (patch is available).
> 
> Gioventhe fact that I wanted that change to go when .28 opens and it
> will really hit users in 6+ months I'd still like to have 3.
>  

I guess we could do something like the below, but it is soooo ugly...

Input: paper over a bug in Synaptics X driver

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/input/evdev.c |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index 2d65411..c78a63b 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -736,8 +736,10 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
 			if ((_IOC_NR(cmd) & ~EV_MAX) == _IOC_NR(EVIOCGBIT(0, 0))) {
 
 				unsigned long *bits;
+				unsigned int buf_len = _IOC_SIZE(cmd);
 				int len;
 
+
 				switch (_IOC_NR(cmd) & EV_MAX) {
 
 				case      0: bits = dev->evbit;  len = EV_MAX;  break;
@@ -751,7 +753,17 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
 				case EV_SW:  bits = dev->swbit;  len = SW_MAX;  break;
 				default: return -EINVAL;
 				}
-				return bits_to_user(bits, len, _IOC_SIZE(cmd), p, compat_mode);
+
+				if ((_IOC_NR(cmd) & EV_MAX) == EV_KEY && buf_len == 0x1ff) {
+					printk(KERN_WARNING
+						"evdev.c(EVIOCGBIT): Detected suspicious "
+						"buffer size 0x1ff, limiting output to 64 "
+						"bytes. Make sure you are not using "
+						"EVIOCGBIT(EV_KEY, KEY_MAX)\n");
+					buf_len = 64; 
+				}
+
+				return bits_to_user(bits, len, buf_len, p, compat_mode);
 			}
 
 			if (_IOC_NR(cmd) == _IOC_NR(EVIOCGKEY(0)))

Re: linux-next: Tree for July 30

[Linus Torvalds]

On Thu, 31 Jul 2008, Dmitry Torokhov wrote:
> 
> Does it have to be papered over in the kernel though?

Yes. It's how we have worked. Asking people to upgrade big core programs 
is not reasonable.

Think of it this way: we absolutely _want_ people to run the latest 
kernel. We want it for their own sake (security etc fixes), but we want it 
even more for *our* own sake (testing, fixes etc).

And if we want to encourage people to upgrade their kernel very 
aggressively (and we absolutely do!), then that means that we have to also 
make sure it doesn't require them upgrading anything else.

> We can only guarantee one thing - ABI. And that is kept intact. But I
> literally have no idea if a kernel breaks a random program out there
> that happens to have a bug.

There are gray areas, yes. For example, timing changes do mean that a new 
kenrel can easily break a program that used to work. We cannot handle 
_everyting_. 

But when the ABI in question is some very specific one, that some 
important program uses (even if the "uses" is "misuses") then it really 
isn't a gray area any more.

And quite frankly, the ABI was apparently pretty bad to begin with, if 
user space got an array back but didn't get to specify the size. So you 
may want to say that user space was broken, but on the other hand, it's 
equally arguable that the ABI was crap.

(Which is something you can pretty much take for granted with ioctl's, of 
course. DO NOT CHANGE IOCTL'S. EVER!)

> We have 3 options now:
> 
> 1. Never change KEY_MAX and dont add any new key definitions.
> 2. Introduce a new ioctl and have all wel-behaving programs rewritten
>    to support it.
> 3. Fix userspace driver (patch is available).

You ignore the obvious choice, which is how we _usually_ do it:

 - help fix up the userspace driver regardless

 - a year down the line, maybe breakage will be a non-issue.

 - but at least _think_ about the fact that yes, most ioctl interfaces are 
   pure and utter sh*t, and the problem was probably not so much the user 
   space driver as the crap interface to begin with!

and discuss whether KEY_MAX really needs to be changed that much. I 
suspect that the change was done without even realizing just how painful 
it was, and that if you look at the original reason for it with the 
hindsight of knowing that it was painful, maybe it wasn't that critical to 
do it after all?

		Linus

Re: linux-next: Tree for July 30

[Rafael J. Wysocki]

On Thursday, 31 of July 2008, Dmitry Torokhov wrote:
> On Thu, Jul 31, 2008 at 08:48:56PM +0200, Rafael J. Wysocki wrote:
> > On Thursday, 31 of July 2008, Dmitry Torokhov wrote:
> > > On Thu, Jul 31, 2008 at 10:44:37AM -0700, Andrew Morton wrote:
> > > > On Thu, 31 Jul 2008 11:56:48 -0400 Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > > > 
> > > > > On Thu, Jul 31, 2008 at 05:36:16PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > > > > > On Thu, Jul 31, 2008 at 4:07 PM, Dmitry Torokhov
> > > > > > <dmitry.torokhov@gmail.com> wrote:
> > > > > > > On Wed, Jul 30, 2008 at 11:10:29PM -0700, Andrew Morton wrote:
> > > > > > >> On Wed, 30 Jul 2008 17:06:35 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > > > > > >>
> > > > > > >> > I have created today's linux-next tree at
> > > > > > >> > git://git.kernel.org/pub/scm/linux/kernel/git/sfr/linux-next.git
> > > > > > >>
> > > > > > >> The X server broke on my FC8 t61p thinkpad.  Mainline is OK.
> > > > > > >>
> > > > > > >> Various information is at http://userweb.kernel.org/~akpm/mo/
> > > > > > >>
> > > > > > >> I'm suspecting the input layer - my synaptics device seems to have
> > > > > > >> disappeared?  See http://userweb.kernel.org/~akpm/mo/Xorg-log-diff.txt
> > > > > > >>
> > > > > > >
> > > > > > > I think this patch should help with Synaptics:
> > > > > > 
> > > > > > Which unfortunately doesn't help all people running with older synaptics
> > > > > > user-space after commit 0571c5d20aca71c735222132b02aebddf593045c
> > > > > > ("Input: expand keycode space").
> > > > > > 
> > > > > > Can't this be solved without breaking Xorg on newer kernels running
> > > > > > older synaptics?
> > > > > > 
> > > > > 
> > > > > No. The X driver is broken. It tells kernel to use buffer bugger than
> > > > > allocated and gets its stack smashed. Tslib has also soma funkiness
> > > > > in the ioctl handling as well... *shrug*
> > > > > 
> > > > > We have a couple months to get distros updated...
> > > > > 
> > > > 
> > > > aaarrrrgggggghhh.  I don't think this is practical.  This means that
> > > > (for example) FC5 machines (of which I happen to have one) are dead. 
> > > > And lots of other older-distro-based systems.
> > > > 
> > > > Is there some userspace workaround which doesn't require an X server
> > > > update?
> > > > 
> > > > Surely it must be possible to make the kernel contiue to support these
> > > > servers?
> > > > 
> > > 
> > > Andrew,
> > > 
> > > It is not like we broke ABI here. The progam (synaptics driver) had a
> > > grave bug. Older kernels happened to paper over the bug because they
> > > did not fill the whole buffer that was advertised as available. Now
> > > that we have more data to report the bug bit us. What do you want me
> > > to do?
> > > 
> > > Synaptics driver is a small package and takes 2 minutes to recompile.
> > > You don't have to update entire X server with it (in fact I don't think
> > > it is even part of X distribution because it is GPL).
> > 
> > Well, we're not supposed to break user space that we used to work with, even
> > if it is known to be buggy.
> 
> No, I am sorry. We are not supposed to break userspace ABI, but that
> is it. Can you vouch that 2.6.25 did not break a single userspace
> program out there?

It probably did break some, but that doesn't imply it's acceptable.

> >  Many people use the older user space on their
> > test systems which are not practical to upgrade.
> > 
> 
> I don't understand this - it is expected that everyone jumps and
> upgrades their kernels with ease but updating broken userspace
> bits is super-hard... Plus, in this case the fixed driver will
> happily work with old kernels.

Please look at it from a different angle.

Kernel testers usually don't expect the kernel to break their X servers,
whatever the reason.  So, if they are not warned to expect the breakage, they
will search for the reason of it, as Andrew has just done, losing their time.
To prevent such losses of time from happening, it's better not to break user
space.

Still, if you really think there's no other way to go (like deferring the change
in question until everyone can be safely assumed to have upgraded their user
space already), please do something to keep people informed.

Thanks,
Rafael

Re: linux-next: Tree for July 30

[Dmitry Torokhov]

On Thu, Jul 31, 2008 at 12:44:04PM -0700, Linus Torvalds wrote:
> 
> 
> On Thu, 31 Jul 2008, Dmitry Torokhov wrote:
> > 
> > Does it have to be papered over in the kernel though?
> 
> Yes. It's how we have worked. Asking people to upgrade big core programs 
> is not reasonable.
>
> Think of it this way: we absolutely _want_ people to run the latest 
> kernel. We want it for their own sake (security etc fixes), but we want it 
> even more for *our* own sake (testing, fixes etc).
> 
> And if we want to encourage people to upgrade their kernel very 
> aggressively (and we absolutely do!), then that means that we have to also 
> make sure it doesn't require them upgrading anything else.

Sometimes we do need to upgrade userspace though. Can we make
Documentation/Changes more prominent? Maybe have it published on
kernel.org?

> 
> > We can only guarantee one thing - ABI. And that is kept intact. But I
> > literally have no idea if a kernel breaks a random program out there
> > that happens to have a bug.
> 
> There are gray areas, yes. For example, timing changes do mean that a new 
> kenrel can easily break a program that used to work. We cannot handle 
> _everyting_. 
> 
> But when the ABI in question is some very specific one, that some 
> important program uses (even if the "uses" is "misuses") then it really 
> isn't a gray area any more.
> 
> And quite frankly, the ABI was apparently pretty bad to begin with, if 
> user space got an array back but didn't get to specify the size. So you 
> may want to say that user space was broken, but on the other hand, it's 
> equally arguable that the ABI was crap.

It did specify the size. Something 448 more bytes than it allocated:

    unsigned long evbits[NBITS(KEY_MAX)];

    /* Check for ABS_X, ABS_Y, ABS_PRESSURE and BTN_TOOL_FINGER */

    SYSCALL(ret = ioctl(fd, EVIOCGBIT(0, KEY_MAX), evbits));

So we allocate 64 bytes on stack and then as kernel to fill it with
511 bytes worth of data.

> 
> (Which is something you can pretty much take for granted with ioctl's, of 
> course. DO NOT CHANGE IOCTL'S. EVER!)
> 
> > We have 3 options now:
> > 
> > 1. Never change KEY_MAX and dont add any new key definitions.
> > 2. Introduce a new ioctl and have all wel-behaving programs rewritten
> >    to support it.
> > 3. Fix userspace driver (patch is available).
> 
> You ignore the obvious choice, which is how we _usually_ do it:
> 
>  - help fix up the userspace driver regardless

In progress.
> 
>  - a year down the line, maybe breakage will be a non-issue.
>

Around when 2.6.28 is released, right? ;)
 
>  - but at least _think_ about the fact that yes, most ioctl interfaces are 
>    pure and utter sh*t, and the problem was probably not so much the user 
>    space driver as the crap interface to begin with!
> 
> and discuss whether KEY_MAX really needs to be changed that much. I 
> suspect that the change was done without even realizing just how painful 
> it was, and that if you look at the original reason for it with the 
> hindsight of knowing that it was painful, maybe it wasn't that critical to 
> do it after all?
>

We do need more keycodes. People are coming wioth more and more. The
patch following the one in question adds about 10 new kodes for remote
controls/phones. And we will get more.

-- 
Dmitry

Re: linux-next: Tree for July 30

[Andrew Morton]

On Thu, 31 Jul 2008 15:42:07 -0400
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:

> -				return bits_to_user(bits, len, _IOC_SIZE(cmd), p, compat_mode);
> +
> +				if ((_IOC_NR(cmd) & EV_MAX) == EV_KEY && buf_len == 0x1ff) {
> +					printk(KERN_WARNING
> +						"evdev.c(EVIOCGBIT): Detected suspicious "
> +						"buffer size 0x1ff, limiting output to 64 "
> +						"bytes. Make sure you are not using "
> +						"EVIOCGBIT(EV_KEY, KEY_MAX)\n");
> +					buf_len = 64; 
> +				}

If that works then great.  But I think the printk could be improved. 
Please provide sufficient information so that users (not programmers)
can go off and fix things up without needing to email kernel developers.

One suitable approach would be

	printk("see http://userweb.kernel.org/~dtor/read-this.txt")

Re: linux-next: Tree for July 30

[Linus Torvalds]

On Thu, 31 Jul 2008, Dmitry Torokhov wrote:
> 
> Sometimes we do need to upgrade userspace though. Can we make
> Documentation/Changes more prominent? Maybe have it published on
> kernel.org?

We basically _never_ have to upgrade userspace that aggressively. We can 
have a Changes file that talks about things that will eventually break 
when we remove support for it eventually, but it should never EVER be used 
as an excuse for "I needed to break it now".

So no, I refuse to make it any more prominent. Because it would just be 
used as an excuse for behavior that I consider unacceptable. It was 
different back when we had 3-year development windows and people upgrading 
from 2.4.x to 2.6.x had to learn new things, but for 2.6.26 to .27 or 
similar it's simply not acceptable.

Look at the VFS layer. Look at how we have multiple different versions of 
"readdir()" (well, getdents, really), and "stat()". Exactly because we 
don't break user space.

> It did specify the size. Something 448 more bytes than it allocated:
> 
>     unsigned long evbits[NBITS(KEY_MAX)];
> 
>     /* Check for ABS_X, ABS_Y, ABS_PRESSURE and BTN_TOOL_FINGER */
> 
>     SYSCALL(ret = ioctl(fd, EVIOCGBIT(0, KEY_MAX), evbits));
> 
> So we allocate 64 bytes on stack and then as kernel to fill it with
> 511 bytes worth of data.

Ok, I can see how it's confused, asking for KEY_MAX _bits_. If this is the 
main user, why not just change the definition to be in bits?

> >  - help fix up the userspace driver regardless
> 
> In progress.
> > 
> >  - a year down the line, maybe breakage will be a non-issue.
> 
> Around when 2.6.28 is released, right? ;)

A year down the line would be 2.6.30 or so.

> We do need more keycodes. People are coming wioth more and more. The
> patch following the one in question adds about 10 new kodes for remote
> controls/phones. And we will get more.

Maybe the problem is a bad design that encourages people to just create 
new keycodes when they really shouldn't? 

			Linus

Re: linux-next: Tree for July 30

[Linus Torvalds]

On Thu, 31 Jul 2008, Linus Torvalds wrote:
> >     /* Check for ABS_X, ABS_Y, ABS_PRESSURE and BTN_TOOL_FINGER */
> > 
> >     SYSCALL(ret = ioctl(fd, EVIOCGBIT(0, KEY_MAX), evbits));
> > 
> > So we allocate 64 bytes on stack and then as kernel to fill it with
> > 511 bytes worth of data.
> 
> Ok, I can see how it's confused, asking for KEY_MAX _bits_. If this is the 
> main user, why not just change the definition to be in bits?

Or just say that "if the buffer is really much too big, maybe they meant 
bits"?

IOW, something like this?

(And no, I'm not seriously proposing _this_ patch, but you get the idea)

		Linus
---
 drivers/input/evdev.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index 2d65411..e45451d 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -734,7 +734,7 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
 		if (_IOC_DIR(cmd) == _IOC_READ) {
 
 			if ((_IOC_NR(cmd) & ~EV_MAX) == _IOC_NR(EVIOCGBIT(0, 0))) {
-
+				unsigned int size = _IOC_SIZE(cmd);
 				unsigned long *bits;
 				int len;
 
@@ -751,7 +751,12 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
 				case EV_SW:  bits = dev->swbit;  len = SW_MAX;  break;
 				default: return -EINVAL;
 				}
-				return bits_to_user(bits, len, _IOC_SIZE(cmd), p, compat_mode);
+
+				/* Some people get confused about size in bits vs bytes */
+				if (size >= len/8)
+					size = size/8;
+
+				return bits_to_user(bits, len, size, p, compat_mode);
 			}
 
 			if (_IOC_NR(cmd) == _IOC_NR(EVIOCGKEY(0)))

Re: linux-next: Tree for July 30

[Dmitry Torokhov]

On Thu, Jul 31, 2008 at 01:16:52PM -0700, Linus Torvalds wrote:
> 
> 
> On Thu, 31 Jul 2008, Dmitry Torokhov wrote:
> > 
> > Sometimes we do need to upgrade userspace though. Can we make
> > Documentation/Changes more prominent? Maybe have it published on
> > kernel.org?
> 
> We basically _never_ have to upgrade userspace that aggressively. We can 
> have a Changes file that talks about things that will eventually break 
> when we remove support for it eventually, but it should never EVER be used 
> as an excuse for "I needed to break it now".
> 
> So no, I refuse to make it any more prominent. 

OK.

> Because it would just be 
> used as an excuse for behavior that I consider unacceptable. It was 
> different back when we had 3-year development windows and people upgrading 
> from 2.4.x to 2.6.x had to learn new things, but for 2.6.26 to .27 or 
> similar it's simply not acceptable.
> 
> Look at the VFS layer. Look at how we have multiple different versions of 
> "readdir()" (well, getdents, really), and "stat()". Exactly because we 
> don't break user space.

Here we don't extend the interface though.
> 
> > It did specify the size. Something 448 more bytes than it allocated:
> > 
> >     unsigned long evbits[NBITS(KEY_MAX)];
> > 
> >     /* Check for ABS_X, ABS_Y, ABS_PRESSURE and BTN_TOOL_FINGER */
> > 
> >     SYSCALL(ret = ioctl(fd, EVIOCGBIT(0, KEY_MAX), evbits));
> > 
> > So we allocate 64 bytes on stack and then as kernel to fill it with
> > 511 bytes worth of data.
> 
> Ok, I can see how it's confused, asking for KEY_MAX _bits_. If this is the 
> main user, why not just change the definition to be in bits?
> 

Because X proper, HAL, DirectFB and many other users got it right and
changing it to mean bits would break _them_.

> > >  - help fix up the userspace driver regardless
> > 
> > In progress.
> > > 
> > >  - a year down the line, maybe breakage will be a non-issue.
> > 
> > Around when 2.6.28 is released, right? ;)
> 
> A year down the line would be 2.6.30 or so.
> 

I guess that means that we have to have that patch that spits warning
and reduces size of returned data of it detects 01ff buffer size.
Still, its uuuuugllllyyy.

> > We do need more keycodes. People are coming wioth more and more. The
> > patch following the one in question adds about 10 new kodes for remote
> > controls/phones. And we will get more.
> 
> Maybe the problem is a bad design that encourages people to just create 
> new keycodes when they really shouldn't? 

That is bigger topic. HID spec has much more events for differect
things though. FOr example the new key definitions for the phones - we
want to have a separate # key and not try to combine "shift" and "3"
and also have separate numeric keys taht don't depend on register and
NumLock state. If we don't have such keycodes we have trouble with
some european users that have numbers in upper register...

-- 
Dmitry

Re: linux-next: Tree for July 30

[Dmitry Torokhov]

On Thu, Jul 31, 2008 at 01:28:37PM -0700, Linus Torvalds wrote:
> 
> +				/* Some people get confused about size in bits vs bytes */
> +				if (size >= len/8)
> +					size = size/8;

People usually allocate single buffer (big enough) and the do a bunch
of tests so this condiion is likely to fire on EV_SW and similar event
types. I think it is safer to explicitely test for EV_KEY/0x1ff.

-- 
Dmitry

Re: linux-next: Tree for July 30

[Linus Torvalds]

Ok, I apparently missed this whole subthread yesterday, only getting back 
to it when going over my old queues.

On Thu, 31 Jul 2008, Dmitry Torokhov wrote:
> 
> Input: paper over a bug in Synaptics X driver

Yeah, it's not pretty, but how about moving that EV_KEY thing into the 
switch() statement? Also, the printk could certainly be a bit more useful.

But before doing _any_ of that, the first thing to do should be to not 
have that four-deep indentation by just splitting that horrible function 
up a bit.

IOW, start off with a patch like the appended, and _then_ add the special 
case handling to the EV_KEY thing. It would probably be most easily done 
by just literally limiting "len" to OLD_KEY_MAX. Something like

   #define OLD_KEY_MAX 0x1ff
   ...
	case EV_KEY:
		bits = dev->keybit;
		len = KEY_MAX;
		/* Hacky workaround for old bug in Xorg */
		if (buf_len == OLD_KEY_MAX) 
			len = OLD_KEY_MAX;
		break;
  ...

or similar.

Yeah, it's not pretty, but pragmatism before beauty.

		Linus
---
 drivers/input/evdev.c |   44 ++++++++++++++++++++++++--------------------
 1 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index 2d65411..ef8c2ed 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -647,6 +647,28 @@ static int str_to_user(const char *str, unsigned int maxlen, void __user *p)
 	return copy_to_user(p, str, len) ? -EFAULT : len;
 }
 
+static int handle_eviocgbit(struct input_dev *dev, unsigned int cmd, void __user *p, int compat_mode)
+{
+	unsigned long *bits;
+	int len;
+
+	switch (_IOC_NR(cmd) & EV_MAX) {
+
+	case      0: bits = dev->evbit;  len = EV_MAX;  break;
+	case EV_KEY: bits = dev->keybit; len = KEY_MAX; break;
+	case EV_REL: bits = dev->relbit; len = REL_MAX; break;
+	case EV_ABS: bits = dev->absbit; len = ABS_MAX; break;
+	case EV_MSC: bits = dev->mscbit; len = MSC_MAX; break;
+	case EV_LED: bits = dev->ledbit; len = LED_MAX; break;
+	case EV_SND: bits = dev->sndbit; len = SND_MAX; break;
+	case EV_FF:  bits = dev->ffbit;  len = FF_MAX;  break;
+	case EV_SW:  bits = dev->swbit;  len = SW_MAX;  break;
+	default: return -EINVAL;
+	}
+	return bits_to_user(bits, len, _IOC_SIZE(cmd), p, compat_mode);
+}
+
+
 static long evdev_do_ioctl(struct file *file, unsigned int cmd,
 			   void __user *p, int compat_mode)
 {
@@ -733,26 +755,8 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
 
 		if (_IOC_DIR(cmd) == _IOC_READ) {
 
-			if ((_IOC_NR(cmd) & ~EV_MAX) == _IOC_NR(EVIOCGBIT(0, 0))) {
-
-				unsigned long *bits;
-				int len;
-
-				switch (_IOC_NR(cmd) & EV_MAX) {
-
-				case      0: bits = dev->evbit;  len = EV_MAX;  break;
-				case EV_KEY: bits = dev->keybit; len = KEY_MAX; break;
-				case EV_REL: bits = dev->relbit; len = REL_MAX; break;
-				case EV_ABS: bits = dev->absbit; len = ABS_MAX; break;
-				case EV_MSC: bits = dev->mscbit; len = MSC_MAX; break;
-				case EV_LED: bits = dev->ledbit; len = LED_MAX; break;
-				case EV_SND: bits = dev->sndbit; len = SND_MAX; break;
-				case EV_FF:  bits = dev->ffbit;  len = FF_MAX;  break;
-				case EV_SW:  bits = dev->swbit;  len = SW_MAX;  break;
-				default: return -EINVAL;
-				}
-				return bits_to_user(bits, len, _IOC_SIZE(cmd), p, compat_mode);
-			}
+			if ((_IOC_NR(cmd) & ~EV_MAX) == _IOC_NR(EVIOCGBIT(0, 0)))
+				return handle_eviocgbit(dev, cmd, p, compat_mode);
 
 			if (_IOC_NR(cmd) == _IOC_NR(EVIOCGKEY(0)))
 				return bits_to_user(dev->key, KEY_MAX, _IOC_SIZE(cmd),

Re: linux-next: Tree for July 30

[Dmitry Torokhov]

On Fri, Aug 01, 2008 at 12:12:19PM -0700, Linus Torvalds wrote:
> 
> Ok, I apparently missed this whole subthread yesterday, only getting back 
> to it when going over my old queues.
> 
> On Thu, 31 Jul 2008, Dmitry Torokhov wrote:
> > 
> > Input: paper over a bug in Synaptics X driver
> 
> Yeah, it's not pretty, but how about moving that EV_KEY thing into the 
> switch() statement?

I want to to be visually separated. It is not a hot path so extra
comparison won't hurt us.

> Also, the printk could certainly be a bit more useful.

I think I will follow Andrew's advice and point people to a page on
kernel.org (to be written).

> have that four-deep indentation by just splitting that horrible function 
> up a bit.
> 
> IOW, start off with a patch like the appended,

Makes sense. Signed-off-by please ;)

-- 
Dmitry

Re: linux-next: Tree for July 30

[Linus Torvalds]

On Fri, 1 Aug 2008, Dmitry Torokhov wrote:
> 
> Makes sense. Signed-off-by please ;)

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

		Linus

Re: linux-next: Tree for July 30

[Stephen Rothwell]

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

Hi Andrew, Dmitry,

On Wed, 30 Jul 2008 23:10:29 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Wed, 30 Jul 2008 17:06:35 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> 
> > I have created today's linux-next tree at
> > git://git.kernel.org/pub/scm/linux/kernel/git/sfr/linux-next.git
> 
> The X server broke on my FC8 t61p thinkpad.  Mainline is OK.
> 
> Various information is at http://userweb.kernel.org/~akpm/mo/
> 
> I'm suspecting the input layer - my synaptics device seems to have
> disappeared?  See http://userweb.kernel.org/~akpm/mo/Xorg-log-diff.txt

I have reverted commit 03bac96fae0efdb25e2059e5accbe4f3ee6328dd ("Input:
expand keycode space") from linux-next until we have some resolution of
this problem.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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

Re: linux-next: Tree for July 30

[Dmitry Torokhov]

On Thu, Jul 31, 2008 at 01:10:56PM -0700, Andrew Morton wrote:
> 
> If that works then great.  But I think the printk could be improved. 
> Please provide sufficient information so that users (not programmers)
> can go off and fix things up without needing to email kernel developers.
> 
> One suitable approach would be
> 
> 	printk("see http://userweb.kernel.org/~dtor/read-this.txt")
> 

Ok, I made a small page here:

	http://userweb.kernel.org/~dtor/eviocgbit-bug.html

If you think this is sufficient I'd like to put the patch with the
warning in 2.6.27 so people and distributions could start updarting
affected programs.

-- 
Dmitry

Re: linux-next: Tree for July 30

[Andrew Morton]

On Thu, 7 Aug 2008 14:11:09 -0400
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:

> On Thu, Jul 31, 2008 at 01:10:56PM -0700, Andrew Morton wrote:
> > 
> > If that works then great.  But I think the printk could be improved. 
> > Please provide sufficient information so that users (not programmers)
> > can go off and fix things up without needing to email kernel developers.
> > 
> > One suitable approach would be
> > 
> > 	printk("see http://userweb.kernel.org/~dtor/read-this.txt")
> > 
> 
> Ok, I made a small page here:
> 
> 	http://userweb.kernel.org/~dtor/eviocgbit-bug.html

You need to mv that file.  It's actually at
http://userweb.kernel.org/~dtor/eviogcbit-bug.html

And

     evdev.c(EVIOCGBIT): Suspicious buffer size 511, limiting output to 64
     bytes. See http://userweb.kernel.org/~dtor/eviocgbit-bug.html 

will get a 404.

typo: s/distribbution/distribution/

The text looks good.  A user would come away wondering which packages
need to be updated.  Do we have a suitable text pattern whcih will help
them?

On a fedora system I have

y:/home/akpm> rpm -qa|grep -i syna
synaptics-0.14.6-2.fc8

So "the synaptics package" would be a suitably distro-neutral description.

`rpm -qa|grep -i tslib' comes up blank so I don't know about that one.

> If you think this is sufficient I'd like to put the patch with the
> warning in 2.6.27 so people and distributions could start updarting
> affected programs.

Sure.  It'd make sense to get that warning into 2.6.25.x and 2.6.26.x
as well, to accelerate the process a bit.

Re: linux-next: Tree for July 30

[Rafael J. Wysocki]

On Thursday, 7 of August 2008, Dmitry Torokhov wrote:
> On Thu, Jul 31, 2008 at 01:10:56PM -0700, Andrew Morton wrote:
> > 
> > If that works then great.  But I think the printk could be improved. 
> > Please provide sufficient information so that users (not programmers)
> > can go off and fix things up without needing to email kernel developers.
> > 
> > One suitable approach would be
> > 
> > 	printk("see http://userweb.kernel.org/~dtor/read-this.txt")
> > 
> 
> Ok, I made a small page here:
> 
> 	http://userweb.kernel.org/~dtor/eviocgbit-bug.html

404: Not Found

> If you think this is sufficient I'd like to put the patch with the
> warning in 2.6.27 so people and distributions could start updarting
> affected programs.

Re: linux-next: Tree for July 30

[Dmitry Torokhov]

On Thu, Aug 07, 2008 at 11:50:31AM -0700, Andrew Morton wrote:
> On Thu, 7 Aug 2008 14:11:09 -0400
> Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> 
> > On Thu, Jul 31, 2008 at 01:10:56PM -0700, Andrew Morton wrote:
> > > 
> > > If that works then great.  But I think the printk could be improved. 
> > > Please provide sufficient information so that users (not programmers)
> > > can go off and fix things up without needing to email kernel developers.
> > > 
> > > One suitable approach would be
> > > 
> > > 	printk("see http://userweb.kernel.org/~dtor/read-this.txt")
> > > 
> > 
> > Ok, I made a small page here:
> > 
> > 	http://userweb.kernel.org/~dtor/eviocgbit-bug.html
> 
> You need to mv that file.  It's actually at
> http://userweb.kernel.org/~dtor/eviogcbit-bug.html
> 
> And
> 
>      evdev.c(EVIOCGBIT): Suspicious buffer size 511, limiting output to 64
>      bytes. See http://userweb.kernel.org/~dtor/eviocgbit-bug.html 
> 
> will get a 404.
> 

Fixed.

> typo: s/distribbution/distribution/
> 

Fixed.

> The text looks good.  A user would come away wondering which packages
> need to be updated.  Do we have a suitable text pattern whcih will help
> them?
> 
> On a fedora system I have
> 
> y:/home/akpm> rpm -qa|grep -i syna
> synaptics-0.14.6-2.fc8
> 
> So "the synaptics package" would be a suitably distro-neutral description.
> 

There is apparently Synaptic package manager - quite similar name - so
I want to emphasize that we are concerned with a driver for X.

> `rpm -qa|grep -i tslib' comes up blank so I don't know about that one.

This is a touchscreen library that is used by embedded people.
Unfortunately I have no idea where the authoritative source is.

> 
> > If you think this is sufficient I'd like to put the patch with the
> > warning in 2.6.27 so people and distributions could start updarting
> > affected programs.
> 
> Sure.  It'd make sense to get that warning into 2.6.25.x and 2.6.26.x
> as well, to accelerate the process a bit.
> 

Ok, good.

-- 
Dmitry