Skip to contentSkip to conversation

fix console change race exposed by CFS

PATCH In review Jan Luebbe 2 participants 4 messages Sep 29, 2007 – Sep 30, 2007

lore
Share & tools
lore

Jan Luebbe

author
#0 lore raw

PATCH 1/1 fix console change race exposed by CFS

From: Jan Lübbe <[email protected]>
The new behaviour of CFS exposes a race which occurs if a switch is requested when vt_mode.mode is VT_PROCESS.

The process with vc->vt_pid is signaled before vc->vt_newvt is set. This causes the switch to fail when triggered by the monitoing process because the target is still -1.

Index: linux-2.6.22/drivers/char/vt_ioctl.c ===================================================================--- linux-2.6.22.orig/drivers/char/vt_ioctl.c +-+-+- linux-2.6.22/drivers/char/vt_ioctl.c
@@-1208,15 +-1208,18 @@ /* *Send the signal as privileged - kill_pid() will
*tell us if the process has gone or something else
- *is awry

+-		 *is awry.
+-		 *+-		 *We need to set vt_newvt *before*sending the signal or we
+-		 *have a race.

*/
+- vc->vt_newvt =new_vc->vc_num; if (kill_pid(vc->vt_pid, vc->vt_mode.relsig, 1) ==0) { /* *It worked. Mark the vt to switch to and
*return. The process needs to send us a *VT_RELDISP ioctl to complete the switch. */
- vc->vt_newvt =new_vc->vc_num; return; }

Acked-by on v1 1/1
#1 lore raw
Quoted 10 lines · Jan Luebbe

From: Jan Lübbe <[email protected]>

The new behaviour of CFS exposes a race which occurs if a switch is requested when vt_mode.mode is VT_PROCESS.

The process with vc->vt_pid is signaled before vc->vt_newvt is set. This causes the switch to fail when triggered by the monitoing process because the target is still -1.

nifty! I'm wondering what the symptoms of this bug were - how did you notice it? Also, i'm wondering how you managed to debug this - looks quite hard to trigger this race, right?

Ingo

author
#2 lore raw
Quoted 9 lines · Ingo Molnar
Quoted 8 lines · *Jan Luebbe

From: Jan Lübbe <[email protected]>>>
The new behaviour of CFS exposes a race which occurs if a switch is requested when vt_mode.mode is VT_PROCESS.

The process with vc->vt_pid is signaled before vc->vt_newvt is set. This causes the switch to fail when triggered by the monitoing process because the target is still -1.

Quoted 7 lines
Quoted 1 line

nifty!I'm wondering what the symptoms of this bug were - how did you notice it? Also, i'm wondering how you managed to debug this - looks quite hard to trigger this race, right?

Ingo

I'm currenly porting the OpenMoko patchset to 2.6.23-rc8. We are using psplash and kdrive. When booting, I got an error from psplash about being unable to switch consoles. Strangely, it was possible to start kdrive manually. So i set up qemu for kernel debugging, set some interesting breakpoints and then saw that it was scheduling away directly after the kill_pid. Then it was quite obvious.

It was triggering every time, on the device and in qemu identically. Most people testing the RCs probably don't run psplash (or any other program which sets vt_mode.mode to VT_PROCESS), so it wasn't noticed earlier.

Jan

#3 lore raw
Quoted 11 lines · Jan Luebbe

I'm currenly porting the OpenMoko patchset to 2.6.23-rc8. We are using psplash and kdrive. When booting, I got an error from psplash about being unable to switch consoles. Strangely, it was possible to start kdrive manually. So i set up qemu for kernel debugging, set some interesting breakpoints and then saw that it was scheduling away directly after the kill_pid. Then it was quite obvious.

It was triggering every time, on the device and in qemu identically. Most people testing the RCs probably don't run psplash (or any other program which sets vt_mode.mode to VT_PROCESS), so it wasn't noticed earlier.

thx for the description. I did some searching on google.com/codesearch, and here are some other console apps that make active use of VT_PROCESS:

1diskxwin-2.0.0/X11/programs/Xserver/hw/kdrive/linux/linux.c usr/include/dev/wscons/wsdisplay_compat_usl.c busybox-1.01/loginutils/vlock.c allegro-4.2.2/src/linux/vtswitch.c tvision/classes/linux/linuxkey.cc xc/programs/Xserver/hw/xfree86/os-support/pmax/pmax_init.c aalib-1.4.0/src/aalinuxkbd.c qt-embedded-opensource-4.0.0-b2/src/gui/embedded/qkbdtty_qws.cpp mc-4.5.55/src/cons.handler.c psplash-0.1/psplash-console.c quakeforge-0.5.5/libs/video/targets/vid_fbdev.c

just in case someone has a regression with any of them, on an older kernel. To me it appears X should have triggered this too. Perhaps people dont switch between VGA and X all that frequently anymore ...

Ingo