* Re: linux-next: Tree for July 30 [not found] <20080730170635.f737ffe9.sfr@canb.auug.org.au> @ 2008-07-31 6:10 ` Andrew Morton 2008-07-31 14:07 ` Dmitry Torokhov 2008-08-04 5:47 ` Stephen Rothwell 0 siblings, 2 replies; 33+ messages in thread From: Andrew Morton @ 2008-07-31 6:10 UTC (permalink / raw) To: Stephen Rothwell; +Cc: linux-next, LKML, Dmitry Torokhov, linux-input 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: linux-next: Tree for July 30 2008-07-31 6:10 ` linux-next: Tree for July 30 Andrew Morton @ 2008-07-31 14:07 ` Dmitry Torokhov 2008-07-31 15:36 ` Bartlomiej Zolnierkiewicz 2008-08-04 5:47 ` Stephen Rothwell 1 sibling, 1 reply; 33+ messages in thread From: Dmitry Torokhov @ 2008-07-31 14:07 UTC (permalink / raw) To: Andrew Morton; +Cc: Stephen Rothwell, linux-next, LKML, linux-input 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 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: linux-next: Tree for July 30 2008-07-31 14:07 ` Dmitry Torokhov @ 2008-07-31 15:36 ` Bartlomiej Zolnierkiewicz 2008-07-31 15:56 ` Dmitry Torokhov 0 siblings, 1 reply; 33+ messages in thread From: Bartlomiej Zolnierkiewicz @ 2008-07-31 15:36 UTC (permalink / raw) To: Dmitry Torokhov Cc: Andrew Morton, Stephen Rothwell, linux-next, LKML, linux-input 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: linux-next: Tree for July 30 2008-07-31 15:36 ` Bartlomiej Zolnierkiewicz @ 2008-07-31 15:56 ` Dmitry Torokhov 2008-07-31 17:44 ` Andrew Morton 0 siblings, 1 reply; 33+ messages in thread From: Dmitry Torokhov @ 2008-07-31 15:56 UTC (permalink / raw) To: Bartlomiej Zolnierkiewicz Cc: Andrew Morton, Stephen Rothwell, linux-next, LKML, linux-input 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: linux-next: Tree for July 30 2008-07-31 15:56 ` Dmitry Torokhov @ 2008-07-31 17:44 ` Andrew Morton 2008-07-31 18:17 ` Dmitry Torokhov 0 siblings, 1 reply; 33+ messages in thread From: Andrew Morton @ 2008-07-31 17:44 UTC (permalink / raw) To: Dmitry Torokhov Cc: Bartlomiej Zolnierkiewicz, Stephen Rothwell, linux-next, LKML, linux-input 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? ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: linux-next: Tree for July 30 2008-07-31 17:44 ` Andrew Morton @ 2008-07-31 18:17 ` Dmitry Torokhov 2008-07-31 18:26 ` Andrew Morton 2008-07-31 18:48 ` Rafael J. Wysocki 0 siblings, 2 replies; 33+ messages in thread From: Dmitry Torokhov @ 2008-07-31 18:17 UTC (permalink / raw) To: Andrew Morton Cc: Bartlomiej Zolnierkiewicz, Stephen Rothwell, linux-next, LKML, linux-input 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: linux-next: Tree for July 30 2008-07-31 18:17 ` Dmitry Torokhov @ 2008-07-31 18:26 ` Andrew Morton 2008-07-31 18:34 ` Dmitry Torokhov 2008-07-31 18:48 ` Rafael J. Wysocki 1 sibling, 1 reply; 33+ messages in thread From: Andrew Morton @ 2008-07-31 18:26 UTC (permalink / raw) To: Dmitry Torokhov Cc: Bartlomiej Zolnierkiewicz, Stephen Rothwell, linux-next, LKML, linux-input 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? ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: linux-next: Tree for July 30 2008-07-31 18:26 ` Andrew Morton @ 2008-07-31 18:34 ` Dmitry Torokhov 2008-07-31 18:55 ` Andrew Morton 0 siblings, 1 reply; 33+ messages in thread From: Dmitry Torokhov @ 2008-07-31 18:34 UTC (permalink / raw) To: Andrew Morton Cc: Bartlomiej Zolnierkiewicz, Stephen Rothwell, linux-next, LKML, linux-input 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: linux-next: Tree for July 30 2008-07-31 18:34 ` Dmitry Torokhov @ 2008-07-31 18:55 ` Andrew Morton 2008-07-31 19:03 ` Dmitry Torokhov 2008-07-31 19:20 ` Hugh Dickins 0 siblings, 2 replies; 33+ messages in thread From: Andrew Morton @ 2008-07-31 18:55 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: bzolnier, sfr, linux-next, linux-kernel, linux-input 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. ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: linux-next: Tree for July 30 2008-07-31 18:55 ` Andrew Morton @ 2008-07-31 19:03 ` Dmitry Torokhov 2008-07-31 19:20 ` Hugh Dickins 1 sibling, 0 replies; 33+ messages in thread From: Dmitry Torokhov @ 2008-07-31 19:03 UTC (permalink / raw) To: Andrew Morton; +Cc: bzolnier, sfr, linux-next, linux-kernel, linux-input 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: linux-next: Tree for July 30 2008-07-31 18:55 ` Andrew Morton 2008-07-31 19:03 ` Dmitry Torokhov @ 2008-07-31 19:20 ` Hugh Dickins 1 sibling, 0 replies; 33+ messages in thread From: Hugh Dickins @ 2008-07-31 19:20 UTC (permalink / raw) To: Andrew Morton Cc: Dmitry Torokhov, bzolnier, sfr, linux-next, linux-kernel, linux-input 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: linux-next: Tree for July 30 2008-07-31 18:17 ` Dmitry Torokhov 2008-07-31 18:26 ` Andrew Morton @ 2008-07-31 18:48 ` Rafael J. Wysocki 2008-07-31 18:54 ` Dmitry Torokhov 1 sibling, 1 reply; 33+ messages in thread From: Rafael J. Wysocki @ 2008-07-31 18:48 UTC (permalink / raw) To: Dmitry Torokhov Cc: Andrew Morton, Bartlomiej Zolnierkiewicz, Stephen Rothwell, linux-next, LKML, linux-input, Linus Torvalds 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: linux-next: Tree for July 30 2008-07-31 18:48 ` Rafael J. Wysocki @ 2008-07-31 18:54 ` Dmitry Torokhov 2008-07-31 19:10 ` Linus Torvalds ` (2 more replies) 0 siblings, 3 replies; 33+ messages in thread From: Dmitry Torokhov @ 2008-07-31 18:54 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Andrew Morton, Bartlomiej Zolnierkiewicz, Stephen Rothwell, linux-next, LKML, linux-input, Linus Torvalds 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: linux-next: Tree for July 30 2008-07-31 18:54 ` Dmitry Torokhov @ 2008-07-31 19:10 ` Linus Torvalds 2008-07-31 19:24 ` Dmitry Torokhov 2008-07-31 19:13 ` Andrew Morton 2008-07-31 19:57 ` Rafael J. Wysocki 2 siblings, 1 reply; 33+ messages in thread From: Linus Torvalds @ 2008-07-31 19:10 UTC (permalink / raw) To: Dmitry Torokhov Cc: Rafael J. Wysocki, Andrew Morton, Bartlomiej Zolnierkiewicz, Stephen Rothwell, linux-next, LKML, linux-input 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: linux-next: Tree for July 30 2008-07-31 19:10 ` Linus Torvalds @ 2008-07-31 19:24 ` Dmitry Torokhov 2008-07-31 19:42 ` Dmitry Torokhov 2008-07-31 19:44 ` Linus Torvalds 0 siblings, 2 replies; 33+ messages in thread From: Dmitry Torokhov @ 2008-07-31 19:24 UTC (permalink / raw) To: Linus Torvalds Cc: Rafael J. Wysocki, Andrew Morton, Bartlomiej Zolnierkiewicz, Stephen Rothwell, linux-next, LKML, linux-input 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: linux-next: Tree for July 30 2008-07-31 19:24 ` Dmitry Torokhov @ 2008-07-31 19:42 ` Dmitry Torokhov 2008-07-31 20:10 ` Andrew Morton 2008-08-01 19:12 ` Linus Torvalds 2008-07-31 19:44 ` Linus Torvalds 1 sibling, 2 replies; 33+ messages in thread From: Dmitry Torokhov @ 2008-07-31 19:42 UTC (permalink / raw) To: Linus Torvalds Cc: Rafael J. Wysocki, Andrew Morton, Bartlomiej Zolnierkiewicz, Stephen Rothwell, linux-next, LKML, linux-input 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))) ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: linux-next: Tree for July 30 2008-07-31 19:42 ` Dmitry Torokhov @ 2008-07-31 20:10 ` Andrew Morton 2008-08-07 18:11 ` Dmitry Torokhov 2008-08-01 19:12 ` Linus Torvalds 1 sibling, 1 reply; 33+ messages in thread From: Andrew Morton @ 2008-07-31 20:10 UTC (permalink / raw) To: Dmitry Torokhov Cc: torvalds, rjw, bzolnier, sfr, linux-next, linux-kernel, linux-input 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") ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: linux-next: Tree for July 30 2008-07-31 20:10 ` Andrew Morton @ 2008-08-07 18:11 ` Dmitry Torokhov 2008-08-07 18:50 ` Andrew Morton 2008-08-07 18:55 ` Rafael J. Wysocki 0 siblings, 2 replies; 33+ messages in thread From: Dmitry Torokhov @ 2008-08-07 18:11 UTC (permalink / raw) To: Andrew Morton Cc: torvalds, rjw, bzolnier, sfr, linux-next, linux-kernel, linux-input 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: linux-next: Tree for July 30 2008-08-07 18:11 ` Dmitry Torokhov @ 2008-08-07 18:50 ` Andrew Morton 2008-08-07 19:06 ` Dmitry Torokhov 2008-08-07 18:55 ` Rafael J. Wysocki 1 sibling, 1 reply; 33+ messages in thread From: Andrew Morton @ 2008-08-07 18:50 UTC (permalink / raw) To: Dmitry Torokhov Cc: torvalds, rjw, bzolnier, sfr, linux-next, linux-kernel, linux-input, stable 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. ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: linux-next: Tree for July 30 2008-08-07 18:50 ` Andrew Morton @ 2008-08-07 19:06 ` Dmitry Torokhov 0 siblings, 0 replies; 33+ messages in thread From: Dmitry Torokhov @ 2008-08-07 19:06 UTC (permalink / raw) To: Andrew Morton Cc: torvalds, rjw, bzolnier, sfr, linux-next, linux-kernel, linux-input, stable 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: linux-next: Tree for July 30 2008-08-07 18:11 ` Dmitry Torokhov 2008-08-07 18:50 ` Andrew Morton @ 2008-08-07 18:55 ` Rafael J. Wysocki 1 sibling, 0 replies; 33+ messages in thread From: Rafael J. Wysocki @ 2008-08-07 18:55 UTC (permalink / raw) To: Dmitry Torokhov Cc: Andrew Morton, torvalds, bzolnier, sfr, linux-next, linux-kernel, linux-input 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. ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: linux-next: Tree for July 30 2008-07-31 19:42 ` Dmitry Torokhov 2008-07-31 20:10 ` Andrew Morton @ 2008-08-01 19:12 ` Linus Torvalds 2008-08-01 19:23 ` Dmitry Torokhov 1 sibling, 1 reply; 33+ messages in thread From: Linus Torvalds @ 2008-08-01 19:12 UTC (permalink / raw) To: Dmitry Torokhov Cc: Rafael J. Wysocki, Andrew Morton, Bartlomiej Zolnierkiewicz, Stephen Rothwell, linux-next, LKML, linux-input 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), ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: linux-next: Tree for July 30 2008-08-01 19:12 ` Linus Torvalds @ 2008-08-01 19:23 ` Dmitry Torokhov 2008-08-01 19:26 ` Linus Torvalds 0 siblings, 1 reply; 33+ messages in thread From: Dmitry Torokhov @ 2008-08-01 19:23 UTC (permalink / raw) To: Linus Torvalds Cc: Rafael J. Wysocki, Andrew Morton, Bartlomiej Zolnierkiewicz, Stephen Rothwell, linux-next, LKML, linux-input 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: linux-next: Tree for July 30 2008-08-01 19:23 ` Dmitry Torokhov @ 2008-08-01 19:26 ` Linus Torvalds 0 siblings, 0 replies; 33+ messages in thread From: Linus Torvalds @ 2008-08-01 19:26 UTC (permalink / raw) To: Dmitry Torokhov Cc: Rafael J. Wysocki, Andrew Morton, Bartlomiej Zolnierkiewicz, Stephen Rothwell, linux-next, LKML, linux-input On Fri, 1 Aug 2008, Dmitry Torokhov wrote: > > Makes sense. Signed-off-by please ;) Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> Linus ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: linux-next: Tree for July 30 2008-07-31 19:24 ` Dmitry Torokhov 2008-07-31 19:42 ` Dmitry Torokhov @ 2008-07-31 19:44 ` Linus Torvalds 2008-07-31 20:05 ` Dmitry Torokhov 1 sibling, 1 reply; 33+ messages in thread From: Linus Torvalds @ 2008-07-31 19:44 UTC (permalink / raw) To: Dmitry Torokhov Cc: Rafael J. Wysocki, Andrew Morton, Bartlomiej Zolnierkiewicz, Stephen Rothwell, linux-next, LKML, linux-input 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: linux-next: Tree for July 30 2008-07-31 19:44 ` Linus Torvalds @ 2008-07-31 20:05 ` Dmitry Torokhov 2008-07-31 20:16 ` Linus Torvalds 0 siblings, 1 reply; 33+ messages in thread From: Dmitry Torokhov @ 2008-07-31 20:05 UTC (permalink / raw) To: Linus Torvalds Cc: Rafael J. Wysocki, Andrew Morton, Bartlomiej Zolnierkiewicz, Stephen Rothwell, linux-next, LKML, linux-input 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: linux-next: Tree for July 30 2008-07-31 20:05 ` Dmitry Torokhov @ 2008-07-31 20:16 ` Linus Torvalds 2008-07-31 20:28 ` Linus Torvalds 2008-07-31 20:28 ` Dmitry Torokhov 0 siblings, 2 replies; 33+ messages in thread From: Linus Torvalds @ 2008-07-31 20:16 UTC (permalink / raw) To: Dmitry Torokhov Cc: Rafael J. Wysocki, Andrew Morton, Bartlomiej Zolnierkiewicz, Stephen Rothwell, linux-next, LKML, linux-input 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: linux-next: Tree for July 30 2008-07-31 20:16 ` Linus Torvalds @ 2008-07-31 20:28 ` Linus Torvalds 2008-07-31 20:39 ` Dmitry Torokhov 2008-07-31 20:28 ` Dmitry Torokhov 1 sibling, 1 reply; 33+ messages in thread From: Linus Torvalds @ 2008-07-31 20:28 UTC (permalink / raw) To: Dmitry Torokhov Cc: Rafael J. Wysocki, Andrew Morton, Bartlomiej Zolnierkiewicz, Stephen Rothwell, linux-next, LKML, linux-input 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))) ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: linux-next: Tree for July 30 2008-07-31 20:28 ` Linus Torvalds @ 2008-07-31 20:39 ` Dmitry Torokhov 0 siblings, 0 replies; 33+ messages in thread From: Dmitry Torokhov @ 2008-07-31 20:39 UTC (permalink / raw) To: Linus Torvalds Cc: Rafael J. Wysocki, Andrew Morton, Bartlomiej Zolnierkiewicz, Stephen Rothwell, linux-next, LKML, linux-input 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: linux-next: Tree for July 30 2008-07-31 20:16 ` Linus Torvalds 2008-07-31 20:28 ` Linus Torvalds @ 2008-07-31 20:28 ` Dmitry Torokhov 1 sibling, 0 replies; 33+ messages in thread From: Dmitry Torokhov @ 2008-07-31 20:28 UTC (permalink / raw) To: Linus Torvalds Cc: Rafael J. Wysocki, Andrew Morton, Bartlomiej Zolnierkiewicz, Stephen Rothwell, linux-next, LKML, linux-input 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: linux-next: Tree for July 30 2008-07-31 18:54 ` Dmitry Torokhov 2008-07-31 19:10 ` Linus Torvalds @ 2008-07-31 19:13 ` Andrew Morton 2008-07-31 19:57 ` Rafael J. Wysocki 2 siblings, 0 replies; 33+ messages in thread From: Andrew Morton @ 2008-07-31 19:13 UTC (permalink / raw) To: Dmitry Torokhov Cc: rjw, bzolnier, sfr, linux-next, linux-kernel, linux-input, torvalds 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? ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: linux-next: Tree for July 30 2008-07-31 18:54 ` Dmitry Torokhov 2008-07-31 19:10 ` Linus Torvalds 2008-07-31 19:13 ` Andrew Morton @ 2008-07-31 19:57 ` Rafael J. Wysocki 2 siblings, 0 replies; 33+ messages in thread From: Rafael J. Wysocki @ 2008-07-31 19:57 UTC (permalink / raw) To: Dmitry Torokhov Cc: Andrew Morton, Bartlomiej Zolnierkiewicz, Stephen Rothwell, linux-next, LKML, linux-input, Linus Torvalds 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: linux-next: Tree for July 30 2008-07-31 6:10 ` linux-next: Tree for July 30 Andrew Morton 2008-07-31 14:07 ` Dmitry Torokhov @ 2008-08-04 5:47 ` Stephen Rothwell 1 sibling, 0 replies; 33+ messages in thread From: Stephen Rothwell @ 2008-08-04 5:47 UTC (permalink / raw) To: Andrew Morton; +Cc: linux-next, LKML, Dmitry Torokhov, linux-input [-- 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 --] ^ permalink raw reply [flat|nested] 33+ messages in thread
end of thread, other threads:[~2008-08-07 19:06 UTC | newest] Thread overview: 33+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- [not found] <20080730170635.f737ffe9.sfr@canb.auug.org.au> 2008-07-31 6:10 ` linux-next: Tree for July 30 Andrew Morton 2008-07-31 14:07 ` Dmitry Torokhov 2008-07-31 15:36 ` Bartlomiej Zolnierkiewicz 2008-07-31 15:56 ` Dmitry Torokhov 2008-07-31 17:44 ` Andrew Morton 2008-07-31 18:17 ` Dmitry Torokhov 2008-07-31 18:26 ` Andrew Morton 2008-07-31 18:34 ` Dmitry Torokhov 2008-07-31 18:55 ` Andrew Morton 2008-07-31 19:03 ` Dmitry Torokhov 2008-07-31 19:20 ` Hugh Dickins 2008-07-31 18:48 ` Rafael J. Wysocki 2008-07-31 18:54 ` Dmitry Torokhov 2008-07-31 19:10 ` Linus Torvalds 2008-07-31 19:24 ` Dmitry Torokhov 2008-07-31 19:42 ` Dmitry Torokhov 2008-07-31 20:10 ` Andrew Morton 2008-08-07 18:11 ` Dmitry Torokhov 2008-08-07 18:50 ` Andrew Morton 2008-08-07 19:06 ` Dmitry Torokhov 2008-08-07 18:55 ` Rafael J. Wysocki 2008-08-01 19:12 ` Linus Torvalds 2008-08-01 19:23 ` Dmitry Torokhov 2008-08-01 19:26 ` Linus Torvalds 2008-07-31 19:44 ` Linus Torvalds 2008-07-31 20:05 ` Dmitry Torokhov 2008-07-31 20:16 ` Linus Torvalds 2008-07-31 20:28 ` Linus Torvalds 2008-07-31 20:39 ` Dmitry Torokhov 2008-07-31 20:28 ` Dmitry Torokhov 2008-07-31 19:13 ` Andrew Morton 2008-07-31 19:57 ` Rafael J. Wysocki 2008-08-04 5:47 ` Stephen Rothwell
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).