2003-06-02 19:56:52

by Hollis Blanchard

[permalink] [raw]

Subject: [CHECKER][PATCH] pnpbios dereferencing user pointer

---

Another simple case of a memcpy that should be copy_from_user...

--
Hollis Blanchard
IBM Linux Technology Center

---

Attachments:

pnpbios-memcpy.diff (475.00 B)

2003-06-02 20:53:45

by Andrew Morton

[permalink] [raw]

Subject: Re: [CHECKER][PATCH] pnpbios dereferencing user pointer

---

Hollis Blanchard <[email protected]> wrote:
>
> Another simple case of a memcpy that should be copy_from_user...

There are also a bunch of memory leaks in there. I modified your patch
thusly:


diff -puN drivers/pnp/pnpbios/proc.c~pnpbios-oops-leak-fix drivers/pnp/pnpbios/proc.c
--- 25/drivers/pnp/pnpbios/proc.c~pnpbios-oops-leak-fix Mon Jun 2 13:59:29 2003
+++ 25-akpm/drivers/pnp/pnpbios/proc.c Mon Jun 2 14:02:26 2003
@@ -178,18 +178,31 @@ static int proc_write_node(struct file *
struct pnp_bios_node *node;
int boot = (long)data >> 8;
u8 nodenum = (long)data;
+ int ret = count;

node = pnpbios_kmalloc(node_info.max_node_size, GFP_KERNEL);
- if (!node) return -ENOMEM;
- if ( pnp_bios_get_dev_node(&nodenum, boot, node) )
- return -EIO;
- if (count != node->size - sizeof(struct pnp_bios_node))
- return -EINVAL;
- memcpy(node->data, buf, count);
- if (pnp_bios_set_dev_node(node->handle, boot, node) != 0)
- return -EINVAL;
+ if (!node)
+ return -ENOMEM;
+ if (pnp_bios_get_dev_node(&nodenum, boot, node)) {
+ ret -EIO;
+ goto out;
+ }
+ if (count != node->size - sizeof(struct pnp_bios_node)) {
+ ret = -EINVAL;
+ goto out;
+ }
+ if (copy_from_user(node->data, buf, count)) {
+ ret = -EFAULT;
+ goto out;
+ }
+ if (pnp_bios_set_dev_node(node->handle, boot, node) != 0) {
+ ret = -EINVAL;
+ goto out;
+ }
+ ret = count;
+out:
kfree(node);
- return count;
+ return ret;
}

int pnpbios_interface_attach_device(struct pnp_bios_node * node)

_