* [PATCH v2] PCI: Prevent out of bounds access in numa_node override - part 2
@ 2015-11-09 19:00 Mathias Krause
2015-11-24 18:49 ` Bjorn Helgaas
0 siblings, 1 reply; 4+ messages in thread
From: Mathias Krause @ 2015-11-09 19:00 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: linux-pci, Mathias Krause, Sasha Levin, Prarit Bhargava
Commit 1266963170f5 ("PCI: Prevent out of bounds access in numa_node
override") missed that the user provided node could also be negative.
Handle this case as well to avoid out-of-bounds accesses to the
node_states[] array. However, allow the special value -1, i.e.
NUMA_NO_NODE, to be able to set the 'no specific node' configuration.
Fixes: 1266963170f5 ("PCI: Prevent out of bounds access in numa_node...")
Signed-off-by: Mathias Krause <minipli@googlemail.com>
Cc: Sasha Levin <sasha.levin@oracle.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: stable@vger.kernel.org # v3.19+
---
v2: allow NUMA_NO_NODE
drivers/pci/pci-sysfs.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 92618686604c..6e9818227b19 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -216,7 +216,10 @@ static ssize_t numa_node_store(struct device *dev,
if (ret)
return ret;
- if (node >= MAX_NUMNODES || !node_online(node))
+ if (node < NUMA_NO_NODE || node >= MAX_NUMNODES)
+ return -EINVAL;
+
+ if (node != NUMA_NO_NODE && !node_online(node))
return -EINVAL;
add_taint(TAINT_FIRMWARE_WORKAROUND, LOCKDEP_STILL_OK);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v2] PCI: Prevent out of bounds access in numa_node override - part 2
2015-11-09 19:00 [PATCH v2] PCI: Prevent out of bounds access in numa_node override - part 2 Mathias Krause
@ 2015-11-24 18:49 ` Bjorn Helgaas
2015-11-24 19:27 ` Mathias Krause
0 siblings, 1 reply; 4+ messages in thread
From: Bjorn Helgaas @ 2015-11-24 18:49 UTC (permalink / raw)
To: Mathias Krause; +Cc: Bjorn Helgaas, linux-pci, Sasha Levin, Prarit Bhargava
On Mon, Nov 09, 2015 at 08:00:27PM +0100, Mathias Krause wrote:
> Commit 1266963170f5 ("PCI: Prevent out of bounds access in numa_node
> override") missed that the user provided node could also be negative.
> Handle this case as well to avoid out-of-bounds accesses to the
> node_states[] array. However, allow the special value -1, i.e.
> NUMA_NO_NODE, to be able to set the 'no specific node' configuration.
>
> Fixes: 1266963170f5 ("PCI: Prevent out of bounds access in numa_node...")
> Signed-off-by: Mathias Krause <minipli@googlemail.com>
> Cc: Sasha Levin <sasha.levin@oracle.com>
> Cc: Prarit Bhargava <prarit@redhat.com>
> Cc: stable@vger.kernel.org # v3.19+
Applied as tweaked below to for-linus for v4.4, thanks! As written,
if NUMA_NO_NODE were defined as -2, we would incorrectly accept -1.
Let me know if you disagree with my fix.
> ---
> v2: allow NUMA_NO_NODE
>
> drivers/pci/pci-sysfs.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 92618686604c..6e9818227b19 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -216,7 +216,10 @@ static ssize_t numa_node_store(struct device *dev,
> if (ret)
> return ret;
>
> - if (node >= MAX_NUMNODES || !node_online(node))
> + if (node < NUMA_NO_NODE || node >= MAX_NUMNODES)
> + return -EINVAL;
> +
> + if (node != NUMA_NO_NODE && !node_online(node))
> return -EINVAL;
>
> add_taint(TAINT_FIRMWARE_WORKAROUND, LOCKDEP_STILL_OK);
commit 2a35194c5a45fbb9ca1d88bc56804dfb51a75233
Author: Mathias Krause <minipli@googlemail.com>
Date: Mon Nov 9 20:00:27 2015 +0100
PCI: Prevent out of bounds access in numa_node override
Commit 1266963170f5 ("PCI: Prevent out of bounds access in numa_node
override") missed that the user-provided node could also be negative.
Handle this case as well to avoid out-of-bounds accesses to the
node_states[] array. However, allow the special value -1, i.e.
NUMA_NO_NODE, to be able to set the 'no specific node' configuration.
[bhelgaas: remove assumption that NUMA_NO_NODE == -1]
Fixes: 1266963170f5 ("PCI: Prevent out of bounds access in numa_node override")
Fixes: 63692df103e9 ("PCI: Allow numa_node override via sysfs")
Signed-off-by: Mathias Krause <minipli@googlemail.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
CC: Sasha Levin <sasha.levin@oracle.com>
CC: Prarit Bhargava <prarit@redhat.com>
CC: stable@vger.kernel.org # v3.19+
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 9261868..50f4747 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -216,7 +216,12 @@ static ssize_t numa_node_store(struct device *dev,
if (ret)
return ret;
- if (node >= MAX_NUMNODES || !node_online(node))
+ if (node < 0 || node >= MAX_NUMNODES) {
+ if (node != NUMA_NO_NODE)
+ return -EINVAL;
+ }
+
+ if (node != NUMA_NO_NODE && !node_online(node))
return -EINVAL;
add_taint(TAINT_FIRMWARE_WORKAROUND, LOCKDEP_STILL_OK);
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v2] PCI: Prevent out of bounds access in numa_node override - part 2
2015-11-24 18:49 ` Bjorn Helgaas
@ 2015-11-24 19:27 ` Mathias Krause
2015-11-24 20:05 ` Bjorn Helgaas
0 siblings, 1 reply; 4+ messages in thread
From: Mathias Krause @ 2015-11-24 19:27 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: Bjorn Helgaas, linux-pci, Sasha Levin, Prarit Bhargava
On 24 November 2015 at 19:49, Bjorn Helgaas <helgaas@kernel.org> wrote:
> Applied as tweaked below to for-linus for v4.4, thanks! As written,
> if NUMA_NO_NODE were defined as -2, we would incorrectly accept -1.
> Let me know if you disagree with my fix.
I don't think the value of NUMA_NO_NODE will (or even has to) ever
change, as we're already exporting that value to userland via sysfs.
But you're right, the code shouldn't make any assumptions about the
concrete value of NUMA_NO_NODE and just handle it as a special
symbolic value.
> commit 2a35194c5a45fbb9ca1d88bc56804dfb51a75233
> Author: Mathias Krause <minipli@googlemail.com>
> Date: Mon Nov 9 20:00:27 2015 +0100
>
> PCI: Prevent out of bounds access in numa_node override
>
> Commit 1266963170f5 ("PCI: Prevent out of bounds access in numa_node
> override") missed that the user-provided node could also be negative.
> Handle this case as well to avoid out-of-bounds accesses to the
> node_states[] array. However, allow the special value -1, i.e.
> NUMA_NO_NODE, to be able to set the 'no specific node' configuration.
>
> [bhelgaas: remove assumption that NUMA_NO_NODE == -1]
> Fixes: 1266963170f5 ("PCI: Prevent out of bounds access in numa_node override")
> Fixes: 63692df103e9 ("PCI: Allow numa_node override via sysfs")
> Signed-off-by: Mathias Krause <minipli@googlemail.com>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> CC: Sasha Levin <sasha.levin@oracle.com>
> CC: Prarit Bhargava <prarit@redhat.com>
> CC: stable@vger.kernel.org # v3.19+
>
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 9261868..50f4747 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -216,7 +216,12 @@ static ssize_t numa_node_store(struct device *dev,
> if (ret)
> return ret;
>
> - if (node >= MAX_NUMNODES || !node_online(node))
> + if (node < 0 || node >= MAX_NUMNODES) {
> + if (node != NUMA_NO_NODE)
> + return -EINVAL;
> + }
I would have written something like this:
if ((node < 0 && node != NUMA_NO_NODE) || node >= MAX_NUMNODES)
return -EINVAL;
It's more compact, but your solution is essentially the same, so I'm
fine with it.
> +
> + if (node != NUMA_NO_NODE && !node_online(node))
> return -EINVAL;
>
> add_taint(TAINT_FIRMWARE_WORKAROUND, LOCKDEP_STILL_OK);
Thanks,
Mathias
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] PCI: Prevent out of bounds access in numa_node override - part 2
2015-11-24 19:27 ` Mathias Krause
@ 2015-11-24 20:05 ` Bjorn Helgaas
0 siblings, 0 replies; 4+ messages in thread
From: Bjorn Helgaas @ 2015-11-24 20:05 UTC (permalink / raw)
To: Mathias Krause; +Cc: Bjorn Helgaas, linux-pci, Sasha Levin, Prarit Bhargava
On Tue, Nov 24, 2015 at 08:27:04PM +0100, Mathias Krause wrote:
> On 24 November 2015 at 19:49, Bjorn Helgaas <helgaas@kernel.org> wrote:
> > Applied as tweaked below to for-linus for v4.4, thanks! As written,
> > if NUMA_NO_NODE were defined as -2, we would incorrectly accept -1.
> > Let me know if you disagree with my fix.
>
> I don't think the value of NUMA_NO_NODE will (or even has to) ever
> change, as we're already exporting that value to userland via sysfs.
> But you're right, the code shouldn't make any assumptions about the
> concrete value of NUMA_NO_NODE and just handle it as a special
> symbolic value.
>
> > commit 2a35194c5a45fbb9ca1d88bc56804dfb51a75233
> > Author: Mathias Krause <minipli@googlemail.com>
> > Date: Mon Nov 9 20:00:27 2015 +0100
> >
> > PCI: Prevent out of bounds access in numa_node override
> >
> > Commit 1266963170f5 ("PCI: Prevent out of bounds access in numa_node
> > override") missed that the user-provided node could also be negative.
> > Handle this case as well to avoid out-of-bounds accesses to the
> > node_states[] array. However, allow the special value -1, i.e.
> > NUMA_NO_NODE, to be able to set the 'no specific node' configuration.
> >
> > [bhelgaas: remove assumption that NUMA_NO_NODE == -1]
> > Fixes: 1266963170f5 ("PCI: Prevent out of bounds access in numa_node override")
> > Fixes: 63692df103e9 ("PCI: Allow numa_node override via sysfs")
> > Signed-off-by: Mathias Krause <minipli@googlemail.com>
> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> > CC: Sasha Levin <sasha.levin@oracle.com>
> > CC: Prarit Bhargava <prarit@redhat.com>
> > CC: stable@vger.kernel.org # v3.19+
> >
> > diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> > index 9261868..50f4747 100644
> > --- a/drivers/pci/pci-sysfs.c
> > +++ b/drivers/pci/pci-sysfs.c
> > @@ -216,7 +216,12 @@ static ssize_t numa_node_store(struct device *dev,
> > if (ret)
> > return ret;
> >
> > - if (node >= MAX_NUMNODES || !node_online(node))
> > + if (node < 0 || node >= MAX_NUMNODES) {
> > + if (node != NUMA_NO_NODE)
> > + return -EINVAL;
> > + }
>
> I would have written something like this:
>
> if ((node < 0 && node != NUMA_NO_NODE) || node >= MAX_NUMNODES)
> return -EINVAL;
I adopted that, thanks!
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-11-24 20:05 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-09 19:00 [PATCH v2] PCI: Prevent out of bounds access in numa_node override - part 2 Mathias Krause
2015-11-24 18:49 ` Bjorn Helgaas
2015-11-24 19:27 ` Mathias Krause
2015-11-24 20:05 ` Bjorn Helgaas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).