From: Alan Cox <alan@lxorguk.ukuu.org.uk>
To: linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org
Subject: [PATCH 2/6] vt: push down the tty lock so we can see what is left to tackle
Date: Thu, 01 Mar 2012 19:50:16 +0000 [thread overview]
Message-ID: <20120301195005.11322.78572.stgit@bob.linux.org.uk> (raw)
In-Reply-To: <20120301194831.11322.38295.stgit@bob.linux.org.uk>
From: Alan Cox <alan@linux.intel.com>
At this point we have the tty_lock guarding a couple of oddities, plus the
translation and unimap still.
We also extend the console_lock in a couple of spots where coverage is wrong
and switch vcs_open to use the right lock !
Signed-off-by: Alan Cox <alan@linux.intel.com>
---
drivers/tty/vt/vc_screen.c | 4 +-
drivers/tty/vt/vt_ioctl.c | 87 +++++++++++++++++++++++++++-----------------
2 files changed, 55 insertions(+), 36 deletions(-)
diff --git a/drivers/tty/vt/vc_screen.c b/drivers/tty/vt/vc_screen.c
index 7a367ff..fa7268a 100644
--- a/drivers/tty/vt/vc_screen.c
+++ b/drivers/tty/vt/vc_screen.c
@@ -608,10 +608,10 @@ vcs_open(struct inode *inode, struct file *filp)
unsigned int currcons = iminor(inode) & 127;
int ret = 0;
- tty_lock();
+ console_lock();
if(currcons && !vc_cons_allocated(currcons-1))
ret = -ENXIO;
- tty_unlock();
+ console_unlock();
return ret;
}
diff --git a/drivers/tty/vt/vt_ioctl.c b/drivers/tty/vt/vt_ioctl.c
index 28ca0aa..16ad235 100644
--- a/drivers/tty/vt/vt_ioctl.c
+++ b/drivers/tty/vt/vt_ioctl.c
@@ -281,7 +281,6 @@ int vt_ioctl(struct tty_struct *tty,
console = vc->vc_num;
- tty_lock();
if (!vc_cons_allocated(console)) { /* impossible? */
ret = -ENOIOCTLCMD;
@@ -299,16 +298,18 @@ int vt_ioctl(struct tty_struct *tty,
switch (cmd) {
case TIOCLINUX:
+ tty_lock();
ret = tioclinux(tty, arg);
+ tty_unlock();
break;
case KIOCSOUND:
if (!perm)
- goto eperm;
+ return -EPERM;
/*
* The use of PIT_TICK_RATE is historic, it used to be
* the platform-dependent CLOCK_TICK_RATE between 2.6.12
* and 2.6.36, which was a minor but unfortunate ABI
- * change.
+ * change. kd_mksound is locked by the input layer.
*/
if (arg)
arg = PIT_TICK_RATE / arg;
@@ -317,7 +318,7 @@ int vt_ioctl(struct tty_struct *tty,
case KDMKTONE:
if (!perm)
- goto eperm;
+ return -EPERM;
{
unsigned int ticks, count;
@@ -335,7 +336,7 @@ int vt_ioctl(struct tty_struct *tty,
case KDGKBTYPE:
/*
- * this is naive.
+ * this is naïve.
*/
ucval = KB_101;
ret = put_user(ucval, (char __user *)arg);
@@ -353,6 +354,8 @@ int vt_ioctl(struct tty_struct *tty,
/*
* KDADDIO and KDDELIO may be able to add ports beyond what
* we reject here, but to be safe...
+ *
+ * These are locked internally via sys_ioperm
*/
if (arg < GPFIRST || arg > GPLAST) {
ret = -EINVAL;
@@ -375,7 +378,7 @@ int vt_ioctl(struct tty_struct *tty,
struct kbd_repeat kbrep;
if (!capable(CAP_SYS_TTY_CONFIG))
- goto eperm;
+ return -EPERM;
if (copy_from_user(&kbrep, up, sizeof(struct kbd_repeat))) {
ret = -EFAULT;
@@ -399,7 +402,7 @@ int vt_ioctl(struct tty_struct *tty,
* need to restore their engine state. --BenH
*/
if (!perm)
- goto eperm;
+ return -EPERM;
switch (arg) {
case KD_GRAPHICS:
break;
@@ -412,6 +415,7 @@ int vt_ioctl(struct tty_struct *tty,
ret = -EINVAL;
goto out;
}
+ /* FIXME: this needs the console lock extending */
if (vc->vc_mode == (unsigned char) arg)
break;
vc->vc_mode = (unsigned char) arg;
@@ -443,7 +447,7 @@ int vt_ioctl(struct tty_struct *tty,
case KDSKBMODE:
if (!perm)
- goto eperm;
+ return -EPERM;
ret = vt_do_kdskbmode(console, arg);
if (ret == 0)
tty_ldisc_flush(tty);
@@ -512,7 +516,7 @@ int vt_ioctl(struct tty_struct *tty,
case KDSIGACCEPT:
{
if (!perm || !capable(CAP_KILL))
- goto eperm;
+ return -EPERM;
if (!valid_signal(arg) || arg < 1 || arg == SIGKILL)
ret = -EINVAL;
else {
@@ -530,7 +534,7 @@ int vt_ioctl(struct tty_struct *tty,
struct vt_mode tmp;
if (!perm)
- goto eperm;
+ return -EPERM;
if (copy_from_user(&tmp, up, sizeof(struct vt_mode))) {
ret = -EFAULT;
goto out;
@@ -576,6 +580,7 @@ int vt_ioctl(struct tty_struct *tty,
struct vt_stat __user *vtstat = up;
unsigned short state, mask;
+ /* Review: FIXME: Console lock ? */
if (put_user(fg_console + 1, &vtstat->v_active))
ret = -EFAULT;
else {
@@ -593,6 +598,7 @@ int vt_ioctl(struct tty_struct *tty,
* Returns the first available (non-opened) console.
*/
case VT_OPENQRY:
+ /* FIXME: locking ? - but then this is a stupid API */
for (i = 0; i < MAX_NR_CONSOLES; ++i)
if (! VT_IS_IN_USE(i))
break;
@@ -606,7 +612,7 @@ int vt_ioctl(struct tty_struct *tty,
*/
case VT_ACTIVATE:
if (!perm)
- goto eperm;
+ return -EPERM;
if (arg == 0 || arg > MAX_NR_CONSOLES)
ret = -ENXIO;
else {
@@ -625,7 +631,7 @@ int vt_ioctl(struct tty_struct *tty,
struct vt_setactivate vsa;
if (!perm)
- goto eperm;
+ return -EPERM;
if (copy_from_user(&vsa, (struct vt_setactivate __user *)arg,
sizeof(struct vt_setactivate))) {
@@ -653,6 +659,7 @@ int vt_ioctl(struct tty_struct *tty,
if (ret)
break;
/* Commence switch and lock */
+ /* Review set_console locks */
set_console(vsa.console);
}
break;
@@ -663,7 +670,7 @@ int vt_ioctl(struct tty_struct *tty,
*/
case VT_WAITACTIVE:
if (!perm)
- goto eperm;
+ return -EPERM;
if (arg == 0 || arg > MAX_NR_CONSOLES)
ret = -ENXIO;
else
@@ -682,16 +689,17 @@ int vt_ioctl(struct tty_struct *tty,
*/
case VT_RELDISP:
if (!perm)
- goto eperm;
+ return -EPERM;
+ console_lock();
if (vc->vt_mode.mode != VT_PROCESS) {
+ console_unlock();
ret = -EINVAL;
break;
}
/*
* Switching-from response
*/
- console_lock();
if (vc->vt_newvt >= 0) {
if (arg == 0)
/*
@@ -768,7 +776,7 @@ int vt_ioctl(struct tty_struct *tty,
ushort ll,cc;
if (!perm)
- goto eperm;
+ return -EPERM;
if (get_user(ll, &vtsizes->v_rows) ||
get_user(cc, &vtsizes->v_cols))
ret = -EFAULT;
@@ -779,6 +787,7 @@ int vt_ioctl(struct tty_struct *tty,
if (vc) {
vc->vc_resize_user = 1;
+ /* FIXME: review v tty lock */
vc_resize(vc_cons[i].d, cc, ll);
}
}
@@ -792,7 +801,7 @@ int vt_ioctl(struct tty_struct *tty,
struct vt_consize __user *vtconsize = up;
ushort ll,cc,vlin,clin,vcol,ccol;
if (!perm)
- goto eperm;
+ return -EPERM;
if (!access_ok(VERIFY_READ, vtconsize,
sizeof(struct vt_consize))) {
ret = -EFAULT;
@@ -848,7 +857,7 @@ int vt_ioctl(struct tty_struct *tty,
case PIO_FONT: {
if (!perm)
- goto eperm;
+ return -EPERM;
op.op = KD_FONT_OP_SET;
op.flags = KD_FONT_FLAG_OLD | KD_FONT_FLAG_DONT_RECALC; /* Compatibility */
op.width = 8;
@@ -889,7 +898,7 @@ int vt_ioctl(struct tty_struct *tty,
case PIO_FONTRESET:
{
if (!perm)
- goto eperm;
+ return -EPERM;
#ifdef BROKEN_GRAPHICS_PROGRAMS
/* With BROKEN_GRAPHICS_PROGRAMS defined, the default
@@ -915,7 +924,7 @@ int vt_ioctl(struct tty_struct *tty,
break;
}
if (!perm && op.op != KD_FONT_OP_GET)
- goto eperm;
+ return -EPERM;
ret = con_font_op(vc, &op);
if (ret)
break;
@@ -927,50 +936,65 @@ int vt_ioctl(struct tty_struct *tty,
case PIO_SCRNMAP:
if (!perm)
ret = -EPERM;
- else
+ else {
+ tty_lock();
ret = con_set_trans_old(up);
+ tty_unlock();
+ }
break;
case GIO_SCRNMAP:
+ tty_lock();
ret = con_get_trans_old(up);
+ tty_unlock();
break;
case PIO_UNISCRNMAP:
if (!perm)
ret = -EPERM;
- else
+ else {
+ tty_lock();
ret = con_set_trans_new(up);
+ tty_unlock();
+ }
break;
case GIO_UNISCRNMAP:
+ tty_lock();
ret = con_get_trans_new(up);
+ tty_unlock();
break;
case PIO_UNIMAPCLR:
{ struct unimapinit ui;
if (!perm)
- goto eperm;
+ return -EPERM;
ret = copy_from_user(&ui, up, sizeof(struct unimapinit));
if (ret)
ret = -EFAULT;
- else
+ else {
+ tty_lock();
con_clear_unimap(vc, &ui);
+ tty_unlock();
+ }
break;
}
case PIO_UNIMAP:
case GIO_UNIMAP:
+ tty_lock();
ret = do_unimap_ioctl(cmd, up, perm, vc);
+ tty_unlock();
break;
case VT_LOCKSWITCH:
if (!capable(CAP_SYS_TTY_CONFIG))
- goto eperm;
+ return -EPERM;
vt_dont_switch = 1;
break;
case VT_UNLOCKSWITCH:
if (!capable(CAP_SYS_TTY_CONFIG))
- goto eperm;
+ return -EPERM;
vt_dont_switch = 0;
break;
case VT_GETHIFONTMASK:
@@ -984,11 +1008,7 @@ int vt_ioctl(struct tty_struct *tty,
ret = -ENOIOCTLCMD;
}
out:
- tty_unlock();
return ret;
-eperm:
- ret = -EPERM;
- goto out;
}
void reset_vc(struct vc_data *vc)
@@ -1150,8 +1170,6 @@ long vt_compat_ioctl(struct tty_struct *tty,
console = vc->vc_num;
- tty_lock();
-
if (!vc_cons_allocated(console)) { /* impossible? */
ret = -ENOIOCTLCMD;
goto out;
@@ -1180,7 +1198,9 @@ long vt_compat_ioctl(struct tty_struct *tty,
case PIO_UNIMAP:
case GIO_UNIMAP:
+ tty_lock();
ret = compat_unimap_ioctl(cmd, up, perm, vc);
+ tty_unlock();
break;
/*
@@ -1217,11 +1237,9 @@ long vt_compat_ioctl(struct tty_struct *tty,
goto fallback;
}
out:
- tty_unlock();
return ret;
fallback:
- tty_unlock();
return vt_ioctl(tty, cmd, arg);
}
@@ -1407,6 +1425,7 @@ int vt_move_to_console(unsigned int vt, int alloc)
return -EIO;
}
console_unlock();
+ /* Review: I don't see why we need tty_lock here FIXME */
tty_lock();
if (vt_waitactive(vt + 1)) {
pr_debug("Suspend: Can't switch VCs.");
next prev parent reply other threads:[~2012-03-01 19:36 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-03-01 19:49 [PATCH 0/6] Remove tty_lock from the console drivers Alan Cox
2012-03-01 19:49 ` [PATCH 1/6] vt: sort out locking for font handling Alan Cox
2012-03-01 19:50 ` Alan Cox [this message]
2012-03-01 20:31 ` [PATCH 2/6] vt: push down the tty lock so we can see what is left to tackle Jiri Slaby
2012-03-01 21:51 ` Arnd Bergmann
2012-03-01 19:51 ` [PATCH 3/6] vt: push down tioclinux cases Alan Cox
2012-03-01 19:51 ` [PATCH 4/6] vt: waitevent is self locked so drop the tty_lock Alan Cox
2012-03-01 19:52 ` [PATCH 5/6] vt: tackle the main part of the selection logic Alan Cox
2012-03-01 19:52 ` [PATCH 6/6] vt: push the tty_lock down into the map handling Alan Cox
2012-03-01 22:10 ` [PATCH 0/6] Remove tty_lock from the console drivers Arnd Bergmann
2012-03-02 14:58 [PATCH 0/6] Series short description Alan Cox
2012-03-02 14:59 ` [PATCH 2/6] vt: push down the tty lock so we can see what is left to tackle Alan Cox
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20120301195005.11322.78572.stgit@bob.linux.org.uk \
--to=alan@lxorguk.ukuu.org.uk \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-serial@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).