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@xxxxxxxxxxxxxxxxxxx> æ 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