Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 5385af2f authored by Hyunwoo Kim's avatar Hyunwoo Kim Committed by Greg Kroah-Hartman
Browse files

fbdev: smscufx: Fix several use-after-free bugs 



commit cc67482c9e5f2c80d62f623bcc347c29f9f648e1 upstream.

Several types of UAFs can occur when physically removing a USB device.

Adds ufx_ops_destroy() function to .fb_destroy of fb_ops, and
in this function, there is kref_put() that finally calls ufx_free().

This fix prevents multiple UAFs.

Signed-off-by: default avatarHyunwoo Kim <imv4bel@gmail.com>
Link: https://lore.kernel.org/linux-fbdev/20221011153436.GA4446@ubuntu/


Cc: <stable@vger.kernel.org>
Signed-off-by: default avatarHelge Deller <deller@gmx.de>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 07ef3be6

drivers/video/fbdev/smscufx.c

+30 −25
Original line number Diff line number Diff line
@@ -97,7 +97,6 @@ struct ufx_data { 
	struct kref kref;

	int fb_count;

	bool virtualized; /* true when physical usb device not present */

	struct delayed_work free_framebuffer_work;

	atomic_t usb_active; /* 0 = update virtual buffer, but no usb traffic */

	atomic_t lost_pixels; /* 1 = a render op failed. Need screen refresh */

	u8 *edid; /* null until we read edid from hw or get from sysfs */
@@ -1116,15 +1115,24 @@ static void ufx_free(struct kref *kref) 
{

	struct ufx_data *dev = container_of(kref, struct ufx_data, kref);



	/* this function will wait for all in-flight urbs to complete */

	if (dev->urbs.count > 0)

		ufx_free_urb_list(dev);

	kfree(dev);

}



	pr_debug("freeing ufx_data %p", dev);

static void ufx_ops_destory(struct fb_info *info)

{

	struct ufx_data *dev = info->par;

	int node = info->node;



	kfree(dev);

	/* Assume info structure is freed after this point */

	framebuffer_release(info);



	pr_debug("fb_info for /dev/fb%d has been freed", node);



	/* release reference taken by kref_init in probe() */

	kref_put(&dev->kref, ufx_free);

}





static void ufx_release_urb_work(struct work_struct *work)

{

	struct urb_node *unode = container_of(work, struct urb_node,
@@ -1133,14 +1141,9 @@ static void ufx_release_urb_work(struct work_struct *work) 
	up(&unode->dev->urbs.limit_sem);

}



static void ufx_free_framebuffer_work(struct work_struct *work)

static void ufx_free_framebuffer(struct ufx_data *dev)

{

	struct ufx_data *dev = container_of(work, struct ufx_data,

					    free_framebuffer_work.work);

	struct fb_info *info = dev->info;

	int node = info->node;



	unregister_framebuffer(info);



	if (info->cmap.len != 0)

		fb_dealloc_cmap(&info->cmap);
@@ -1152,11 +1155,6 @@ static void ufx_free_framebuffer_work(struct work_struct *work) 


	dev->info = NULL;



	/* Assume info structure is freed after this point */

	framebuffer_release(info);



	pr_debug("fb_info for /dev/fb%d has been freed", node);



	/* ref taken in probe() as part of registering framebfufer */

	kref_put(&dev->kref, ufx_free);

}
@@ -1168,11 +1166,13 @@ static int ufx_ops_release(struct fb_info *info, int user) 
{

	struct ufx_data *dev = info->par;



	mutex_lock(&disconnect_mutex);



	dev->fb_count--;



	/* We can't free fb_info here - fbmem will touch it when we return */

	if (dev->virtualized && (dev->fb_count == 0))

		schedule_delayed_work(&dev->free_framebuffer_work, HZ);

		ufx_free_framebuffer(dev);



	if ((dev->fb_count == 0) && (info->fbdefio)) {

		fb_deferred_io_cleanup(info);
@@ -1186,6 +1186,8 @@ static int ufx_ops_release(struct fb_info *info, int user) 


	kref_put(&dev->kref, ufx_free);



	mutex_unlock(&disconnect_mutex);



	return 0;

}
@@ -1292,6 +1294,7 @@ static struct fb_ops ufx_ops = { 
	.fb_blank = ufx_ops_blank,

	.fb_check_var = ufx_ops_check_var,

	.fb_set_par = ufx_ops_set_par,

	.fb_destroy = ufx_ops_destory,

};



/* Assumes &info->lock held by caller
@@ -1673,9 +1676,6 @@ static int ufx_usb_probe(struct usb_interface *interface, 
		goto destroy_modedb;

	}



	INIT_DELAYED_WORK(&dev->free_framebuffer_work,

			  ufx_free_framebuffer_work);



	retval = ufx_reg_read(dev, 0x3000, &id_rev);

	check_warn_goto_error(retval, "error %d reading 0x3000 register from device", retval);

	dev_dbg(dev->gdev, "ID_REV register value 0x%08x", id_rev);
@@ -1748,10 +1748,12 @@ static int ufx_usb_probe(struct usb_interface *interface, 
static void ufx_usb_disconnect(struct usb_interface *interface)

{

	struct ufx_data *dev;

	struct fb_info *info;



	mutex_lock(&disconnect_mutex);



	dev = usb_get_intfdata(interface);

	info = dev->info;



	pr_debug("USB disconnect starting\n");
@@ -1765,12 +1767,15 @@ static void ufx_usb_disconnect(struct usb_interface *interface) 


	/* if clients still have us open, will be freed on last close */

	if (dev->fb_count == 0)

		schedule_delayed_work(&dev->free_framebuffer_work, 0);

		ufx_free_framebuffer(dev);



	/* release reference taken by kref_init in probe() */

	kref_put(&dev->kref, ufx_free);

	/* this function will wait for all in-flight urbs to complete */

	if (dev->urbs.count > 0)

		ufx_free_urb_list(dev);



	/* consider ufx_data freed */

	pr_debug("freeing ufx_data %p", dev);



	unregister_framebuffer(info);



	mutex_unlock(&disconnect_mutex);

}