Messages in this thread | | | Date | Sat, 30 Mar 2019 19:45:50 +0100 | From | Greg KH <> | Subject | Re: [PATCH] drivers/tty: fix kernel address leaks in rp_ioctl |
| |
On Sat, Mar 30, 2019 at 06:33:44PM +0800, Fuqian Huang wrote: > The RCKP_GET_STRUCT case in rp_ioctl will copy a kernel > pointer(info->port.ops) to user space. The info->port.ops points to a > constant object 'rocket_port_ops' during the initialization. (init_r_port > in drivers/tty/rocket.c:633) > > Add a function clear_pointer_fields to set the pointer fields of struct > r_port to NULL before copy_to_user. > > Signed-off-by: Fuqian Huang <huangfq.daxian@gmail.com> > Reported-by: Fuqian Huang <huangfq.daxian@gmail.com> > -- > diff --git a/drivers/tty/rocket.c b/drivers/tty/rocket.c > index b121d8f..28016e1 100644 > --- a/drivers/tty/rocket.c > +++ b/drivers/tty/rocket.c > @@ -1271,21 +1271,42 @@ static int get_version(struct r_port *info, struct > rocket_version __user *retver > return 0; > } > > +static void clear_pointer_fields(struct r_port *old, struct r_port *new) > +{ > + memcpy(new, old, sizeof (struct r_port)); > + new->port.tty = NULL; > + new->port.itty = NULL; > + new->port.ops = NULL; > + new->port.client_ops = NULL; > + memset(&new->port.open_wait.head, 0, sizeof(struct list_head)); > + memset(&new->port.delta_msr_wait.head, 0, sizeof(struct list_head)); > + memset(&new->port.mutex.wait_list, 0, sizeof(struct list_head)); > + memset(&new->port.buf_mutex.wait_list, 0, sizeof(struct list_head)); > + new->port.xmit_buf = NULL; > + new->port.client_data = NULL; > + new->ctlp = NULL; > + new->xmit_buf = NULL; > + memset(&new->write_mtx.wait_list, 0, sizeof(struct list_head)); > +} > + > /* 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; > + struct r_port *masked_info; > 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))) > + masked_info = kzalloc(sizeof (struct r_port), GFP_KERNEL); > + clear_pointer_fields(info, masked_info); > + if (copy_to_user(argp, masked_info, sizeof (struct r_port))) > ret = -EFAULT; > + kfree(masked_info); > break; > case RCKP_GET_CONFIG: > ret = get_config(info, argp);
The patch is totally whitespace corrupted and can not be applied :(
You can not cut/paste in gmail when sending a patch.
Also, your change leaks memory on the error path :(
Please fix up and try again.
thanks,
greg k-h
| |