Add compat_ioctl method for tty code to allow processing
of 32 bit ioctl calls on 64 bit systems by tty core,
tty drivers, and line disciplines.
Based on patch by Arnd Bergmann:
http://www.uwsg.iu.edu/hypermail/linux/kernel/0511.0/1732.html
Signed-off-by: Paul Fulghum <[email protected]>
CC: Arnd Bergmann <[email protected]>
--- a/drivers/char/n_tty.c 2007-04-25 22:08:32.000000000 -0500
+++ b/drivers/char/n_tty.c 2007-05-01 11:00:47.000000000 -0500
@@ -1544,21 +1544,18 @@ static unsigned int normal_poll(struct t
}
struct tty_ldisc tty_ldisc_N_TTY = {
- TTY_LDISC_MAGIC, /* magic */
- "n_tty", /* name */
- 0, /* num */
- 0, /* flags */
- n_tty_open, /* open */
- n_tty_close, /* close */
- n_tty_flush_buffer, /* flush_buffer */
- n_tty_chars_in_buffer, /* chars_in_buffer */
- read_chan, /* read */
- write_chan, /* write */
- n_tty_ioctl, /* ioctl */
- n_tty_set_termios, /* set_termios */
- normal_poll, /* poll */
- NULL, /* hangup */
- n_tty_receive_buf, /* receive_buf */
- n_tty_write_wakeup /* write_wakeup */
+ .magic = TTY_LDISC_MAGIC,
+ .name = "n_tty",
+ .open = n_tty_open,
+ .close = n_tty_close,
+ .flush_buffer = n_tty_flush_buffer,
+ .chars_in_buffer = n_tty_chars_in_buffer,
+ .read = read_chan,
+ .write = write_chan,
+ .ioctl = n_tty_ioctl,
+ .set_termios = n_tty_set_termios,
+ .poll = normal_poll,
+ .receive_buf = n_tty_receive_buf,
+ .write_wakeup = n_tty_write_wakeup
};
--- a/include/linux/tty_driver.h 2007-04-25 22:08:32.000000000 -0500
+++ b/include/linux/tty_driver.h 2007-05-03 10:03:52.000000000 -0500
@@ -52,6 +52,11 @@
* This routine allows the tty driver to implement
* device-specific ioctl's. If the ioctl number passed in cmd
* is not recognized by the driver, it should return ENOIOCTLCMD.
+ *
+ * long (*compat_ioctl)(struct tty_struct *tty, struct file * file,
+ * unsigned int cmd, unsigned long arg);
+ *
+ * implement ioctl processing for 32 bit process on 64 bit system
*
* void (*set_termios)(struct tty_struct *tty, struct ktermios * old);
*
@@ -132,6 +137,8 @@ struct tty_operations {
int (*chars_in_buffer)(struct tty_struct *tty);
int (*ioctl)(struct tty_struct *tty, struct file * file,
unsigned int cmd, unsigned long arg);
+ long (*compat_ioctl)(struct tty_struct *tty, struct file * file,
+ unsigned int cmd, unsigned long arg);
void (*set_termios)(struct tty_struct *tty, struct ktermios * old);
void (*throttle)(struct tty_struct * tty);
void (*unthrottle)(struct tty_struct * tty);
@@ -193,6 +200,8 @@ struct tty_driver {
int (*chars_in_buffer)(struct tty_struct *tty);
int (*ioctl)(struct tty_struct *tty, struct file * file,
unsigned int cmd, unsigned long arg);
+ long (*compat_ioctl)(struct tty_struct *tty, struct file * file,
+ unsigned int cmd, unsigned long arg);
void (*set_termios)(struct tty_struct *tty, struct ktermios * old);
void (*throttle)(struct tty_struct * tty);
void (*unthrottle)(struct tty_struct * tty);
--- a/drivers/char/tty_io.c 2007-04-25 22:08:32.000000000 -0500
+++ b/drivers/char/tty_io.c 2007-05-03 10:42:35.000000000 -0500
@@ -153,6 +153,11 @@ static int tty_open(struct inode *, stru
static int tty_release(struct inode *, struct file *);
int tty_ioctl(struct inode * inode, struct file * file,
unsigned int cmd, unsigned long arg);
+#ifdef CONFIG_COMPAT
+long tty_compat_ioctl(struct file * file, unsigned int cmd, unsigned long arg);
+#else
+#define tty_compat_ioctl NULL
+#endif
static int tty_fasync(int fd, struct file * filp, int on);
static void release_tty(struct tty_struct *tty, int idx);
static struct pid *__proc_set_tty(struct task_struct *tsk,
@@ -1145,8 +1150,8 @@ static unsigned int hung_up_tty_poll(str
return POLLIN | POLLOUT | POLLERR | POLLHUP | POLLRDNORM | POLLWRNORM;
}
-static int hung_up_tty_ioctl(struct inode * inode, struct file * file,
- unsigned int cmd, unsigned long arg)
+static long hung_up_tty_ioctl(struct file * file,
+ unsigned int cmd, unsigned long arg)
{
return cmd == TIOCSPGRP ? -ENOTTY : -EIO;
}
@@ -1157,6 +1162,7 @@ static const struct file_operations tty_
.write = tty_write,
.poll = tty_poll,
.ioctl = tty_ioctl,
+ .compat_ioctl = tty_compat_ioctl,
.open = tty_open,
.release = tty_release,
.fasync = tty_fasync,
@@ -1169,6 +1175,7 @@ static const struct file_operations ptmx
.write = tty_write,
.poll = tty_poll,
.ioctl = tty_ioctl,
+ .compat_ioctl = tty_compat_ioctl,
.open = ptmx_open,
.release = tty_release,
.fasync = tty_fasync,
@@ -1181,6 +1188,7 @@ static const struct file_operations cons
.write = redirected_tty_write,
.poll = tty_poll,
.ioctl = tty_ioctl,
+ .compat_ioctl = tty_compat_ioctl,
.open = tty_open,
.release = tty_release,
.fasync = tty_fasync,
@@ -1191,7 +1199,8 @@ static const struct file_operations hung
.read = hung_up_tty_read,
.write = hung_up_tty_write,
.poll = hung_up_tty_poll,
- .ioctl = hung_up_tty_ioctl,
+ .unlocked_ioctl = hung_up_tty_ioctl,
+ .compat_ioctl = hung_up_tty_ioctl,
.release = tty_release,
};
@@ -3353,6 +3362,31 @@ int tty_ioctl(struct inode * inode, stru
return retval;
}
+#ifdef CONFIG_COMPAT
+long tty_compat_ioctl(struct file * file, unsigned int cmd, unsigned long arg)
+{
+ struct inode *inode = file->f_dentry->d_inode;
+ struct tty_struct *tty = file->private_data;
+ struct tty_ldisc *ld;
+ int retval = -ENOIOCTLCMD;
+
+ if (tty_paranoia_check(tty, inode, "tty_ioctl"))
+ return -EINVAL;
+
+ if (tty->driver->compat_ioctl) {
+ retval = (tty->driver->compat_ioctl)(tty, file, cmd, arg);
+ if (retval != -ENOIOCTLCMD)
+ return retval;
+ }
+
+ ld = tty_ldisc_ref_wait(tty);
+ if (ld->compat_ioctl)
+ retval = ld->compat_ioctl(tty, file, cmd, arg);
+ tty_ldisc_deref(ld);
+
+ return retval;
+}
+#endif
/*
* This implements the "Secure Attention Key" --- the idea is to
@@ -3685,6 +3719,7 @@ void tty_set_operations(struct tty_drive
driver->write_room = op->write_room;
driver->chars_in_buffer = op->chars_in_buffer;
driver->ioctl = op->ioctl;
+ driver->compat_ioctl = op->compat_ioctl;
driver->set_termios = op->set_termios;
driver->throttle = op->throttle;
driver->unthrottle = op->unthrottle;
--- a/include/linux/tty_ldisc.h 2007-04-25 22:08:32.000000000 -0500
+++ b/include/linux/tty_ldisc.h 2007-05-03 10:53:30.000000000 -0500
@@ -59,6 +59,11 @@
* low-level driver can "grab" an ioctl request before the line
* discpline has a chance to see it.
*
+ * long (*compat_ioctl)(struct tty_struct * tty, struct file * file,
+ * unsigned int cmd, unsigned long arg);
+ *
+ * Process ioctl calls from 32-bit process on 64-bit system
+ *
* void (*set_termios)(struct tty_struct *tty, struct ktermios * old);
*
* This function notifies the line discpline that a change has
@@ -118,6 +123,8 @@ struct tty_ldisc {
const unsigned char * buf, size_t nr);
int (*ioctl)(struct tty_struct * tty, struct file * file,
unsigned int cmd, unsigned long arg);
+ long (*compat_ioctl)(struct tty_struct * tty, struct file * file,
+ unsigned int cmd, unsigned long arg);
void (*set_termios)(struct tty_struct *tty, struct ktermios * old);
unsigned int (*poll)(struct tty_struct *, struct file *,
struct poll_table_struct *);
Arnd Bergmann wrote:
> - The return value of the new compat_ioctl methods should probably
> 'int', not 'long'. We've had the discussion before and then
> decided not to change the existing compat_ioctl and
> unlocked_ioctl functions -- even though int is more appropriate,
> but having the same prototype has the advantage that a driver
> can use the same function for both ->ioctl and ->compat_ioctl
> if all calls are compatible.
I noticed that but thought the change in return value type
had some higher purpose I had not perceived. If it can be int
that would be the way to go.
> - In your driver you don't get the big kernel lock in the
> compat_ioctl function. I assume that this is correct for
> the particular driver, but it may be nice if you could
> consequently also add an unlocked_ioctl function that can
> be used without the BKL for native ioctls. It would be good
> to hear an opinon on this from someone who has an insight
> in tty locking issues though, so I'm Cc:ing some people
> who have touched that recently.
I don't count on higher level locking for
synchronization issues specific to the driver.
I thought the current compat_ioctl() was already
meant to *not* have the BKL just like unlocked_ioctl.
My thought was that any driver getting a recent update
like compat_ioctl() would need to be reviewed for BKL
safety and take the lock manually if necessary.
Drivers that are falling behind wont have a compat_ioctl
defined at all.
--
Paul
Paul Fulghum wrote:
> Arnd Bergmann wrote:
>> - In your driver you don't get the big kernel lock in the
>> compat_ioctl function. I assume that this is correct for
>> the particular driver, but it may be nice if you could
>> consequently also add an unlocked_ioctl function that can
>> be used without the BKL for native ioctls. It would be good
>> to hear an opinon on this from someone who has an insight
>> in tty locking issues though, so I'm Cc:ing some people
>> who have touched that recently.
>
> I don't count on higher level locking for
> synchronization issues specific to the driver.
>
> I thought the current compat_ioctl() was already
> meant to *not* have the BKL just like unlocked_ioctl.
> My thought was that any driver getting a recent update
> like compat_ioctl() would need to be reviewed for BKL
> safety and take the lock manually if necessary.
Nevermind. I misread what you wrote (I'm tired).
Yes, adding an unlocked_ioctl() makes sense.
--
Paul
Arnd Bergmann wrote:
> - The return value of the new compat_ioctl methods should probably
> 'int', not 'long'. We've had the discussion before and then
> decided not to change the existing compat_ioctl and
> unlocked_ioctl functions -- even though int is more appropriate,
> but having the same prototype has the advantage that a driver
> can use the same function for both ->ioctl and ->compat_ioctl
> if all calls are compatible.
Any comments on this or should I assume that
compat_ioctl() will stay with ulong return type?
--
Paul Fulghum
Microgate Systems, Ltd.