From patchwork Thu Oct 20 20:01:13 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: V4bel X-Patchwork-Id: 13013990 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 39007C4332F for ; Thu, 20 Oct 2022 20:01:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229991AbiJTUBk (ORCPT ); Thu, 20 Oct 2022 16:01:40 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47370 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229975AbiJTUBh (ORCPT ); Thu, 20 Oct 2022 16:01:37 -0400 Received: from mail-pf1-x436.google.com (mail-pf1-x436.google.com [IPv6:2607:f8b0:4864:20::436]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EED8D181C8C for ; Thu, 20 Oct 2022 13:01:28 -0700 (PDT) Received: by mail-pf1-x436.google.com with SMTP id g28so539348pfk.8 for ; Thu, 20 Oct 2022 13:01:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-disposition:mime-version:message-id:subject:cc:to:from:date :from:to:cc:subject:date:message-id:reply-to; bh=8QFkj9eVIkbxCy5NQnLk1d8ujRFVsl1WFy880Uu5V7E=; b=WZUDgTN58FA+GtBOYb6eRU53jCH7UOyrFhWuxtC6Tp3ugyacaOvTN9QxqtqonzBcoO YLZewmFcQVINhpOWQfg43DproYrPvKdm+0FwH5YWoSakWub7XqKrvmcURRa8rp3HRj/j bvmJKo6j/1uECgeAdu5t4U6HpzoFtGRM1PgEcZRt9KmDynvKN+FBbeot6+NMTJKU1hsB kQ4RD37sTHiq4iwi0MgB428L8Z1+JChBb5TcPhyWTkvw3U7CIZ6jLhIJdF3v5Evh2Ygj iOz58KzFeVYa9CzLyXfa9k+RT+LnssPa+/UHkrzYeeoTP4rmDdOh+dQOFdvRy74Lef2d Dydg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-disposition:mime-version:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=8QFkj9eVIkbxCy5NQnLk1d8ujRFVsl1WFy880Uu5V7E=; b=i+HKhqxBfc1Sz9gtHqsb+HAMRSQ2/ipFMew53+bphfPhV0KKVpJff5+S2Aaat7tlV/ WgQrefGu0jdu/6VHxbIweoNw+sme2uohFEH/Lko9kNYl71FtvUEle5osIXgDIn1/ef0g 5K5DjoZ/L8OCHfNMRP3SivaticA6x/ki1qsvMyf4KuVdGh31jALZRXh0YBmTHRK4+UBf pZNNiw7m8BK0C7+QMp8M9WLAgSJn+evlnkT5TiZNwjGhjYK/LhN0me1siTQCd8hX2U9t 4XxvLb5os8Oo2xZPMCeoy1egjg10gZRjeQuc4qs1iUQOKVUwUJm/wXydjzECJ/qsZ4aN GMgg== X-Gm-Message-State: ACrzQf0evWVcJZZv2E9gDYMJ0gr4p/omWgHbmO2CyCULySEAaosc8Cok bSg6GzpCm2ZMD0n9zOpF0+EL7G56uc4= X-Google-Smtp-Source: AMsMyM5jQbpBV5B3hwnc9fqao2gemLLhACd/mP9bu23pKklolM7clIOtQr/VrArKSgki9KGXNpSnSA== X-Received: by 2002:a17:902:d48d:b0:185:115c:b165 with SMTP id c13-20020a170902d48d00b00185115cb165mr15697503plg.86.1666296077277; Thu, 20 Oct 2022 13:01:17 -0700 (PDT) Received: from ubuntu ([175.124.254.119]) by smtp.gmail.com with ESMTPSA id y8-20020a17090a6c8800b0020a9af6bb1asm207950pjj.32.2022.10.20.13.01.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 20 Oct 2022 13:01:16 -0700 (PDT) Date: Thu, 20 Oct 2022 13:01:13 -0700 From: Hyunwoo Kim To: steve.glendinning@shawell.net, deller@gmx.de Cc: linux-fbdev@vger.kernel.org, dri-devel@lists.freedesktop.org, imv4bel@gmail.com Subject: [PATCH] video: fbdev: smscufx: Fixed several use-after-free bugs Message-ID: <20221020200113.GA320044@ubuntu> MIME-Version: 1.0 Content-Disposition: inline Precedence: bulk List-ID: X-Mailing-List: linux-fbdev@vger.kernel.org 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: Hyunwoo Kim Link: https://lore.kernel.org/linux-fbdev/20221011153436.GA4446@ubuntu/ --- drivers/video/fbdev/smscufx.c | 43 +++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/drivers/video/fbdev/smscufx.c b/drivers/video/fbdev/smscufx.c index e65bdc499c23..9f90c02c6533 100644 --- a/drivers/video/fbdev/smscufx.c +++ b/drivers/video/fbdev/smscufx.c @@ -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 */ @@ -1117,15 +1116,18 @@ 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; - kfree(dev); + /* 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, @@ -1134,15 +1136,11 @@ 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); if (info->monspecs.modedb) @@ -1169,11 +1167,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 const 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); }