Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1171155 > unrolled thread
| Started by | Jiang Liu <jiang.liu@linux.intel.com> |
|---|---|
| First post | 2015-06-24 09:50 +0200 |
| Last post | 2015-06-24 10:40 +0200 |
| Articles | 3 — 3 participants |
Back to article view | Back to linux.kernel
[Bugfix v2] PCI, ACPI: Fix regressions caused by resource_size_t overflow with 32bit kernel Jiang Liu <jiang.liu@linux.intel.com> - 2015-06-24 09:50 +0200
Re: [Bugfix v2] PCI, ACPI: Fix regressions caused by resource_size_t overflow with 32bit kernel Boszormenyi Zoltan <zboszor@pr.hu> - 2015-06-24 10:30 +0200
Re: [Bugfix v2] PCI, ACPI: Fix regressions caused by resource_size_t overflow with 32bit kernel Ingo Molnar <mingo@kernel.org> - 2015-06-24 10:40 +0200
| From | Jiang Liu <jiang.liu@linux.intel.com> |
|---|---|
| Date | 2015-06-24 09:50 +0200 |
| Subject | [Bugfix v2] PCI, ACPI: Fix regressions caused by resource_size_t overflow with 32bit kernel |
| Message-ID | <pENBw-3q6-13@gated-at.bofh.it> |
Since commit 593669c2ac0f ("x86/PCI/ACPI: Use common ACPI resource
interfaces to simplify implementation"), x86 PCI ACPI host bridge driver
validates ACPI resources by first converting an ACPI resource to
a 'struct resource' structure and then applying checks against the
converted resource structure. The 'start' and 'end' fields in 'struct
resource' are defined to be type of resource_size_t, which may be 32 bits
or 64 bits depending on CONFIG_PHYS_ADDR_T_64BIT.
This may cause incorrect resource validation results with 32 bit kernels
because 64bit ACPI resource descriptors may get truncated when converting
to 32bit 'start' and 'end' fields in 'struct resource'. And eventually
affects PCI resource allocation subsystem and causes some PCI devices
unusable.
So enhance the ACPI resource parsing interfaces to ignore ACPI resource
descriptors with address/offset observe 4G when running in 32bit mode.
This reverts to the behavior before commit 593669c2ac0f.
This issue was triggered on a platform running 32bit kernel with an
ACPI resource descriptor with address range [0x400000000-0xfffffffff].
Please refer to https://lkml.org/lkml/2015/6/19/277 for more information.
Reported-by: Boszormenyi Zoltan <zboszor@pr.hu>
Fixes: 593669c2ac0f ("x86/PCI/ACPI: Use common ACPI resource interfaces to simplify implementation")
Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
Cc: stable@vger.kernel.org # 4.0
---
Hi Zoltan,
Could you please help to test this patch against the latest kernel?
Thanks!
Gerry
---
drivers/acpi/resource.c | 24 +++++++++++++++---------
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index 8244f013f210..f1c966e05078 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -193,6 +193,7 @@ static bool acpi_decode_space(struct resource_win *win,
u8 iodec = attr->granularity == 0xfff ? ACPI_DECODE_10 : ACPI_DECODE_16;
bool wp = addr->info.mem.write_protect;
u64 len = attr->address_length;
+ u64 start, end, offset = 0;
struct resource *res = &win->res;
/*
@@ -204,9 +205,6 @@ static bool acpi_decode_space(struct resource_win *win,
pr_debug("ACPI: Invalid address space min_addr_fix %d, max_addr_fix %d, len %llx\n",
addr->min_address_fixed, addr->max_address_fixed, len);
- res->start = attr->minimum;
- res->end = attr->maximum;
-
/*
* For bridges that translate addresses across the bridge,
* translation_offset is the offset that must be added to the
@@ -214,12 +212,22 @@ static bool acpi_decode_space(struct resource_win *win,
* primary side. Non-bridge devices must list 0 for all Address
* Translation offset bits.
*/
- if (addr->producer_consumer == ACPI_PRODUCER) {
- res->start += attr->translation_offset;
- res->end += attr->translation_offset;
- } else if (attr->translation_offset) {
+ if (addr->producer_consumer == ACPI_PRODUCER)
+ offset = attr->translation_offset;
+ else if (attr->translation_offset)
pr_debug("ACPI: translation_offset(%lld) is invalid for non-bridge device.\n",
attr->translation_offset);
+ start = attr->minimum + offset;
+ end = attr->maximum + offset;
+
+ win->offset = offset;
+ res->start = start;
+ res->end = end;
+ if (sizeof(resource_size_t) < sizeof(u64) &&
+ (offset != win->offset || start != res->start || end != res->end)) {
+ pr_warn("acpi resource window ([%#llx-%#llx] ignored, not CPU addressable)\n",
+ attr->minimum, attr->maximum);
+ return false;
}
switch (addr->resource_type) {
@@ -236,8 +244,6 @@ static bool acpi_decode_space(struct resource_win *win,
return false;
}
- win->offset = attr->translation_offset;
-
if (addr->producer_consumer == ACPI_PRODUCER)
res->flags |= IORESOURCE_WINDOW;
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Boszormenyi Zoltan <zboszor@pr.hu> |
|---|---|
| Date | 2015-06-24 10:30 +0200 |
| Subject | Re: [Bugfix v2] PCI, ACPI: Fix regressions caused by resource_size_t overflow with 32bit kernel |
| Message-ID | <pEOef-4ou-27@gated-at.bofh.it> |
| In reply to | #1171155 |
2015-06-24 09:43 keltezéssel, Jiang Liu írta:
> Since commit 593669c2ac0f ("x86/PCI/ACPI: Use common ACPI resource
> interfaces to simplify implementation"), x86 PCI ACPI host bridge driver
> validates ACPI resources by first converting an ACPI resource to
> a 'struct resource' structure and then applying checks against the
> converted resource structure. The 'start' and 'end' fields in 'struct
> resource' are defined to be type of resource_size_t, which may be 32 bits
> or 64 bits depending on CONFIG_PHYS_ADDR_T_64BIT.
>
> This may cause incorrect resource validation results with 32 bit kernels
> because 64bit ACPI resource descriptors may get truncated when converting
> to 32bit 'start' and 'end' fields in 'struct resource'. And eventually
> affects PCI resource allocation subsystem and causes some PCI devices
> unusable.
>
> So enhance the ACPI resource parsing interfaces to ignore ACPI resource
> descriptors with address/offset observe 4G when running in 32bit mode.
> This reverts to the behavior before commit 593669c2ac0f.
>
> This issue was triggered on a platform running 32bit kernel with an
> ACPI resource descriptor with address range [0x400000000-0xfffffffff].
> Please refer to https://lkml.org/lkml/2015/6/19/277 for more information.
>
> Reported-by: Boszormenyi Zoltan <zboszor@pr.hu>
> Fixes: 593669c2ac0f ("x86/PCI/ACPI: Use common ACPI resource interfaces to simplify implementation")
> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> Cc: stable@vger.kernel.org # 4.0
> ---
>
> Hi Zoltan,
> Could you please help to test this patch against the latest kernel?
> Thanks!
> Gerry
I will, thanks.
Best regards,
Zoltán
>
> ---
> drivers/acpi/resource.c | 24 +++++++++++++++---------
> 1 file changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> index 8244f013f210..f1c966e05078 100644
> --- a/drivers/acpi/resource.c
> +++ b/drivers/acpi/resource.c
> @@ -193,6 +193,7 @@ static bool acpi_decode_space(struct resource_win *win,
> u8 iodec = attr->granularity == 0xfff ? ACPI_DECODE_10 : ACPI_DECODE_16;
> bool wp = addr->info.mem.write_protect;
> u64 len = attr->address_length;
> + u64 start, end, offset = 0;
> struct resource *res = &win->res;
>
> /*
> @@ -204,9 +205,6 @@ static bool acpi_decode_space(struct resource_win *win,
> pr_debug("ACPI: Invalid address space min_addr_fix %d, max_addr_fix %d, len %llx\n",
> addr->min_address_fixed, addr->max_address_fixed, len);
>
> - res->start = attr->minimum;
> - res->end = attr->maximum;
> -
> /*
> * For bridges that translate addresses across the bridge,
> * translation_offset is the offset that must be added to the
> @@ -214,12 +212,22 @@ static bool acpi_decode_space(struct resource_win *win,
> * primary side. Non-bridge devices must list 0 for all Address
> * Translation offset bits.
> */
> - if (addr->producer_consumer == ACPI_PRODUCER) {
> - res->start += attr->translation_offset;
> - res->end += attr->translation_offset;
> - } else if (attr->translation_offset) {
> + if (addr->producer_consumer == ACPI_PRODUCER)
> + offset = attr->translation_offset;
> + else if (attr->translation_offset)
> pr_debug("ACPI: translation_offset(%lld) is invalid for non-bridge device.\n",
> attr->translation_offset);
> + start = attr->minimum + offset;
> + end = attr->maximum + offset;
> +
> + win->offset = offset;
> + res->start = start;
> + res->end = end;
> + if (sizeof(resource_size_t) < sizeof(u64) &&
> + (offset != win->offset || start != res->start || end != res->end)) {
> + pr_warn("acpi resource window ([%#llx-%#llx] ignored, not CPU addressable)\n",
> + attr->minimum, attr->maximum);
> + return false;
> }
>
> switch (addr->resource_type) {
> @@ -236,8 +244,6 @@ static bool acpi_decode_space(struct resource_win *win,
> return false;
> }
>
> - win->offset = attr->translation_offset;
> -
> if (addr->producer_consumer == ACPI_PRODUCER)
> res->flags |= IORESOURCE_WINDOW;
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Ingo Molnar <mingo@kernel.org> |
|---|---|
| Date | 2015-06-24 10:40 +0200 |
| Subject | Re: [Bugfix v2] PCI, ACPI: Fix regressions caused by resource_size_t overflow with 32bit kernel |
| Message-ID | <pEOnT-4zK-5@gated-at.bofh.it> |
| In reply to | #1171155 |
* Jiang Liu <jiang.liu@linux.intel.com> wrote:
> Since commit 593669c2ac0f ("x86/PCI/ACPI: Use common ACPI resource interfaces to
> simplify implementation"), x86 PCI ACPI host bridge driver validates ACPI
> resources by first converting an ACPI resource to a 'struct resource' structure
> and then applying checks against the converted resource structure. The 'start'
> and 'end' fields in 'struct resource' are defined to be type of resource_size_t,
> which may be 32 bits or 64 bits depending on CONFIG_PHYS_ADDR_T_64BIT.
>
> This may cause incorrect resource validation results with 32 bit kernels because
> 64bit ACPI resource descriptors may get truncated when converting to 32bit
> 'start' and 'end' fields in 'struct resource'. And eventually affects PCI
> resource allocation subsystem and causes some PCI devices unusable.
s/causes some PCI devices unusuable.
makes some PCI devices unusuable.
Also, this description is still pretty vague. What exactly happened? Did some PCI
devices not show up during bootup? Or did they hang? Or did something else happen?
This is _by far_ the most important part of the changelog and determines whether a
patch gets backported or not. Why does a usable regression description have to be
coaxed out of you like pulling teeth??
> So enhance the ACPI resource parsing interfaces to ignore ACPI resource
> descriptors with address/offset observe 4G when running in 32bit mode. This
> reverts to the behavior before commit 593669c2ac0f.
>
> This issue was triggered on a platform running 32bit kernel with an ACPI
> resource descriptor with address range [0x400000000-0xfffffffff]. Please refer
> to https://lkml.org/lkml/2015/6/19/277 for more information.
s/32bit/32-bit
s/64bit/64-bit
s/32 bit/32-bit
s/64 bit/64-bit
Thanks,
Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web