From patchwork Mon Feb 8 15:13:42 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Matt Fleming X-Patchwork-Id: 8250751 Return-Path: X-Original-To: patchwork-linux-fsdevel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 30D249F319 for ; Mon, 8 Feb 2016 15:13:52 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id F3FCE200E8 for ; Mon, 8 Feb 2016 15:13:50 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 9657A2039E for ; Mon, 8 Feb 2016 15:13:48 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753621AbcBHPNr (ORCPT ); Mon, 8 Feb 2016 10:13:47 -0500 Received: from mail-wm0-f45.google.com ([74.125.82.45]:38248 "EHLO mail-wm0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753604AbcBHPNp (ORCPT ); Mon, 8 Feb 2016 10:13:45 -0500 Received: by mail-wm0-f45.google.com with SMTP id p63so119997145wmp.1 for ; Mon, 08 Feb 2016 07:13:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=console-pimps-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=C7Tz7jZMjiCngV3KKPM0/RBDVnENHyV3hdoNs/LXFtI=; b=el8ck80dSXQugFkvhifU5XWn9yBGCnltcMtKmXy2K8EHpeYMMb2h0fdwoZXnkv4chO O4t949rxq5UMVOgcG+sqDPNrpe0AfkapBwZEzc4k213dSxpUYMMZ0CxbBaqfaICEIYFd h1KO6frKHxZS7X49Re3iGZbHI45EF6BrIOmNhf51XBcF9y0jxDLzi5er6tLivFLwpv4z pNqnsfpffc55Abs8mir8iwNLB8ITy8YUazHKRmLHFAiJlD8nMhEABOgC7M/HEM8h6C3R SYZcqqWHo9zLu4fdtTb/qEZFXRBFuLpnrnrY1EIx2J8e5rVl/3B315Qn+TdawtnES6g6 qcow== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-type:content-disposition:in-reply-to :user-agent; bh=C7Tz7jZMjiCngV3KKPM0/RBDVnENHyV3hdoNs/LXFtI=; b=jNnYMO4uX/0WEB9IboVr5SmoGw30zvWpVU2nPhN8/HCUAf8L520ipxkA1xwXjNaFMj mANxPreeLE/aym4x9XqvHPoswIds1opGFRR6SU9PuCiAQI5tuFTtpSO94NQ/9tc7JKA6 emtBluOKJA4nH8r9rD1FQe+sAc6A+FO5VeIeOA+M2jDJGpM3dhfjPUkd1HIZSzr3x5Jb 8b6rWbpAJx8J3Ydsr1NskUsKfPG158EMEIZl9wI8RgS5kp1O8thVEkhUUABAY+KYOb64 d6YRgsHEzqS0PDmA/UuSRKXMJpULvK45KKlB9w0hbPtzss3Wimzfw9mp3m075Mxop0BP QImw== X-Gm-Message-State: AG10YOTA10VFMQCxki4f+Pa20dIvJMUXYTLa3LBQNXBKiOJykYyPR2D+WrsbUSGWi7Zs9A== X-Received: by 10.28.89.195 with SMTP id n186mr31991752wmb.49.1454944423915; Mon, 08 Feb 2016 07:13:43 -0800 (PST) Received: from localhost (5ec16434.skybroadband.com. [94.193.100.52]) by smtp.gmail.com with ESMTPSA id gt7sm30257280wjc.1.2016.02.08.07.13.42 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 08 Feb 2016 07:13:43 -0800 (PST) Date: Mon, 8 Feb 2016 15:13:42 +0000 From: Matt Fleming To: "Kweh, Hock Leong" Cc: Greg Kroah-Hartman , Ong Boon Leong , LKML , linux-efi@vger.kernel.org, Sam Protsenko , Peter Jones , Andy Lutomirski , Roy Franz , Borislav Petkov , James Bottomley , Linux FS Devel , Anvin H Peter , Bryan O'Donoghue Subject: Re: [PATCH v11 1/1] efi: a misc char interface for user to update efi firmware Message-ID: <20160208151342.GB2413@codeblueprint.co.uk> References: <1454042394-21507-1-git-send-email-hock.leong.kweh@intel.com> <1454042394-21507-2-git-send-email-hock.leong.kweh@intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1454042394-21507-2-git-send-email-hock.leong.kweh@intel.com> User-Agent: Mutt/1.5.24+41 (02bc14ed1569) (2015-08-30) Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Spam-Status: No, score=-7.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,T_DKIM_INVALID,UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Fri, 29 Jan, at 12:39:54PM, Kweh Hock Leong wrote: > From: "Kweh, Hock Leong" > > Introducing a kernel module to expose capsule loader interface > (misc char device file note) for users to upload capsule binaries. > > Example: > cat firmware.bin > /dev/efi_capsule_loader > > This patch also export efi_capsule_supported() function symbol for > verifying the submitted capsule header in this kernel module. > > Cc: Matt Fleming > Signed-off-by: Kweh, Hock Leong > --- > drivers/firmware/efi/Kconfig | 9 + > drivers/firmware/efi/Makefile | 1 + > drivers/firmware/efi/capsule-loader.c | 334 +++++++++++++++++++++++++++++++++ > drivers/firmware/efi/capsule.c | 1 + > 4 files changed, 345 insertions(+) > create mode 100644 drivers/firmware/efi/capsule-loader.c OK, I've picked this up. I did make some modifications based on Bryan's comments from v10 of this patch. You can see a diff of the changes below, but roughly, 1. Use Bryan's comments for changelog and expand it a bit 2. Add more detail to Kconfig entry, most people don't need this 3. Sprinkled comments from Bryan in the kerneldoc comments 4. Deleted a level of indentation in efi_capsule_setup_info() 5. Local variable instead of indexing ->pages in efi_capsule_write() 6. Fix memory leak in efi_capsule_open() My plan is to include this patch in the EFI capsule series, and resend the whole thing out. --- the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig index 73c14af00c19..de221bbde9c9 100644 --- a/drivers/firmware/efi/Kconfig +++ b/drivers/firmware/efi/Kconfig @@ -92,9 +92,10 @@ config EFI_CAPSULE_LOADER depends on EFI help This option exposes a loader interface "/dev/efi_capsule_loader" for - users to load EFI capsules. + users to load EFI capsules. This driver requires working runtime + capsule support in the firmware, which many OEMs do not provide. - If unsure, say N. + Most users should say N. endmenu diff --git a/drivers/firmware/efi/capsule-loader.c b/drivers/firmware/efi/capsule-loader.c index acffd767223e..9a43b1568234 100644 --- a/drivers/firmware/efi/capsule-loader.c +++ b/drivers/firmware/efi/capsule-loader.c @@ -33,7 +33,7 @@ struct capsule_info { * efi_free_all_buff_pages - free all previous allocated buffer pages * @cap_info: pointer to current instance of capsule_info structure * - * In addition of freeing buffer pages, it flags NO_FURTHER_WRITE_ACTION + * In addition to freeing buffer pages, it flags NO_FURTHER_WRITE_ACTION * to cease processing data in subsequent write(2) calls until close(2) * is called. **/ @@ -46,7 +46,7 @@ static void efi_free_all_buff_pages(struct capsule_info *cap_info) } /** - * efi_capsule_setup_info - to obtain the efi capsule header in the binary and + * efi_capsule_setup_info - obtain the efi capsule header in the binary and * setup capsule_info structure * @cap_info: pointer to current instance of capsule_info structure * @kbuff: a mapped first page buffer pointer @@ -55,44 +55,46 @@ static void efi_free_all_buff_pages(struct capsule_info *cap_info) static ssize_t efi_capsule_setup_info(struct capsule_info *cap_info, void *kbuff, size_t hdr_bytes) { + efi_capsule_header_t *cap_hdr; + size_t pages_needed; int ret; void *temp_page; - /* Only process data block that is larger than a efi header size */ - if (hdr_bytes >= sizeof(efi_capsule_header_t)) { - /* Reset back to the correct offset of header */ - efi_capsule_header_t *cap_hdr = kbuff - cap_info->count; - size_t pages_needed = ALIGN(cap_hdr->imagesize, PAGE_SIZE) >> - PAGE_SHIFT; + /* Only process data block that is larger than efi header size */ + if (hdr_bytes < sizeof(efi_capsule_header_t)) + return 0; - if (pages_needed == 0) { - pr_err("%s: pages count invalid\n", __func__); - return -EINVAL; - } + /* Reset back to the correct offset of header */ + cap_hdr = kbuff - cap_info->count; + pages_needed = ALIGN(cap_hdr->imagesize, PAGE_SIZE) >> PAGE_SHIFT; - /* Check if the capsule binary supported */ - ret = efi_capsule_supported(cap_hdr->guid, cap_hdr->flags, - cap_hdr->imagesize, - &cap_info->reset_type); - if (ret) { - pr_err("%s: efi_capsule_supported() failed\n", - __func__); - return ret; - } + if (pages_needed == 0) { + pr_err("%s: pages count invalid\n", __func__); + return -EINVAL; + } - cap_info->total_size = cap_hdr->imagesize; - temp_page = krealloc(cap_info->pages, - pages_needed * sizeof(void *), - GFP_KERNEL | __GFP_ZERO); - if (!temp_page) { - pr_debug("%s: krealloc() failed\n", __func__); - return -ENOMEM; - } + /* Check if the capsule binary supported */ + ret = efi_capsule_supported(cap_hdr->guid, cap_hdr->flags, + cap_hdr->imagesize, + &cap_info->reset_type); + if (ret) { + pr_err("%s: efi_capsule_supported() failed\n", + __func__); + return ret; + } - cap_info->pages = temp_page; - cap_info->header_obtained = true; + cap_info->total_size = cap_hdr->imagesize; + temp_page = krealloc(cap_info->pages, + pages_needed * sizeof(void *), + GFP_KERNEL | __GFP_ZERO); + if (!temp_page) { + pr_debug("%s: krealloc() failed\n", __func__); + return -ENOMEM; } + cap_info->pages = temp_page; + cap_info->header_obtained = true; + return 0; } @@ -137,21 +139,22 @@ static ssize_t efi_capsule_submit_update(struct capsule_info *cap_info) * @offp: not used * * Expectation: - * - User space tool should start at the beginning of capsule binary and + * - A user space tool should start at the beginning of capsule binary and * pass data in sequentially. * - Users should close and re-open this file note in order to upload more * capsules. * - After an error returned, user should close the file and restart the * operation for the next try otherwise -EIO will be returned until the * file is closed. - * - EFI capsule header must be located at the beginning of capsule binary - * file and passed in as first block data of write operation. + * - An EFI capsule header must be located at the beginning of capsule + * binary file and passed in as first block data of write operation. **/ static ssize_t efi_capsule_write(struct file *file, const char __user *buff, size_t count, loff_t *offp) { int ret = 0; struct capsule_info *cap_info = file->private_data; + struct page *page; void *kbuff = NULL; size_t write_byte; @@ -164,16 +167,20 @@ static ssize_t efi_capsule_write(struct file *file, const char __user *buff, /* Only alloc a new page when previous page is full */ if (!cap_info->page_bytes_remain) { - cap_info->pages[cap_info->index++] = alloc_page(GFP_KERNEL); - if (!cap_info->pages[cap_info->index - 1]) { + page = alloc_page(GFP_KERNEL); + if (!page) { pr_debug("%s: alloc_page() failed\n", __func__); ret = -ENOMEM; goto failed; } + + cap_info->pages[cap_info->index++] = page; cap_info->page_bytes_remain = PAGE_SIZE; } - kbuff = kmap(cap_info->pages[cap_info->index - 1]); + page = cap_info->pages[cap_info->index - 1]; + + kbuff = kmap(page); if (!kbuff) { pr_debug("%s: kmap() failed\n", __func__); ret = -EFAULT; @@ -199,7 +206,7 @@ static ssize_t efi_capsule_write(struct file *file, const char __user *buff, } cap_info->count += write_byte; - kunmap(cap_info->pages[cap_info->index - 1]); + kunmap(page); /* Submit the full binary to efi_capsule_update() API */ if (cap_info->header_obtained && @@ -219,7 +226,7 @@ static ssize_t efi_capsule_write(struct file *file, const char __user *buff, return write_byte; fail_unmap: - kunmap(cap_info->pages[cap_info->index - 1]); + kunmap(page); failed: efi_free_all_buff_pages(cap_info); return ret; @@ -253,8 +260,8 @@ static int efi_capsule_flush(struct file *file, fl_owner_t id) * @inode: not used * @file: file pointer * - * We would not free successfully submitted pages since efi update require - * data to be maintained across system reboot. + * We will not free successfully submitted pages since efi update + * requires data to be maintained across system reboot. **/ static int efi_capsule_release(struct inode *inode, struct file *file) { @@ -285,8 +292,10 @@ static int efi_capsule_open(struct inode *inode, struct file *file) return -ENOMEM; cap_info->pages = kzalloc(sizeof(void *), GFP_KERNEL); - if (!cap_info->pages) + if (!cap_info->pages) { + kfree(cap_info); return -ENOMEM; + } file->private_data = cap_info; -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in