Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1640781 > unrolled thread

[PATCH] efi-pstore: Fix read iter after pstore API refactor

Back to article view | Back to linux.kernel

Contents

  [PATCH] efi-pstore: Fix read iter after pstore API refactor Kees Cook <keescook@chromium.org> - 2017-05-13 00:00 +0200
    Re: [PATCH] efi-pstore: Fix read iter after pstore API refactor Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2017-05-18 12:40 +0200
      Re: [PATCH] efi-pstore: Fix read iter after pstore API refactor Kees Cook <keescook@chromium.org> - 2017-05-18 15:10 +0200
        Re: [PATCH] efi-pstore: Fix read iter after pstore API refactor Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2017-05-18 18:20 +0200
          Re: [PATCH] efi-pstore: Fix read iter after pstore API refactor Kees Cook <keescook@chromium.org> - 2017-05-18 18:50 +0200
            Re: [PATCH] efi-pstore: Fix read iter after pstore API refactor Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2017-05-19 15:40 +0200

#1640781 — [PATCH] efi-pstore: Fix read iter after pstore API refactor

During the internal pstore API refactoring, the EFI vars read entry was
accidentally made to update a stack variable instead of the pstore
private data pointer. This corrects the problem (and removes the now
needless argument).

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 drivers/firmware/efi/efi-pstore.c | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/drivers/firmware/efi/efi-pstore.c b/drivers/firmware/efi/efi-pstore.c
index 93d8cdbe7ef4..9e6f14c354f1 100644
--- a/drivers/firmware/efi/efi-pstore.c
+++ b/drivers/firmware/efi/efi-pstore.c
@@ -155,25 +155,20 @@ static int efi_pstore_scan_sysfs_exit(struct efivar_entry *pos,
  * efi_pstore_sysfs_entry_iter
  *
  * @record: pstore record to pass to callback
- * @pos: entry to begin iterating from
  *
  * You MUST call efivar_enter_iter_begin() before this function, and
  * efivar_entry_iter_end() afterwards.
  *
- * It is possible to begin iteration from an arbitrary entry within
- * the list by passing @pos. @pos is updated on return to point to
- * the next entry of the last one passed to efi_pstore_read_func().
- * To begin iterating from the beginning of the list @pos must be %NULL.
  */
-static int efi_pstore_sysfs_entry_iter(struct pstore_record *record,
-				       struct efivar_entry **pos)
+static int efi_pstore_sysfs_entry_iter(struct pstore_record *record)
 {
+	struct efivar_entry *pos = (struct efivar_entry *)record->psi->data;
 	struct efivar_entry *entry, *n;
 	struct list_head *head = &efivar_sysfs_list;
 	int size = 0;
 	int ret;
 
-	if (!*pos) {
+	if (!pos) {
 		list_for_each_entry_safe(entry, n, head, list) {
 			efi_pstore_scan_sysfs_enter(entry, n, head);
 
@@ -185,21 +180,21 @@ static int efi_pstore_sysfs_entry_iter(struct pstore_record *record,
 			if (size)
 				break;
 		}
-		*pos = n;
+		pos = n;
 		return size;
 	}
 
-	list_for_each_entry_safe_from((*pos), n, head, list) {
-		efi_pstore_scan_sysfs_enter((*pos), n, head);
+	list_for_each_entry_safe_from(pos, n, head, list) {
+		efi_pstore_scan_sysfs_enter(pos, n, head);
 
-		size = efi_pstore_read_func((*pos), record);
-		ret = efi_pstore_scan_sysfs_exit((*pos), n, head, size < 0);
+		size = efi_pstore_read_func(pos, record);
+		ret = efi_pstore_scan_sysfs_exit(pos, n, head, size < 0);
 		if (ret)
 			return ret;
 		if (size)
 			break;
 	}
-	*pos = n;
+	pos = n;
 	return size;
 }
 
@@ -218,7 +213,6 @@ static int efi_pstore_sysfs_entry_iter(struct pstore_record *record,
  */
 static ssize_t efi_pstore_read(struct pstore_record *record)
 {
-	struct efivar_entry *entry = (struct efivar_entry *)record->psi->data;
 	ssize_t size;
 
 	record->buf = kzalloc(EFIVARS_DATA_SIZE_MAX, GFP_KERNEL);
@@ -229,7 +223,7 @@ static ssize_t efi_pstore_read(struct pstore_record *record)
 		size = -EINTR;
 		goto out;
 	}
-	size = efi_pstore_sysfs_entry_iter(record, &entry);
+	size = efi_pstore_sysfs_entry_iter(record);
 	efivar_entry_iter_end();
 
 out:
-- 
2.7.4


-- 
Kees Cook
Pixel Security

[toc] | [next] | [standalone]

#1644062

On 12 May 2017 at 22:58, Kees Cook <keescook@chromium.org> wrote:
> During the internal pstore API refactoring, the EFI vars read entry was
> accidentally made to update a stack variable instead of the pstore
> private data pointer. This corrects the problem (and removes the now
> needless argument).
>
> Signed-off-by: Kees Cook <keescook@chromium.org>

Does this need a cc stable?

> ---
>  drivers/firmware/efi/efi-pstore.c | 26 ++++++++++----------------
>  1 file changed, 10 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/firmware/efi/efi-pstore.c b/drivers/firmware/efi/efi-pstore.c
> index 93d8cdbe7ef4..9e6f14c354f1 100644
> --- a/drivers/firmware/efi/efi-pstore.c
> +++ b/drivers/firmware/efi/efi-pstore.c
> @@ -155,25 +155,20 @@ static int efi_pstore_scan_sysfs_exit(struct efivar_entry *pos,
>   * efi_pstore_sysfs_entry_iter
>   *
>   * @record: pstore record to pass to callback
> - * @pos: entry to begin iterating from
>   *
>   * You MUST call efivar_enter_iter_begin() before this function, and
>   * efivar_entry_iter_end() afterwards.
>   *
> - * It is possible to begin iteration from an arbitrary entry within
> - * the list by passing @pos. @pos is updated on return to point to
> - * the next entry of the last one passed to efi_pstore_read_func().
> - * To begin iterating from the beginning of the list @pos must be %NULL.
>   */
> -static int efi_pstore_sysfs_entry_iter(struct pstore_record *record,
> -                                      struct efivar_entry **pos)
> +static int efi_pstore_sysfs_entry_iter(struct pstore_record *record)
>  {
> +       struct efivar_entry *pos = (struct efivar_entry *)record->psi->data;
>         struct efivar_entry *entry, *n;
>         struct list_head *head = &efivar_sysfs_list;
>         int size = 0;
>         int ret;
>
> -       if (!*pos) {
> +       if (!pos) {
>                 list_for_each_entry_safe(entry, n, head, list) {
>                         efi_pstore_scan_sysfs_enter(entry, n, head);
>
> @@ -185,21 +180,21 @@ static int efi_pstore_sysfs_entry_iter(struct pstore_record *record,
>                         if (size)
>                                 break;
>                 }
> -               *pos = n;
> +               pos = n;
>                 return size;
>         }
>
> -       list_for_each_entry_safe_from((*pos), n, head, list) {
> -               efi_pstore_scan_sysfs_enter((*pos), n, head);
> +       list_for_each_entry_safe_from(pos, n, head, list) {
> +               efi_pstore_scan_sysfs_enter(pos, n, head);
>
> -               size = efi_pstore_read_func((*pos), record);
> -               ret = efi_pstore_scan_sysfs_exit((*pos), n, head, size < 0);
> +               size = efi_pstore_read_func(pos, record);
> +               ret = efi_pstore_scan_sysfs_exit(pos, n, head, size < 0);
>                 if (ret)
>                         return ret;
>                 if (size)
>                         break;
>         }
> -       *pos = n;
> +       pos = n;
>         return size;
>  }
>
> @@ -218,7 +213,6 @@ static int efi_pstore_sysfs_entry_iter(struct pstore_record *record,
>   */
>  static ssize_t efi_pstore_read(struct pstore_record *record)
>  {
> -       struct efivar_entry *entry = (struct efivar_entry *)record->psi->data;
>         ssize_t size;
>
>         record->buf = kzalloc(EFIVARS_DATA_SIZE_MAX, GFP_KERNEL);
> @@ -229,7 +223,7 @@ static ssize_t efi_pstore_read(struct pstore_record *record)
>                 size = -EINTR;
>                 goto out;
>         }
> -       size = efi_pstore_sysfs_entry_iter(record, &entry);
> +       size = efi_pstore_sysfs_entry_iter(record);
>         efivar_entry_iter_end();
>
>  out:
> --
> 2.7.4
>
>
> --
> Kees Cook
> Pixel Security

[toc] | [prev] | [next] | [standalone]

#1644462

On Thu, May 18, 2017 at 3:35 AM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> On 12 May 2017 at 22:58, Kees Cook <keescook@chromium.org> wrote:
>> During the internal pstore API refactoring, the EFI vars read entry was
>> accidentally made to update a stack variable instead of the pstore
>> private data pointer. This corrects the problem (and removes the now
>> needless argument).
>>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>
> Does this need a cc stable?

No, the refactor first appeared in 4.12-rc1.

-Kees

-- 
Kees Cook
Pixel Security

[toc] | [prev] | [next] | [standalone]

#1644722

On 18 May 2017 at 14:01, Kees Cook <keescook@chromium.org> wrote:
> On Thu, May 18, 2017 at 3:35 AM, Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
>> On 12 May 2017 at 22:58, Kees Cook <keescook@chromium.org> wrote:
>>> During the internal pstore API refactoring, the EFI vars read entry was
>>> accidentally made to update a stack variable instead of the pstore
>>> private data pointer. This corrects the problem (and removes the now
>>> needless argument).
>>>
>>> Signed-off-by: Kees Cook <keescook@chromium.org>
>>
>> Does this need a cc stable?
>
> No, the refactor first appeared in 4.12-rc1.
>

OK. Applied.

[toc] | [prev] | [next] | [standalone]

#1644748

On Thu, May 18, 2017 at 9:18 AM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> On 18 May 2017 at 14:01, Kees Cook <keescook@chromium.org> wrote:
>> On Thu, May 18, 2017 at 3:35 AM, Ard Biesheuvel
>> <ard.biesheuvel@linaro.org> wrote:
>>> On 12 May 2017 at 22:58, Kees Cook <keescook@chromium.org> wrote:
>>>> During the internal pstore API refactoring, the EFI vars read entry was
>>>> accidentally made to update a stack variable instead of the pstore
>>>> private data pointer. This corrects the problem (and removes the now
>>>> needless argument).
>>>>
>>>> Signed-off-by: Kees Cook <keescook@chromium.org>
>>>
>>> Does this need a cc stable?
>>
>> No, the refactor first appeared in 4.12-rc1.
>>
>
> OK. Applied.

Err, eek, no, it's already gone into Linus's tree.

-Kees

-- 
Kees Cook
Pixel Security

[toc] | [prev] | [next] | [standalone]

#1645605

On 18 May 2017 at 17:41, Kees Cook <keescook@chromium.org> wrote:
> On Thu, May 18, 2017 at 9:18 AM, Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
>> On 18 May 2017 at 14:01, Kees Cook <keescook@chromium.org> wrote:
>>> On Thu, May 18, 2017 at 3:35 AM, Ard Biesheuvel
>>> <ard.biesheuvel@linaro.org> wrote:
>>>> On 12 May 2017 at 22:58, Kees Cook <keescook@chromium.org> wrote:
>>>>> During the internal pstore API refactoring, the EFI vars read entry was
>>>>> accidentally made to update a stack variable instead of the pstore
>>>>> private data pointer. This corrects the problem (and removes the now
>>>>> needless argument).
>>>>>
>>>>> Signed-off-by: Kees Cook <keescook@chromium.org>
>>>>
>>>> Does this need a cc stable?
>>>
>>> No, the refactor first appeared in 4.12-rc1.
>>>
>>
>> OK. Applied.
>
> Err, eek, no, it's already gone into Linus's tree.
>

OK, I dropped it again :-)

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web