Re: [PATCH] drivers/tty: fix kernel address leaks in rp_ioctl

[Greg KH]

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@xxxxxxxxx>
> Reported-by: Fuqian Huang <huangfq.daxian@xxxxxxxxx>
> --
> 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