diff options
author | Serge E. Hallyn <serue@us.ibm.com> | 2009-05-20 11:15:28 -0400 |
---|---|---|
committer | Greg Kroah-Hartman <gregkh@suse.de> | 2009-06-19 14:00:54 -0400 |
commit | f82ebea5c8ef9ed9378fc6402051a4860a805397 (patch) | |
tree | 505076547f6129a053f1972716ddf10a0fb93ce1 | |
parent | 0f51010e87636ed93338f4d9a987a466ca0d6969 (diff) |
staging: p9auth: prevent some oopses and memory leaks
Before all testcases, do:
mknod /dev/caphash c 253 0
mknod /dev/capuse c 253 1
This patch does the following:
1. caphash write of > CAP_NODE_SIZE bytes overruns node_ptr->data
(test: cat /etc/mime.types > /dev/caphash)
2. make sure we don't dereference a NULL cap_devices[0].head
(test: cat serge@root@abab > /dev/capuse)
3. don't let strlen dereference a NULL target_user etc
(test: echo ab > /dev/capuse)
4. Don't leak a bunch of memory in cap_write(). Note that
technically node_ptr is not needed for the capuse write case.
As a result I have a much more extensive patch splitting up
cap_write(), but I thought a smaller patch that is easier to test
and verify would be a better start. To test:
cnt=0
while [ 1 ]; do
echo /etc/mime.types > /dev/capuse
if [ $((cnt%25)) -eq 0 ]; then
head -2 /proc/meminfo
fi
cnt=$((cnt+1))
sleep 0.3
done
Without this patch, it MemFree steadily drops. With the patch,
it does not.
I have *not* tested this driver (with or without these patches)
with factotum or anything - only using the tests described above.
Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
-rw-r--r-- | drivers/staging/p9auth/p9auth.c | 24 |
1 files changed, 23 insertions, 1 deletions
diff --git a/drivers/staging/p9auth/p9auth.c b/drivers/staging/p9auth/p9auth.c index 3cac89b26faf..9111dcba37a1 100644 --- a/drivers/staging/p9auth/p9auth.c +++ b/drivers/staging/p9auth/p9auth.c | |||
@@ -180,8 +180,12 @@ static ssize_t cap_write(struct file *filp, const char __user *buf, | |||
180 | if (down_interruptible(&dev->sem)) | 180 | if (down_interruptible(&dev->sem)) |
181 | return -ERESTARTSYS; | 181 | return -ERESTARTSYS; |
182 | 182 | ||
183 | user_buf_running = NULL; | ||
184 | hash_str = NULL; | ||
183 | node_ptr = kmalloc(sizeof(struct cap_node), GFP_KERNEL); | 185 | node_ptr = kmalloc(sizeof(struct cap_node), GFP_KERNEL); |
184 | user_buf = kzalloc(count, GFP_KERNEL); | 186 | user_buf = kzalloc(count, GFP_KERNEL); |
187 | if (!node_ptr || !user_buf) | ||
188 | goto out; | ||
185 | 189 | ||
186 | if (copy_from_user(user_buf, buf, count)) { | 190 | if (copy_from_user(user_buf, buf, count)) { |
187 | retval = -EFAULT; | 191 | retval = -EFAULT; |
@@ -193,11 +197,21 @@ static ssize_t cap_write(struct file *filp, const char __user *buf, | |||
193 | * hashed capability supplied by the user to the list of hashes | 197 | * hashed capability supplied by the user to the list of hashes |
194 | */ | 198 | */ |
195 | if (0 == iminor(filp->f_dentry->d_inode)) { | 199 | if (0 == iminor(filp->f_dentry->d_inode)) { |
200 | if (count > CAP_NODE_SIZE) { | ||
201 | retval = -EINVAL; | ||
202 | goto out; | ||
203 | } | ||
196 | printk(KERN_INFO "Capability being written to /dev/caphash : \n"); | 204 | printk(KERN_INFO "Capability being written to /dev/caphash : \n"); |
197 | hexdump(user_buf, count); | 205 | hexdump(user_buf, count); |
198 | memcpy(node_ptr->data, user_buf, count); | 206 | memcpy(node_ptr->data, user_buf, count); |
199 | list_add(&(node_ptr->list), &(dev->head->list)); | 207 | list_add(&(node_ptr->list), &(dev->head->list)); |
208 | node_ptr = NULL; | ||
200 | } else { | 209 | } else { |
210 | if (!cap_devices[0].head || | ||
211 | list_empty(&(cap_devices[0].head->list))) { | ||
212 | retval = -EINVAL; | ||
213 | goto out; | ||
214 | } | ||
201 | /* | 215 | /* |
202 | * break the supplied string into tokens with @ as the | 216 | * break the supplied string into tokens with @ as the |
203 | * delimiter If the string is "user1@user2@randomstring" we | 217 | * delimiter If the string is "user1@user2@randomstring" we |
@@ -208,6 +222,10 @@ static ssize_t cap_write(struct file *filp, const char __user *buf, | |||
208 | source_user = strsep(&user_buf_running, "@"); | 222 | source_user = strsep(&user_buf_running, "@"); |
209 | target_user = strsep(&user_buf_running, "@"); | 223 | target_user = strsep(&user_buf_running, "@"); |
210 | rand_str = strsep(&user_buf_running, "@"); | 224 | rand_str = strsep(&user_buf_running, "@"); |
225 | if (!source_user || !target_user || !rand_str) { | ||
226 | retval = -EINVAL; | ||
227 | goto out; | ||
228 | } | ||
211 | 229 | ||
212 | /* hash the string user1@user2 with rand_str as the key */ | 230 | /* hash the string user1@user2 with rand_str as the key */ |
213 | len = strlen(source_user) + strlen(target_user) + 1; | 231 | len = strlen(source_user) + strlen(target_user) + 1; |
@@ -224,7 +242,7 @@ static ssize_t cap_write(struct file *filp, const char __user *buf, | |||
224 | retval = -EFAULT; | 242 | retval = -EFAULT; |
225 | goto out; | 243 | goto out; |
226 | } | 244 | } |
227 | memcpy(node_ptr->data, result, CAP_NODE_SIZE); | 245 | memcpy(node_ptr->data, result, CAP_NODE_SIZE); /* why? */ |
228 | /* Change the process's uid if the hash is present in the | 246 | /* Change the process's uid if the hash is present in the |
229 | * list of hashes | 247 | * list of hashes |
230 | */ | 248 | */ |
@@ -299,6 +317,10 @@ static ssize_t cap_write(struct file *filp, const char __user *buf, | |||
299 | dev->size = *f_pos; | 317 | dev->size = *f_pos; |
300 | 318 | ||
301 | out: | 319 | out: |
320 | kfree(node_ptr); | ||
321 | kfree(user_buf); | ||
322 | kfree(user_buf_running); | ||
323 | kfree(hash_str); | ||
302 | up(&dev->sem); | 324 | up(&dev->sem); |
303 | return retval; | 325 | return retval; |
304 | } | 326 | } |