lkml.org 
[lkml]   [2023]   [May]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH AUTOSEL 6.3 28/67] media: dvb-core: Fix use-after-free due to race at dvb_register_device()
    Date
    From: Hyunwoo Kim <imv4bel@gmail.com>

    [ Upstream commit 627bb528b086b4136315c25d6a447a98ea9448d3 ]

    dvb_register_device() dynamically allocates fops with kmemdup()
    to set the fops->owner.
    And these fops are registered in 'file->f_ops' using replace_fops()
    in the dvb_device_open() process, and kfree()d in dvb_free_device().

    However, it is not common to use dynamically allocated fops instead
    of 'static const' fops as an argument of replace_fops(),
    and UAF may occur.
    These UAFs can occur on any dvb type using dvb_register_device(),
    such as dvb_dvr, dvb_demux, dvb_frontend, dvb_net, etc.

    So, instead of kfree() the fops dynamically allocated in
    dvb_register_device() in dvb_free_device() called during the
    .disconnect() process, kfree() it collectively in exit_dvbdev()
    called when the dvbdev.c module is removed.

    Link: https://lore.kernel.org/linux-media/20221117045925.14297-4-imv4bel@gmail.com
    Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
    Reported-by: kernel test robot <lkp@intel.com>
    Reported-by: Dan Carpenter <error27@gmail.com>
    Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
    Signed-off-by: Sasha Levin <sashal@kernel.org>
    ---
    drivers/media/dvb-core/dvbdev.c | 84 ++++++++++++++++++++++++---------
    include/media/dvbdev.h | 15 ++++++
    2 files changed, 78 insertions(+), 21 deletions(-)

    diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
    index 0ed087caf7f3b..73d296f27ff92 100644
    --- a/drivers/media/dvb-core/dvbdev.c
    +++ b/drivers/media/dvb-core/dvbdev.c
    @@ -27,6 +27,7 @@
    #include <media/tuner.h>

    static DEFINE_MUTEX(dvbdev_mutex);
    +static LIST_HEAD(dvbdevfops_list);
    static int dvbdev_debug;

    module_param(dvbdev_debug, int, 0644);
    @@ -453,14 +454,15 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
    enum dvb_device_type type, int demux_sink_pads)
    {
    struct dvb_device *dvbdev;
    - struct file_operations *dvbdevfops;
    + struct file_operations *dvbdevfops = NULL;
    + struct dvbdevfops_node *node = NULL, *new_node = NULL;
    struct device *clsdev;
    int minor;
    int id, ret;

    mutex_lock(&dvbdev_register_lock);

    - if ((id = dvbdev_get_free_id (adap, type)) < 0){
    + if ((id = dvbdev_get_free_id (adap, type)) < 0) {
    mutex_unlock(&dvbdev_register_lock);
    *pdvbdev = NULL;
    pr_err("%s: couldn't find free device id\n", __func__);
    @@ -468,18 +470,45 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
    }

    *pdvbdev = dvbdev = kzalloc(sizeof(*dvbdev), GFP_KERNEL);
    -
    if (!dvbdev){
    mutex_unlock(&dvbdev_register_lock);
    return -ENOMEM;
    }

    - dvbdevfops = kmemdup(template->fops, sizeof(*dvbdevfops), GFP_KERNEL);
    + /*
    + * When a device of the same type is probe()d more than once,
    + * the first allocated fops are used. This prevents memory leaks
    + * that can occur when the same device is probe()d repeatedly.
    + */
    + list_for_each_entry(node, &dvbdevfops_list, list_head) {
    + if (node->fops->owner == adap->module &&
    + node->type == type &&
    + node->template == template) {
    + dvbdevfops = node->fops;
    + break;
    + }
    + }

    - if (!dvbdevfops){
    - kfree (dvbdev);
    - mutex_unlock(&dvbdev_register_lock);
    - return -ENOMEM;
    + if (dvbdevfops == NULL) {
    + dvbdevfops = kmemdup(template->fops, sizeof(*dvbdevfops), GFP_KERNEL);
    + if (!dvbdevfops) {
    + kfree(dvbdev);
    + mutex_unlock(&dvbdev_register_lock);
    + return -ENOMEM;
    + }
    +
    + new_node = kzalloc(sizeof(struct dvbdevfops_node), GFP_KERNEL);
    + if (!new_node) {
    + kfree(dvbdevfops);
    + kfree(dvbdev);
    + mutex_unlock(&dvbdev_register_lock);
    + return -ENOMEM;
    + }
    +
    + new_node->fops = dvbdevfops;
    + new_node->type = type;
    + new_node->template = template;
    + list_add_tail (&new_node->list_head, &dvbdevfops_list);
    }

    memcpy(dvbdev, template, sizeof(struct dvb_device));
    @@ -490,20 +519,20 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
    dvbdev->priv = priv;
    dvbdev->fops = dvbdevfops;
    init_waitqueue_head (&dvbdev->wait_queue);
    -
    dvbdevfops->owner = adap->module;
    -
    list_add_tail (&dvbdev->list_head, &adap->device_list);
    -
    down_write(&minor_rwsem);
    #ifdef CONFIG_DVB_DYNAMIC_MINORS
    for (minor = 0; minor < MAX_DVB_MINORS; minor++)
    if (dvb_minors[minor] == NULL)
    break;
    -
    if (minor == MAX_DVB_MINORS) {
    + if (new_node) {
    + list_del (&new_node->list_head);
    + kfree(dvbdevfops);
    + kfree(new_node);
    + }
    list_del (&dvbdev->list_head);
    - kfree(dvbdevfops);
    kfree(dvbdev);
    up_write(&minor_rwsem);
    mutex_unlock(&dvbdev_register_lock);
    @@ -512,41 +541,47 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
    #else
    minor = nums2minor(adap->num, type, id);
    #endif
    -
    dvbdev->minor = minor;
    dvb_minors[minor] = dvb_device_get(dvbdev);
    up_write(&minor_rwsem);
    -
    ret = dvb_register_media_device(dvbdev, type, minor, demux_sink_pads);
    if (ret) {
    pr_err("%s: dvb_register_media_device failed to create the mediagraph\n",
    __func__);
    -
    + if (new_node) {
    + list_del (&new_node->list_head);
    + kfree(dvbdevfops);
    + kfree(new_node);
    + }
    dvb_media_device_free(dvbdev);
    list_del (&dvbdev->list_head);
    - kfree(dvbdevfops);
    kfree(dvbdev);
    mutex_unlock(&dvbdev_register_lock);
    return ret;
    }

    - mutex_unlock(&dvbdev_register_lock);
    -
    clsdev = device_create(dvb_class, adap->device,
    MKDEV(DVB_MAJOR, minor),
    dvbdev, "dvb%d.%s%d", adap->num, dnames[type], id);
    if (IS_ERR(clsdev)) {
    pr_err("%s: failed to create device dvb%d.%s%d (%ld)\n",
    __func__, adap->num, dnames[type], id, PTR_ERR(clsdev));
    + if (new_node) {
    + list_del (&new_node->list_head);
    + kfree(dvbdevfops);
    + kfree(new_node);
    + }
    dvb_media_device_free(dvbdev);
    list_del (&dvbdev->list_head);
    - kfree(dvbdevfops);
    kfree(dvbdev);
    + mutex_unlock(&dvbdev_register_lock);
    return PTR_ERR(clsdev);
    }
    +
    dprintk("DVB: register adapter%d/%s%d @ minor: %i (0x%02x)\n",
    adap->num, dnames[type], id, minor, minor);

    + mutex_unlock(&dvbdev_register_lock);
    return 0;
    }
    EXPORT_SYMBOL(dvb_register_device);
    @@ -575,7 +610,6 @@ static void dvb_free_device(struct kref *ref)
    {
    struct dvb_device *dvbdev = container_of(ref, struct dvb_device, ref);

    - kfree (dvbdev->fops);
    kfree (dvbdev);
    }

    @@ -1081,9 +1115,17 @@ static int __init init_dvbdev(void)

    static void __exit exit_dvbdev(void)
    {
    + struct dvbdevfops_node *node, *next;
    +
    class_destroy(dvb_class);
    cdev_del(&dvb_device_cdev);
    unregister_chrdev_region(MKDEV(DVB_MAJOR, 0), MAX_DVB_MINORS);
    +
    + list_for_each_entry_safe(node, next, &dvbdevfops_list, list_head) {
    + list_del (&node->list_head);
    + kfree(node->fops);
    + kfree(node);
    + }
    }

    subsys_initcall(init_dvbdev);
    diff --git a/include/media/dvbdev.h b/include/media/dvbdev.h
    index 29d25c8a6f13f..8958e5e2fc5b7 100644
    --- a/include/media/dvbdev.h
    +++ b/include/media/dvbdev.h
    @@ -193,6 +193,21 @@ struct dvb_device {
    void *priv;
    };

    +/**
    + * struct dvbdevfops_node - fops nodes registered in dvbdevfops_list
    + *
    + * @fops: Dynamically allocated fops for ->owner registration
    + * @type: type of dvb_device
    + * @template: dvb_device used for registration
    + * @list_head: list_head for dvbdevfops_list
    + */
    +struct dvbdevfops_node {
    + struct file_operations *fops;
    + enum dvb_device_type type;
    + const struct dvb_device *template;
    + struct list_head list_head;
    +};
    +
    /**
    * dvb_device_get - Increase dvb_device reference
    *
    --
    2.39.2
    \
     
     \ /
      Last update: 2023-05-25 22:39    [W:4.180 / U:0.100 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site