Re: [GIT PULL] x86 fixes for 3.9
On 26/04/13 10:02, Michel Lespinasse wrote:
> On Fri, Apr 26, 2013 at 1:49 AM, Matt Fleming wrote:
>> On 26/04/13 08:43, Michel Lespinasse wrote:
>>> Still seeing the crash.
>>>
>>> I went and compared the crash dump with the vmlinux disassembly; the
>>> issue is a NULL pointer dereference in list_for_each_entry_safe().
>>> list_empty() checks that the head node points to itself, but here the
>>> head node has NULL. I think this may be due to gsmi_init() being
>>> called before efivars_init(). Not sure what's the proper fix though.
>>
>> Ohh... I see what you mean. The bug is in variable_is_present() because
>> it accesses __efivars directly, which a) isn't the struct efivars gsmi.c
>> uses and b) hasn't been initialised. Something like this might work.
>
> [... skipping patch ...]
>
> Yes, this one fixes it. Thanks !
Thanks for testing!
Linus, did you want to take this one directly?
---
>From f576b789b29dab31ddc1c7d37af63f10fb203fb7 Mon Sep 17 00:00:00 2001
From: Matt Fleming
Date: Fri, 26 Apr 2013 10:10:55 +0100
Subject: [PATCH] efivars: only check for duplicates on the registered list
variable_is_present() accesses '__efivars' directly, but when called
via gsmi_init() Michel reports observing the following crash,
BUG: unable to handle kernel NULL pointer dereference at (null)
IP: [] variable_is_present+0x55/0x170
Call Trace:
[] register_efivars+0x106/0x370
[] ? firmware_map_add_early+0xb1/0xb1
[] gsmi_init+0x2ad/0x3da
[] do_one_initcall+0x3f/0x170
The reason for the crash is that '__efivars' hasn't been initialised nor
has it been registered with register_efivars() by the time the google
EFI SMI driver runs. The gsmi code uses its own struct efivars, and
therefore, a different variable list. Fix the above crash by passing the
registered struct efivars to variable_is_present(), so that we traverse
the correct list.
Reported-by: Michel Lespinasse
Tested-by: Michel Lespinasse
Cc: Mike Waychison
Cc: Matthew Garrett
Cc: Seiji Aguchi
Signed-off-by: Matt Fleming
---
drivers/firmware/efivars.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
index 182ce94..f4baa11 100644
--- a/drivers/firmware/efivars.c
+++ b/drivers/firmware/efivars.c
@@ -1628,10 +1628,11 @@ static ssize_t efivar_delete(struct file *filp, struct
kobject *kobj,
return count;
}
-static bool variable_is_present(efi_char16_t *variable_name, efi_guid_t
*vendor)
+static bool variable_is_present(struct efivars *efivars,
+ efi_char16_t *variable_name,
+ efi_guid_t *vendor)
{
struct efivar_entry *entry, *n;
- struct efivars *efivars = &__efivars;
unsigned long strsize1, strsize2;
bool found = false;
@@ -1703,8 +1704,8 @@ static void efivar_update_sysfs_entries(struct
work_struct *work)
if (status != EFI_SUCCESS) {
break;
} else {
- if (!variable_is_present(variable_name,
- )) {
+ if (!variable_is_present(efivars,
+ variable_name, )) {
found = true;
break;
}
@@ -2008,7 +2009,8 @@ int register_efivars(struct efivars *efivars,
* we'll ever see a different variable name,
* and may end up looping here forever.
*/
- if (variable_is_present(variable_name, _guid)) {
+ if (variable_is_present(efivars, variable_name,
+ _guid)) {
dup_variable_bug(variable_name, _guid,
variable_name_size);
status = EFI_NOT_FOUND;
--
1.7.10.4
--
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: [GIT PULL] x86 fixes for 3.9
On 26/04/13 08:29, Ingo Molnar wrote: > I'm worried about the fragility of this code - this is firmware code ... > > I think firmware code should be fundamentally paranoid and robust, and in > this case treat all EFI-provided data as hostile and do a much sanity > checking of it as possible - and provide an actionable error message if > the checks fail, not just 'crash'. > > Even if no-one complained, yet. I'm not sure how much more robust checking for NULL makes it, it's not going to save us from garbage pointers, etc. Instead of the pointer being NULL a more likely bug is that query_variable_info() exists (for those firmware that implement the relevant spec version), but doesn't work correctly. That's the kind of bug we've been seeing in other functions. To be fair to Josh, his original submission did include the NULL check, but I asked him to remove it. If someone really wants to re-add the check for a NULL pointer, I'm not super opposed to it. -- 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: [GIT PULL] x86 fixes for 3.9
On Fri, Apr 26, 2013 at 1:49 AM, Matt Fleming wrote: > On 26/04/13 08:43, Michel Lespinasse wrote: >> Still seeing the crash. >> >> I went and compared the crash dump with the vmlinux disassembly; the >> issue is a NULL pointer dereference in list_for_each_entry_safe(). >> list_empty() checks that the head node points to itself, but here the >> head node has NULL. I think this may be due to gsmi_init() being >> called before efivars_init(). Not sure what's the proper fix though. > > Ohh... I see what you mean. The bug is in variable_is_present() because > it accesses __efivars directly, which a) isn't the struct efivars gsmi.c > uses and b) hasn't been initialised. Something like this might work. [... skipping patch ...] Yes, this one fixes it. Thanks ! -- Michel "Walken" Lespinasse A program is never fully debugged until the last user dies. -- 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: [GIT PULL] x86 fixes for 3.9
On 26/04/13 08:43, Michel Lespinasse wrote:
> Still seeing the crash.
>
> I went and compared the crash dump with the vmlinux disassembly; the
> issue is a NULL pointer dereference in list_for_each_entry_safe().
> list_empty() checks that the head node points to itself, but here the
> head node has NULL. I think this may be due to gsmi_init() being
> called before efivars_init(). Not sure what's the proper fix though.
Ohh... I see what you mean. The bug is in variable_is_present() because
it accesses __efivars directly, which a) isn't the struct efivars gsmi.c
uses and b) hasn't been initialised. Something like this might work.
---
diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
index 182ce94..f4baa11 100644
--- a/drivers/firmware/efivars.c
+++ b/drivers/firmware/efivars.c
@@ -1628,10 +1628,11 @@ static ssize_t efivar_delete(struct file *filp, struct
kobject *kobj,
return count;
}
-static bool variable_is_present(efi_char16_t *variable_name, efi_guid_t
*vendor)
+static bool variable_is_present(struct efivars *efivars,
+ efi_char16_t *variable_name,
+ efi_guid_t *vendor)
{
struct efivar_entry *entry, *n;
- struct efivars *efivars = &__efivars;
unsigned long strsize1, strsize2;
bool found = false;
@@ -1703,8 +1704,8 @@ static void efivar_update_sysfs_entries(struct
work_struct *work)
if (status != EFI_SUCCESS) {
break;
} else {
- if (!variable_is_present(variable_name,
- )) {
+ if (!variable_is_present(efivars,
+ variable_name, )) {
found = true;
break;
}
@@ -2008,7 +2009,8 @@ int register_efivars(struct efivars *efivars,
* we'll ever see a different variable name,
* and may end up looping here forever.
*/
- if (variable_is_present(variable_name, _guid)) {
+ if (variable_is_present(efivars, variable_name,
+ _guid)) {
dup_variable_bug(variable_name, _guid,
variable_name_size);
status = EFI_NOT_FOUND;
--
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: [GIT PULL] x86 fixes for 3.9
On Fri, Apr 26, 2013 at 12:12 AM, Matt Fleming wrote:
> On 26/04/13 00:11, Michel Lespinasse wrote:
>> On Thu, Apr 25, 2013 at 3:54 PM, H. Peter Anvin wrote:
>>> On 04/25/2013 03:53 PM, Michel Lespinasse wrote:
Well, I don't know if this is related, but commit e971318bbed6 broke
the google EFI SMI driver with
BUG: unable to handle kernel NULL pointer dereference at (null)
IP: [] variable_is_present+0x55/0x170
Call Trace:
[] register_efivars+0x106/0x370
[] ? firmware_map_add_early+0xb1/0xb1
[] gsmi_init+0x2ad/0x3da
[] do_one_initcall+0x3f/0x170
...
>>>
>>> I don't know either. Could you test this patch and see if it does anything?
>>
>> Nope, still seeing the crash with this patch applied.
>
> Could you try the following patch against Linus' tree? The bug you've
> found and the changes in the pull request are unrelated.
>
> diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
> index 182ce94..2a4f619 100644
> --- a/drivers/firmware/efivars.c
> +++ b/drivers/firmware/efivars.c
> @@ -1635,6 +1635,9 @@ static bool variable_is_present(efi_char16_t
> *variable_name, efi_guid_t *vendor)
> unsigned long strsize1, strsize2;
> bool found = false;
>
> + if (list_empty(>list))
> + return false;
> +
> strsize1 = ucs2_strsize(variable_name, 1024);
> list_for_each_entry_safe(entry, n, >list, list) {
> strsize2 = ucs2_strsize(entry->var.VariableName, 1024);
Still seeing the crash.
I went and compared the crash dump with the vmlinux disassembly; the
issue is a NULL pointer dereference in list_for_each_entry_safe().
list_empty() checks that the head node points to itself, but here the
head node has NULL. I think this may be due to gsmi_init() being
called before efivars_init(). Not sure what's the proper fix though.
--
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.
--
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: [GIT PULL] x86 fixes for 3.9
* Matthew Garrett wrote: > On Thu, 2013-04-25 at 15:20 -0700, Linus Torvalds wrote: > > On Thu, Apr 25, 2013 at 2:44 PM, H. Peter Anvin > > wrote: > > > > > > - if (!sys_table->runtime->query_variable_info) > > > + if (sys_table->runtime->hdr.revision < > > > EFI_2_00_SYSTEM_TABLE_REVISION) > > > return EFI_UNSUPPORTED; > > > > Is a EFI 2.00 system table *guaranteed* to have that > > "query_variable_info" function? The above adds the version check, but > > removes the check for a NULL pointer. > > As far as the spec's concerned, yes. As far as reality's concerned - if > anything doesn't provide it, we're already crashing when > efi_virt_query_variable_info() gets called. Nobody's complained so far. I'm worried about the fragility of this code - this is firmware code ... I think firmware code should be fundamentally paranoid and robust, and in this case treat all EFI-provided data as hostile and do a much sanity checking of it as possible - and provide an actionable error message if the checks fail, not just 'crash'. Even if no-one complained, yet. Thanks, Ingo -- 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: [GIT PULL] x86 fixes for 3.9
On 26/04/13 00:11, Michel Lespinasse wrote:
> On Thu, Apr 25, 2013 at 3:54 PM, H. Peter Anvin wrote:
>> On 04/25/2013 03:53 PM, Michel Lespinasse wrote:
>>> On Thu, Apr 25, 2013 at 3:23 PM, Matthew Garrett
>>> wrote:
On Thu, 2013-04-25 at 15:20 -0700, Linus Torvalds wrote:
> On Thu, Apr 25, 2013 at 2:44 PM, H. Peter Anvin
> wrote:
>>
>> - if (!sys_table->runtime->query_variable_info)
>> + if (sys_table->runtime->hdr.revision <
>> EFI_2_00_SYSTEM_TABLE_REVISION)
>> return EFI_UNSUPPORTED;
>
> Is a EFI 2.00 system table *guaranteed* to have that
> "query_variable_info" function? The above adds the version check, but
> removes the check for a NULL pointer.
As far as the spec's concerned, yes. As far as reality's concerned - if
anything doesn't provide it, we're already crashing when
efi_virt_query_variable_info() gets called. Nobody's complained so far.
>>>
>>> Well, I don't know if this is related, but commit e971318bbed6 broke
>>> the google EFI SMI driver with
>>> BUG: unable to handle kernel NULL pointer dereference at (null)
>>> IP: [] variable_is_present+0x55/0x170
>>> Call Trace:
>>> [] register_efivars+0x106/0x370
>>> [] ? firmware_map_add_early+0xb1/0xb1
>>> [] gsmi_init+0x2ad/0x3da
>>> [] do_one_initcall+0x3f/0x170
>>> ...
>>
>> I don't know either. Could you test this patch and see if it does anything?
>
> Nope, still seeing the crash with this patch applied.
Could you try the following patch against Linus' tree? The bug you've
found and the changes in the pull request are unrelated.
---
diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
index 182ce94..2a4f619 100644
--- a/drivers/firmware/efivars.c
+++ b/drivers/firmware/efivars.c
@@ -1635,6 +1635,9 @@ static bool variable_is_present(efi_char16_t
*variable_name, efi_guid_t *vendor)
unsigned long strsize1, strsize2;
bool found = false;
+ if (list_empty(>list))
+ return false;
+
strsize1 = ucs2_strsize(variable_name, 1024);
list_for_each_entry_safe(entry, n, >list, list) {
strsize2 = ucs2_strsize(entry->var.VariableName, 1024);
--
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: [GIT PULL] x86 fixes for 3.9
On 26/04/13 00:11, Michel Lespinasse wrote:
On Thu, Apr 25, 2013 at 3:54 PM, H. Peter Anvin h...@zytor.com wrote:
On 04/25/2013 03:53 PM, Michel Lespinasse wrote:
On Thu, Apr 25, 2013 at 3:23 PM, Matthew Garrett
matthew.garr...@nebula.com wrote:
On Thu, 2013-04-25 at 15:20 -0700, Linus Torvalds wrote:
On Thu, Apr 25, 2013 at 2:44 PM, H. Peter Anvin h...@linux.intel.com
wrote:
- if (!sys_table-runtime-query_variable_info)
+ if (sys_table-runtime-hdr.revision
EFI_2_00_SYSTEM_TABLE_REVISION)
return EFI_UNSUPPORTED;
Is a EFI 2.00 system table *guaranteed* to have that
query_variable_info function? The above adds the version check, but
removes the check for a NULL pointer.
As far as the spec's concerned, yes. As far as reality's concerned - if
anything doesn't provide it, we're already crashing when
efi_virt_query_variable_info() gets called. Nobody's complained so far.
Well, I don't know if this is related, but commit e971318bbed6 broke
the google EFI SMI driver with
BUG: unable to handle kernel NULL pointer dereference at (null)
IP: [814a7245] variable_is_present+0x55/0x170
Call Trace:
[814a9936] register_efivars+0x106/0x370
[818ff430] ? firmware_map_add_early+0xb1/0xb1
[818ff6dd] gsmi_init+0x2ad/0x3da
[8100020f] do_one_initcall+0x3f/0x170
...
I don't know either. Could you test this patch and see if it does anything?
Nope, still seeing the crash with this patch applied.
Could you try the following patch against Linus' tree? The bug you've
found and the changes in the pull request are unrelated.
---
diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
index 182ce94..2a4f619 100644
--- a/drivers/firmware/efivars.c
+++ b/drivers/firmware/efivars.c
@@ -1635,6 +1635,9 @@ static bool variable_is_present(efi_char16_t
*variable_name, efi_guid_t *vendor)
unsigned long strsize1, strsize2;
bool found = false;
+ if (list_empty(efivars-list))
+ return false;
+
strsize1 = ucs2_strsize(variable_name, 1024);
list_for_each_entry_safe(entry, n, efivars-list, list) {
strsize2 = ucs2_strsize(entry-var.VariableName, 1024);
--
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: [GIT PULL] x86 fixes for 3.9
* Matthew Garrett matthew.garr...@nebula.com wrote: On Thu, 2013-04-25 at 15:20 -0700, Linus Torvalds wrote: On Thu, Apr 25, 2013 at 2:44 PM, H. Peter Anvin h...@linux.intel.com wrote: - if (!sys_table-runtime-query_variable_info) + if (sys_table-runtime-hdr.revision EFI_2_00_SYSTEM_TABLE_REVISION) return EFI_UNSUPPORTED; Is a EFI 2.00 system table *guaranteed* to have that query_variable_info function? The above adds the version check, but removes the check for a NULL pointer. As far as the spec's concerned, yes. As far as reality's concerned - if anything doesn't provide it, we're already crashing when efi_virt_query_variable_info() gets called. Nobody's complained so far. I'm worried about the fragility of this code - this is firmware code ... I think firmware code should be fundamentally paranoid and robust, and in this case treat all EFI-provided data as hostile and do a much sanity checking of it as possible - and provide an actionable error message if the checks fail, not just 'crash'. Even if no-one complained, yet. Thanks, Ingo -- 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: [GIT PULL] x86 fixes for 3.9
On Fri, Apr 26, 2013 at 12:12 AM, Matt Fleming matt.flem...@intel.com wrote:
On 26/04/13 00:11, Michel Lespinasse wrote:
On Thu, Apr 25, 2013 at 3:54 PM, H. Peter Anvin h...@zytor.com wrote:
On 04/25/2013 03:53 PM, Michel Lespinasse wrote:
Well, I don't know if this is related, but commit e971318bbed6 broke
the google EFI SMI driver with
BUG: unable to handle kernel NULL pointer dereference at (null)
IP: [814a7245] variable_is_present+0x55/0x170
Call Trace:
[814a9936] register_efivars+0x106/0x370
[818ff430] ? firmware_map_add_early+0xb1/0xb1
[818ff6dd] gsmi_init+0x2ad/0x3da
[8100020f] do_one_initcall+0x3f/0x170
...
I don't know either. Could you test this patch and see if it does anything?
Nope, still seeing the crash with this patch applied.
Could you try the following patch against Linus' tree? The bug you've
found and the changes in the pull request are unrelated.
diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
index 182ce94..2a4f619 100644
--- a/drivers/firmware/efivars.c
+++ b/drivers/firmware/efivars.c
@@ -1635,6 +1635,9 @@ static bool variable_is_present(efi_char16_t
*variable_name, efi_guid_t *vendor)
unsigned long strsize1, strsize2;
bool found = false;
+ if (list_empty(efivars-list))
+ return false;
+
strsize1 = ucs2_strsize(variable_name, 1024);
list_for_each_entry_safe(entry, n, efivars-list, list) {
strsize2 = ucs2_strsize(entry-var.VariableName, 1024);
Still seeing the crash.
I went and compared the crash dump with the vmlinux disassembly; the
issue is a NULL pointer dereference in list_for_each_entry_safe().
list_empty() checks that the head node points to itself, but here the
head node has NULL. I think this may be due to gsmi_init() being
called before efivars_init(). Not sure what's the proper fix though.
--
Michel Walken Lespinasse
A program is never fully debugged until the last user dies.
--
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: [GIT PULL] x86 fixes for 3.9
On 26/04/13 08:43, Michel Lespinasse wrote:
Still seeing the crash.
I went and compared the crash dump with the vmlinux disassembly; the
issue is a NULL pointer dereference in list_for_each_entry_safe().
list_empty() checks that the head node points to itself, but here the
head node has NULL. I think this may be due to gsmi_init() being
called before efivars_init(). Not sure what's the proper fix though.
Ohh... I see what you mean. The bug is in variable_is_present() because
it accesses __efivars directly, which a) isn't the struct efivars gsmi.c
uses and b) hasn't been initialised. Something like this might work.
---
diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
index 182ce94..f4baa11 100644
--- a/drivers/firmware/efivars.c
+++ b/drivers/firmware/efivars.c
@@ -1628,10 +1628,11 @@ static ssize_t efivar_delete(struct file *filp, struct
kobject *kobj,
return count;
}
-static bool variable_is_present(efi_char16_t *variable_name, efi_guid_t
*vendor)
+static bool variable_is_present(struct efivars *efivars,
+ efi_char16_t *variable_name,
+ efi_guid_t *vendor)
{
struct efivar_entry *entry, *n;
- struct efivars *efivars = __efivars;
unsigned long strsize1, strsize2;
bool found = false;
@@ -1703,8 +1704,8 @@ static void efivar_update_sysfs_entries(struct
work_struct *work)
if (status != EFI_SUCCESS) {
break;
} else {
- if (!variable_is_present(variable_name,
- vendor)) {
+ if (!variable_is_present(efivars,
+ variable_name, vendor)) {
found = true;
break;
}
@@ -2008,7 +2009,8 @@ int register_efivars(struct efivars *efivars,
* we'll ever see a different variable name,
* and may end up looping here forever.
*/
- if (variable_is_present(variable_name, vendor_guid)) {
+ if (variable_is_present(efivars, variable_name,
+ vendor_guid)) {
dup_variable_bug(variable_name, vendor_guid,
variable_name_size);
status = EFI_NOT_FOUND;
--
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: [GIT PULL] x86 fixes for 3.9
On 26/04/13 08:29, Ingo Molnar wrote: I'm worried about the fragility of this code - this is firmware code ... I think firmware code should be fundamentally paranoid and robust, and in this case treat all EFI-provided data as hostile and do a much sanity checking of it as possible - and provide an actionable error message if the checks fail, not just 'crash'. Even if no-one complained, yet. I'm not sure how much more robust checking for NULL makes it, it's not going to save us from garbage pointers, etc. Instead of the pointer being NULL a more likely bug is that query_variable_info() exists (for those firmware that implement the relevant spec version), but doesn't work correctly. That's the kind of bug we've been seeing in other functions. To be fair to Josh, his original submission did include the NULL check, but I asked him to remove it. If someone really wants to re-add the check for a NULL pointer, I'm not super opposed to it. -- 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: [GIT PULL] x86 fixes for 3.9
On Fri, Apr 26, 2013 at 1:49 AM, Matt Fleming matt.flem...@intel.com wrote: On 26/04/13 08:43, Michel Lespinasse wrote: Still seeing the crash. I went and compared the crash dump with the vmlinux disassembly; the issue is a NULL pointer dereference in list_for_each_entry_safe(). list_empty() checks that the head node points to itself, but here the head node has NULL. I think this may be due to gsmi_init() being called before efivars_init(). Not sure what's the proper fix though. Ohh... I see what you mean. The bug is in variable_is_present() because it accesses __efivars directly, which a) isn't the struct efivars gsmi.c uses and b) hasn't been initialised. Something like this might work. [... skipping patch ...] Yes, this one fixes it. Thanks ! -- Michel Walken Lespinasse A program is never fully debugged until the last user dies. -- 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: [GIT PULL] x86 fixes for 3.9
On 26/04/13 10:02, Michel Lespinasse wrote:
On Fri, Apr 26, 2013 at 1:49 AM, Matt Fleming matt.flem...@intel.com wrote:
On 26/04/13 08:43, Michel Lespinasse wrote:
Still seeing the crash.
I went and compared the crash dump with the vmlinux disassembly; the
issue is a NULL pointer dereference in list_for_each_entry_safe().
list_empty() checks that the head node points to itself, but here the
head node has NULL. I think this may be due to gsmi_init() being
called before efivars_init(). Not sure what's the proper fix though.
Ohh... I see what you mean. The bug is in variable_is_present() because
it accesses __efivars directly, which a) isn't the struct efivars gsmi.c
uses and b) hasn't been initialised. Something like this might work.
[... skipping patch ...]
Yes, this one fixes it. Thanks !
Thanks for testing!
Linus, did you want to take this one directly?
---
From f576b789b29dab31ddc1c7d37af63f10fb203fb7 Mon Sep 17 00:00:00 2001
From: Matt Fleming matt.flem...@intel.com
Date: Fri, 26 Apr 2013 10:10:55 +0100
Subject: [PATCH] efivars: only check for duplicates on the registered list
variable_is_present() accesses '__efivars' directly, but when called
via gsmi_init() Michel reports observing the following crash,
BUG: unable to handle kernel NULL pointer dereference at (null)
IP: [814a7245] variable_is_present+0x55/0x170
Call Trace:
[814a9936] register_efivars+0x106/0x370
[818ff430] ? firmware_map_add_early+0xb1/0xb1
[818ff6dd] gsmi_init+0x2ad/0x3da
[8100020f] do_one_initcall+0x3f/0x170
The reason for the crash is that '__efivars' hasn't been initialised nor
has it been registered with register_efivars() by the time the google
EFI SMI driver runs. The gsmi code uses its own struct efivars, and
therefore, a different variable list. Fix the above crash by passing the
registered struct efivars to variable_is_present(), so that we traverse
the correct list.
Reported-by: Michel Lespinasse wal...@google.com
Tested-by: Michel Lespinasse wal...@google.com
Cc: Mike Waychison mi...@google.com
Cc: Matthew Garrett matthew.garr...@nebula.com
Cc: Seiji Aguchi seiji.agu...@hds.com
Signed-off-by: Matt Fleming matt.flem...@intel.com
---
drivers/firmware/efivars.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
index 182ce94..f4baa11 100644
--- a/drivers/firmware/efivars.c
+++ b/drivers/firmware/efivars.c
@@ -1628,10 +1628,11 @@ static ssize_t efivar_delete(struct file *filp, struct
kobject *kobj,
return count;
}
-static bool variable_is_present(efi_char16_t *variable_name, efi_guid_t
*vendor)
+static bool variable_is_present(struct efivars *efivars,
+ efi_char16_t *variable_name,
+ efi_guid_t *vendor)
{
struct efivar_entry *entry, *n;
- struct efivars *efivars = __efivars;
unsigned long strsize1, strsize2;
bool found = false;
@@ -1703,8 +1704,8 @@ static void efivar_update_sysfs_entries(struct
work_struct *work)
if (status != EFI_SUCCESS) {
break;
} else {
- if (!variable_is_present(variable_name,
- vendor)) {
+ if (!variable_is_present(efivars,
+ variable_name, vendor)) {
found = true;
break;
}
@@ -2008,7 +2009,8 @@ int register_efivars(struct efivars *efivars,
* we'll ever see a different variable name,
* and may end up looping here forever.
*/
- if (variable_is_present(variable_name, vendor_guid)) {
+ if (variable_is_present(efivars, variable_name,
+ vendor_guid)) {
dup_variable_bug(variable_name, vendor_guid,
variable_name_size);
status = EFI_NOT_FOUND;
--
1.7.10.4
--
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: [GIT PULL] x86 fixes for 3.9
On Thu, Apr 25, 2013 at 3:54 PM, H. Peter Anvin wrote: > On 04/25/2013 03:53 PM, Michel Lespinasse wrote: >> On Thu, Apr 25, 2013 at 3:23 PM, Matthew Garrett >> wrote: >>> On Thu, 2013-04-25 at 15:20 -0700, Linus Torvalds wrote: On Thu, Apr 25, 2013 at 2:44 PM, H. Peter Anvin wrote: > > - if (!sys_table->runtime->query_variable_info) > + if (sys_table->runtime->hdr.revision < > EFI_2_00_SYSTEM_TABLE_REVISION) > return EFI_UNSUPPORTED; Is a EFI 2.00 system table *guaranteed* to have that "query_variable_info" function? The above adds the version check, but removes the check for a NULL pointer. >>> >>> As far as the spec's concerned, yes. As far as reality's concerned - if >>> anything doesn't provide it, we're already crashing when >>> efi_virt_query_variable_info() gets called. Nobody's complained so far. >> >> Well, I don't know if this is related, but commit e971318bbed6 broke >> the google EFI SMI driver with >> BUG: unable to handle kernel NULL pointer dereference at (null) >> IP: [] variable_is_present+0x55/0x170 >> Call Trace: >> [] register_efivars+0x106/0x370 >> [] ? firmware_map_add_early+0xb1/0xb1 >> [] gsmi_init+0x2ad/0x3da >> [] do_one_initcall+0x3f/0x170 >> ... > > I don't know either. Could you test this patch and see if it does anything? Nope, still seeing the crash with this patch applied. -- Michel "Walken" Lespinasse A program is never fully debugged until the last user dies. -- 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: [GIT PULL] x86 fixes for 3.9
On 04/25/2013 03:53 PM, Michel Lespinasse wrote: > On Thu, Apr 25, 2013 at 3:23 PM, Matthew Garrett > wrote: >> On Thu, 2013-04-25 at 15:20 -0700, Linus Torvalds wrote: >>> On Thu, Apr 25, 2013 at 2:44 PM, H. Peter Anvin >>> wrote: - if (!sys_table->runtime->query_variable_info) + if (sys_table->runtime->hdr.revision < EFI_2_00_SYSTEM_TABLE_REVISION) return EFI_UNSUPPORTED; >>> >>> Is a EFI 2.00 system table *guaranteed* to have that >>> "query_variable_info" function? The above adds the version check, but >>> removes the check for a NULL pointer. >> >> As far as the spec's concerned, yes. As far as reality's concerned - if >> anything doesn't provide it, we're already crashing when >> efi_virt_query_variable_info() gets called. Nobody's complained so far. > > Well, I don't know if this is related, but commit e971318bbed6 broke > the google EFI SMI driver with > BUG: unable to handle kernel NULL pointer dereference at (null) > IP: [] variable_is_present+0x55/0x170 > Call Trace: > [] register_efivars+0x106/0x370 > [] ? firmware_map_add_early+0xb1/0xb1 > [] gsmi_init+0x2ad/0x3da > [] do_one_initcall+0x3f/0x170 > ... > I don't know either. Could you test this patch and see if it does anything? -hpa -- 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: [GIT PULL] x86 fixes for 3.9
On Thu, Apr 25, 2013 at 3:23 PM, Matthew Garrett wrote: > On Thu, 2013-04-25 at 15:20 -0700, Linus Torvalds wrote: >> On Thu, Apr 25, 2013 at 2:44 PM, H. Peter Anvin wrote: >> > >> > - if (!sys_table->runtime->query_variable_info) >> > + if (sys_table->runtime->hdr.revision < >> > EFI_2_00_SYSTEM_TABLE_REVISION) >> > return EFI_UNSUPPORTED; >> >> Is a EFI 2.00 system table *guaranteed* to have that >> "query_variable_info" function? The above adds the version check, but >> removes the check for a NULL pointer. > > As far as the spec's concerned, yes. As far as reality's concerned - if > anything doesn't provide it, we're already crashing when > efi_virt_query_variable_info() gets called. Nobody's complained so far. Well, I don't know if this is related, but commit e971318bbed6 broke the google EFI SMI driver with BUG: unable to handle kernel NULL pointer dereference at (null) IP: [] variable_is_present+0x55/0x170 Call Trace: [] register_efivars+0x106/0x370 [] ? firmware_map_add_early+0xb1/0xb1 [] gsmi_init+0x2ad/0x3da [] do_one_initcall+0x3f/0x170 ... -- Michel "Walken" Lespinasse A program is never fully debugged until the last user dies. -- 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: [GIT PULL] x86 fixes for 3.9
On Thu, 2013-04-25 at 15:20 -0700, Linus Torvalds wrote:
> On Thu, Apr 25, 2013 at 2:44 PM, H. Peter Anvin wrote:
> >
> > - if (!sys_table->runtime->query_variable_info)
> > + if (sys_table->runtime->hdr.revision <
> > EFI_2_00_SYSTEM_TABLE_REVISION)
> > return EFI_UNSUPPORTED;
>
> Is a EFI 2.00 system table *guaranteed* to have that
> "query_variable_info" function? The above adds the version check, but
> removes the check for a NULL pointer.
As far as the spec's concerned, yes. As far as reality's concerned - if
anything doesn't provide it, we're already crashing when
efi_virt_query_variable_info() gets called. Nobody's complained so far.
> And why the hell don't we have a real structure that has been filled
> out properly, and instead apparently just do this "point to random
> memory that doesn't necessarily have the full structure?
This is early boot code, we're not in the kernel proper yet. All we have
is the structure that's handed to us by the firmware, and the size of
that structure varies depending on its version.
--
Matthew Garrett | mj...@srcf.ucam.org
N�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z� �w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i
Re: [GIT PULL] x86 fixes for 3.9
On Thu, Apr 25, 2013 at 2:44 PM, H. Peter Anvin wrote: > > - if (!sys_table->runtime->query_variable_info) > + if (sys_table->runtime->hdr.revision < EFI_2_00_SYSTEM_TABLE_REVISION) > return EFI_UNSUPPORTED; Is a EFI 2.00 system table *guaranteed* to have that "query_variable_info" function? The above adds the version check, but removes the check for a NULL pointer. And why the hell don't we have a real structure that has been filled out properly, and instead apparently just do this "point to random memory that doesn't necessarily have the full structure? Linus -- 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/
[GIT PULL] x86 fixes for 3.9
Hi Linus, The following changes since commit 0fbd06761f5c17cc9b20e02af60fd7ee9c895996: Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/sparc (2013-04-24 17:10:18 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86-urgent-for-linus This is exclusively two EFI fixes: * The EFI variable anti-bricking algorithm merged in -rc8 broke booting on some Apple machines because they implement EFI spec 1.10, which doesn't provide a QueryVariableInfo() runtime function and the logic used to check for the existence of that function was insufficient. Fix from Josh Boyer. * The anti-bricking algorithm also introduced a compiler warning on 32-bit. Fix from Borislav Petkov. Borislav Petkov (1): x86, efi: Fix a build warning H. Peter Anvin (1): Merge tag 'efi-urgent' into x86/urgent Josh Boyer (1): efi: Check EFI revision in setup_efi_vars arch/x86/boot/compressed/eboot.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c index 8615f75..35ee62f 100644 --- a/arch/x86/boot/compressed/eboot.c +++ b/arch/x86/boot/compressed/eboot.c @@ -258,7 +258,7 @@ static efi_status_t setup_efi_vars(struct boot_params *params) u64 store_size, remaining_size, var_size; efi_status_t status; - if (!sys_table->runtime->query_variable_info) + if (sys_table->runtime->hdr.revision < EFI_2_00_SYSTEM_TABLE_REVISION) return EFI_UNSUPPORTED; data = (struct setup_data *)(unsigned long)params->hdr.setup_data; @@ -266,7 +266,7 @@ static efi_status_t setup_efi_vars(struct boot_params *params) while (data && data->next) data = (struct setup_data *)(unsigned long)data->next; - status = efi_call_phys4(sys_table->runtime->query_variable_info, + status = efi_call_phys4((void *)sys_table->runtime->query_variable_info, EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS, _size, -- 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/
[GIT PULL] x86 fixes for 3.9
Hi Linus, The following changes since commit 0fbd06761f5c17cc9b20e02af60fd7ee9c895996: Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/sparc (2013-04-24 17:10:18 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86-urgent-for-linus This is exclusively two EFI fixes: * The EFI variable anti-bricking algorithm merged in -rc8 broke booting on some Apple machines because they implement EFI spec 1.10, which doesn't provide a QueryVariableInfo() runtime function and the logic used to check for the existence of that function was insufficient. Fix from Josh Boyer. * The anti-bricking algorithm also introduced a compiler warning on 32-bit. Fix from Borislav Petkov. Borislav Petkov (1): x86, efi: Fix a build warning H. Peter Anvin (1): Merge tag 'efi-urgent' into x86/urgent Josh Boyer (1): efi: Check EFI revision in setup_efi_vars arch/x86/boot/compressed/eboot.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c index 8615f75..35ee62f 100644 --- a/arch/x86/boot/compressed/eboot.c +++ b/arch/x86/boot/compressed/eboot.c @@ -258,7 +258,7 @@ static efi_status_t setup_efi_vars(struct boot_params *params) u64 store_size, remaining_size, var_size; efi_status_t status; - if (!sys_table-runtime-query_variable_info) + if (sys_table-runtime-hdr.revision EFI_2_00_SYSTEM_TABLE_REVISION) return EFI_UNSUPPORTED; data = (struct setup_data *)(unsigned long)params-hdr.setup_data; @@ -266,7 +266,7 @@ static efi_status_t setup_efi_vars(struct boot_params *params) while (data data-next) data = (struct setup_data *)(unsigned long)data-next; - status = efi_call_phys4(sys_table-runtime-query_variable_info, + status = efi_call_phys4((void *)sys_table-runtime-query_variable_info, EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS, store_size, -- 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: [GIT PULL] x86 fixes for 3.9
On Thu, Apr 25, 2013 at 2:44 PM, H. Peter Anvin h...@linux.intel.com wrote: - if (!sys_table-runtime-query_variable_info) + if (sys_table-runtime-hdr.revision EFI_2_00_SYSTEM_TABLE_REVISION) return EFI_UNSUPPORTED; Is a EFI 2.00 system table *guaranteed* to have that query_variable_info function? The above adds the version check, but removes the check for a NULL pointer. And why the hell don't we have a real structure that has been filled out properly, and instead apparently just do this point to random memory that doesn't necessarily have the full structure? Linus -- 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: [GIT PULL] x86 fixes for 3.9
On Thu, 2013-04-25 at 15:20 -0700, Linus Torvalds wrote:
On Thu, Apr 25, 2013 at 2:44 PM, H. Peter Anvin h...@linux.intel.com wrote:
- if (!sys_table-runtime-query_variable_info)
+ if (sys_table-runtime-hdr.revision
EFI_2_00_SYSTEM_TABLE_REVISION)
return EFI_UNSUPPORTED;
Is a EFI 2.00 system table *guaranteed* to have that
query_variable_info function? The above adds the version check, but
removes the check for a NULL pointer.
As far as the spec's concerned, yes. As far as reality's concerned - if
anything doesn't provide it, we're already crashing when
efi_virt_query_variable_info() gets called. Nobody's complained so far.
And why the hell don't we have a real structure that has been filled
out properly, and instead apparently just do this point to random
memory that doesn't necessarily have the full structure?
This is early boot code, we're not in the kernel proper yet. All we have
is the structure that's handed to us by the firmware, and the size of
that structure varies depending on its version.
--
Matthew Garrett | mj...@srcf.ucam.org
N�r��yb�X��ǧv�^�){.n�+{zX����ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z� �w���?��)ߢf��^jǫy�m��@A�a���
0��h���i
Re: [GIT PULL] x86 fixes for 3.9
On Thu, Apr 25, 2013 at 3:23 PM, Matthew Garrett matthew.garr...@nebula.com wrote: On Thu, 2013-04-25 at 15:20 -0700, Linus Torvalds wrote: On Thu, Apr 25, 2013 at 2:44 PM, H. Peter Anvin h...@linux.intel.com wrote: - if (!sys_table-runtime-query_variable_info) + if (sys_table-runtime-hdr.revision EFI_2_00_SYSTEM_TABLE_REVISION) return EFI_UNSUPPORTED; Is a EFI 2.00 system table *guaranteed* to have that query_variable_info function? The above adds the version check, but removes the check for a NULL pointer. As far as the spec's concerned, yes. As far as reality's concerned - if anything doesn't provide it, we're already crashing when efi_virt_query_variable_info() gets called. Nobody's complained so far. Well, I don't know if this is related, but commit e971318bbed6 broke the google EFI SMI driver with BUG: unable to handle kernel NULL pointer dereference at (null) IP: [814a7245] variable_is_present+0x55/0x170 Call Trace: [814a9936] register_efivars+0x106/0x370 [818ff430] ? firmware_map_add_early+0xb1/0xb1 [818ff6dd] gsmi_init+0x2ad/0x3da [8100020f] do_one_initcall+0x3f/0x170 ... -- Michel Walken Lespinasse A program is never fully debugged until the last user dies. -- 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: [GIT PULL] x86 fixes for 3.9
On 04/25/2013 03:53 PM, Michel Lespinasse wrote: On Thu, Apr 25, 2013 at 3:23 PM, Matthew Garrett matthew.garr...@nebula.com wrote: On Thu, 2013-04-25 at 15:20 -0700, Linus Torvalds wrote: On Thu, Apr 25, 2013 at 2:44 PM, H. Peter Anvin h...@linux.intel.com wrote: - if (!sys_table-runtime-query_variable_info) + if (sys_table-runtime-hdr.revision EFI_2_00_SYSTEM_TABLE_REVISION) return EFI_UNSUPPORTED; Is a EFI 2.00 system table *guaranteed* to have that query_variable_info function? The above adds the version check, but removes the check for a NULL pointer. As far as the spec's concerned, yes. As far as reality's concerned - if anything doesn't provide it, we're already crashing when efi_virt_query_variable_info() gets called. Nobody's complained so far. Well, I don't know if this is related, but commit e971318bbed6 broke the google EFI SMI driver with BUG: unable to handle kernel NULL pointer dereference at (null) IP: [814a7245] variable_is_present+0x55/0x170 Call Trace: [814a9936] register_efivars+0x106/0x370 [818ff430] ? firmware_map_add_early+0xb1/0xb1 [818ff6dd] gsmi_init+0x2ad/0x3da [8100020f] do_one_initcall+0x3f/0x170 ... I don't know either. Could you test this patch and see if it does anything? -hpa -- 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: [GIT PULL] x86 fixes for 3.9
On Thu, Apr 25, 2013 at 3:54 PM, H. Peter Anvin h...@zytor.com wrote: On 04/25/2013 03:53 PM, Michel Lespinasse wrote: On Thu, Apr 25, 2013 at 3:23 PM, Matthew Garrett matthew.garr...@nebula.com wrote: On Thu, 2013-04-25 at 15:20 -0700, Linus Torvalds wrote: On Thu, Apr 25, 2013 at 2:44 PM, H. Peter Anvin h...@linux.intel.com wrote: - if (!sys_table-runtime-query_variable_info) + if (sys_table-runtime-hdr.revision EFI_2_00_SYSTEM_TABLE_REVISION) return EFI_UNSUPPORTED; Is a EFI 2.00 system table *guaranteed* to have that query_variable_info function? The above adds the version check, but removes the check for a NULL pointer. As far as the spec's concerned, yes. As far as reality's concerned - if anything doesn't provide it, we're already crashing when efi_virt_query_variable_info() gets called. Nobody's complained so far. Well, I don't know if this is related, but commit e971318bbed6 broke the google EFI SMI driver with BUG: unable to handle kernel NULL pointer dereference at (null) IP: [814a7245] variable_is_present+0x55/0x170 Call Trace: [814a9936] register_efivars+0x106/0x370 [818ff430] ? firmware_map_add_early+0xb1/0xb1 [818ff6dd] gsmi_init+0x2ad/0x3da [8100020f] do_one_initcall+0x3f/0x170 ... I don't know either. Could you test this patch and see if it does anything? Nope, still seeing the crash with this patch applied. -- Michel Walken Lespinasse A program is never fully debugged until the last user dies. -- 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/
[GIT PULL] x86 fixes for 3.9-rc4
Hi Linus,
A collection of minor fixes, more EFI variables paranoia
(anti-bricking) plus the ability to disable the pstore either as a
runtime default or completely, due to bricking concerns.
The following changes since commit a937536b868b8369b98967929045f1df54234323:
Linux 3.9-rc3 (2013-03-17 15:59:32 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/urgent
for you to fetch changes up to b9726d9df8263fe756d2293ff01906f41f7d2776:
Merge tag 'efi-for-3.9-rc4' into x86/urgent (2013-03-23 21:49:51 -0700)
Ben Hutchings (1):
efivars: Fix check for CONFIG_EFI_VARS_PSTORE_DEFAULT_DISABLE
CQ Tang (1):
x86-64: Fix the failure case in copy_user_handle_tail()
Fenghua Yu (1):
x86-32, microcode_intel_early: Fix crash with CONFIG_DEBUG_VIRTUAL
H. Peter Anvin (3):
x86, microcode_intel_early: Mark apply_microcode_early() as cpuinit
Merge tag 'efi-for-3.9-rc3' into x86/urgent
Merge tag 'efi-for-3.9-rc4' into x86/urgent
Matt Fleming (2):
efivars: explicitly calculate length of VariableName
efivars: Handle duplicate names from get_next_variable()
Seth Forshee (2):
efivars: Allow disabling use as a pstore backend
efivars: Add module parameter to disable use as a pstore backend
arch/x86/kernel/microcode_intel_early.c | 30 +++
arch/x86/lib/usercopy_64.c | 4 +-
drivers/firmware/Kconfig| 18
drivers/firmware/efivars.c | 150 ++--
4 files changed, 139 insertions(+), 63 deletions(-)
diff --git a/arch/x86/kernel/microcode_intel_early.c
b/arch/x86/kernel/microcode_intel_early.c
index 7890bc8..d893e8e 100644
--- a/arch/x86/kernel/microcode_intel_early.c
+++ b/arch/x86/kernel/microcode_intel_early.c
@@ -90,13 +90,13 @@ microcode_phys(struct microcode_intel **mc_saved_tmp,
struct microcode_intel ***mc_saved;
mc_saved = (struct microcode_intel ***)
- __pa_symbol(_saved_data->mc_saved);
+ __pa_nodebug(_saved_data->mc_saved);
for (i = 0; i < mc_saved_data->mc_saved_count; i++) {
struct microcode_intel *p;
p = *(struct microcode_intel **)
- __pa(mc_saved_data->mc_saved + i);
- mc_saved_tmp[i] = (struct microcode_intel *)__pa(p);
+ __pa_nodebug(mc_saved_data->mc_saved + i);
+ mc_saved_tmp[i] = (struct microcode_intel *)__pa_nodebug(p);
}
}
#endif
@@ -562,7 +562,7 @@ scan_microcode(unsigned long start, unsigned long end,
struct cpio_data cd;
long offset = 0;
#ifdef CONFIG_X86_32
- char *p = (char *)__pa_symbol(ucode_name);
+ char *p = (char *)__pa_nodebug(ucode_name);
#else
char *p = ucode_name;
#endif
@@ -630,8 +630,8 @@ static void __cpuinit print_ucode(struct ucode_cpu_info
*uci)
if (mc_intel == NULL)
return;
- delay_ucode_info_p = (int *)__pa_symbol(_ucode_info);
- current_mc_date_p = (int *)__pa_symbol(_mc_date);
+ delay_ucode_info_p = (int *)__pa_nodebug(_ucode_info);
+ current_mc_date_p = (int *)__pa_nodebug(_mc_date);
*delay_ucode_info_p = 1;
*current_mc_date_p = mc_intel->hdr.date;
@@ -659,8 +659,8 @@ static inline void __cpuinit print_ucode(struct
ucode_cpu_info *uci)
}
#endif
-static int apply_microcode_early(struct mc_saved_data *mc_saved_data,
-struct ucode_cpu_info *uci)
+static int __cpuinit apply_microcode_early(struct mc_saved_data *mc_saved_data,
+ struct ucode_cpu_info *uci)
{
struct microcode_intel *mc_intel;
unsigned int val[2];
@@ -741,15 +741,15 @@ load_ucode_intel_bsp(void)
#ifdef CONFIG_X86_32
struct boot_params *boot_params_p;
- boot_params_p = (struct boot_params *)__pa_symbol(_params);
+ boot_params_p = (struct boot_params *)__pa_nodebug(_params);
ramdisk_image = boot_params_p->hdr.ramdisk_image;
ramdisk_size = boot_params_p->hdr.ramdisk_size;
initrd_start_early = ramdisk_image;
initrd_end_early = initrd_start_early + ramdisk_size;
_load_ucode_intel_bsp(
- (struct mc_saved_data *)__pa_symbol(_saved_data),
- (unsigned long *)__pa_symbol(_saved_in_initrd),
+ (struct mc_saved_data *)__pa_nodebug(_saved_data),
+ (unsigned long *)__pa_nodebug(_saved_in_initrd),
initrd_start_early, initrd_end_early, );
#else
ramdisk_image = boot_params.hdr.ramdisk_image;
@@ -772,10 +772,10 @@ void __cpuinit load_ucode_intel_ap(void)
unsigned long *initrd_start_p;
mc_saved_in_initrd_p =
- (unsigned long *)__pa_symbol(mc_saved_in_initrd);
- mc_saved_data_p = (struct mc_saved_data
[GIT PULL] x86 fixes for 3.9-rc4
Hi Linus,
A collection of minor fixes, more EFI variables paranoia
(anti-bricking) plus the ability to disable the pstore either as a
runtime default or completely, due to bricking concerns.
The following changes since commit a937536b868b8369b98967929045f1df54234323:
Linux 3.9-rc3 (2013-03-17 15:59:32 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/urgent
for you to fetch changes up to b9726d9df8263fe756d2293ff01906f41f7d2776:
Merge tag 'efi-for-3.9-rc4' into x86/urgent (2013-03-23 21:49:51 -0700)
Ben Hutchings (1):
efivars: Fix check for CONFIG_EFI_VARS_PSTORE_DEFAULT_DISABLE
CQ Tang (1):
x86-64: Fix the failure case in copy_user_handle_tail()
Fenghua Yu (1):
x86-32, microcode_intel_early: Fix crash with CONFIG_DEBUG_VIRTUAL
H. Peter Anvin (3):
x86, microcode_intel_early: Mark apply_microcode_early() as cpuinit
Merge tag 'efi-for-3.9-rc3' into x86/urgent
Merge tag 'efi-for-3.9-rc4' into x86/urgent
Matt Fleming (2):
efivars: explicitly calculate length of VariableName
efivars: Handle duplicate names from get_next_variable()
Seth Forshee (2):
efivars: Allow disabling use as a pstore backend
efivars: Add module parameter to disable use as a pstore backend
arch/x86/kernel/microcode_intel_early.c | 30 +++
arch/x86/lib/usercopy_64.c | 4 +-
drivers/firmware/Kconfig| 18
drivers/firmware/efivars.c | 150 ++--
4 files changed, 139 insertions(+), 63 deletions(-)
diff --git a/arch/x86/kernel/microcode_intel_early.c
b/arch/x86/kernel/microcode_intel_early.c
index 7890bc8..d893e8e 100644
--- a/arch/x86/kernel/microcode_intel_early.c
+++ b/arch/x86/kernel/microcode_intel_early.c
@@ -90,13 +90,13 @@ microcode_phys(struct microcode_intel **mc_saved_tmp,
struct microcode_intel ***mc_saved;
mc_saved = (struct microcode_intel ***)
- __pa_symbol(mc_saved_data-mc_saved);
+ __pa_nodebug(mc_saved_data-mc_saved);
for (i = 0; i mc_saved_data-mc_saved_count; i++) {
struct microcode_intel *p;
p = *(struct microcode_intel **)
- __pa(mc_saved_data-mc_saved + i);
- mc_saved_tmp[i] = (struct microcode_intel *)__pa(p);
+ __pa_nodebug(mc_saved_data-mc_saved + i);
+ mc_saved_tmp[i] = (struct microcode_intel *)__pa_nodebug(p);
}
}
#endif
@@ -562,7 +562,7 @@ scan_microcode(unsigned long start, unsigned long end,
struct cpio_data cd;
long offset = 0;
#ifdef CONFIG_X86_32
- char *p = (char *)__pa_symbol(ucode_name);
+ char *p = (char *)__pa_nodebug(ucode_name);
#else
char *p = ucode_name;
#endif
@@ -630,8 +630,8 @@ static void __cpuinit print_ucode(struct ucode_cpu_info
*uci)
if (mc_intel == NULL)
return;
- delay_ucode_info_p = (int *)__pa_symbol(delay_ucode_info);
- current_mc_date_p = (int *)__pa_symbol(current_mc_date);
+ delay_ucode_info_p = (int *)__pa_nodebug(delay_ucode_info);
+ current_mc_date_p = (int *)__pa_nodebug(current_mc_date);
*delay_ucode_info_p = 1;
*current_mc_date_p = mc_intel-hdr.date;
@@ -659,8 +659,8 @@ static inline void __cpuinit print_ucode(struct
ucode_cpu_info *uci)
}
#endif
-static int apply_microcode_early(struct mc_saved_data *mc_saved_data,
-struct ucode_cpu_info *uci)
+static int __cpuinit apply_microcode_early(struct mc_saved_data *mc_saved_data,
+ struct ucode_cpu_info *uci)
{
struct microcode_intel *mc_intel;
unsigned int val[2];
@@ -741,15 +741,15 @@ load_ucode_intel_bsp(void)
#ifdef CONFIG_X86_32
struct boot_params *boot_params_p;
- boot_params_p = (struct boot_params *)__pa_symbol(boot_params);
+ boot_params_p = (struct boot_params *)__pa_nodebug(boot_params);
ramdisk_image = boot_params_p-hdr.ramdisk_image;
ramdisk_size = boot_params_p-hdr.ramdisk_size;
initrd_start_early = ramdisk_image;
initrd_end_early = initrd_start_early + ramdisk_size;
_load_ucode_intel_bsp(
- (struct mc_saved_data *)__pa_symbol(mc_saved_data),
- (unsigned long *)__pa_symbol(mc_saved_in_initrd),
+ (struct mc_saved_data *)__pa_nodebug(mc_saved_data),
+ (unsigned long *)__pa_nodebug(mc_saved_in_initrd),
initrd_start_early, initrd_end_early, uci);
#else
ramdisk_image = boot_params.hdr.ramdisk_image;
@@ -772,10 +772,10 @@ void __cpuinit load_ucode_intel_ap(void)
unsigned long *initrd_start_p;
mc_saved_in_initrd_p =
- (unsigned long *)__pa_symbol(mc_saved_in_initrd);
- mc_saved_data_p =
