On Wed 05-01-11 12:59:59, Andrew Morton wrote:
> On Wed, 5 Jan 2011 09:43:57 +0100
> Michal Hocko <mhocko@xxxxxxx> wrote:
[...]
> > --- a/mm/hugetlb.c
> > +++ b/mm/hugetlb.c
> > @@ -1928,7 +1928,8 @@ static int hugetlb_sysctl_handler_common(bool obey_mempolicy,
> >
> > table->data = &tmp;
> > table->maxlen = sizeof(unsigned long);
> > - proc_doulongvec_minmax(table, write, buffer, length, ppos);
> > + if (proc_doulongvec_minmax(table, write, buffer, length, ppos))
> > + return -EINVAL;
>
> proc_doulongvec_minmax() can return -EFAULT or -ENOMEM. It is
> incorrect to unconditionally convert those into -EINVAL.
You are right, I have missed that. Thanks for fixing that up
>
> > if (write) {
> > NODEMASK_ALLOC(nodemask_t, nodes_allowed,
>
> hm, the code doesn't check that NODEMASK_ALLOC succeeded. That
> NODEMASK_ALLOC conversion was quite sloppy.
What do you think about the patch bellow? I have based it on top of
you mm patches (I was CCed):
hugetlb-check-the-return-value-of-string-conversion-in-sysctl-handler.patch
hugetlb-check-the-return-value-of-string-conversion-in-sysctl-handler-fix.patch
hugetlb-do-not-allow-pagesize-=-max_order-pool-adjustment.patch
hugetlb-do-not-allow-pagesize-=-max_order-pool-adjustment-fix.patch
hugetlb-fix-handling-of-parse-errors-in-sysfs.patch
Some of them didn't apply cleanly so I had to tweak them a bit so maybe
I am missing some other patches.
---
>From e2cb35db086f133bea87fc8190fc8cca03557606 Mon Sep 17 00:00:00 2001
From: Michal Hocko <mhocko@xxxxxxx>
Date: Thu, 6 Jan 2011 10:57:31 +0100
Subject: [PATCH] hugetlb: handle NODEMASK_ALLOC failure correctly
NODEMASK_ALLOC can use kmalloc if nodemask_t > 256 bytes so it might
fail with NULL as a result. Let's check the resulting variable and
fail with -ENOMEM if NODEMASK_ALLOC failed.
Signed-off-by: Michal Hocko <mhocko@xxxxxxx>
---
mm/hugetlb.c | 18 +++++++++++++++---
1 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 4c0606c..21f31b2 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -1439,14 +1439,19 @@ static ssize_t nr_hugepages_store_common(bool obey_mempolicy,
struct hstate *h;
NODEMASK_ALLOC(nodemask_t, nodes_allowed, GFP_KERNEL | __GFP_NORETRY);
+ if (!nodes_allowed) {
+ err = -ENOMEM;
+ goto out;
+ }
+
err = strict_strtoul(buf, 10, &count);
if (err)
- goto out;
+ goto out_free_mask;
h = kobj_to_hstate(kobj, &nid);
if (h->order >= MAX_ORDER) {
err = -EINVAL;
- goto out;
+ goto out_free_mask;
}
if (nid == NUMA_NO_NODE) {
@@ -1474,8 +1479,9 @@ static ssize_t nr_hugepages_store_common(bool obey_mempolicy,
NODEMASK_FREE(nodes_allowed);
return len;
-out:
+out_free_mask:
NODEMASK_FREE(nodes_allowed);
+out:
return err;
}
@@ -1951,6 +1957,12 @@ static int hugetlb_sysctl_handler_common(bool obey_mempolicy,
if (write) {
NODEMASK_ALLOC(nodemask_t, nodes_allowed,
GFP_KERNEL | __GFP_NORETRY);
+
+ if (!nodes_allowed) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
if (!(obey_mempolicy &&
init_nodemask_of_mempolicy(nodes_allowed))) {
NODEMASK_FREE(nodes_allowed);
--
1.7.2.3
--
Michal Hocko
L3 team
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9
Czech Republic
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxxx For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>