linux-kernel - Re: [RFC] tty: Add get- ioctls to fetch tty status

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20120922180639.GC11610@moon>
Date:	Sat, 22 Sep 2012 22:06:39 +0400
From:	Cyrill Gorcunov <gorcunov@...nvz.org>
To:	Alan Cox <alan@...rguk.ukuu.org.uk>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	"H. Peter Anvin" <hpa@...or.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Pavel Emelyanov <xemul@...allels.com>,
	Jiri Slaby <jslaby@...e.cz>
Subject: Re: [RFC] tty: Add get- ioctls to fetch tty status

On Thu, Sep 13, 2012 at 05:25:07PM +0100, Alan Cox wrote:
> On Thu, 13 Sep 2012 16:54:01 +0400
> Cyrill Gorcunov <gorcunov@...nvz.org> wrote:
> 
> > On Thu, Sep 13, 2012 at 01:51:31PM +0100, Alan Cox wrote:
> > > > +static int pty_get_lock(struct tty_struct *tty, int __user *arg)
> > > > +{
> > > > +	int locked = test_bit(TTY_PTY_LOCK, &tty->flags);
> > > > +	if (put_user(locked, arg))
> > > > +		return -EFAULT;
> > > 
> > > Now explain exactly how this doesn't race with another thread chanigng
> > > the lock setting ?
> > 
> > It's the same as to set/clear this bit, isn't it? Please correct me
> > if I'm wrong.
> 
> So by the time you've finished the test bit and returned it to user space
> the answer may have changed ?
> 
> > > The other comment I have is that it might be better put these in now
> > > there are sysfs patches for the tty layer bouncing about to provide the
> > > needed infrastructure ?
> > 
> > Alan, could you please point me where these patches are living, so I would
> > take a look and check them out
> 
> linux-serial or check tty-next as I think Greg has now merged the test
> patch.

Guys, you mean something like below? Look, I must admit I'm not really
sure if I've done all locking right, and there is no need for additional
kref counting on tty_struct. Could you please check if it looks more-less
sane (I've tested it but still...)

---
From: Cyrill Gorcunov <gorcunov@...nvz.org>
Subject: tty, pty: Add pty_state attribute to fetch tty flags

For checkpoint/restore we need to know if tty has
exclusive or packet mode set, as well as if pty
is currently locked (just to be able to restore
this characteristics).

To serve this the pty_state attribute is introduced
for pty devices. A typical output looks like

 | [root@...tune ~]# cat /sys/devices/virtual/tty/ptmp0/pty_state
 | locked: 0 exclusive: 0 packet: 0

Signed-off-by: Cyrill Gorcunov <gorcunov@...nvz.org>
CC: Alan Cox <alan@...rguk.ukuu.org.uk>
CC: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
CC: "H. Peter Anvin" <hpa@...or.com>
CC: Jiri Slaby <jslaby@...e.cz>
CC: Pavel Emelyanov <xemul@...allels.com>
---
 drivers/tty/pty.c |   45 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 44 insertions(+), 1 deletion(-)

Index: tty.git/drivers/tty/pty.c
===================================================================
--- tty.git.orig/drivers/tty/pty.c
+++ tty.git/drivers/tty/pty.c
@@ -283,6 +283,46 @@ done:
 	return 0;
 }
 
+static ssize_t pty_show_state(struct device *dev,
+			      struct device_attribute *attr,
+			      char *buf)
+{
+	struct tty_struct *tty = dev_get_drvdata(dev);
+	int locked, exclusive, packet;
+
+	tty_lock(tty);
+	locked = test_bit(TTY_PTY_LOCK, &tty->flags);
+	exclusive = test_bit(TTY_EXCLUSIVE, &tty->flags);
+	packet = tty->packet;
+	tty_unlock(tty);
+
+	return snprintf(buf, PAGE_SIZE, "locked: %d exclusive: %d packet: %d\n",
+			locked, exclusive, packet);
+}
+
+static DEVICE_ATTR(pty_state, S_IRUSR | S_IRGRP, pty_show_state, NULL);
+
+static struct attribute *pty_state_dev_attrs[] = {
+	&dev_attr_pty_state.attr,
+	NULL,
+};
+
+static const struct attribute_group pty_dev_attr_group = {
+	.attrs = pty_state_dev_attrs,
+};
+
+static const struct attribute_group *pty_dev_attr_groups[] = {
+	&pty_dev_attr_group,
+	NULL,
+};
+
+static int pty_register_attr(struct tty_driver *driver, struct tty_struct *tty)
+{
+	struct device *dev = tty_register_device_attr(driver, tty->index, tty->dev,
+						      (void *)tty, pty_dev_attr_groups);
+	return PTR_RET(dev);
+}
+
 /**
  *	pty_common_install		-	set up the pty pair
  *	@driver: the pty driver
@@ -351,7 +391,9 @@ static int pty_common_install(struct tty
 
 	tty_driver_kref_get(driver);
 	tty->count++;
-	return 0;
+
+	return pty_register_attr(driver, tty);
+
 err_free_termios:
 	if (legacy)
 		tty_free_termios(tty);
@@ -368,6 +410,7 @@ err:
 
 static void pty_cleanup(struct tty_struct *tty)
 {
+	tty_unregister_device(tty->driver, tty->index);
 	kfree(tty->port);
 }
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/