From patchwork Fri Oct 19 20:39:17 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Wenwen Wang X-Patchwork-Id: 10651767 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 223B013A4 for ; Mon, 22 Oct 2018 07:09:18 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0FE862880A for ; Mon, 22 Oct 2018 07:09:18 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 040A02881A; Mon, 22 Oct 2018 07:09:18 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 97F242880A for ; Mon, 22 Oct 2018 07:09:17 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 735A889CD8; Mon, 22 Oct 2018 07:06:25 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mta-p5.oit.umn.edu (mta-p5.oit.umn.edu [134.84.196.205]) by gabe.freedesktop.org (Postfix) with ESMTPS id 2C3E489CBC for ; Fri, 19 Oct 2018 20:39:28 +0000 (UTC) Received: from localhost (unknown [127.0.0.1]) by mta-p5.oit.umn.edu (Postfix) with ESMTP id A8F10D48 for ; Fri, 19 Oct 2018 20:39:27 +0000 (UTC) X-Virus-Scanned: amavisd-new at umn.edu Received: from mta-p5.oit.umn.edu ([127.0.0.1]) by localhost (mta-p5.oit.umn.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id voFP3ePwijPc for ; Fri, 19 Oct 2018 15:39:27 -0500 (CDT) Received: from mail-io1-f71.google.com (mail-io1-f71.google.com [209.85.166.71]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mta-p5.oit.umn.edu (Postfix) with ESMTPS id 7C43DB15 for ; Fri, 19 Oct 2018 15:39:26 -0500 (CDT) Received: by mail-io1-f71.google.com with SMTP id z20-v6so32010031ioh.2 for ; Fri, 19 Oct 2018 13:39:26 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=8xYoiRynmnEoMoJLcYQvBlb5siR4tDJjtLBRz6IrP84=; b=FAm4aHfBYOhFeC23SE4sx9dIwwBP3qZNKKF9It6bUVBC0UV3wR7jJaGxshdOg7aRVa pSeTq+xXBB2dG2KuK5fswxc4e19ITAO0ICJDc9xjmvZf/lchawr4Y62mEO8xV5TLJKq8 vwiFBUJhnNkeLjhHEFg8eTW9xBnOjI07Jwvxfoe7Nj0zPjhYRO3fiB6pBDUlKaODzDgn 7nVqbZ6kQ261Dg30AtTk66uSQr4HC8hptLDmw6EPt3K4WEis4jlvZw4nqpK6WEkCmDic QbeqwErwSs/EW3pXsG81zAyKiiPBoCSSisgPjYDjLfBaZRSnC+5YRnGm9a630fvh22w7 FTtQ== X-Gm-Message-State: AGRZ1gKlD0XuSUfGi0e8rNs4z6yhfe2WZ5FCXMEyrB+kz2GkL9rLf56V gcCRKJwgVn9J+cRWbwdptHZJhsD/+7O50Kmx/QTnfwVQBMtknQ9t1llE3yjRJhSWYXJGhOwFHXb rhLwwL1HyIECm7MJKfYoqvwD1YuxzJP0/ X-Received: by 2002:a5e:dd0a:: with SMTP id t10-v6mr3964763iop.62.1539981566037; Fri, 19 Oct 2018 13:39:26 -0700 (PDT) X-Google-Smtp-Source: AJdET5e9D2AwVhI+4ifydxZ1szmWyEZb8Y/yYPv7TgFl9kHsKXwqwRsV16mH7W/yz9VE6stCb570SA== X-Received: by 2002:a5e:dd0a:: with SMTP id t10-v6mr3964752iop.62.1539981565780; Fri, 19 Oct 2018 13:39:25 -0700 (PDT) Received: from cs-u-cslp16.cs.umn.edu (cs-u-cslp16.cs.umn.edu. [134.84.121.95]) by smtp.gmail.com with ESMTPSA id 82-v6sm1595304ita.17.2018.10.19.13.39.24 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 19 Oct 2018 13:39:24 -0700 (PDT) From: Wenwen Wang To: Wenwen Wang Subject: [PATCH] video: fbdev: sis: fix a missing-check bug Date: Fri, 19 Oct 2018 15:39:17 -0500 Message-Id: <1539981557-13590-1-git-send-email-wang6495@umn.edu> X-Mailer: git-send-email 2.7.4 X-Mailman-Approved-At: Mon, 22 Oct 2018 07:06:20 +0000 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "open list:FRAMEBUFFER LAYER" , Bartlomiej Zolnierkiewicz , Thomas Winischhofer , Kangjie Lu , open list , "open list:FRAMEBUFFER LAYER" MIME-Version: 1.0 Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP In sisfb_find_rom(), the official pci ROM is checked to see whether it contains the expected value at specific locations. This is done by firstly mapping the rom into the IO memory region 'rom_base' and then invoking sisfb_check_rom() to perform the checks. If the checks succeed, i.e., sisfb_check_rom() returns 1, the whole content of the rom is then copied to 'myrombase' through memcpy_fromio(). The problem here is that the checks are conducted on the IO region 'rom_base' directly. Given that the device also has the permission to access the IO region, it is possible that a malicious device controlled by an attacker can race to modify the values in the region after the checks but before memcpy_fromio(). By doing so, the attacker can supply unexpected roms, which can cause undefined behavior of the kernel and introduce potential security risk. The following for loop also has a similar issue. To avoid the above issue, this patch firstly copies the content of the rom and then performs the checks on the copied version. If all the checks are satisfied, the copied version will then be returned. Signed-off-by: Wenwen Wang --- drivers/video/fbdev/sis/sis_main.c | 52 ++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 30 deletions(-) diff --git a/drivers/video/fbdev/sis/sis_main.c b/drivers/video/fbdev/sis/sis_main.c index 20aff90..a2d8051 100644 --- a/drivers/video/fbdev/sis/sis_main.c +++ b/drivers/video/fbdev/sis/sis_main.c @@ -4089,29 +4089,29 @@ static int __init sisfb_setup(char *options) } #endif -static int sisfb_check_rom(void __iomem *rom_base, +static int sisfb_check_rom(unsigned char *rom_base, struct sis_video_info *ivideo) { - void __iomem *rom; + unsigned char *rom; int romptr; - if((readb(rom_base) != 0x55) || (readb(rom_base + 1) != 0xaa)) + if ((*rom_base != 0x55) || (*(rom_base + 1) != 0xaa)) return 0; - romptr = (readb(rom_base + 0x18) | (readb(rom_base + 0x19) << 8)); + romptr = (*(rom_base + 0x18) | (*(rom_base + 0x19) << 8)); if(romptr > (0x10000 - 8)) return 0; rom = rom_base + romptr; - if((readb(rom) != 'P') || (readb(rom + 1) != 'C') || - (readb(rom + 2) != 'I') || (readb(rom + 3) != 'R')) + if ((*(rom) != 'P') || (*(rom + 1) != 'C') || + (*(rom + 2) != 'I') || (*(rom + 3) != 'R')) return 0; - if((readb(rom + 4) | (readb(rom + 5) << 8)) != ivideo->chip_vendor) + if ((*(rom + 4) | (*(rom + 5) << 8)) != ivideo->chip_vendor) return 0; - if((readb(rom + 6) | (readb(rom + 7) << 8)) != ivideo->chip_id) + if ((*(rom + 6) | (*(rom + 7) << 8)) != ivideo->chip_id) return 0; return 1; @@ -4124,6 +4124,10 @@ static unsigned char *sisfb_find_rom(struct pci_dev *pdev) unsigned char *myrombase = NULL; size_t romsize; + myrombase = vmalloc(65536); + if (!myrombase) + return NULL; + /* First, try the official pci ROM functions (except * on integrated chipsets which have no ROM). */ @@ -4131,20 +4135,15 @@ static unsigned char *sisfb_find_rom(struct pci_dev *pdev) if(!ivideo->nbridge) { if((rom_base = pci_map_rom(pdev, &romsize))) { - - if(sisfb_check_rom(rom_base, ivideo)) { - - if((myrombase = vmalloc(65536))) { - memcpy_fromio(myrombase, rom_base, - (romsize > 65536) ? 65536 : romsize); - } - } + memcpy_fromio(myrombase, rom_base, + (romsize > 65536) ? 65536 : romsize); pci_unmap_rom(pdev, rom_base); + + if (sisfb_check_rom(myrombase, ivideo)) + return myrombase; } } - if(myrombase) return myrombase; - /* Otherwise do it the conventional way. */ #if defined(__i386__) || defined(__x86_64__) @@ -4156,24 +4155,17 @@ static unsigned char *sisfb_find_rom(struct pci_dev *pdev) rom_base = ioremap(temp, 65536); if (!rom_base) continue; - - if (!sisfb_check_rom(rom_base, ivideo)) { - iounmap(rom_base); - continue; - } - - if ((myrombase = vmalloc(65536))) - memcpy_fromio(myrombase, rom_base, 65536); - + memcpy_fromio(myrombase, rom_base, 65536); iounmap(rom_base); - break; + if (sisfb_check_rom(myrombase, ivideo)) + return myrombase; } } #endif - - return myrombase; + vfree(myrombase); + return NULL; } static void sisfb_post_map_vram(struct sis_video_info *ivideo,