Use string bound functions (take #2)

PATCH In review Christoph Egger 2 participants 3 messages Jan 29, 2007

lore

Share & tools

lore

Christoph Egger

author

2007-01-29 14:25 UTC #0 lore raw

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

arch independent code.

Signed-off-by Christoph Egger <[email protected]>

xen/common/gdbstub.c +1 -1

diff -r f8ddcb758117 xen/common/gdbstub.c
@@ -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);

xen/common/kernel.c +7 -8

diff -r f8ddcb758117 xen/common/kernel.c
@@ -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;

xen/common/kexec.c +1 -1

diff -r f8ddcb758117 xen/common/kexec.c
@@ -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;

xen/common/keyhandler.c +2 -4

diff -r f8ddcb758117 xen/common/keyhandler.c
@@ -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)

xen/common/libelf/libelf-dominfo.c +8 -8

diff -r f8ddcb758117 xen/common/libelf/libelf-dominfo.c
@@ -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]"))

xen/common/perfc.c +1 -2

diff -r f8ddcb758117 xen/common/perfc.c
@@ -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 )
             {

xen/common/rangeset.c +2 -3

diff -r f8ddcb758117 xen/common/rangeset.c
@@ -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 )

xen/common/symbols.c +6 -4

diff -r f8ddcb758117 xen/common/symbols.c
@@ -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);
 }

xen/drivers/char/console.c +2 -2

diff -r f8ddcb758117 xen/drivers/char/console.c
@@ -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);

xen/include/xen/string.h +0 -4

diff -r f8ddcb758117 xen/include/xen/string.h
@@ -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_ */

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Anil Madhavapeddy

2007-01-29 14:37 UTC #1 lore raw

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

Quoted 1 line · Christoph Egger

<xen_stringbound.diff>

Christoph Egger

author

2007-01-29 14:51 UTC #2 lore raw

Quoted 3 lines · 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).

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

Christoph

Quoted 5 lines

-anil
Quoted 1 line · Christoph Egger
<xen_stringbound.diff>