Re: [RFC] /sys/kernel/mm/hugepages/hugepages-1048576kB/nr_overcommit_hugepages
From: Andrew Morton <akpm@linux-foundation.org>
Date: 2011-01-05 21:00:02
On Wed, 5 Jan 2011 09:43:57 +0100 Michal Hocko [off-list ref] wrote:
quoted hunk ↗ jump to hunk
... proc_doulongvec_minmax may fail if the given buffer doesn't represent a valid number. If we provide something invalid we will initialize the resulting value (nr_overcommit_huge_pages in this case) to a random value from the stack. The issue was introduced by a3d0c6aa when the default handler has been replaced by the helper function where we do not check the return value. Reproducer: echo "" > /proc/sys/vm/nr_overcommit_hugepages ...--- 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.
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.
--- a/mm/hugetlb.c~hugetlb-check-the-return-value-of-string-conversion-in-sysctl-handler-fix
+++ a/mm/hugetlb.c@@ -1859,14 +1859,16 @@ static int hugetlb_sysctl_handler_common { struct hstate *h = &default_hstate; unsigned long tmp; + int ret; if (!write) tmp = h->max_huge_pages; table->data = &tmp; table->maxlen = sizeof(unsigned long); - if (proc_doulongvec_minmax(table, write, buffer, length, ppos)) - return -EINVAL; + ret = proc_doulongvec_minmax(table, write, buffer, length, ppos); + if (ret) + goto out; if (write) { NODEMASK_ALLOC(nodemask_t, nodes_allowed,
@@ -1881,8 +1883,8 @@ static int hugetlb_sysctl_handler_common if (nodes_allowed != &node_states[N_HIGH_MEMORY]) NODEMASK_FREE(nodes_allowed); } - - return 0; +out: + return ret; } int hugetlb_sysctl_handler(struct ctl_table *table, int write,
@@ -1920,22 +1922,24 @@ int hugetlb_overcommit_handler(struct ct { struct hstate *h = &default_hstate; unsigned long tmp; + int ret; if (!write) tmp = h->nr_overcommit_huge_pages; table->data = &tmp; table->maxlen = sizeof(unsigned long); - if (proc_doulongvec_minmax(table, write, buffer, length, ppos)) - return -EINVAL; + ret = proc_doulongvec_minmax(table, write, buffer, length, ppos); + if (ret) + goto out; if (write) { spin_lock(&hugetlb_lock); h->nr_overcommit_huge_pages = tmp; spin_unlock(&hugetlb_lock); } - - return 0; +out: + return ret; } #endif /* CONFIG_SYSCTL */
_ -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. 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@kvack.org"> email@kvack.org </a>