RE: [PATCH v10 1/1] efi: a misc char interface for user to update efi firmware
> -Original Message- > From: Matt Fleming [mailto:m...@codeblueprint.co.uk] > Sent: Thursday, January 28, 2016 8:16 PM > > On Tue, 26 Jan, at 03:10:03AM, Kweh Hock Leong wrote: > > > > > > This mutex is not needed. It doesn't protect anything in your code. > > > > This is to synchronize/serializes one of the instant is calling > efi_capsule_supported() > > and the other one is calling efi_capsule_update() which they are exported > symbol > > from capsule.c. > > You don't need to synchronise them. > > Looking at my original capsule patches I can see why you might be > doing this locking, but it's unnecessary. You don't need to serialise > access to efi_capsule_supported() and efi_capsule_update(). > > Internally to the core EFI capsule code we need to ensure that we > don't allow capsules with conflicting reset types to be sent to the > firmware (how would we know the type of reset to perform?), but that > should be entirely transparent to your driver. > > I'm planning on re-sending my capsule patches, so all this should > become clearer. > Ok. Noted. Will remove it and submit for v11. > > > This function would be much more simple if you handled the above > > > condition differently. > > > > > > Instead of having code in efi_capsule_setup_info() to allocate the > > > initial ->pages memory, why not just allocate it in efi_capsule_open() > > > at the same time as you allocate the private_data? You can then > > > free it in efi_capsule_release() (you're leaking it at the moment). > > > > > > You are always going to need at least one element in ->pages for > > > successful operation, so you might as well allocate it up front. > > > > Just want to double check I understand you correctly. Do you mean > > I should free ->pages during close(2) but not free the ->pages[X] if > > there is any successfully submitted? > > Yes, that's correct. Ok. Noted. Thanks & Regards, Wilson
Re: [PATCH v10 1/1] efi: a misc char interface for user to update efi firmware
On Tue, 26 Jan, at 03:10:03AM, Kweh Hock Leong wrote: > > > > This mutex is not needed. It doesn't protect anything in your code. > > This is to synchronize/serializes one of the instant is calling > efi_capsule_supported() > and the other one is calling efi_capsule_update() which they are exported > symbol > from capsule.c. You don't need to synchronise them. Looking at my original capsule patches I can see why you might be doing this locking, but it's unnecessary. You don't need to serialise access to efi_capsule_supported() and efi_capsule_update(). Internally to the core EFI capsule code we need to ensure that we don't allow capsules with conflicting reset types to be sent to the firmware (how would we know the type of reset to perform?), but that should be entirely transparent to your driver. I'm planning on re-sending my capsule patches, so all this should become clearer. > > This function would be much more simple if you handled the above > > condition differently. > > > > Instead of having code in efi_capsule_setup_info() to allocate the > > initial ->pages memory, why not just allocate it in efi_capsule_open() > > at the same time as you allocate the private_data? You can then > > free it in efi_capsule_release() (you're leaking it at the moment). > > > > You are always going to need at least one element in ->pages for > > successful operation, so you might as well allocate it up front. > > Just want to double check I understand you correctly. Do you mean > I should free ->pages during close(2) but not free the ->pages[X] if > there is any successfully submitted? Yes, that's correct.
RE: [PATCH v10 1/1] efi: a misc char interface for user to update efi firmware
> -Original Message-
> From: Matt Fleming [mailto:m...@console-pimps.org]
> Sent: Thursday, January 21, 2016 7:52 PM
>
> On Fri, 18 Dec, at 08:13:01PM, Kweh Hock Leong wrote:
> > From: "Kweh, Hock Leong"
> >
> > Introducing a kernel module to expose capsule loader interface
> > (misc char device file note) for user to upload capsule binaries.
> >
> > Example method to load the capsule binary:
> > 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 | 10
> > drivers/firmware/efi/Makefile |1
> > drivers/firmware/efi/capsule-loader.c | 355
> +
> > drivers/firmware/efi/capsule.c|1
> > 4 files changed, 367 insertions(+)
> > create mode 100644 drivers/firmware/efi/capsule-loader.c
>
> [...]
>
> > +#define pr_fmt(fmt) "EFI: " fmt
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#define NO_FURTHER_WRITE_ACTION -1
> > +
> > +struct capsule_info {
> > + boolheader_obtained;
> > + int reset_type;
> > + longindex;
> > + size_t count;
> > + size_t total_size;
> > + struct page **pages;
> > + size_t page_bytes_remain;
> > +};
> > +
> > +static DEFINE_MUTEX(capsule_loader_lock);
>
> This mutex is not needed. It doesn't protect anything in your code.
This is to synchronize/serializes one of the instant is calling
efi_capsule_supported()
and the other one is calling efi_capsule_update() which they are exported symbol
from capsule.c.
>
> > +/**
> > + * efi_free_all_buff_pages - free all previous allocated buffer pages
> > + * @cap_info: pointer to current instance of capsule_info structure
> > + *
> > + * Besides freeing the buffer pages, it also flagged an
> > + * NO_FURTHER_WRITE_ACTION flag for indicating to the next write
> entries
> > + * that there is a flaw happened and any subsequence actions are not
> > + * valid unless close(2).
> > + **/
> > +static void efi_free_all_buff_pages(struct capsule_info *cap_info)
> > +{
> > + while (cap_info->index > 0)
> > + __free_page(cap_info->pages[--cap_info->index]);
> > +
> > + kfree(cap_info->pages);
> > +
> > + cap_info->index = NO_FURTHER_WRITE_ACTION;
> > +}
> > +
> > +/**
> > + * efi_capsule_setup_info - to 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 1st page buffer pointer
> > + * @hdr_bytes: the total bytes of received efi header
> > + **/
> > +static ssize_t efi_capsule_setup_info(struct capsule_info *cap_info,
> > + void *kbuff, size_t hdr_bytes)
> > +{
> > + int ret;
> > + void *temp_page;
> > +
> > + /* Handling 1st block is less than header size */
> > + if (hdr_bytes < sizeof(efi_capsule_header_t)) {
> > + if (!cap_info->pages) {
> > + cap_info->pages = kzalloc(sizeof(void *),
> GFP_KERNEL);
> > + if (!cap_info->pages) {
> > + pr_debug("%s: kzalloc() failed\n", __func__);
> > + return -ENOMEM;
> > + }
> > + }
>
> This function would be much more simple if you handled the above
> condition differently.
>
> Instead of having code in efi_capsule_setup_info() to allocate the
> initial ->pages memory, why not just allocate it in efi_capsule_open()
> at the same time as you allocate the private_data? You can then
> free it in efi_capsule_release() (you're leaking it at the moment).
>
> You are always going to need at least one element in ->pages for
> successful operation, so you might as well allocate it up front.
Just want to double check I understand you correctly. Do you mean
I should free ->pages during close(2) but not free the ->pages[X] if
there is any successfully submitted?
>
> > + } else {
> > + /* 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;
> > +
> > + if (pages_needed == 0) {
> > + pr_err("%s: pages count invalid\n", __func__);
> > + return -EINVAL;
> > + }
> > +
> > + /* Check if the capsule binary supported */
> > + mutex_lock(&capsule_loader_lock);
> > + ret = efi_capsule_supported(cap_hdr->guid, cap_hdr->flags,
> > + cap_hdr->imagesize,
> > + &cap_info->reset_typ
RE: [PATCH v10 1/1] efi: a misc char interface for user to update efi firmware
> -Original Message-
> From: Bryan O'Donoghue [mailto:pure.lo...@nexus-software.ie]
> Sent: Tuesday, December 22, 2015 1:04 AM
>
> Small nit.
>
Hi, Sorry for the late response. Happy New Year.
> On Fri, 2015-12-18 at 20:13 +0800, Kweh, Hock Leong wrote:
> > From: "Kweh, Hock Leong"
> >
> > Introducing a kernel module to expose capsule loader interface
> > (misc char device file note) for user to upload capsule binaries.
>
> This patch ? introduces a kernel module to expose a capsule loader
> interface... for a user ...
>
> >
> > Example method to load the capsule binary:
>
> Example:
>
Noted.
> > 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.
>
> 1. Should be a separate patch
> 2. Suggested, rewording for that patch log
>
> 2/2 Title
> This patch exports efi_capsule_supported to enable verification of the
> capsule header.
>
> That said - who is the user of this symbol ? Why is it needed ? Anyway
> please consider splitting and rewording.
>
Answered by Borislav.
> >
> > +config EFI_CAPSULE_LOADER
> > + tristate "EFI capsule loader"
> > + depends on EFI
> > + help
> > + This option exposes a loader interface
> > "/dev/efi_capsule_loader" for
> > + user to load EFI capsule binary and update the EFI
> > firmware. After
> > + a successful loading, a system reboot is required.
> > +
> > + If unsure, say N.
> > +
>
> This option exposes a loader interface... for a user to load an EFI
> capsule.
>
> A capsule isn't necessarily exclusively for updating of firmware either
> AFAIK - so I suggest dropping.
>
> http://www.intel.com/content/www/us/en/architecture-and-
> technology/unif
> ied-extensible-firmware-interface/efi-capsule-specification.html
>
> Quote "Capsules were designed for and are intended to be the major
> vehicle for delivering firmware volume changes. There is no requirement
> that capsules be limited to that function."
>
Noted.
> >
> > +
> > +/**
> > + * efi_free_all_buff_pages - free all previous allocated buffer
> > pages
> > + * @cap_info: pointer to current instance of capsule_info structure
> > + *
> > + * Besides freeing the buffer pages, it also flagged an
> > + * NO_FURTHER_WRITE_ACTION flag for indicating to the next
> > write entries
> > + * that there is a flaw happened and any subsequence actions
> > are not
> > + * valid unless close(2).
> > + **/
>
> The description needs to be reworded.
>
> In addition to freeing buffer pages, we flag NO_FURTHER_WRITE_ACTION on
> error and will cease to process data in subsequent efi_capsule_write()
> calls.
>
> (or similar)
>
Noted.
> > +
> > +/**
> > + * efi_capsule_setup_info - to 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 1st page buffer pointer
>
> first not 1st
>
Noted.
> > + * @hdr_bytes: the total bytes of received efi header
>
> the total number of received bytes of the EFI header
>
Noted.
> > + **/
> > +static ssize_t efi_capsule_setup_info(struct capsule_info *cap_info,
> > + void *kbuff, size_t hdr_bytes)
> > +{
> > + int ret;
> > + void *temp_page;
> > +
> > + /* Handling 1st block is less than header size */
> first or initial
> I think you mean to say "Ensure first
>
No. I mean handling. I will change to "first"
> > +
> > + /* Check if the capsule binary supported */
> > + mutex_lock(&capsule_loader_lock);
> > + ret = efi_capsule_supported(cap_hdr->guid, cap_hdr
> > ->flags,
> > + cap_hdr->imagesize,
> > + &cap_info->reset_type);
> > + mutex_unlock(&capsule_loader_lock);
>
> What does this mutex really do ? You hold it here around
> efi_capsule_supported() in the call
>
> chain efi_capsule_write() => efi_capsule_setup_info()
>
> and then again inside of efi_capsule_submit_update() the call chain
>
> chain efi_capsule_write() => efi_capsule_submit_update()
>
> So if its valid to ensure you are synchronizing parallel contexts with
> -respect to calls into efi_capsule_supported() and
> efi_capsule_submit_update() why aren't you holding that lock for the
> duration of efi_capsule_write() - couldn't cap_info->page_bytes_remain
> as an example equally be racy ?
>
This is to protect multiple instant calling open(2) and still able to
synchronize
the calling to efi_capsule_supported() and efi_capsule_update() when they
are uploading the capsule concurrently.
>
> > +
> > +/**
> > + * efi_capsule_submit_update - invoke the efi_capsule_update API
> > once binary
> > + *upload done
> > + * @cap_info: pointer to current instance of capsule_info structure
> > + **/
> > +static ssize_t efi_capsule_submi
Re: [PATCH v10 1/1] efi: a misc char interface for user to update efi firmware
On Mon, 21 Dec, at 05:04:11PM, Bryan O'Donoghue wrote:
> > +static int efi_capsule_open(struct inode *inode, struct file *file)
> > +{
> > + struct capsule_info *cap_info;
> > +
> > + cap_info = kzalloc(sizeof(*cap_info), GFP_KERNEL);
> > + if (!cap_info)
> > + return -ENOMEM;
> > +
> > + file->private_data = cap_info;
> > +
> > + return 0;
> > +}
>
> You have a memory leak here don't you ?
>
> if I do
> for (i = 0; i < N; i++) {
> fd = open(/dev/your_node);
> close(fd);
> }
>
> You'll leak that kzalloc...
Nope, it gets freed in efi_capsule_release(). Though the code does
leak cap_info->pages if a capsule is successfully submitted to the
firmware.
Re: [PATCH v10 1/1] efi: a misc char interface for user to update efi firmware
On Fri, 18 Dec, at 08:13:01PM, Kweh Hock Leong wrote:
> From: "Kweh, Hock Leong"
>
> Introducing a kernel module to expose capsule loader interface
> (misc char device file note) for user to upload capsule binaries.
>
> Example method to load the capsule binary:
> 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 | 10
> drivers/firmware/efi/Makefile |1
> drivers/firmware/efi/capsule-loader.c | 355
> +
> drivers/firmware/efi/capsule.c|1
> 4 files changed, 367 insertions(+)
> create mode 100644 drivers/firmware/efi/capsule-loader.c
[...]
> +#define pr_fmt(fmt) "EFI: " fmt
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define NO_FURTHER_WRITE_ACTION -1
> +
> +struct capsule_info {
> + boolheader_obtained;
> + int reset_type;
> + longindex;
> + size_t count;
> + size_t total_size;
> + struct page **pages;
> + size_t page_bytes_remain;
> +};
> +
> +static DEFINE_MUTEX(capsule_loader_lock);
This mutex is not needed. It doesn't protect anything in your code.
> +/**
> + * efi_free_all_buff_pages - free all previous allocated buffer pages
> + * @cap_info: pointer to current instance of capsule_info structure
> + *
> + * Besides freeing the buffer pages, it also flagged an
> + * NO_FURTHER_WRITE_ACTION flag for indicating to the next write entries
> + * that there is a flaw happened and any subsequence actions are not
> + * valid unless close(2).
> + **/
> +static void efi_free_all_buff_pages(struct capsule_info *cap_info)
> +{
> + while (cap_info->index > 0)
> + __free_page(cap_info->pages[--cap_info->index]);
> +
> + kfree(cap_info->pages);
> +
> + cap_info->index = NO_FURTHER_WRITE_ACTION;
> +}
> +
> +/**
> + * efi_capsule_setup_info - to 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 1st page buffer pointer
> + * @hdr_bytes: the total bytes of received efi header
> + **/
> +static ssize_t efi_capsule_setup_info(struct capsule_info *cap_info,
> + void *kbuff, size_t hdr_bytes)
> +{
> + int ret;
> + void *temp_page;
> +
> + /* Handling 1st block is less than header size */
> + if (hdr_bytes < sizeof(efi_capsule_header_t)) {
> + if (!cap_info->pages) {
> + cap_info->pages = kzalloc(sizeof(void *), GFP_KERNEL);
> + if (!cap_info->pages) {
> + pr_debug("%s: kzalloc() failed\n", __func__);
> + return -ENOMEM;
> + }
> + }
This function would be much more simple if you handled the above
condition differently.
Instead of having code in efi_capsule_setup_info() to allocate the
initial ->pages memory, why not just allocate it in efi_capsule_open()
at the same time as you allocate the private_data? You can then
free it in efi_capsule_release() (you're leaking it at the moment).
You are always going to need at least one element in ->pages for
successful operation, so you might as well allocate it up front.
> + } else {
> + /* 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;
> +
> + if (pages_needed == 0) {
> + pr_err("%s: pages count invalid\n", __func__);
> + return -EINVAL;
> + }
> +
> + /* Check if the capsule binary supported */
> + mutex_lock(&capsule_loader_lock);
> + ret = efi_capsule_supported(cap_hdr->guid, cap_hdr->flags,
> + cap_hdr->imagesize,
> + &cap_info->reset_type);
> + mutex_unlock(&capsule_loader_lock);
> + if (ret) {
> + pr_err("%s: efi_capsule_supported() failed\n",
> +__func__);
> + return ret;
> + }
> +
> + 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;
>
Re: [PATCH v10 1/1] efi: a misc char interface for user to update efi firmware
On Mon, 2015-12-21 at 18:37 +0100, Borislav Petkov wrote: > On Mon, Dec 21, 2015 at 05:04:11PM +, Bryan O'Donoghue wrote: > > > This patch also export efi_capsule_supported() function symbol > > > for > > > verifying the submitted capsule header in this kernel module. > > > > 1. Should be a separate patch > > 2. Suggested, rewording for that patch log > > > > 2/2 Title > > This patch exports efi_capsule_supported to enable verification of > > the > > capsule header. > > > > That said - who is the user of this symbol ? Why is it needed ? > > Anyway > > please consider splitting and rewording. > > Huh? > > I still don't really see the reasoning for splitting out the export. > > When you do the export and use it in a single patch it is *obvious* > why > it is being exported. > > And there's no need to mention in the commit message that you're > exporting a symbol. People export symbols all the time. > Yes - saw the export down the end of the patchset. Feel free to ignore that comment. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v10 1/1] efi: a misc char interface for user to update efi firmware
On Mon, Dec 21, 2015 at 05:04:11PM +, Bryan O'Donoghue wrote: > > This patch also export efi_capsule_supported() function symbol for > > verifying the submitted capsule header in this kernel module. > > 1. Should be a separate patch > 2. Suggested, rewording for that patch log > > 2/2 Title > This patch exports efi_capsule_supported to enable verification of the > capsule header. > > That said - who is the user of this symbol ? Why is it needed ? Anyway > please consider splitting and rewording. Huh? I still don't really see the reasoning for splitting out the export. When you do the export and use it in a single patch it is *obvious* why it is being exported. And there's no need to mention in the commit message that you're exporting a symbol. People export symbols all the time. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v10 1/1] efi: a misc char interface for user to update efi firmware
Small nit.
On Fri, 2015-12-18 at 20:13 +0800, Kweh, Hock Leong wrote:
> From: "Kweh, Hock Leong"
>
> Introducing a kernel module to expose capsule loader interface
> (misc char device file note) for user to upload capsule binaries.
This patch ? introduces a kernel module to expose a capsule loader
interface... for a user ...
>
> Example method to load the capsule binary:
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.
1. Should be a separate patch
2. Suggested, rewording for that patch log
2/2 Title
This patch exports efi_capsule_supported to enable verification of the
capsule header.
That said - who is the user of this symbol ? Why is it needed ? Anyway
please consider splitting and rewording.
>
> Cc: Matt Fleming
> Signed-off-by: Kweh, Hock Leong
> ---
> drivers/firmware/efi/Kconfig | 10
> drivers/firmware/efi/Makefile |1
> drivers/firmware/efi/capsule-loader.c | 355
> +
> drivers/firmware/efi/capsule.c|1
> 4 files changed, 367 insertions(+)
> create mode 100644 drivers/firmware/efi/capsule-loader.c
>
> diff --git a/drivers/firmware/efi/Kconfig
> b/drivers/firmware/efi/Kconfig
> index e1670d5..dc2a912 100644
> --- a/drivers/firmware/efi/Kconfig
> +++ b/drivers/firmware/efi/Kconfig
> @@ -87,6 +87,16 @@ config EFI_RUNTIME_WRAPPERS
> config EFI_ARMSTUB
> bool
>
> +config EFI_CAPSULE_LOADER
> + tristate "EFI capsule loader"
> + depends on EFI
> + help
> + This option exposes a loader interface
> "/dev/efi_capsule_loader" for
> + user to load EFI capsule binary and update the EFI
> firmware. After
> + a successful loading, a system reboot is required.
> +
> + If unsure, say N.
> +
This option exposes a loader interface... for a user to load an EFI
capsule.
A capsule isn't necessarily exclusively for updating of firmware either
AFAIK - so I suggest dropping.
http://www.intel.com/content/www/us/en/architecture-and-technology/unif
ied-extensible-firmware-interface/efi-capsule-specification.html
Quote "Capsules were designed for and are intended to be the major
vehicle for delivering firmware volume changes. There is no requirement
that capsules be limited to that function."
> endmenu
>
> config UEFI_CPER
> diff --git a/drivers/firmware/efi/Makefile
> b/drivers/firmware/efi/Makefile
> index fabf801..cf9d9d3 100644
> --- a/drivers/firmware/efi/Makefile
> +++ b/drivers/firmware/efi/Makefile
> @@ -18,3 +18,4 @@ obj-$(CONFIG_EFI_RUNTIME_MAP) +=
> runtime-map.o
> obj-$(CONFIG_EFI_RUNTIME_WRAPPERS) += runtime-wrappers.o
> obj-$(CONFIG_EFI_STUB) += libstub/
> obj-$(CONFIG_EFI_FAKE_MEMMAP)+= fake_mem.o
> +obj-$(CONFIG_EFI_CAPSULE_LOADER) += capsule-loader.o
> diff --git a/drivers/firmware/efi/capsule-loader.c
> b/drivers/firmware/efi/capsule-loader.c
> new file mode 100644
> index 000..e1214bb
> --- /dev/null
> +++ b/drivers/firmware/efi/capsule-loader.c
> @@ -0,0 +1,355 @@
> +/*
> + * EFI capsule loader driver.
> + *
> + * Copyright 2015 Intel Corporation
> + *
> + * This file is part of the Linux kernel, and is made available
> under
> + * the terms of the GNU General Public License version 2.
> + */
> +
> +#define pr_fmt(fmt) "EFI: " fmt
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define NO_FURTHER_WRITE_ACTION -1
> +
> +struct capsule_info {
> + boolheader_obtained;
> + int reset_type;
> + longindex;
> + size_t count;
> + size_t total_size;
> + struct page **pages;
> + size_t page_bytes_remain;
> +};
> +
> +static DEFINE_MUTEX(capsule_loader_lock);
> +
> +/**
> + * efi_free_all_buff_pages - free all previous allocated buffer
> pages
> + * @cap_info: pointer to current instance of capsule_info structure
> + *
> + * Besides freeing the buffer pages, it also flagged an
> + * NO_FURTHER_WRITE_ACTION flag for indicating to the next
> write entries
> + * that there is a flaw happened and any subsequence actions
> are not
> + * valid unless close(2).
> + **/
The description needs to be reworded.
In addition to freeing buffer pages, we flag NO_FURTHER_WRITE_ACTION on
error and will cease to process data in subsequent efi_capsule_write()
calls.
(or similar)
> +static void efi_free_all_buff_pages(struct capsule_info *cap_info)
> +{
> + while (cap_info->index > 0)
> + __free_page(cap_info->pages[--cap_info->index]);
> +
> + kfree(cap_info->pages);
> +
> + cap_info->index = NO_FURTHER_WRITE_ACTION;
> +}
> +
> +/**
> + * efi_capsule_setup_info - to obtain the efi capsule header in the
> binary and
> + * setup capsule_info structure
> + * @cap_info: pointer to current instance
