a kernel address leak via copy_to_user in drivers/tty/rocket.c

a kernel address leak via copy_to_user in drivers/tty/rocket.c

[Fuqian Huang]

Hi, recently I found that there is a kernel address leaks to user
space via copy_to_user in
drivers/tty/rocket.c:1287 (linux-5.0.5)
static int rp_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned
long arg) {
  ...
  case RCKP_GET_STRUCT:
      if (copy_to_user(argp, info, sizeof(struct r_port))
  ...
}
The `info` is a struct r_port. and the field `r_port.port.ops` is an
constant pointer,
and it points to a constant object `rocket_port_ops` during the initialization.
(function init_r_port) (drivers/tty/rocket.c:633)

patch suggestion:
set the pointer field to null before the copy to user call.

Re: a kernel address leak via copy_to_user in drivers/tty/rocket.c

[Greg KH]

On Sat, Mar 30, 2019 at 03:05:11PM +0800, Fuqian Huang wrote:
> Hi, recently I found that there is a kernel address leaks to user
> space via copy_to_user in
> drivers/tty/rocket.c:1287 (linux-5.0.5)
> static int rp_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned
> long arg) {
>   ...
>   case RCKP_GET_STRUCT:
>       if (copy_to_user(argp, info, sizeof(struct r_port))
>   ...
> }
> The `info` is a struct r_port. and the field `r_port.port.ops` is an
> constant pointer,
> and it points to a constant object `rocket_port_ops` during the initialization.
> (function init_r_port) (drivers/tty/rocket.c:633)
> 
> patch suggestion:
> set the pointer field to null before the copy to user call.

Great, can you send a patch to fix this so that you get the proper
credit for finding and resolving it?

thanks,

greg k-h

Re: a kernel address leak via copy_to_user in drivers/tty/rocket.c

[Fuqian Huang]

I add a function to mask all pointer fields of struct r_port. (Other
pointer fields may have a similar issue, so I set all pointer fields
to NULL);
The modified code is marked with ">"

>1274 static void mask_pointer(struct r_port *info, struct r_port *masked_info) {
>1275   memcpy(masked_info, info, sizeof (struct r_port));
>1276   masked_info->port.tty = NULL;
>1277   masked_info->port.itty = NULL;
>1278   masked_info->port.ops = NULL;
>1279   masked_info->port.client_ops = NULL;
>1280   memset(&masked_info->port.open_wait.head, 0, sizeof(struct list_head));
>1281   memset(&masked_info->port.delta_msr_wait.head, 0, sizeof(struct list_head));
>1282   memset(&masked_info->port.mutex.wait_list, 0, sizeof(struct list_head));
>1283   memset(&masked_info->port.buf_mutex.wait_list, 0, sizeof(struct list_head));
>1284   masked_info->port.xmit_buf = NULL;
>1285   masked_info->port.client_data = NULL;
>1286   masked_info->ctlp = NULL;
>1287   masked_info->xmit_buf = NULL;
>1288   memset(&masked_info->write_mtx.wait_list, 0, sizeof(struct list_head));
>1289 }
1290
1291 /*  IOCTL call handler into the driver */
1292 static int rp_ioctl(struct tty_struct *tty,
1293         unsigned int cmd, unsigned long arg)
1294 {
1295   struct r_port *info = tty->driver_data;
>1296   struct r_port *masked_info;
1297   void __user *argp = (void __user *)arg;
1298   int ret = 0;
1299
1300   if (cmd != RCKP_GET_PORTS && rocket_paranoia_check(info, "rp_ioctl"))
1301     return -ENXIO;
1302   switch (cmd) {
1303   case RCKP_GET_STRUCT:
>1304     masked_info = kzalloc(sizeof (struct r_port), GFP_KERNEL);
>1305     mask_pointer(info, masked_info);
>1306     if (copy_to_user(argp, masked_info, sizeof (struct r_port)))
1307       ret = -EFAULT;

Greg KH <gregkh@linuxfoundation.org> 於 2019年3月30日週六 下午3:14寫道：
>
> On Sat, Mar 30, 2019 at 03:05:11PM +0800, Fuqian Huang wrote:
> > Hi, recently I found that there is a kernel address leaks to user
> > space via copy_to_user in
> > drivers/tty/rocket.c:1287 (linux-5.0.5)
> > static int rp_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned
> > long arg) {
> >   ...
> >   case RCKP_GET_STRUCT:
> >       if (copy_to_user(argp, info, sizeof(struct r_port))
> >   ...
> > }
> > The `info` is a struct r_port. and the field `r_port.port.ops` is an
> > constant pointer,
> > and it points to a constant object `rocket_port_ops` during the initialization.
> > (function init_r_port) (drivers/tty/rocket.c:633)
> >
> > patch suggestion:
> > set the pointer field to null before the copy to user call.
>
> Great, can you send a patch to fix this so that you get the proper
> credit for finding and resolving it?
>
> thanks,
>
> greg k-h

Re: a kernel address leak via copy_to_user in drivers/tty/rocket.c

[Greg KH]

On Sat, Mar 30, 2019 at 04:02:38PM +0800, Fuqian Huang wrote:
> I add a function to mask all pointer fields of struct r_port. (Other
> pointer fields may have a similar issue, so I set all pointer fields
> to NULL);
> The modified code is marked with ">"
> 
> >1274 static void mask_pointer(struct r_port *info, struct r_port *masked_info) {
> >1275   memcpy(masked_info, info, sizeof (struct r_port));
> >1276   masked_info->port.tty = NULL;
> >1277   masked_info->port.itty = NULL;
> >1278   masked_info->port.ops = NULL;
> >1279   masked_info->port.client_ops = NULL;
> >1280   memset(&masked_info->port.open_wait.head, 0, sizeof(struct list_head));
> >1281   memset(&masked_info->port.delta_msr_wait.head, 0, sizeof(struct list_head));
> >1282   memset(&masked_info->port.mutex.wait_list, 0, sizeof(struct list_head));
> >1283   memset(&masked_info->port.buf_mutex.wait_list, 0, sizeof(struct list_head));
> >1284   masked_info->port.xmit_buf = NULL;
> >1285   masked_info->port.client_data = NULL;
> >1286   masked_info->ctlp = NULL;
> >1287   masked_info->xmit_buf = NULL;
> >1288   memset(&masked_info->write_mtx.wait_list, 0, sizeof(struct list_head));
> >1289 }
> 1290
> 1291 /*  IOCTL call handler into the driver */
> 1292 static int rp_ioctl(struct tty_struct *tty,
> 1293         unsigned int cmd, unsigned long arg)
> 1294 {
> 1295   struct r_port *info = tty->driver_data;
> >1296   struct r_port *masked_info;
> 1297   void __user *argp = (void __user *)arg;
> 1298   int ret = 0;
> 1299
> 1300   if (cmd != RCKP_GET_PORTS && rocket_paranoia_check(info, "rp_ioctl"))
> 1301     return -ENXIO;
> 1302   switch (cmd) {
> 1303   case RCKP_GET_STRUCT:
> >1304     masked_info = kzalloc(sizeof (struct r_port), GFP_KERNEL);
> >1305     mask_pointer(info, masked_info);
> >1306     if (copy_to_user(argp, masked_info, sizeof (struct r_port)))
> 1307       ret = -EFAULT;

Please read Documentation/SubmittingPatches for the proper way to create
and submit a patch such that we can apply it.

thanks!

greg k-h