media: dvbdev: adopts refcnt to avoid UAF

dvb_unregister_device() is known that prone to use-after-free.
That is, the cleanup from dvb_unregister_device() releases the dvb_device
even if there are pointers stored in file->private_data still refer to it.

This patch adds a reference counter into struct dvb_device and delays its
deallocation until no pointer refers to the object.

Link: https://lore.kernel.org/linux-media/20220807145952.10368-1-linma@zju.edu.cn
Signed-off-by: Lin Ma <linma@zju.edu.cn>
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
This commit is contained in:
Lin Ma 2022-08-07 15:59:52 +01:00 committed by Mauro Carvalho Chehab
parent 9b7de3c2da
commit 0fc044b2b5
4 changed files with 44 additions and 23 deletions

View File

@ -157,7 +157,7 @@ static void dvb_ca_private_free(struct dvb_ca_private *ca)
{ {
unsigned int i; unsigned int i;
dvb_free_device(ca->dvbdev); dvb_device_put(ca->dvbdev);
for (i = 0; i < ca->slot_count; i++) for (i = 0; i < ca->slot_count; i++)
vfree(ca->slot_info[i].rx_buffer.data); vfree(ca->slot_info[i].rx_buffer.data);

View File

@ -136,7 +136,7 @@ static void __dvb_frontend_free(struct dvb_frontend *fe)
struct dvb_frontend_private *fepriv = fe->frontend_priv; struct dvb_frontend_private *fepriv = fe->frontend_priv;
if (fepriv) if (fepriv)
dvb_free_device(fepriv->dvbdev); dvb_device_put(fepriv->dvbdev);
dvb_frontend_invoke_release(fe, fe->ops.release); dvb_frontend_invoke_release(fe, fe->ops.release);

View File

@ -97,7 +97,7 @@ static int dvb_device_open(struct inode *inode, struct file *file)
new_fops = fops_get(dvbdev->fops); new_fops = fops_get(dvbdev->fops);
if (!new_fops) if (!new_fops)
goto fail; goto fail;
file->private_data = dvbdev; file->private_data = dvb_device_get(dvbdev);
replace_fops(file, new_fops); replace_fops(file, new_fops);
if (file->f_op->open) if (file->f_op->open)
err = file->f_op->open(inode, file); err = file->f_op->open(inode, file);
@ -161,6 +161,9 @@ int dvb_generic_release(struct inode *inode, struct file *file)
} }
dvbdev->users++; dvbdev->users++;
dvb_device_put(dvbdev);
return 0; return 0;
} }
EXPORT_SYMBOL(dvb_generic_release); EXPORT_SYMBOL(dvb_generic_release);
@ -479,6 +482,7 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
return -ENOMEM; return -ENOMEM;
} }
kref_init(&dvbdev->ref);
memcpy(dvbdev, template, sizeof(struct dvb_device)); memcpy(dvbdev, template, sizeof(struct dvb_device));
dvbdev->type = type; dvbdev->type = type;
dvbdev->id = id; dvbdev->id = id;
@ -510,7 +514,7 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
#endif #endif
dvbdev->minor = minor; dvbdev->minor = minor;
dvb_minors[minor] = dvbdev; dvb_minors[minor] = dvb_device_get(dvbdev);
up_write(&minor_rwsem); up_write(&minor_rwsem);
ret = dvb_register_media_device(dvbdev, type, minor, demux_sink_pads); ret = dvb_register_media_device(dvbdev, type, minor, demux_sink_pads);
@ -555,6 +559,7 @@ void dvb_remove_device(struct dvb_device *dvbdev)
down_write(&minor_rwsem); down_write(&minor_rwsem);
dvb_minors[dvbdev->minor] = NULL; dvb_minors[dvbdev->minor] = NULL;
dvb_device_put(dvbdev);
up_write(&minor_rwsem); up_write(&minor_rwsem);
dvb_media_device_free(dvbdev); dvb_media_device_free(dvbdev);
@ -566,21 +571,34 @@ void dvb_remove_device(struct dvb_device *dvbdev)
EXPORT_SYMBOL(dvb_remove_device); EXPORT_SYMBOL(dvb_remove_device);
void dvb_free_device(struct dvb_device *dvbdev) static void dvb_free_device(struct kref *ref)
{ {
if (!dvbdev) struct dvb_device *dvbdev = container_of(ref, struct dvb_device, ref);
return;
kfree (dvbdev->fops); kfree (dvbdev->fops);
kfree (dvbdev); kfree (dvbdev);
} }
EXPORT_SYMBOL(dvb_free_device);
struct dvb_device *dvb_device_get(struct dvb_device *dvbdev)
{
kref_get(&dvbdev->ref);
return dvbdev;
}
EXPORT_SYMBOL(dvb_device_get);
void dvb_device_put(struct dvb_device *dvbdev)
{
if (dvbdev)
kref_put(&dvbdev->ref, dvb_free_device);
}
void dvb_unregister_device(struct dvb_device *dvbdev) void dvb_unregister_device(struct dvb_device *dvbdev)
{ {
dvb_remove_device(dvbdev); dvb_remove_device(dvbdev);
dvb_free_device(dvbdev); dvb_device_put(dvbdev);
} }
EXPORT_SYMBOL(dvb_unregister_device); EXPORT_SYMBOL(dvb_unregister_device);

