2017-03-13 22:09:58

by Okash Khawaja

[permalink] [raw]
Subject: [patch 1/7] tty_port: allow a port to be opened with a tty that has no file handle

Allow access to TTY device from kernel. This is based on Alan Cox's patch
(http://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1215095.html),
with description quoted below.

"tty_port: allow a port to be opened with a tty that has no file handle

Let us create tty objects entirely in kernel space.

With this a kernel created non file backed tty object could be used to handle
data, and set terminal modes. Not all ldiscs can cope with this as N_TTY in
particular has to work back to the fs/tty layer.

The tty_port code is however otherwise clean of file handles as far as I can
tell as is the low level tty port write path used by the ldisc, the
configuration low level interfaces and most of the ldiscs.

Currently you don't have any exposure to see tty hangups because those are
built around the file layer. However a) it's a fixed port so you probably
don't care about that b) if you do we can add a callback and c) you almost
certainly don't want the userspace tear down/rebuild behaviour anyway.

This should however be sufficient if we wanted for example to enumerate all
the bluetooth bound fixed ports via ACPI and make them directly available.

It doesn't deal with the case of a user opening a port that's also kernel
opened and that would need some locking out (so it returned EBUSY if bound
to a kernel device of some kind). That needs resolving along with how you
"up" or "down" your new bluetooth device, or enumerate it while providing
the existing tty API to avoid regressions (and to debug)."

Signed-off-by: Okash Khawaja <[email protected]>

Reviewed-by: Samuel Thibault <[email protected]>

Index: linux-4.10.1/drivers/tty/tty_io.c
===================================================================
--- linux-4.10.1.orig/drivers/tty/tty_io.c
+++ linux-4.10.1/drivers/tty/tty_io.c
@@ -855,7 +855,7 @@

int tty_hung_up_p(struct file *filp)
{
- return (filp->f_op == &hung_up_tty_fops);
+ return (filp && filp->f_op == &hung_up_tty_fops);
}

EXPORT_SYMBOL(tty_hung_up_p);
@@ -1368,7 +1368,10 @@
struct tty_struct *tty;

if (driver->ops->lookup)
- tty = driver->ops->lookup(driver, file, idx);
+ if (!file)
+ tty = ERR_PTR(-EIO);
+ else
+ tty = driver->ops->lookup(driver, file, idx);
else
tty = driver->ttys[idx];

