[PATCH] strndup_user: convert keyctl

Copies user-space string with strndup_user() and moves the type string duplication code to a function (thus fixing a wrong check on the length of the type.) Signed-off-by: Davi Arnaut <davi.arnaut@gmail.com> Cc: David Howells <dhowells@redhat.com> Signed-off-by: Andrew Morton <akpm@osdl.org> Signed-off-by: Linus Torvalds <torvalds@osdl.org>

[PATCH] strndup_user: convert keyctl
Copies user-space string with strndup_user() and moves the type string duplication code to a function (thus fixing a wrong check on the length of the type.) Signed-off-by: Davi Arnaut <davi.arnaut@gmail.com> Cc: David Howells <dhowells@redhat.com> Signed-off-by: Andrew Morton <akpm@osdl.org> Signed-off-by: Linus Torvalds <torvalds@osdl.org>
Davi Arnaut · Linus Torvalds
1 parent 24277dda3a
Showing 1 changed file with 50 additions and 105 deletions Side-by-side Diff
security/keys/keyctl.c
@@ -17,10 +17,33 @@
 #include <linux/keyctl.h>
 #include <linux/fs.h>
 #include <linux/capability.h>
+#include <linux/string.h>
 #include <linux/err.h>
 #include <asm/uaccess.h>
 #include "internal.h"
  
+static int key_get_type_from_user(char *type,
+				  const char __user *_type,
+				  unsigned len)
+{
+	int ret;
+
+	ret = strncpy_from_user(type, _type, len);
+
+	if (ret < 0)
+		return -EFAULT;
+
+	if (ret == 0 || ret >= len)
+		return -EINVAL;
+
+	if (type[0] == '.')
+		return -EPERM;
+
+	type[len - 1] = '\0';
+
+	return 0;
+}
+
 /*****************************************************************************/
 /*
  * extract the description of a new key from userspace and either add it as a
  
  
  
  
  
@@ -38,41 +61,23 @@
 	key_ref_t keyring_ref, key_ref;
 	char type[32], *description;
 	void *payload;
-	long dlen, ret;
+	long ret;
  
 	ret = -EINVAL;
 	if (plen > 32767)
 		goto error;
  
 	/* draw all the data into kernel space */
-	ret = strncpy_from_user(type, _type, sizeof(type) - 1);
+	ret = key_get_type_from_user(type, _type, sizeof(type));
 	if (ret < 0)
 		goto error;
-	type[31] = '\0';
  
-	ret = -EPERM;
-	if (type[0] == '.')
+	description = strndup_user(_description, PAGE_SIZE);
+	if (IS_ERR(description)) {
+		ret = PTR_ERR(description);
 		goto error;
+	}
  
-	ret = -EFAULT;
-	dlen = strnlen_user(_description, PAGE_SIZE - 1);
-	if (dlen <= 0)
-		goto error;
-
-	ret = -EINVAL;
-	if (dlen > PAGE_SIZE - 1)
-		goto error;
-
-	ret = -ENOMEM;
-	description = kmalloc(dlen + 1, GFP_KERNEL);
-	if (!description)
-		goto error;
-	description[dlen] = '\0';
-
-	ret = -EFAULT;
-	if (copy_from_user(description, _description, dlen) != 0)
-		goto error2;
-
 	/* pull the payload in if one was supplied */
 	payload = NULL;
  
  
  
  
  
  
  
  
  
@@ -136,59 +141,28 @@
 	struct key *key;
 	key_ref_t dest_ref;
 	char type[32], *description, *callout_info;
-	long dlen, ret;
+	long ret;
  
 	/* pull the type into kernel space */
-	ret = strncpy_from_user(type, _type, sizeof(type) - 1);
+	ret = key_get_type_from_user(type, _type, sizeof(type));
 	if (ret < 0)
 		goto error;
-	type[31] = '\0';
  
-	ret = -EPERM;
-	if (type[0] == '.')
-		goto error;
-
 	/* pull the description into kernel space */
-	ret = -EFAULT;
-	dlen = strnlen_user(_description, PAGE_SIZE - 1);
-	if (dlen <= 0)
+	description = strndup_user(_description, PAGE_SIZE);
+	if (IS_ERR(description)) {
+		ret = PTR_ERR(description);
 		goto error;
+	}
  