View File

@ -160,6 +160,7 @@ struct dvb_adapter {
*/ */
struct dvb_device { struct dvb_device {
struct list_head list_head; struct list_head list_head;
struct kref ref;
const struct file_operations *fops; const struct file_operations *fops;
struct dvb_adapter *adapter; struct dvb_adapter *adapter;
enum dvb_device_type type; enum dvb_device_type type;
@ -191,6 +192,20 @@ struct dvb_device {
void *priv; void *priv;
}; };
/**
* dvb_device_get - Increase dvb_device reference
*
* @dvbdev: pointer to struct dvb_device
*/
struct dvb_device *dvb_device_get(struct dvb_device *dvbdev);
/**
* dvb_device_get - Decrease dvb_device reference
*
* @dvbdev: pointer to struct dvb_device
*/
void dvb_device_put(struct dvb_device *dvbdev);
/** /**
* dvb_register_adapter - Registers a new DVB adapter * dvb_register_adapter - Registers a new DVB adapter
* *
@ -235,29 +250,17 @@ int dvb_register_device(struct dvb_adapter *adap,
/** /**
* dvb_remove_device - Remove a registered DVB device * dvb_remove_device - Remove a registered DVB device
* *
* This does not free memory. To do that, call dvb_free_device(). * This does not free memory. dvb_free_device() will do that when
* reference counter is empty
* *
* @dvbdev: pointer to struct dvb_device * @dvbdev: pointer to struct dvb_device
*/ */
void dvb_remove_device(struct dvb_device *dvbdev); void dvb_remove_device(struct dvb_device *dvbdev);
/**
* dvb_free_device - Free memory occupied by a DVB device.
*
* Call dvb_unregister_device() before calling this function.
*
* @dvbdev: pointer to struct dvb_device
*/
void dvb_free_device(struct dvb_device *dvbdev);
/** /**
* dvb_unregister_device - Unregisters a DVB device * dvb_unregister_device - Unregisters a DVB device
* *
* This is a combination of dvb_remove_device() and dvb_free_device().
* Using this function is usually a mistake, and is often an indicator
* for a use-after-free bug (when a userspace process keeps a file
* handle to a detached device).
*
* @dvbdev: pointer to struct dvb_device * @dvbdev: pointer to struct dvb_device
*/ */
void dvb_unregister_device(struct dvb_device *dvbdev); void dvb_unregister_device(struct dvb_device *dvbdev);