Hi,
On 12/04/2014 12:25 AM, Andrew Morton wrote:
On Wed, 03 Dec 2014 15:41:21 +0300 Andrey Ryabinin <a.ryabi...@samsung.com>
wrote:
Use the 'unsigned long' type for 'zero' variable to fix this.
Changing type to 'unsigned long' shouldn't affect any other users
of this variable.
Reported-by: Dmitry Vyukov <dvyu...@google.com>
Fixes: ed4d4902ebdd ("mm, hugetlb: remove hugetlb_zero and hugetlb_infinity")
Signed-off-by: Andrey Ryabinin <a.ryabi...@samsung.com>
---
kernel/sysctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 15f2511..45c45c9 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -120,7 +120,7 @@ static int sixty = 60;
static int __maybe_unused neg_one = -1;
-static int zero;
+static unsigned long zero;
After some (useless) playing around (see the attached patch):
Using
> .extra1=zero,
for proc_doulongvec_minmax doesn't make any sense:
__do_proc_doulongvec_minmax() internally contains
if ((min && val < *min) || (max && val > *max))
continue;
What about just deleting the offending .extra1=zero line?
.extra1=NULL has the same effect as .extra1=&zero.
--
Manfred
>From 194e5d4758bb30531bad0907f06f3518002cd8b4 Mon Sep 17 00:00:00 2001
From: Manfred Spraul <manf...@colorfullife.com>
Date: Sat, 13 Dec 2014 21:25:27 +0100
Subject: [PATCH] kernel/sysctl.c: Type safe macros
struct ctl_table is used for creating entries in e.g. /proc/sys/kernel.
The structure contains three void * entries and a function pointer, thus
there is the risk of incorrectly mixing types.
The patch create a macro that prevents accidential mixing, it enforces
that the type expected by the function pointer and the three void * are
all of the same type.
Notes:
- From my first impression, most proc entries mix types, and it works,
because (int)1 and (unsigned int)1 are identical.
Thus I'm not sure if this is the right approach.
- the code doesn't compile, it contains an intentional incompatible type
What do you think?
--
Manfred
---
kernel/sysctl.c | 113 ++++++++++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 97 insertions(+), 16 deletions(-)
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 15f2511..bc446a6 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -276,6 +276,101 @@ static int min_extfrag_threshold;
static int max_extfrag_threshold = 1000;
#endif
+/*
+ * Type safe macros for creating the sysctl table entries:
+ *
+ * TODO:
+ * - 1) It works.
+ * I.e. it complains when _dointvec is used to assign
+ * to an unsigned int. Unfortunately, this is very common:
+ * * _minmax is used, with min=0 and e.g. max=int_max.
+ * * the variable is actually used as a boolean, thus it doesn't matter
+ * if the user space interface returns -1 or 0xffffffff.
+ * * unsigned int zero is used as min
+ * * ...
+ * Thus most error messages would be false positives.
+ *
+ * - 2) The error message is difficult to understand
+ * error: initializer element is not constant
+ */
+
+#define ASSIGN_TYPE_SAFE(param, type) \
+ __builtin_choose_expr(__builtin_types_compatible_p(typeof(param), type), \
+ param, \
+ /* The void expression results in a compile-time error \
+ when assigning the result to something. */ \
+ ((void)0) \
+ )
+
+#define SYSCTL_BUILD_INT_ENTRY_MINMAX(name, entry, min_ptr, max_ptr) \
+ { \
+ .procname = (name), \
+ .data = ASSIGN_TYPE_SAFE(&(entry), int *), \
+ .maxlen = sizeof( (entry) ), \
+ .mode = 0644, \
+ .proc_handler = proc_dointvec_minmax, \
+ .extra1 = ASSIGN_TYPE_SAFE( (min_ptr), int *), \
+ .extra2 = ASSIGN_TYPE_SAFE( (max_ptr), int *) \
+ }
+
+#define SYSCTL_BUILD_ULONG_ENTRY_MINMAX(name, entry, min_ptr, max_ptr) \
+ { \
+ .procname = (name), \
+ .data = ASSIGN_TYPE_SAFE(&(entry), unsigned long *), \
+ .maxlen = sizeof( (entry) ), \
+ .mode = 0644, \
+ .proc_handler = proc_doulongvec_minmax, \
+ .extra1 = ASSIGN_TYPE_SAFE( (min_ptr), unsigned long *), \
+ .extra2 = ASSIGN_TYPE_SAFE( (max_ptr), unsigned long *) \
+ }
+
+#define SYSCTL_BUILD_ENTRY_MINMAX(name, entry, min_ptr, max_ptr) \
+ { \
+ .procname = (name), \
+ .data = &(entry), \
+ .maxlen = sizeof( (entry) ), \
+ .mode = 0644, \
+ .proc_handler = __builtin_choose_expr(__builtin_types_compatible_p(typeof( &(entry)), int *), \
+ proc_dointvec_minmax, \
+ __builtin_choose_expr(__builtin_types_compatible_p(typeof( &(entry)), unsigned long *), \
+ proc_doulongvec_minmax, \
+ ((void)0) ) ), \
+ .extra1 = ASSIGN_TYPE_SAFE( (min_ptr), typeof( &(entry) )), \
+ .extra2 = ASSIGN_TYPE_SAFE( (max_ptr), typeof( &(entry) )) \
+ }
+
+#define SYSCTL_BUILD_INT_ENTRY(name, entry) \
+ { \
+ .procname = (name), \
+ .data = ASSIGN_TYPE_SAFE( &(entry), int *), \
+ .maxlen = sizeof( (entry) ), \
+ .mode = 0644, \
+ .proc_handler = proc_dointvec, \
+ }
+
+#define SYSCTL_BUILD_ULONG_ENTRY(name, entry) \
+ { \
+ .procname = (name), \
+ .data = ASSIGN_TYPE_SAFE( &(entry), unsigned long *), \
+ .maxlen = sizeof( (entry) ), \
+ .mode = 0644, \
+ .proc_handler = proc_doulongvec, \
+ }
+
+#define SYSCTL_BUILD_ENTRY(name, entry) \
+ { \
+ .procname = (name), \
+ .data = &(entry), \
+ .maxlen = sizeof( (entry) ), \
+ .mode = 0644, \
+ .proc_handler = __builtin_choose_expr(__builtin_types_compatible_p(typeof( &(entry)), int *), \
+ proc_dointvec, \
+ __builtin_choose_expr(__builtin_types_compatible_p(typeof( &(entry)), unsigned long *), \
+ proc_doulongvec, \
+ ((void)0) ) ) \
+ }
+
+
static struct ctl_table kern_table[] = {
{
.procname = "sched_child_runs_first",
@@ -1344,22 +1439,8 @@ static struct ctl_table vm_table[] = {
.mode = 0644,
.proc_handler = proc_dointvec_jiffies,
},
- {
- .procname = "block_dump",
- .data = &block_dump,
- .maxlen = sizeof(block_dump),
- .mode = 0644,
- .proc_handler = proc_dointvec,
- .extra1 = &zero,
- },
- {
- .procname = "vfs_cache_pressure",
- .data = &sysctl_vfs_cache_pressure,
- .maxlen = sizeof(sysctl_vfs_cache_pressure),
- .mode = 0644,
- .proc_handler = proc_dointvec,
- .extra1 = &zero,
- },
+ SYSCTL_BUILD_ENTRY_MINMAX("block_dump", block_dump, &zero, (unsigned int *)NULL),
+ SYSCTL_BUILD_ENTRY_MINMAX("vfs_cache_pressure", sysctl_vfs_cache_pressure, &zero, (int *)NULL),
#ifdef HAVE_ARCH_PICK_MMAP_LAYOUT
{
.procname = "legacy_va_layout",
--
2.1.0
