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 | } |