@@ -1986,7 +1989,7 @@
struct tty_driver *console_driver = console_device(index);
if (console_driver) {
driver = tty_driver_kref_get(console_driver);
- if (driver) {
+ if (driver && filp) {
/* Don't let /dev/console block */
filp->f_flags |= O_NONBLOCK;
break;
@@ -2019,7 +2022,7 @@
* - concurrent tty driver removal w/ lookup
* - concurrent tty removal from driver table
*/
-static struct tty_struct *tty_open_by_driver(dev_t device, struct inode *inode,
+struct tty_struct *tty_open_by_driver(dev_t device, struct inode *inode,
struct file *filp)
{
struct tty_struct *tty;
@@ -2064,6 +2067,7 @@
tty_driver_kref_put(driver);
return tty;
}
+EXPORT_SYMBOL(tty_open_by_driver);

/**
* tty_open - open a tty device
Index: linux-4.10.1/drivers/tty/tty_port.c
===================================================================
--- linux-4.10.1.orig/drivers/tty/tty_port.c
+++ linux-4.10.1/drivers/tty/tty_port.c
@@ -335,7 +335,7 @@
* tty_port_block_til_ready - Waiting logic for tty open
* @port: the tty port being opened
* @tty: the tty device being bound
- * @filp: the file pointer of the opener
+ * @filp: the file pointer of the opener or NULL
*
* Implement the core POSIX/SuS tty behaviour when opening a tty device.
* Handles:
@@ -369,7 +369,7 @@
tty_port_set_active(port, 1);
return 0;
}
- if (filp->f_flags & O_NONBLOCK) {
+ if (filp == NULL || filp->f_flags & O_NONBLOCK) {
/* Indicate we are open */
if (C_BAUD(tty))
tty_port_raise_dtr_rts(port);
Index: linux-4.10.1/include/linux/tty.h
===================================================================
--- linux-4.10.1.orig/include/linux/tty.h
+++ linux-4.10.1/include/linux/tty.h
@@ -394,6 +394,8 @@
/* tty_io.c */
extern int __init tty_init(void);
extern const char *tty_name(const struct tty_struct *tty);
+extern struct tty_struct *tty_open_by_driver(dev_t device, struct inode *inode,
+ struct file *filp);
#else
static inline void console_init(void)
{ }


2017-03-13 22:13:08

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [patch 1/7] tty_port: allow a port to be opened with a tty that has no file handle

On Mon, Mar 13, 2017 at 10:05:52PM +0000, [email protected] wrote:
> Allow access to TTY device from kernel. This is based on Alan Cox's patch
> (http://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1215095.html),
> with description quoted below.
>
> "tty_port: allow a port to be opened with a tty that has no file handle
>
> Let us create tty objects entirely in kernel space.
>
> With this a kernel created non file backed tty object could be used to handle
> data, and set terminal modes. Not all ldiscs can cope with this as N_TTY in
> particular has to work back to the fs/tty layer.
>
> The tty_port code is however otherwise clean of file handles as far as I can
> tell as is the low level tty port write path used by the ldisc, the
> configuration low level interfaces and most of the ldiscs.
>
> Currently you don't have any exposure to see tty hangups because those are
> built around the file layer. However a) it's a fixed port so you probably
> don't care about that b) if you do we can add a callback and c) you almost
> certainly don't want the userspace tear down/rebuild behaviour anyway.
>
> This should however be sufficient if we wanted for example to enumerate all
> the bluetooth bound fixed ports via ACPI and make them directly available.
>
> It doesn't deal with the case of a user opening a port that's also kernel
> opened and that would need some locking out (so it returned EBUSY if bound
> to a kernel device of some kind). That needs resolving along with how you
> "up" or "down" your new bluetooth device, or enumerate it while providing
> the existing tty API to avoid regressions (and to debug)."
>
> Signed-off-by: Okash Khawaja <[email protected]>
>
> Reviewed-by: Samuel Thibault <[email protected]>

You do know this is already in 4.11-rc1, right? Please rebase your
patch set on 4.11-rc2 at the least and resend.

thanks,

greg k-h

2017-03-13 22:38:59

by Okash Khawaja

[permalink] [raw]
Subject: Re: [patch 1/7] tty_port: allow a port to be opened with a tty that has no file handle

On Tue, Mar 14, 2017 at 06:12:47AM +0800, Greg Kroah-Hartman wrote:
> On Mon, Mar 13, 2017 at 10:05:52PM +0000, [email protected] wrote:
> > Allow access to TTY device from kernel. This is based on Alan Cox's patch
> > (http://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1215095.html),
> > with description quoted below.
> >
> > "tty_port: allow a port to be opened with a tty that has no file handle
> >
> > Let us create tty objects entirely in kernel space.
> >
> > With this a kernel created non file backed tty object could be used to handle
> > data, and set terminal modes. Not all ldiscs can cope with this as N_TTY in
> > particular has to work back to the fs/tty layer.
> >
> > The tty_port code is however otherwise clean of file handles as far as I can
> > tell as is the low level tty port write path used by the ldisc, the
> > configuration low level interfaces and most of the ldiscs.
> >
> > Currently you don't have any exposure to see tty hangups because those are
> > built around the file layer. However a) it's a fixed port so you probably
> > don't care about that b) if you do we can add a callback and c) you almost
> > certainly don't want the userspace tear down/rebuild behaviour anyway.
> >
> > This should however be sufficient if we wanted for example to enumerate all
> > the bluetooth bound fixed ports via ACPI and make them directly available.
> >
> > It doesn't deal with the case of a user opening a port that's also kernel
> > opened and that would need some locking out (so it returned EBUSY if bound
> > to a kernel device of some kind). That needs resolving along with how you
> > "up" or "down" your new bluetooth device, or enumerate it while providing
> > the existing tty API to avoid regressions (and to debug)."
> >
> > Signed-off-by: Okash Khawaja <[email protected]>
> >
> > Reviewed-by: Samuel Thibault <[email protected]>
>
> You do know this is already in 4.11-rc1, right? Please rebase your
> patch set on 4.11-rc2 at the least and resend.

Didn't realise that! Will resend

2017-03-14 09:14:43

by Dan Carpenter

[permalink] [raw]
Subject: Re: [patch 1/7] tty_port: allow a port to be opened with a tty that has no file handle

Could you fix your From header in your email client?

regards,
dan carpenter