-	ret = -EINVAL;
-	if (dlen > PAGE_SIZE - 1)
-		goto error;
-
-	ret = -ENOMEM;
-	description = kmalloc(dlen + 1, GFP_KERNEL);
-	if (!description)
-		goto error;
-	description[dlen] = '\0';
-
-	ret = -EFAULT;
-	if (copy_from_user(description, _description, dlen) != 0)
-		goto error2;
-
 	/* pull the callout info into kernel space */
 	callout_info = NULL;
 	if (_callout_info) {
-		ret = -EFAULT;
-		dlen = strnlen_user(_callout_info, PAGE_SIZE - 1);
-		if (dlen <= 0)
+		callout_info = strndup_user(_callout_info, PAGE_SIZE);
+		if (IS_ERR(callout_info)) {
+			ret = PTR_ERR(callout_info);
 			goto error2;
-
-		ret = -EINVAL;
-		if (dlen > PAGE_SIZE - 1)
-			goto error2;
-
-		ret = -ENOMEM;
-		callout_info = kmalloc(dlen + 1, GFP_KERNEL);
-		if (!callout_info)
-			goto error2;
-		callout_info[dlen] = '\0';
-
-		ret = -EFAULT;
-		if (copy_from_user(callout_info, _callout_info, dlen) != 0)
-			goto error3;
+		}
 	}
  
 	/* get the destination keyring if specified */
  
  
  
