Re: [PATCH 3/8] dm: prevent possible buffer overflow in ioctl interface

[Kevin Corry <corryk@us.ibm.com>]

On Friday 28 February 2003 08:32, you wrote:
> On Thu, 2003-02-27 at 14:05, Kevin Corry wrote:
> > Unfortunately, Linus seems to have committed that patch already. So here
> > is a patch to fix just that line.
> >
> > Thanks for catching that.
>
> Third time, strlen isn't necessary, it can be done at compile time.
>
> --- a/drivers/md/dm-ioctl.c     2003/02/27 16:29:58
> +++ b/drivers/md/dm-ioctl.c     2003/02/27 17:21:54
> @@ -174,7 +174,7 @@
>  static int register_with_devfs(struct hash_cell *hc)
>  {
>         struct gendisk *disk = dm_disk(hc->md);
> -       char *name = kmalloc(DM_NAME_LEN + strlen(DM_DIR) + 1);
> +       char *name = kmalloc(DM_NAME_LEN + sizeof(DM_DIR));
>         if (!name) {
>                 return -ENOMEM;
>         }

Sorry, I sent the last patch before I got your email.

Also, the "+1" is still necessary, even if we switch to sizeof. The sprintf 
call that follows copies DM_DIR, followed by a slash, followed by the name 
from the hash table into the allocated string. The "+1" is for the slash in 
the middle. The terminating NULL character is accounted for in DM_NAME_LEN.

Linus, here is (yet another!) patch against current BK.

-- 
Kevin Corry
corryk@us.ibm.com
http://evms.sourceforge.net/


--- linux-2.5.63-bk4a/drivers/md/dm-ioctl.c	Fri Feb 28 08:43:19 2003
+++ linux-2.5.63-bk4b/drivers/md/dm-ioctl.c	Fri Feb 28 08:44:08 2003
@@ -174,7 +174,7 @@
 static int register_with_devfs(struct hash_cell *hc)
 {
 	struct gendisk *disk = dm_disk(hc->md);
-	char *name = kmalloc(DM_NAME_LEN + strlen(DM_DIR) + 1, GFP_KERNEL);
+	char *name = kmalloc(DM_NAME_LEN + sizeof(DM_DIR) + 1, GFP_KERNEL);
 	if (!name) {
 		return -ENOMEM;
 	}