Add new string functions strict_strto* and convert kernel params to use them

Currently, for every sysfs node, the callers will be responsible for implementing store operation, so many many callers are doing duplicate things to validate input, they have the same mistakes because they are calling simple_strtol/ul/ll/uul, especially for module params, they are just numeric, but you can echo such values as 0x1234xxx, 07777888 and 1234aaa, for these cases, module params store operation just ignores succesive invalid char and converts prefix part to a numeric although input is acctually invalid. This patch tries to fix the aforementioned issues and implements strict_strtox serial functions, kernel/params.c uses them to strictly validate input, so module params will reject such values as 0x1234xxxx and returns an error: write error: Invalid argument Any modules which export numeric sysfs node can use strict_strtox instead of simple_strtox to reject any invalid input. Here are some test results: Before applying this patch: [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak 4096 [root@yangyi-dev /]# echo 0x1000 > /sys/module/e1000/parameters/copybreak [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak 4096 [root@yangyi-dev /]# echo 0x1000g > /sys/module/e1000/parameters/copybreak [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak 4096 [root@yangyi-dev /]# echo 0x1000gggggggg > /sys/module/e1000/parameters/copybreak [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak 4096 [root@yangyi-dev /]# echo 010000 > /sys/module/e1000/parameters/copybreak [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak 4096 [root@yangyi-dev /]# echo 0100008 > /sys/module/e1000/parameters/copybreak [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak 4096 [root@yangyi-dev /]# echo 010000aaaaa > /sys/module/e1000/parameters/copybreak [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak 4096 [root@yangyi-dev /]# After applying this patch: [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak 4096 [root@yangyi-dev /]# echo 0x1000 > /sys/module/e1000/parameters/copybreak [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak 4096 [root@yangyi-dev /]# echo 0x1000g > /sys/module/e1000/parameters/copybreak -bash: echo: write error: Invalid argument [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak 4096 [root@yangyi-dev /]# echo 0x1000gggggggg > /sys/module/e1000/parameters/copybreak -bash: echo: write error: Invalid argument [root@yangyi-dev /]# echo 010000 > /sys/module/e1000/parameters/copybreak [root@yangyi-dev /]# echo 0100008 > /sys/module/e1000/parameters/copybreak -bash: echo: write error: Invalid argument [root@yangyi-dev /]# echo 010000aaaaa > /sys/module/e1000/parameters/copybreak -bash: echo: write error: Invalid argument [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak 4096 [root@yangyi-dev /]# echo -n 4096 > /sys/module/e1000/parameters/copybreak [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak 4096 [root@yangyi-dev /]# [akpm@linux-foundation.org: fix compiler warnings] [akpm@linux-foundation.org: fix off-by-one found by tiwai@suse.de] Signed-off-by: Yi Yang <yi.y.yang@intel.com> Cc: Greg KH <greg@kroah.com> Cc: "Randy.Dunlap" <rdunlap@xenotime.net> Cc: Takashi Iwai <tiwai@suse.de> Cc: Hugh Dickins <hugh@veritas.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

Add new string functions strict_strto* and convert kernel params to use them
Currently, for every sysfs node, the callers will be responsible for implementing store operation, so many many callers are doing duplicate things to validate input, they have the same mistakes because they are calling simple_strtol/ul/ll/uul, especially for module params, they are just numeric, but you can echo such values as 0x1234xxx, 07777888 and 1234aaa, for these cases, module params store operation just ignores succesive invalid char and converts prefix part to a numeric although input is acctually invalid. This patch tries to fix the aforementioned issues and implements strict_strtox serial functions, kernel/params.c uses them to strictly validate input, so module params will reject such values as 0x1234xxxx and returns an error: write error: Invalid argument Any modules which export numeric sysfs node can use strict_strtox instead of simple_strtox to reject any invalid input. Here are some test results: Before applying this patch: [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak 4096 [root@yangyi-dev /]# echo 0x1000 > /sys/module/e1000/parameters/copybreak [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak 4096 [root@yangyi-dev /]# echo 0x1000g > /sys/module/e1000/parameters/copybreak [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak 4096 [root@yangyi-dev /]# echo 0x1000gggggggg > /sys/module/e1000/parameters/copybreak [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak 4096 [root@yangyi-dev /]# echo 010000 > /sys/module/e1000/parameters/copybreak [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak 4096 [root@yangyi-dev /]# echo 0100008 > /sys/module/e1000/parameters/copybreak [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak 4096 [root@yangyi-dev /]# echo 010000aaaaa > /sys/module/e1000/parameters/copybreak [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak 4096 [root@yangyi-dev /]# After applying this patch: [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak 4096 [root@yangyi-dev /]# echo 0x1000 > /sys/module/e1000/parameters/copybreak [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak 4096 [root@yangyi-dev /]# echo 0x1000g > /sys/module/e1000/parameters/copybreak -bash: echo: write error: Invalid argument [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak 4096 [root@yangyi-dev /]# echo 0x1000gggggggg > /sys/module/e1000/parameters/copybreak -bash: echo: write error: Invalid argument [root@yangyi-dev /]# echo 010000 > /sys/module/e1000/parameters/copybreak [root@yangyi-dev /]# echo 0100008 > /sys/module/e1000/parameters/copybreak -bash: echo: write error: Invalid argument [root@yangyi-dev /]# echo 010000aaaaa > /sys/module/e1000/parameters/copybreak -bash: echo: write error: Invalid argument [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak 4096 [root@yangyi-dev /]# echo -n 4096 > /sys/module/e1000/parameters/copybreak [root@yangyi-dev /]# cat /sys/module/e1000/parameters/copybreak 4096 [root@yangyi-dev /]# [akpm@linux-foundation.org: fix compiler warnings] [akpm@linux-foundation.org: fix off-by-one found by tiwai@suse.de] Signed-off-by: Yi Yang <yi.y.yang@intel.com> Cc: Greg KH <greg@kroah.com> Cc: "Randy.Dunlap" <rdunlap@xenotime.net> Cc: Takashi Iwai <tiwai@suse.de> Cc: Hugh Dickins <hugh@veritas.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Yi Yang · Linus Torvalds
1 parent 10e6f32bdf
Showing 3 changed files with 137 additions and 10 deletions Side-by-side Diff
include/linux/kernel.h
kernel/params.c
lib/vsprintf.c
@@ -141,6 +141,10 @@
 extern long simple_strtol(const char *,char **,unsigned int);
 extern unsigned long long simple_strtoull(const char *,char **,unsigned int);
 extern long long simple_strtoll(const char *,char **,unsigned int);
+extern int strict_strtoul(const char *, unsigned int, unsigned long *);
+extern int strict_strtol(const char *, unsigned int, long *);
+extern int strict_strtoull(const char *, unsigned int, unsigned long long *);
+extern int strict_strtoll(const char *, unsigned int, long long *);
 extern int sprintf(char * buf, const char * fmt, ...)
 	__attribute__ ((format (printf, 2, 3)));
 extern int vsprintf(char *buf, const char *, va_list)
@@ -180,12 +180,12 @@
 #define STANDARD_PARAM_DEF(name, type, format, tmptype, strtolfn)      	\
 	int param_set_##name(const char *val, struct kernel_param *kp)	\
 	{								\
-		char *endp;						\
 		tmptype l;						\
+		int ret;						\
 									\
 		if (!val) return -EINVAL;				\
-		l = strtolfn(val, &endp, 0);				\
-		if (endp == val || ((type)l != l))			\
+		ret = strtolfn(val, 0, &l);				\
+		if (ret == -EINVAL || ((type)l != l))			\
 			return -EINVAL;					\
 		*((type *)kp->arg) = l;					\
 		return 0;						\
@@ -195,13 +195,13 @@
 		return sprintf(buffer, format, *((type *)kp->arg));	\
 	}
  
-STANDARD_PARAM_DEF(byte, unsigned char, "%c", unsigned long, simple_strtoul);
-STANDARD_PARAM_DEF(short, short, "%hi", long, simple_strtol);
-STANDARD_PARAM_DEF(ushort, unsigned short, "%hu", unsigned long, simple_strtoul);
-STANDARD_PARAM_DEF(int, int, "%i", long, simple_strtol);
-STANDARD_PARAM_DEF(uint, unsigned int, "%u", unsigned long, simple_strtoul);
-STANDARD_PARAM_DEF(long, long, "%li", long, simple_strtol);
-STANDARD_PARAM_DEF(ulong, unsigned long, "%lu", unsigned long, simple_strtoul);
+STANDARD_PARAM_DEF(byte, unsigned char, "%c", unsigned long, strict_strtoul);
+STANDARD_PARAM_DEF(short, short, "%hi", long, strict_strtol);
+STANDARD_PARAM_DEF(ushort, unsigned short, "%hu", unsigned long, strict_strtoul);
+STANDARD_PARAM_DEF(int, int, "%i", long, strict_strtol);
+STANDARD_PARAM_DEF(uint, unsigned int, "%u", unsigned long, strict_strtoul);
+STANDARD_PARAM_DEF(long, long, "%li", long, strict_strtol);
+STANDARD_PARAM_DEF(ulong, unsigned long, "%lu", unsigned long, strict_strtoul);
  
 int param_set_charp(const char *val, struct kernel_param *kp)
 {
@@ -126,6 +126,129 @@
 	return simple_strtoull(cp,endp,base);
 }
  
+
+/**
+ * strict_strtoul - convert a string to an unsigned long strictly
+ * @cp: The string to be converted
+ * @base: The number base to use
+ * @res: The converted result value
+ *
+ * strict_strtoul converts a string to an unsigned long only if the
+ * string is really an unsigned long string, any string containing
+ * any invalid char at the tail will be rejected and -EINVAL is returned,
+ * only a newline char at the tail is acceptible because people generally
+ * change a module parameter in the following way:
+ *
+ * 	echo 1024 > /sys/module/e1000/parameters/copybreak
+ *
+ * echo will append a newline to the tail.
+ *
+ * It returns 0 if conversion is successful and *res is set to the converted
+ * value, otherwise it returns -EINVAL and *res is set to 0.
+ *
+ * simple_strtoul just ignores the successive invalid characters and
+ * return the converted value of prefix part of the string.
+ */
+int strict_strtoul(const char *cp, unsigned int base, unsigned long *res);
+
+/**
+ * strict_strtol - convert a string to a long strictly
+ * @cp: The string to be converted
+ * @base: The number base to use
+ * @res: The converted result value
+ *
+ * strict_strtol is similiar to strict_strtoul, but it allows the first
+ * character of a string is '-'.
+ *
+ * It returns 0 if conversion is successful and *res is set to the converted
+ * value, otherwise it returns -EINVAL and *res is set to 0.
+ */
+int strict_strtol(const char *cp, unsigned int base, long *res);
+
+/**
+ * strict_strtoull - convert a string to an unsigned long long strictly
+ * @cp: The string to be converted
+ * @base: The number base to use
+ * @res: The converted result value
+ *
+ * strict_strtoull converts a string to an unsigned long long only if the
+ * string is really an unsigned long long string, any string containing
+ * any invalid char at the tail will be rejected and -EINVAL is returned,
+ * only a newline char at the tail is acceptible because people generally
+ * change a module parameter in the following way:
+ *
+ * 	echo 1024 > /sys/module/e1000/parameters/copybreak
+ *
+ * echo will append a newline to the tail of the string.
+ *
+ * It returns 0 if conversion is successful and *res is set to the converted
+ * value, otherwise it returns -EINVAL and *res is set to 0.
+ *
+ * simple_strtoull just ignores the successive invalid characters and
+ * return the converted value of prefix part of the string.
+ */
+int strict_strtoull(const char *cp, unsigned int base, unsigned long long *res);
+
+/**
+ * strict_strtoll - convert a string to a long long strictly
+ * @cp: The string to be converted
+ * @base: The number base to use
+ * @res: The converted result value
+ *
+ * strict_strtoll is similiar to strict_strtoull, but it allows the first
+ * character of a string is '-'.
+ *
+ * It returns 0 if conversion is successful and *res is set to the converted
+ * value, otherwise it returns -EINVAL and *res is set to 0.
+ */
+int strict_strtoll(const char *cp, unsigned int base, long long *res);
+
+#define define_strict_strtoux(type, valtype)				\
+int strict_strtou##type(const char *cp, unsigned int base, valtype *res)\
+{									\
+	char *tail;							\
+	valtype val;							\
+	size_t len;							\
+									\
+	*res = 0;							\
+	len = strlen(cp);						\
+	if (len == 0)							\
+		return -EINVAL;						\
+									\
+	val = simple_strtoul(cp, &tail, base);				\
+	if ((*tail == '\0') ||						\
+		((len == (size_t)(tail - cp) + 1) && (*tail == '\n'))) {\
+		*res = val;						\
+		return 0;						\
+	}								\
+									\
+	return -EINVAL;							\
+}									\
+
+#define define_strict_strtox(type, valtype)				\
+int strict_strto##type(const char *cp, unsigned int base, valtype *res)	\
+{									\
+	int ret;							\
+	if (*cp == '-') {						\
+		ret = strict_strtou##type(cp+1, base, res);		\
+		if (ret != 0)						\
+			*res = -(*res);					\
+	} else								\
+		ret = strict_strtou##type(cp, base, res);		\
+									\
+	return ret;							\
+}									\
+
+define_strict_strtoux(l, unsigned long)
+define_strict_strtox(l, long)
+define_strict_strtoux(ll, unsigned long long)
+define_strict_strtox(ll, long long)
+
+EXPORT_SYMBOL(strict_strtoul);
+EXPORT_SYMBOL(strict_strtol);
+EXPORT_SYMBOL(strict_strtoll);
+EXPORT_SYMBOL(strict_strtoull);
+
 static int skip_atoi(const char **s)
 {
 	int i=0;
...	...	@@ -141,6 +141,10 @@
141	141	extern long simple_strtol(const char ,char *,unsigned int);
142	142	extern unsigned long long simple_strtoull(const char ,char *,unsigned int);
143	143	extern long long simple_strtoll(const char ,char *,unsigned int);
	144	+extern int strict_strtoul(const char , unsigned int, unsigned long );
	145	+extern int strict_strtol(const char , unsigned int, long );
	146	+extern int strict_strtoull(const char , unsigned int, unsigned long long );
	147	+extern int strict_strtoll(const char , unsigned int, long long );
144	148	extern int sprintf(char * buf, const char * fmt, ...)
145	149	__attribute__ ((format (printf, 2, 3)));
146	150	extern int vsprintf(char buf, const char , va_list)
...	...	@@ -180,12 +180,12 @@
180	180	#define STANDARD_PARAM_DEF(name, type, format, tmptype, strtolfn) \
181	181	int param_set_##name(const char val, struct kernel_param kp) \
182	182	{ \
183		- char *endp; \
184	183	tmptype l; \
	184	+ int ret; \
185	185	\
186	186	if (!val) return -EINVAL; \
187		- l = strtolfn(val, &endp, 0); \
188		- if (endp == val \|\| ((type)l != l)) \
	187	+ ret = strtolfn(val, 0, &l); \
	188	+ if (ret == -EINVAL \|\| ((type)l != l)) \
189	189	return -EINVAL; \
190	190	((type )kp->arg) = l; \
191	191	return 0; \
...	...	@@ -195,13 +195,13 @@
195	195	return sprintf(buffer, format, ((type )kp->arg)); \
196	196	}
197	197
198		-STANDARD_PARAM_DEF(byte, unsigned char, "%c", unsigned long, simple_strtoul);
199		-STANDARD_PARAM_DEF(short, short, "%hi", long, simple_strtol);
200		-STANDARD_PARAM_DEF(ushort, unsigned short, "%hu", unsigned long, simple_strtoul);
201		-STANDARD_PARAM_DEF(int, int, "%i", long, simple_strtol);
202		-STANDARD_PARAM_DEF(uint, unsigned int, "%u", unsigned long, simple_strtoul);
203		-STANDARD_PARAM_DEF(long, long, "%li", long, simple_strtol);
204		-STANDARD_PARAM_DEF(ulong, unsigned long, "%lu", unsigned long, simple_strtoul);
	198	+STANDARD_PARAM_DEF(byte, unsigned char, "%c", unsigned long, strict_strtoul);
	199	+STANDARD_PARAM_DEF(short, short, "%hi", long, strict_strtol);
	200	+STANDARD_PARAM_DEF(ushort, unsigned short, "%hu", unsigned long, strict_strtoul);
	201	+STANDARD_PARAM_DEF(int, int, "%i", long, strict_strtol);
	202	+STANDARD_PARAM_DEF(uint, unsigned int, "%u", unsigned long, strict_strtoul);
	203	+STANDARD_PARAM_DEF(long, long, "%li", long, strict_strtol);
	204	+STANDARD_PARAM_DEF(ulong, unsigned long, "%lu", unsigned long, strict_strtoul);
205	205
206	206	int param_set_charp(const char val, struct kernel_param kp)
207	207	{
...	...	@@ -126,6 +126,129 @@
126	126	return simple_strtoull(cp,endp,base);
127	127	}
128	128
	129	+
	130	+/**
	131	+ * strict_strtoul - convert a string to an unsigned long strictly
	132	+ * @cp: The string to be converted
	133	+ * @base: The number base to use
	134	+ * @res: The converted result value
	135	+ *
	136	+ * strict_strtoul converts a string to an unsigned long only if the
	137	+ * string is really an unsigned long string, any string containing
	138	+ * any invalid char at the tail will be rejected and -EINVAL is returned,
	139	+ * only a newline char at the tail is acceptible because people generally
	140	+ * change a module parameter in the following way:
	141	+ *
	142	+ * echo 1024 > /sys/module/e1000/parameters/copybreak
	143	+ *
	144	+ * echo will append a newline to the tail.
	145	+ *
	146	+ * It returns 0 if conversion is successful and *res is set to the converted
	147	+ * value, otherwise it returns -EINVAL and *res is set to 0.
	148	+ *
	149	+ * simple_strtoul just ignores the successive invalid characters and
	150	+ * return the converted value of prefix part of the string.
	151	+ */
	152	+int strict_strtoul(const char cp, unsigned int base, unsigned long res);
	153	+
	154	+/**
	155	+ * strict_strtol - convert a string to a long strictly
	156	+ * @cp: The string to be converted
	157	+ * @base: The number base to use
	158	+ * @res: The converted result value
	159	+ *
	160	+ * strict_strtol is similiar to strict_strtoul, but it allows the first
	161	+ * character of a string is '-'.
	162	+ *
	163	+ * It returns 0 if conversion is successful and *res is set to the converted
	164	+ * value, otherwise it returns -EINVAL and *res is set to 0.
	165	+ */
	166	+int strict_strtol(const char cp, unsigned int base, long res);
	167	+
	168	+/**
	169	+ * strict_strtoull - convert a string to an unsigned long long strictly
	170	+ * @cp: The string to be converted
	171	+ * @base: The number base to use
	172	+ * @res: The converted result value
	173	+ *
	174	+ * strict_strtoull converts a string to an unsigned long long only if the
	175	+ * string is really an unsigned long long string, any string containing
	176	+ * any invalid char at the tail will be rejected and -EINVAL is returned,
	177	+ * only a newline char at the tail is acceptible because people generally
	178	+ * change a module parameter in the following way:
	179	+ *
	180	+ * echo 1024 > /sys/module/e1000/parameters/copybreak
	181	+ *
	182	+ * echo will append a newline to the tail of the string.
	183	+ *
	184	+ * It returns 0 if conversion is successful and *res is set to the converted
	185	+ * value, otherwise it returns -EINVAL and *res is set to 0.
	186	+ *
	187	+ * simple_strtoull just ignores the successive invalid characters and
	188	+ * return the converted value of prefix part of the string.
	189	+ */
	190	+int strict_strtoull(const char cp, unsigned int base, unsigned long long res);
	191	+
	192	+/**
	193	+ * strict_strtoll - convert a string to a long long strictly
	194	+ * @cp: The string to be converted
	195	+ * @base: The number base to use
	196	+ * @res: The converted result value
	197	+ *
	198	+ * strict_strtoll is similiar to strict_strtoull, but it allows the first
	199	+ * character of a string is '-'.
	200	+ *
	201	+ * It returns 0 if conversion is successful and *res is set to the converted
	202	+ * value, otherwise it returns -EINVAL and *res is set to 0.
	203	+ */
	204	+int strict_strtoll(const char cp, unsigned int base, long long res);
	205	+
	206	+#define define_strict_strtoux(type, valtype) \
	207	+int strict_strtou##type(const char cp, unsigned int base, valtype res)\
	208	+{ \
	209	+ char *tail; \
	210	+ valtype val; \
	211	+ size_t len; \
	212	+ \
	213	+ *res = 0; \
	214	+ len = strlen(cp); \
	215	+ if (len == 0) \
	216	+ return -EINVAL; \
	217	+ \
	218	+ val = simple_strtoul(cp, &tail, base); \
	219	+ if ((*tail == '\0') \|\| \
	220	+ ((len == (size_t)(tail - cp) + 1) && (*tail == '\n'))) {\
	221	+ *res = val; \
	222	+ return 0; \
	223	+ } \
	224	+ \
	225	+ return -EINVAL; \
	226	+} \
	227	+
	228	+#define define_strict_strtox(type, valtype) \
	229	+int strict_strto##type(const char cp, unsigned int base, valtype res) \
	230	+{ \
	231	+ int ret; \
	232	+ if (*cp == '-') { \
	233	+ ret = strict_strtou##type(cp+1, base, res); \
	234	+ if (ret != 0) \
	235	+ res = -(res); \
	236	+ } else \
	237	+ ret = strict_strtou##type(cp, base, res); \
	238	+ \
	239	+ return ret; \
	240	+} \
	241	+
	242	+define_strict_strtoux(l, unsigned long)
	243	+define_strict_strtox(l, long)
	244	+define_strict_strtoux(ll, unsigned long long)
	245	+define_strict_strtox(ll, long long)
	246	+
	247	+EXPORT_SYMBOL(strict_strtoul);
	248	+EXPORT_SYMBOL(strict_strtol);
	249	+EXPORT_SYMBOL(strict_strtoll);
	250	+EXPORT_SYMBOL(strict_strtoull);
	251	+
129	252	static int skip_atoi(const char **s)
130	253	{
131	254	int i=0;