@@ -264,36 +238,21 @@
 long keyctl_join_session_keyring(const char __user *_name)
 {
 	char *name;
-	long nlen, ret;
+	long ret;
  
 	/* fetch the name from userspace */
 	name = NULL;
 	if (_name) {
-		ret = -EFAULT;
-		nlen = strnlen_user(_name, PAGE_SIZE - 1);
-		if (nlen <= 0)
+		name = strndup_user(_name, PAGE_SIZE);
+		if (IS_ERR(name)) {
+			ret = PTR_ERR(name);
 			goto error;
-
-		ret = -EINVAL;
-		if (nlen > PAGE_SIZE - 1)
-			goto error;
-
-		ret = -ENOMEM;
-		name = kmalloc(nlen + 1, GFP_KERNEL);
-		if (!name)
-			goto error;
-		name[nlen] = '\0';
-
-		ret = -EFAULT;
-		if (copy_from_user(name, _name, nlen) != 0)
-			goto error2;
+		}
 	}
  
 	/* join the session */
 	ret = join_session_keyring(name);
  
- error2:
-	kfree(name);
  error:
 	return ret;
  
  
  
  
  
@@ -566,32 +525,18 @@
 	struct key_type *ktype;
 	key_ref_t keyring_ref, key_ref, dest_ref;
 	char type[32], *description;
-	long dlen, ret;
+	long ret;
  
 	/* pull the type and description into kernel space */
-	ret = strncpy_from_user(type, _type, sizeof(type) - 1);
+	ret = key_get_type_from_user(type, _type, sizeof(type));
 	if (ret < 0)
 		goto error;
-	type[31] = '\0';
  
-	ret = -EFAULT;
-	dlen = strnlen_user(_description, PAGE_SIZE - 1);
-	if (dlen <= 0)
+	description = strndup_user(_description, PAGE_SIZE);
+	if (IS_ERR(description)) {
+		ret = PTR_ERR(description);
 		goto error;
-
-	ret = -EINVAL;
-	if (dlen > PAGE_SIZE - 1)
-		goto error;
-
-	ret = -ENOMEM;
-	description = kmalloc(dlen + 1, GFP_KERNEL);
-	if (!description)
-		goto error;
-	description[dlen] = '\0';
-
-	ret = -EFAULT;
-	if (copy_from_user(description, _description, dlen) != 0)
-		goto error2;
+	}
  
 	/* get the keyring at which to begin the search */
 	keyring_ref = lookup_user_key(NULL, ringid, 0, 0, KEY_SEARCH);
...	...	@@ -17,10 +17,33 @@
17	17	#include <linux/keyctl.h>
18	18	#include <linux/fs.h>
19	19	#include <linux/capability.h>
	20	+#include <linux/string.h>
20	21	#include <linux/err.h>
21	22	#include <asm/uaccess.h>
22	23	#include "internal.h"
23	24
	25	+static int key_get_type_from_user(char *type,
	26	+ const char __user *_type,
	27	+ unsigned len)
	28	+{
	29	+ int ret;
	30	+
	31	+ ret = strncpy_from_user(type, _type, len);
	32	+
	33	+ if (ret < 0)
	34	+ return -EFAULT;
	35	+
	36	+ if (ret == 0 \|\| ret >= len)
	37	+ return -EINVAL;
	38	+
	39	+ if (type[0] == '.')
	40	+ return -EPERM;
	41	+
	42	+ type[len - 1] = '\0';
	43	+
	44	+ return 0;
	45	+}
	46	+
24	47	/*****************************************************************************/
25	48	/*
26	49	* extract the description of a new key from userspace and either add it as a
27	50
28	51
29	52
30	53
31	54
...	...	@@ -38,41 +61,23 @@
38	61	key_ref_t keyring_ref, key_ref;
39	62	char type[32], *description;
40	63	void *payload;
41		- long dlen, ret;
	64	+ long ret;
42	65
43	66	ret = -EINVAL;
44	67	if (plen > 32767)
45	68	goto error;
46	69
47	70	/* draw all the data into kernel space */
48		- ret = strncpy_from_user(type, _type, sizeof(type) - 1);
	71	+ ret = key_get_type_from_user(type, _type, sizeof(type));
49	72	if (ret < 0)
50	73	goto error;
51		- type[31] = '\0';
52	74
53		- ret = -EPERM;
54		- if (type[0] == '.')
	75	+ description = strndup_user(_description, PAGE_SIZE);
	76	+ if (IS_ERR(description)) {
	77	+ ret = PTR_ERR(description);
55	78	goto error;
	79	+ }
56	80
57		- ret = -EFAULT;
58		- dlen = strnlen_user(_description, PAGE_SIZE - 1);
59		- if (dlen <= 0)
60		- goto error;
61		-
62		- ret = -EINVAL;
63		- if (dlen > PAGE_SIZE - 1)
64		- goto error;
65		-
66		- ret = -ENOMEM;
67		- description = kmalloc(dlen + 1, GFP_KERNEL);
68		- if (!description)
69		- goto error;
70		- description[dlen] = '\0';
71		-
72		- ret = -EFAULT;
73		- if (copy_from_user(description, _description, dlen) != 0)
74		- goto error2;
75		-
76	81	/* pull the payload in if one was supplied */
77	82	payload = NULL;
78	83
79	84
80	85
81	86
82	87
83	88
84	89
85	90
86	91
...	...	@@ -136,59 +141,28 @@
136	141	struct key *key;
137	142	key_ref_t dest_ref;
138	143	char type[32], description, callout_info;
139		- long dlen, ret;
	144	+ long ret;
140	145
141	146	/* pull the type into kernel space */
142		- ret = strncpy_from_user(type, _type, sizeof(type) - 1);
	147	+ ret = key_get_type_from_user(type, _type, sizeof(type));
143	148	if (ret < 0)
144	149	goto error;
145		- type[31] = '\0';
146	150
147		- ret = -EPERM;
148		- if (type[0] == '.')
149		- goto error;
150		-
151	151	/* pull the description into kernel space */
152		- ret = -EFAULT;
153		- dlen = strnlen_user(_description, PAGE_SIZE - 1);
154		- if (dlen <= 0)
	152	+ description = strndup_user(_description, PAGE_SIZE);
	153	+ if (IS_ERR(description)) {
	154	+ ret = PTR_ERR(description);
155	155	goto error;
	156	+ }
156	157
157		- ret = -EINVAL;
158		- if (dlen > PAGE_SIZE - 1)
159		- goto error;
160		-
161		- ret = -ENOMEM;
162		- description = kmalloc(dlen + 1, GFP_KERNEL);
163		- if (!description)
164		- goto error;
165		- description[dlen] = '\0';
166		-
167		- ret = -EFAULT;
168		- if (copy_from_user(description, _description, dlen) != 0)
169		- goto error2;
170		-
171	158	/* pull the callout info into kernel space */
172	159	callout_info = NULL;
173	160	if (_callout_info) {
174		- ret = -EFAULT;
175		- dlen = strnlen_user(_callout_info, PAGE_SIZE - 1);
176		- if (dlen <= 0)
	161	+ callout_info = strndup_user(_callout_info, PAGE_SIZE);
	162	+ if (IS_ERR(callout_info)) {
	163	+ ret = PTR_ERR(callout_info);
177	164	goto error2;
178		-
179		- ret = -EINVAL;
180		- if (dlen > PAGE_SIZE - 1)
181		- goto error2;
182		-
183		- ret = -ENOMEM;
184		- callout_info = kmalloc(dlen + 1, GFP_KERNEL);
185		- if (!callout_info)
186		- goto error2;
187		- callout_info[dlen] = '\0';
188		-
189		- ret = -EFAULT;
190		- if (copy_from_user(callout_info, _callout_info, dlen) != 0)
191		- goto error3;
	165	+ }
192	166	}
193	167
194	168	/* get the destination keyring if specified */
195	169
196	170
197	171
...	...	@@ -264,36 +238,21 @@
264	238	long keyctl_join_session_keyring(const char __user *_name)
265	239	{
266	240	char *name;
267		- long nlen, ret;
	241	+ long ret;
268	242
269	243	/* fetch the name from userspace */
270	244	name = NULL;
271	245	if (_name) {
272		- ret = -EFAULT;
273		- nlen = strnlen_user(_name, PAGE_SIZE - 1);
274		- if (nlen <= 0)
	246	+ name = strndup_user(_name, PAGE_SIZE);
	247	+ if (IS_ERR(name)) {
	248	+ ret = PTR_ERR(name);
275	249	goto error;
276		-
277		- ret = -EINVAL;
278		- if (nlen > PAGE_SIZE - 1)
279		- goto error;
280		-
281		- ret = -ENOMEM;
282		- name = kmalloc(nlen + 1, GFP_KERNEL);
283		- if (!name)
284		- goto error;
285		- name[nlen] = '\0';
286		-
287		- ret = -EFAULT;
288		- if (copy_from_user(name, _name, nlen) != 0)
289		- goto error2;
	250	+ }
290	251	}
291	252
292	253	/* join the session */
293	254	ret = join_session_keyring(name);
294	255
295		- error2:
296		- kfree(name);
297	256	error:
298	257	return ret;
299	258
300	259
301	260
302	261
303	262
...	...	@@ -566,32 +525,18 @@
566	525	struct key_type *ktype;
567	526	key_ref_t keyring_ref, key_ref, dest_ref;
568	527	char type[32], *description;
569		- long dlen, ret;
	528	+ long ret;
570	529
571	530	/* pull the type and description into kernel space */
572		- ret = strncpy_from_user(type, _type, sizeof(type) - 1);
	531	+ ret = key_get_type_from_user(type, _type, sizeof(type));
573	532	if (ret < 0)
574	533	goto error;
575		- type[31] = '\0';
576	534
577		- ret = -EFAULT;
578		- dlen = strnlen_user(_description, PAGE_SIZE - 1);
579		- if (dlen <= 0)
	535	+ description = strndup_user(_description, PAGE_SIZE);
	536	+ if (IS_ERR(description)) {
	537	+ ret = PTR_ERR(description);
580	538	goto error;
581		-
582		- ret = -EINVAL;
583		- if (dlen > PAGE_SIZE - 1)
584		- goto error;
585		-
586		- ret = -ENOMEM;
587		- description = kmalloc(dlen + 1, GFP_KERNEL);
588		- if (!description)
589		- goto error;
590		- description[dlen] = '\0';
591		-
592		- ret = -EFAULT;
593		- if (copy_from_user(description, _description, dlen) != 0)
594		- goto error2;
	539	+ }
595	540
596	541	/* get the keyring at which to begin the search */
597	542	keyring_ref = lookup_user_key(NULL, ringid, 0, 0, KEY_SEARCH);