* [PATCH v4] tty: rocket: Remove the rp_ioctl
@ 2019-04-16 14:53 Fuqian Huang
2019-04-17 6:27 ` Jiri Slaby
0 siblings, 1 reply; 5+ messages in thread
From: Fuqian Huang @ 2019-04-16 14:53 UTC (permalink / raw)
Cc: Fuqian Huang, Greg Kroah-Hartman, Jiri Slaby, linux-kernel
If the cmd is RCPK_GET_STRUCT, copy_to_user will copy
info to user space. As info->port.ops is the address of
a constant object rocket_port_ops (assigned in init_r_port),
a kernel address leakage happens.
The rp_ioctl seems will be not used by anybody.
Delete the rp_ioctl completely.
Signed-off-by: Fuqian Huang <huangfq.daxian@gmail.com>
---
drivers/tty/rocket.c | 154 +--------------------------------------------------
drivers/tty/rocket.h | 10 ----
2 files changed, 1 insertion(+), 163 deletions(-)
diff --git a/drivers/tty/rocket.c b/drivers/tty/rocket.c
index b121d8f8f3d7..e3b4e8bc2d7a 100644
--- a/drivers/tty/rocket.c
+++ b/drivers/tty/rocket.c
@@ -90,10 +90,6 @@ static void rp_do_poll(struct timer_list *unused);
static struct tty_driver *rocket_driver;
-static struct rocket_version driver_version = {
- ROCKET_VERSION, ROCKET_DATE
-};
-
static struct r_port *rp_table[MAX_RP_PORTS]; /* The main repository of serial port state information. */
static unsigned int xmit_flags[NUM_BOARDS]; /* Bit significant, indicates port had data to transmit. */
/* eg. Bit 0 indicates port 0 has xmit data, ... */
@@ -1160,154 +1156,6 @@ static int rp_tiocmset(struct tty_struct *tty,
return 0;
}
-static int get_config(struct r_port *info, struct rocket_config __user *retinfo)
-{
- struct rocket_config tmp;
-
- memset(&tmp, 0, sizeof (tmp));
- mutex_lock(&info->port.mutex);
- tmp.line = info->line;
- tmp.flags = info->flags;
- tmp.close_delay = info->port.close_delay;
- tmp.closing_wait = info->port.closing_wait;
- tmp.port = rcktpt_io_addr[(info->line >> 5) & 3];
- mutex_unlock(&info->port.mutex);
-
- if (copy_to_user(retinfo, &tmp, sizeof (*retinfo)))
- return -EFAULT;
- return 0;
-}
-
-static int set_config(struct tty_struct *tty, struct r_port *info,
- struct rocket_config __user *new_info)
-{
- struct rocket_config new_serial;
-
- if (copy_from_user(&new_serial, new_info, sizeof (new_serial)))
- return -EFAULT;
-
- mutex_lock(&info->port.mutex);
- if (!capable(CAP_SYS_ADMIN))
- {
- if ((new_serial.flags & ~ROCKET_USR_MASK) != (info->flags & ~ROCKET_USR_MASK)) {
- mutex_unlock(&info->port.mutex);
- return -EPERM;
- }
- info->flags = ((info->flags & ~ROCKET_USR_MASK) | (new_serial.flags & ROCKET_USR_MASK));
- mutex_unlock(&info->port.mutex);
- return 0;
- }
-
- if ((new_serial.flags ^ info->flags) & ROCKET_SPD_MASK) {
- /* warn about deprecation, unless clearing */
- if (new_serial.flags & ROCKET_SPD_MASK)
- dev_warn_ratelimited(tty->dev, "use of SPD flags is deprecated\n");
- }
-
- info->flags = ((info->flags & ~ROCKET_FLAGS) | (new_serial.flags & ROCKET_FLAGS));
- info->port.close_delay = new_serial.close_delay;
- info->port.closing_wait = new_serial.closing_wait;
-
- mutex_unlock(&info->port.mutex);
-
- configure_r_port(tty, info, NULL);
- return 0;
-}
-
-/*
- * This function fills in a rocket_ports struct with information
- * about what boards/ports are in the system. This info is passed
- * to user space. See setrocket.c where the info is used to create
- * the /dev/ttyRx ports.
- */
-static int get_ports(struct r_port *info, struct rocket_ports __user *retports)
-{
- struct rocket_ports tmp;
- int board;
-
- memset(&tmp, 0, sizeof (tmp));
- tmp.tty_major = rocket_driver->major;
-
- for (board = 0; board < 4; board++) {
- tmp.rocketModel[board].model = rocketModel[board].model;
- strcpy(tmp.rocketModel[board].modelString, rocketModel[board].modelString);
- tmp.rocketModel[board].numPorts = rocketModel[board].numPorts;
- tmp.rocketModel[board].loadrm2 = rocketModel[board].loadrm2;
- tmp.rocketModel[board].startingPortNumber = rocketModel[board].startingPortNumber;
- }
- if (copy_to_user(retports, &tmp, sizeof (*retports)))
- return -EFAULT;
- return 0;
-}
-
-static int reset_rm2(struct r_port *info, void __user *arg)
-{
- int reset;
-
- if (!capable(CAP_SYS_ADMIN))
- return -EPERM;
-
- if (copy_from_user(&reset, arg, sizeof (int)))
- return -EFAULT;
- if (reset)
- reset = 1;
-
- if (rcktpt_type[info->board] != ROCKET_TYPE_MODEMII &&
- rcktpt_type[info->board] != ROCKET_TYPE_MODEMIII)
- return -EINVAL;
-
- if (info->ctlp->BusType == isISA)
- sModemReset(info->ctlp, info->chan, reset);
- else
- sPCIModemReset(info->ctlp, info->chan, reset);
-
- return 0;
-}
-
-static int get_version(struct r_port *info, struct rocket_version __user *retvers)
-{
- if (copy_to_user(retvers, &driver_version, sizeof (*retvers)))
- return -EFAULT;
- return 0;
-}
-
-/* IOCTL call handler into the driver */
-static int rp_ioctl(struct tty_struct *tty,
- unsigned int cmd, unsigned long arg)
-{
- struct r_port *info = tty->driver_data;
- void __user *argp = (void __user *)arg;
- int ret = 0;
-
- if (cmd != RCKP_GET_PORTS && rocket_paranoia_check(info, "rp_ioctl"))
- return -ENXIO;
-
- switch (cmd) {
- case RCKP_GET_STRUCT:
- if (copy_to_user(argp, info, sizeof (struct r_port)))
- ret = -EFAULT;
- break;
- case RCKP_GET_CONFIG:
- ret = get_config(info, argp);
- break;
- case RCKP_SET_CONFIG:
- ret = set_config(tty, info, argp);
- break;
- case RCKP_GET_PORTS:
- ret = get_ports(info, argp);
- break;
- case RCKP_RESET_RM2:
- ret = reset_rm2(info, argp);
- break;
- case RCKP_GET_VERSION:
- ret = get_version(info, argp);
- break;
- default:
- ret = -ENOIOCTLCMD;
- }
- return ret;
-}
-
static void rp_send_xchar(struct tty_struct *tty, char ch)
{
struct r_port *info = tty->driver_data;
@@ -2305,7 +2153,7 @@ static const struct tty_operations rocket_ops = {
.write_room = rp_write_room,
.chars_in_buffer = rp_chars_in_buffer,
.flush_buffer = rp_flush_buffer,
- .ioctl = rp_ioctl,
+ .ioctl = NULL,
.throttle = rp_throttle,
.unthrottle = rp_unthrottle,
.set_termios = rp_set_termios,
diff --git a/drivers/tty/rocket.h b/drivers/tty/rocket.h
index d0560203f215..98d20475365b 100644
--- a/drivers/tty/rocket.h
+++ b/drivers/tty/rocket.h
@@ -68,16 +68,6 @@ struct rocket_version {
#define ROCKET_CLOSING_WAIT_NONE ASYNC_CLOSING_WAIT_NONE
#define ROCKET_CLOSING_WAIT_INF ASYNC_CLOSING_WAIT_INF
-/*
- * Rocketport ioctls -- "RP"
- */
-#define RCKP_GET_STRUCT 0x00525001
-#define RCKP_GET_CONFIG 0x00525002
-#define RCKP_SET_CONFIG 0x00525003
-#define RCKP_GET_PORTS 0x00525004
-#define RCKP_RESET_RM2 0x00525005
-#define RCKP_GET_VERSION 0x00525006
-
/* Rocketport Models */
#define MODEL_RP32INTF 0x0001 /* RP 32 port w/external I/F */
#define MODEL_RP8INTF 0x0002 /* RP 8 port w/external I/F */
--
2.11.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v4] tty: rocket: Remove the rp_ioctl
2019-04-16 14:53 [PATCH v4] tty: rocket: Remove the rp_ioctl Fuqian Huang
@ 2019-04-17 6:27 ` Jiri Slaby
2019-04-17 6:46 ` Greg Kroah-Hartman
0 siblings, 1 reply; 5+ messages in thread
From: Jiri Slaby @ 2019-04-17 6:27 UTC (permalink / raw)
To: Fuqian Huang; +Cc: Greg Kroah-Hartman, linux-kernel
On 16. 04. 19, 16:53, Fuqian Huang wrote:
> --- a/drivers/tty/rocket.c
> +++ b/drivers/tty/rocket.c
...
> -/* IOCTL call handler into the driver */
> -static int rp_ioctl(struct tty_struct *tty,
> - unsigned int cmd, unsigned long arg)
> -{
> - struct r_port *info = tty->driver_data;
> - void __user *argp = (void __user *)arg;
> - int ret = 0;
> -
> - if (cmd != RCKP_GET_PORTS && rocket_paranoia_check(info, "rp_ioctl"))
> - return -ENXIO;
> -
> - switch (cmd) {
> - case RCKP_GET_STRUCT:
> - if (copy_to_user(argp, info, sizeof (struct r_port)))
> - ret = -EFAULT;
> - break;
I am not sure what Greg meant, but I had only this particular ioctl on
mind when I was writing about removal. The other ones might be used _in
theory_. Even though they are not exposed to userspace either. So if
they are not source of potential issues, I would keep them:
> - case RCKP_GET_CONFIG:
> - ret = get_config(info, argp);
> - break;
> - case RCKP_SET_CONFIG:
> - ret = set_config(tty, info, argp);
> - break;
> - case RCKP_GET_PORTS:
> - ret = get_ports(info, argp);
> - break;
> - case RCKP_RESET_RM2:
> - ret = reset_rm2(info, argp);
> - break;
> - case RCKP_GET_VERSION:
> - ret = get_version(info, argp);
> - break;
> - default:
> - ret = -ENOIOCTLCMD;
> - }
> - return ret;
> -}
thanks,
--
js
suse labs
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v4] tty: rocket: Remove the rp_ioctl
2019-04-17 6:27 ` Jiri Slaby
@ 2019-04-17 6:46 ` Greg Kroah-Hartman
2019-04-17 6:52 ` Jiri Slaby
0 siblings, 1 reply; 5+ messages in thread
From: Greg Kroah-Hartman @ 2019-04-17 6:46 UTC (permalink / raw)
To: Jiri Slaby; +Cc: Fuqian Huang, linux-kernel
On Wed, Apr 17, 2019 at 08:27:41AM +0200, Jiri Slaby wrote:
> On 16. 04. 19, 16:53, Fuqian Huang wrote:
> > --- a/drivers/tty/rocket.c
> > +++ b/drivers/tty/rocket.c
> ...
> > -/* IOCTL call handler into the driver */
> > -static int rp_ioctl(struct tty_struct *tty,
> > - unsigned int cmd, unsigned long arg)
> > -{
> > - struct r_port *info = tty->driver_data;
> > - void __user *argp = (void __user *)arg;
> > - int ret = 0;
> > -
> > - if (cmd != RCKP_GET_PORTS && rocket_paranoia_check(info, "rp_ioctl"))
> > - return -ENXIO;
> > -
> > - switch (cmd) {
> > - case RCKP_GET_STRUCT:
> > - if (copy_to_user(argp, info, sizeof (struct r_port)))
> > - ret = -EFAULT;
> > - break;
>
> I am not sure what Greg meant, but I had only this particular ioctl on
> mind when I was writing about removal. The other ones might be used _in
> theory_. Even though they are not exposed to userspace either. So if
> they are not source of potential issues, I would keep them:
I was just meaning that one specific ioctl, not the whole thing.
Although dropping all of these custom ioctls is probably the right thing
to do in the long-run, for now, let's just get rid of the problem one.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v4] tty: rocket: Remove the rp_ioctl
2019-04-17 6:46 ` Greg Kroah-Hartman
@ 2019-04-17 6:52 ` Jiri Slaby
2019-04-17 6:56 ` Jiri Slaby
0 siblings, 1 reply; 5+ messages in thread
From: Jiri Slaby @ 2019-04-17 6:52 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Fuqian Huang, linux-kernel
On 17. 04. 19, 8:46, Greg Kroah-Hartman wrote:
> On Wed, Apr 17, 2019 at 08:27:41AM +0200, Jiri Slaby wrote:
>> On 16. 04. 19, 16:53, Fuqian Huang wrote:
>>> --- a/drivers/tty/rocket.c
>>> +++ b/drivers/tty/rocket.c
>> ...
>>> -/* IOCTL call handler into the driver */
>>> -static int rp_ioctl(struct tty_struct *tty,
>>> - unsigned int cmd, unsigned long arg)
>>> -{
>>> - struct r_port *info = tty->driver_data;
>>> - void __user *argp = (void __user *)arg;
>>> - int ret = 0;
>>> -
>>> - if (cmd != RCKP_GET_PORTS && rocket_paranoia_check(info, "rp_ioctl"))
>>> - return -ENXIO;
>>> -
>>> - switch (cmd) {
>>> - case RCKP_GET_STRUCT:
>>> - if (copy_to_user(argp, info, sizeof (struct r_port)))
>>> - ret = -EFAULT;
>>> - break;
>>
>> I am not sure what Greg meant, but I had only this particular ioctl on
>> mind when I was writing about removal. The other ones might be used _in
>> theory_. Even though they are not exposed to userspace either. So if
>> they are not source of potential issues, I would keep them:
>
> I was just meaning that one specific ioctl, not the whole thing.
>
> Although dropping all of these custom ioctls is probably the right thing
> to do in the long-run, for now, let's just get rid of the problem one.
So could you rework the patch to remove only the one? And in a separate
patch, introduce a printk_ratelimited warning to every "case *:" (i.e.
no warning in the "default" case) in rp_ioctl about depreciation of the
ioctl?
thanks,
--
js
suse labs
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v4] tty: rocket: Remove the rp_ioctl
2019-04-17 6:52 ` Jiri Slaby
@ 2019-04-17 6:56 ` Jiri Slaby
0 siblings, 0 replies; 5+ messages in thread
From: Jiri Slaby @ 2019-04-17 6:56 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Fuqian Huang, linux-kernel
On 17. 04. 19, 8:52, Jiri Slaby wrote:
> So could you rework the patch to remove only the one? And in a separate
> patch, introduce a printk_ratelimited warning to every "case *:" (i.e.
Not printk_ratelimited, but "dev_warn_ratelimited(tty->dev" as we have a
device.
> no warning in the "default" case) in rp_ioctl about depreciation of the
> ioctl?
>
> thanks,
--
js
suse labs
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-04-17 6:56 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-16 14:53 [PATCH v4] tty: rocket: Remove the rp_ioctl Fuqian Huang
2019-04-17 6:27 ` Jiri Slaby
2019-04-17 6:46 ` Greg Kroah-Hartman
2019-04-17 6:52 ` Jiri Slaby
2019-04-17 6:56 ` Jiri Slaby
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).