[PATCH 1/3] Use string bound functions (take #2)

[PATCH 1/3] Use string bound functions (take #2)

[Christoph Egger]

[-- Attachment #1: Type: text/plain, Size: 83 bytes --]


arch independent code.

Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>


[-- Attachment #2: xen_stringbound.diff --]
[-- Type: text/x-diff, Size: 8913 bytes --]

diff -r f8ddcb758117 xen/common/gdbstub.c
--- a/xen/common/gdbstub.c	Sun Jan 28 19:02:00 2007 +0000
+++ b/xen/common/gdbstub.c	Mon Jan 29 10:17:23 2007 +0100
@@ -268,7 +268,7 @@ gdb_send_packet(struct gdb_context *ctx)
     char buf[3];
     int count;
 
-    sprintf(buf, "%.02x\n", ctx->out_csum);
+    snprintf(buf, sizeof(buf), "%.02x\n", ctx->out_csum);
 
     gdb_write_to_packet_char('#', ctx);
     gdb_write_to_packet(buf, 2, ctx);
diff -r f8ddcb758117 xen/common/kernel.c
--- a/xen/common/kernel.c	Sun Jan 28 19:02:00 2007 +0000
+++ b/xen/common/kernel.c	Mon Jan 29 14:09:20 2007 +0100
@@ -72,8 +72,7 @@ void cmdline_parse(char *cmdline)
             switch ( param->type )
             {
             case OPT_STR:
-                strncpy(param->var, optval, param->len);
-                ((char *)param->var)[param->len-1] = '\0';
+                strlcpy(param->var, optval, param->len);
                 break;
             case OPT_UINT:
                 *(unsigned int *)param->var =
@@ -142,7 +141,7 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDL
     case XENVER_extraversion:
     {
         xen_extraversion_t extraversion;
-        safe_strcpy(extraversion, xen_extra_version());
+        strlcpy(extraversion, xen_extra_version(), sizeof(extraversion));
         if ( copy_to_guest(arg, (char *)extraversion, sizeof(extraversion)) )
             return -EFAULT;
         return 0;
@@ -151,10 +150,10 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDL
     case XENVER_compile_info:
     {
         struct xen_compile_info info;
-        safe_strcpy(info.compiler,       xen_compiler());
-        safe_strcpy(info.compile_by,     xen_compile_by());
-        safe_strcpy(info.compile_domain, xen_compile_domain());
-        safe_strcpy(info.compile_date,   xen_compile_date());
+        strlcpy(info.compiler,       xen_compiler(), sizeof(info.compiler));
+        strlcpy(info.compile_by,     xen_compile_by(), sizeof(info.compile_by));
+        strlcpy(info.compile_domain, xen_compile_domain(), sizeof(info.compile_domain));
+        strlcpy(info.compile_date,   xen_compile_date(), sizeof(info.compile_date));
         if ( copy_to_guest(arg, &info, 1) )
             return -EFAULT;
         return 0;
@@ -187,7 +186,7 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDL
     case XENVER_changeset:
     {
         xen_changeset_info_t chgset;
-        safe_strcpy(chgset, xen_changeset());
+        strlcpy(chgset, xen_changeset(), sizeof(chgset));
         if ( copy_to_guest(arg, (char *)chgset, sizeof(chgset)) )
             return -EFAULT;
         return 0;
diff -r f8ddcb758117 xen/common/kexec.c
--- a/xen/common/kexec.c	Sun Jan 28 19:02:00 2007 +0000
+++ b/xen/common/kexec.c	Mon Jan 29 14:41:33 2007 +0100
@@ -131,7 +131,7 @@ __initcall(register_crashdump_trigger);
 
 static void setup_note(Elf_Note *n, const char *name, int type, int descsz)
 {
-    strcpy(ELFNOTE_NAME(n), name);
+    strlcpy(ELFNOTE_NAME(n), name, sizeof(ELFNOTE_NAME(n)));
     n->namesz = strlen(name);
     n->descsz = descsz;
     n->type = type;
diff -r f8ddcb758117 xen/common/keyhandler.c
--- a/xen/common/keyhandler.c	Sun Jan 28 19:02:00 2007 +0000
+++ b/xen/common/keyhandler.c	Mon Jan 29 11:17:37 2007 +0100
@@ -67,8 +67,7 @@ void register_keyhandler(
     ASSERT(key_table[key].u.handler == NULL);
     key_table[key].u.handler = handler;
     key_table[key].flags     = 0;
-    strncpy(key_table[key].desc, desc, STR_MAX);
-    key_table[key].desc[STR_MAX-1] = '\0';
+    strlcpy(key_table[key].desc, desc, STR_MAX);
 }
 
 void register_irq_keyhandler(
@@ -77,8 +76,7 @@ void register_irq_keyhandler(
     ASSERT(key_table[key].u.irq_handler == NULL);
     key_table[key].u.irq_handler = handler;
     key_table[key].flags         = KEYHANDLER_IRQ_CALLBACK;
-    strncpy(key_table[key].desc, desc, STR_MAX);
-    key_table[key].desc[STR_MAX-1] = '\0';
+    strlcpy(key_table[key].desc, desc, STR_MAX);
 }
 
 static void show_handlers(unsigned char key)
diff -r f8ddcb758117 xen/common/libelf/libelf-dominfo.c
--- a/xen/common/libelf/libelf-dominfo.c	Sun Jan 28 19:02:00 2007 +0000
+++ b/xen/common/libelf/libelf-dominfo.c	Mon Jan 29 10:08:09 2007 +0100
@@ -128,16 +128,16 @@ int elf_xen_parse_note(struct elf_binary
     switch (type)
     {
     case XEN_ELFNOTE_LOADER:
-	strncpy(parms->loader, str, sizeof(parms->loader));
+	strlcpy(parms->loader, str, sizeof(parms->loader));
 	break;
     case XEN_ELFNOTE_GUEST_OS:
-	strncpy(parms->guest_os, str, sizeof(parms->guest_os));
+	strlcpy(parms->guest_os, str, sizeof(parms->guest_os));
 	break;
     case XEN_ELFNOTE_GUEST_VERSION:
-	strncpy(parms->guest_ver, str, sizeof(parms->guest_ver));
+	strlcpy(parms->guest_ver, str, sizeof(parms->guest_ver));
 	break;
     case XEN_ELFNOTE_XEN_VERSION:
-	strncpy(parms->xen_ver, str, sizeof(parms->xen_ver));
+	strlcpy(parms->xen_ver, str, sizeof(parms->xen_ver));
 	break;
     case XEN_ELFNOTE_PAE_MODE:
 	if (0 == strcmp(str, "yes"))
@@ -224,13 +224,13 @@ int elf_xen_parse_guest_info(struct elf_
 
 	/* strings */
 	if (0 == strcmp(name, "LOADER"))
-	    strncpy(parms->loader, value, sizeof(parms->loader));
+	    strlcpy(parms->loader, value, sizeof(parms->loader));
 	if (0 == strcmp(name, "GUEST_OS"))
-	    strncpy(parms->guest_os, value, sizeof(parms->guest_os));
+	    strlcpy(parms->guest_os, value, sizeof(parms->guest_os));
 	if (0 == strcmp(name, "GUEST_VER"))
-	    strncpy(parms->guest_ver, value, sizeof(parms->guest_ver));
+	    strlcpy(parms->guest_ver, value, sizeof(parms->guest_ver));
 	if (0 == strcmp(name, "XEN_VER"))
-	    strncpy(parms->xen_ver, value, sizeof(parms->xen_ver));
+	    strlcpy(parms->xen_ver, value, sizeof(parms->xen_ver));
 	if (0 == strcmp(name, "PAE"))
 	{
 	    if (0 == strcmp(value, "yes[extended-cr3]"))
diff -r f8ddcb758117 xen/common/perfc.c
--- a/xen/common/perfc.c	Sun Jan 28 19:02:00 2007 +0000
+++ b/xen/common/perfc.c	Mon Jan 29 10:09:19 2007 +0100
@@ -148,9 +148,8 @@ static int perfc_copy_info(XEN_GUEST_HAN
     {
         for ( i = 0; i < NR_PERFCTRS; i++ )
         {
-            strncpy(perfc_d[i].name, perfc_info[i].name,
+            strlcpy(perfc_d[i].name, perfc_info[i].name,
                     sizeof(perfc_d[i].name));
-            perfc_d[i].name[sizeof(perfc_d[i].name)-1] = '\0';
 
             switch ( perfc_info[i].type )
             {
diff -r f8ddcb758117 xen/common/rangeset.c
--- a/xen/common/rangeset.c	Sun Jan 28 19:02:00 2007 +0000
+++ b/xen/common/rangeset.c	Mon Jan 29 10:17:42 2007 +0100
@@ -283,12 +283,11 @@ struct rangeset *rangeset_new(
 
     if ( name != NULL )
     {
-        strncpy(r->name, name, sizeof(r->name));
-        r->name[sizeof(r->name)-1] = '\0';
+        strlcpy(r->name, name, sizeof(r->name));
     }
     else
     {
-        sprintf(r->name, "(no name)");
+        snprintf(r->name, sizeof(r->name), "(no name)");
     }
 
     if ( (r->domain = d) != NULL )
diff -r f8ddcb758117 xen/common/symbols.c
--- a/xen/common/symbols.c	Sun Jan 28 19:02:00 2007 +0000
+++ b/xen/common/symbols.c	Mon Jan 29 10:19:37 2007 +0100
@@ -142,15 +142,17 @@ void __print_symbol(const char *fmt, uns
     const char *name;
     unsigned long offset, size;
     char namebuf[KSYM_NAME_LEN+1];
-    char buffer[sizeof("%s+%#lx/%#lx [%s]") + KSYM_NAME_LEN +
-               2*(BITS_PER_LONG*3/10) + 1];
+
+#define BUFFER_SIZE sizeof("%s+%#lx/%#lx [%s]") + KSYM_NAME_LEN + \
+			2*(BITS_PER_LONG*3/10) + 1
+    char buffer[BUFFER_SIZE];
 
     name = symbols_lookup(address, &size, &offset, namebuf);
 
     if (!name)
-        sprintf(buffer, "???");
+        snprintf(buffer, BUFFER_SIZE, "???");
     else
-        sprintf(buffer, "%s+%#lx/%#lx", name, offset, size);
+        snprintf(buffer, BUFFER_SIZE, "%s+%#lx/%#lx", name, offset, size);
 
     printk(fmt, buffer);
 }
diff -r f8ddcb758117 xen/drivers/char/console.c
--- a/xen/drivers/char/console.c	Sun Jan 28 19:02:00 2007 +0000
+++ b/xen/drivers/char/console.c	Mon Jan 29 15:07:54 2007 +0100
@@ -481,7 +481,7 @@ void printk(const char *fmt, ...)
 
 void set_printk_prefix(const char *prefix)
 {
-    strcpy(printk_prefix, prefix);
+    strlcpy(printk_prefix, prefix, sizeof(printk_prefix));
 }
 
 void init_console(void)
@@ -771,7 +771,7 @@ void debugtrace_printk(const char *fmt, 
 
     ASSERT(debugtrace_buf[debugtrace_bytes - 1] == 0);
 
-    sprintf(buf, "%u ", ++count);
+    snprintf(buf, sizeof(buf), "%u ", ++count);
 
     va_start(args, fmt);
     (void)vsnprintf(buf + strlen(buf), sizeof(buf), fmt, args);
diff -r f8ddcb758117 xen/include/xen/string.h
--- a/xen/include/xen/string.h	Sun Jan 28 19:02:00 2007 +0000
+++ b/xen/include/xen/string.h	Mon Jan 29 14:10:23 2007 +0100
@@ -82,9 +82,5 @@ extern void * memchr(const void *,int,__
 }
 #endif
 
-#define safe_strcpy(d,s)                        \
-do { strncpy((d),(s),sizeof((d)));              \
-     (d)[sizeof((d))-1] = '\0';                 \
-} while (0)
 
 #endif /* _LINUX_STRING_H_ */

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 1/3] Use string bound functions (take #2)

[Anil Madhavapeddy]

I notice these aren't checking for buffer truncation.  Might be worth  
putting checks for this in places where it matters and not silently  
truncate the string (e.g. the domain builder).

-anil

On 29 Jan 2007, at 14:25, Christoph Egger wrote:

> <xen_stringbound.diff>

Re: [PATCH 1/3] Use string bound functions (take #2)

[Christoph Egger]

On Monday 29 January 2007 15:37, Anil Madhavapeddy wrote:
> I notice these aren't checking for buffer truncation.  Might be worth
> putting checks for this in places where it matters and not silently
> truncate the string (e.g. the domain builder).

Yeah. I think, this should be done in another patch. Most important is,
that strlcpy() is used rather strncpy().

Christoph

>
> -anil
>
> On 29 Jan 2007, at 14:25, Christoph Egger wrote:
> > <xen_stringbound.diff>