Date: Tue, 23 Sep 2008 14:02:50 +0400 From: Eygene Ryabinkin <rea-fbsd@codelabs.ru> To: Maksim Yevmenkin <maksim.yevmenkin@gmail.com> Cc: rik@freebsd.org, ed@freebsd.org, current@freebsd.org, bug-followup@freebsd.org Subject: Re: kern/127446: [patch] fix race in sys/dev/kbdmux/kbdmux.c Message-ID: <QqKr2VPu8fyQwhEJHQt9Bg/XOKc@K033dsD74Ht1Pqr5uGITGrg6QIg> In-Reply-To: <XTJ4IILGIQiL7GkTteNpWNqCP5A@puSb6//93PqXpeckgTOA3gBSgL0> References: <20080917161633.9E2F717101@shadow.codelabs.ru> <bb4a86c70809170956x36234893he8894a49127b6b6e@mail.gmail.com> <gaH7cu1zt0Bw5e8Q2XfgrlKSRyY@iXA9ZWPrtc2I2BMzBXoToMd7YdQ> <bb4a86c70809171052t5e961675n139b2848e6addd0@mail.gmail.com> <OBTLESaczqaewZBU9qdf0FWZRHE@7/OI9n2WwGs0JPogavdPqCpU9P8> <OukumQcOIGlPdbI1QmdweY8cFsk@IW4pzQfcswHj%2B/3imThIGjvxQdw> <bb4a86c70809190920l29f40e3cja19f6795e19b02b@mail.gmail.com> <byCZZ2L%2BP/vIU5hBTMuURReOshU@p9DJkXKp%2Br2jequ3/WCPe%2BJakGc> <XTJ4IILGIQiL7GkTteNpWNqCP5A@puSb6//93PqXpeckgTOA3gBSgL0>
next in thread | previous in thread | raw e-mail | index | archive | help
--OwLcNYc0lM97+oe1
Content-Type: multipart/mixed; boundary="5vNYLRcllDrimb99"
Content-Disposition: inline
--5vNYLRcllDrimb99
Content-Type: text/plain; charset=koi8-r
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
Tue, Sep 23, 2008 at 01:09:03PM +0400, Eygene Ryabinkin wrote:
> New patch works on 7.0 and 7.0-PRERELEASE, but currently hangs my
> 7.1-PRERELEASE just before activation of the single-user mode. I am
> investigating -- I did the original patch for the syscons.c 1.453.2.1.
> Changes in 1.453.2.2 look innocently, but 1.453.2.3 changed some
> functionality, may be it is the culprit. Will inform on my findings.
It turned that locking inside sc_cnputc() was redundant and errorneous:
WITNESS quicky advised me not to do it. Forgot to run locking subsystem
checks, sorry for that.
The attached patch was tested on two 7.1-PRERELEASE systems (i386 and
amd64, both with and without X), on 7.0-STABLE and 7.0-RELEASE-p3 (i386,
only console, no X) and on rather old 8-CURRENT from Jul 1, syscons.c
revision 1.459 (amd64 both with and without X). Works fine for me.
8.x needs slightly modified patch due to the naming changes. Also
attached it. Will try a fresher -CURRENT in some hours: Ed did massive
changes due to the MPSAFE tty layer, so may be this patch won't be
needed for the modern 8-CURRENT.
--=20
Eygene
_ ___ _.--. #
\`.|\..----...-'` `-._.-'_.-'` # Remember that it is hard
/ ' ` , __.--' # to read the on-line manual =20
)/' _/ \ `-_, / # while single-stepping the kernel.
`-'" `"\_ ,_.-;_.-\_ ', fsc/as #
_.-'_./ {_.' ; / # -- FreeBSD Developers handbook=20
{_.-``-' {_/ #
--5vNYLRcllDrimb99
Content-Type: text/x-diff; charset=koi8-r
Content-Disposition: attachment; filename="syscons-read-race.FreeBSD-8.patch"
Content-Transfer-Encoding: quoted-printable
Avoids races of high-level syscons code with the sckbdevent handler.
Patch for the syscons.c 1.459 (8-CURRENT from July 1st 2008).
--- sys/dev/syscons/syscons.c.orig 2008-05-25 19:30:27.000000000 +0400
+++ sys/dev/syscons/syscons.c 2008-09-23 13:51:14.000000000 +0400
@@ -1583,6 +1583,7 @@
int s =3D spltty(); /* block sckbdevent and scrn_timer while we poll */
int c;
=20
+ mtx_lock(&Giant);
/* assert(sc_console !=3D NULL) */
=20
/*=20
@@ -1594,11 +1595,13 @@
sccnupdate(scp);
=20
if (fkeycp < fkey.len) {
+ mtx_unlock(&Giant);
splx(s);
return fkey.str[fkeycp++];
}
=20
if (scp->sc->kbd =3D=3D NULL) {
+ mtx_unlock(&Giant);
splx(s);
return -1;
}
@@ -1621,6 +1624,7 @@
scp->kbd_mode =3D cur_mode;
kbdd_ioctl(scp->sc->kbd, KDSKBMODE, (caddr_t)&scp->kbd_mode);
kbdd_disable(scp->sc->kbd);
+ mtx_unlock(&Giant);
splx(s);
=20
switch (KEYFLAGS(c)) {
--5vNYLRcllDrimb99
Content-Type: text/x-diff; charset=koi8-r
Content-Disposition: attachment; filename="syscons-read-race.patch"
Content-Transfer-Encoding: quoted-printable
Avoids races of high-level syscons code with the sckbdevent handler.
Made for FreeBSD 7.x (syscons.c 1.453.2.1); was tested on
7.0-PRERELEASE, 7.0-STABLE, 7.0-RELEASE-p3 and 7.1-PRERELEASE (up to
the revision 1.453.2.3 of syscons.c).
--- sys/dev/syscons/syscons.c.orig 2008-09-23 11:46:45.000000000 +0400
+++ sys/dev/syscons/syscons.c 2008-09-23 12:16:32.000000000 +0400
@@ -1590,6 +1592,7 @@
int s =3D spltty(); /* block sckbdevent and scrn_timer while we poll */
int c;
=20
+ mtx_lock(&Giant);
/* assert(sc_console !=3D NULL) */
=20
/*=20
@@ -1601,11 +1604,13 @@
sccnupdate(scp);
=20
if (fkeycp < fkey.len) {
+ mtx_unlock(&Giant);
splx(s);
return fkey.str[fkeycp++];
}
=20
if (scp->sc->kbd =3D=3D NULL) {
+ mtx_unlock(&Giant);
splx(s);
return -1;
}
@@ -1628,6 +1633,7 @@
scp->kbd_mode =3D cur_mode;
kbd_ioctl(scp->sc->kbd, KDSKBMODE, (caddr_t)&scp->kbd_mode);
kbd_disable(scp->sc->kbd);
+ mtx_unlock(&Giant);
splx(s);
=20
switch (KEYFLAGS(c)) {
--5vNYLRcllDrimb99--
--OwLcNYc0lM97+oe1
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.9 (FreeBSD)
iEYEARECAAYFAkjYvsoACgkQthUKNsbL7YjJFQCffM8DomRrMBOwhQPnqn3ab3Js
e80An0f7H0egC+dEcV0jDgCFK0MmEVKQ
=v79M
-----END PGP SIGNATURE-----
--OwLcNYc0lM97+oe1--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?QqKr2VPu8fyQwhEJHQt9Bg/XOKc>