All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Use string bounded functions
@ 2007-01-29 10:10 Christoph Egger
  2007-01-29 10:52 ` Keir Fraser
  0 siblings, 1 reply; 5+ messages in thread
From: Christoph Egger @ 2007-01-29 10:10 UTC (permalink / raw)
  To: xen-devel

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


Hi!

The attached patch replaces sprintf with snprintf and strncpy with strlcpy.

There are various cases where no NULL-terminated strings are guaranteed
and eventual possible overflows. This patch fixes them.

BTW: Since Xen kernel has its own string functions, can't we just remove
sprintf() and strncpy()? IMO, Xen should not inherit the historical C relicts.

Christoph


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

diff -r f8ddcb758117 xen/arch/x86/cpu/centaur.c
--- a/xen/arch/x86/cpu/centaur.c	Sun Jan 28 19:02:00 2007 +0000
+++ b/xen/arch/x86/cpu/centaur.c	Mon Jan 29 10:21:40 2007 +0100
@@ -437,7 +437,8 @@ static void __init init_centaur(struct c
 				/* Add L1 data and code cache sizes. */
 				c->x86_cache_size = (cc>>24)+(dd>>24);
 			}
-			sprintf( c->x86_model_id, "WinChip %s", name );
+			snprintf( c->x86_model_id, sizeof(c->x86_model_id),
+				"WinChip %s", name );
 			break;
 
 		case 6:
diff -r f8ddcb758117 xen/arch/x86/cpu/common.c
--- a/xen/arch/x86/cpu/common.c	Sun Jan 28 19:02:00 2007 +0000
+++ b/xen/arch/x86/cpu/common.c	Mon Jan 29 10:22:14 2007 +0100
@@ -386,8 +386,8 @@ void __devinit identify_cpu(struct cpuin
 			strcpy(c->x86_model_id, p);
 		else
 			/* Last resort... */
-			sprintf(c->x86_model_id, "%02x/%02x",
-				c->x86_vendor, c->x86_model);
+			snprintf(c->x86_model_id, sizeof(c->x86_model_id),
+				"%02x/%02x", c->x86_vendor, c->x86_model);
 	}
 
 	/* Now the feature flags better reflect actual CPU features! */
diff -r f8ddcb758117 xen/arch/x86/domain_build.c
--- a/xen/arch/x86/domain_build.c	Sun Jan 28 19:02:00 2007 +0000
+++ b/xen/arch/x86/domain_build.c	Mon Jan 29 10:36:30 2007 +0100
@@ -821,7 +821,7 @@ int construct_dom0(struct domain *d,
     si->pt_base      = vpt_start + 2 * PAGE_SIZE * !!IS_COMPAT(d);
     si->nr_pt_frames = nr_pt_pages;
     si->mfn_list     = vphysmap_start;
-    sprintf(si->magic, "xen-%i.%i-x86_%d%s",
+    snprintf(si->magic, sizeof(si->magic), "xen-%i.%i-x86_%d%s",
             xen_major_version(), xen_minor_version(),
             elf_64bit(&elf) ? 64 : 32,
             parms.pae ? "p" : "");
@@ -871,7 +871,7 @@ int construct_dom0(struct domain *d,
 
     memset(si->cmd_line, 0, sizeof(si->cmd_line));
     if ( cmdline != NULL )
-        strncpy((char *)si->cmd_line, cmdline, sizeof(si->cmd_line)-1);
+        strlcpy((char *)si->cmd_line, cmdline, sizeof(si->cmd_line));
 
     if ( fill_console_start_info((void *)(si + 1)) )
     {
diff -r f8ddcb758117 xen/arch/x86/hvm/intercept.c
--- a/xen/arch/x86/hvm/intercept.c	Sun Jan 28 19:02:00 2007 +0000
+++ b/xen/arch/x86/hvm/intercept.c	Mon Jan 29 10:13:16 2007 +0100
@@ -173,7 +173,7 @@ int hvm_register_savevm(struct domain *d
         return -1;
     }
 
-    strncpy(se->idstr, idstr, HVM_SE_IDSTR_LEN);
+    strlcpy(se->idstr, idstr, HVM_SE_IDSTR_LEN);
 
     se->instance_id = instance_id;
     se->version_id = version_id;
diff -r f8ddcb758117 xen/arch/x86/oprofile/nmi_int.c
--- a/xen/arch/x86/oprofile/nmi_int.c	Sun Jan 28 19:02:00 2007 +0000
+++ b/xen/arch/x86/oprofile/nmi_int.c	Mon Jan 29 10:50:02 2007 +0100
@@ -22,6 +22,7 @@
 #include <asm/regs.h>
 #include <asm/current.h>
 #include <xen/delay.h>
+#include <xen/string.h>
  
 #include "op_counter.h"
 #include "op_x86_model.h"
@@ -39,7 +40,6 @@ extern int active_id(struct domain *d);
 extern int active_id(struct domain *d);
 extern int is_profiled(struct domain *d);
 
-extern size_t strlcpy(char *dest, const char *src, size_t size);
 
 
 static int nmi_callback(struct cpu_user_regs *regs, int cpu)
@@ -276,20 +276,20 @@ static int __init p4_init(char * cpu_typ
 	}
 
 #ifndef CONFIG_SMP
-	strncpy (cpu_type, "i386/p4", XENOPROF_CPU_TYPE_SIZE - 1);
+	strlcpy (cpu_type, "i386/p4", XENOPROF_CPU_TYPE_SIZE);
 	model = &op_p4_spec;
 	return 1;
 #else
 	switch (smp_num_siblings) {
 		case 1:
-			strncpy (cpu_type, "i386/p4", 
-				 XENOPROF_CPU_TYPE_SIZE - 1);
+			strlcpy (cpu_type, "i386/p4", 
+				 XENOPROF_CPU_TYPE_SIZE);
 			model = &op_p4_spec;
 			return 1;
 
 		case 2:
-			strncpy (cpu_type, "i386/p4-ht", 
-				 XENOPROF_CPU_TYPE_SIZE - 1);
+			strlcpy (cpu_type, "i386/p4-ht", 
+				 XENOPROF_CPU_TYPE_SIZE);
 			model = &op_p4_ht2_spec;
 			return 1;
 	}
@@ -311,17 +311,17 @@ static int __init ppro_init(char *cpu_ty
 		return 0;
 	}
 	else if (cpu_model == 15)
-		strncpy (cpu_type, "i386/core_2", XENOPROF_CPU_TYPE_SIZE - 1);
+		strlcpy (cpu_type, "i386/core_2", XENOPROF_CPU_TYPE_SIZE);
 	else if (cpu_model == 14)
-		strncpy (cpu_type, "i386/core", XENOPROF_CPU_TYPE_SIZE - 1);
+		strlcpy (cpu_type, "i386/core", XENOPROF_CPU_TYPE_SIZE);
 	else if (cpu_model == 9)
-		strncpy (cpu_type, "i386/p6_mobile", XENOPROF_CPU_TYPE_SIZE - 1);
+		strlcpy (cpu_type, "i386/p6_mobile", XENOPROF_CPU_TYPE_SIZE);
 	else if (cpu_model > 5)
-		strncpy (cpu_type, "i386/piii", XENOPROF_CPU_TYPE_SIZE - 1);
+		strlcpy (cpu_type, "i386/piii", XENOPROF_CPU_TYPE_SIZE);
 	else if (cpu_model > 2)
-		strncpy (cpu_type, "i386/pii", XENOPROF_CPU_TYPE_SIZE - 1);
+		strlcpy (cpu_type, "i386/pii", XENOPROF_CPU_TYPE_SIZE);
 	else
-		strncpy (cpu_type, "i386/ppro", XENOPROF_CPU_TYPE_SIZE - 1);
+		strlcpy (cpu_type, "i386/ppro", XENOPROF_CPU_TYPE_SIZE);
 
 	model = &op_ppro_spec;
 	return 1;
@@ -346,9 +346,6 @@ int nmi_init(int *num_events, int *is_pr
 		}
 	}
  
-	/* Make sure string is NULL terminated */
-	cpu_type[XENOPROF_CPU_TYPE_SIZE - 1] = 0;
-
 	switch (vendor) {
 		case X86_VENDOR_AMD:
 			/* Needs to be at least an Athlon (or hammer in 32bit mode) */
@@ -361,15 +358,15 @@ int nmi_init(int *num_events, int *is_pr
 				return -ENODEV;
 			case 6:
 				model = &op_athlon_spec;
-				strncpy (cpu_type, "i386/athlon", 
-					 XENOPROF_CPU_TYPE_SIZE - 1);
+				strlcpy (cpu_type, "i386/athlon", 
+					 XENOPROF_CPU_TYPE_SIZE);
 				break;
 			case 0xf:
 				model = &op_athlon_spec;
 				/* Actually it could be i386/hammer too, but give
 				   user space an consistent name. */
-				strncpy (cpu_type, "x86-64/hammer", 
-					 XENOPROF_CPU_TYPE_SIZE - 1);
+				strlcpy (cpu_type, "x86-64/hammer", 
+					 XENOPROF_CPU_TYPE_SIZE);
 				break;
 			}
 			break;
diff -r f8ddcb758117 xen/arch/x86/setup.c
--- a/xen/arch/x86/setup.c	Sun Jan 28 19:02:00 2007 +0000
+++ b/xen/arch/x86/setup.c	Mon Jan 29 10:52:04 2007 +0100
@@ -111,8 +111,7 @@ static void parse_acpi_param(char *s)
 static void parse_acpi_param(char *s)
 {
     /* Save the parameter so it can be propagated to domain0. */
-    strncpy(acpi_param, s, sizeof(acpi_param));
-    acpi_param[sizeof(acpi_param)-1] = '\0';
+    strlcpy(acpi_param, s, sizeof(acpi_param));
 
     /* Interpret the parameter for use within Xen. */
     if ( !strcmp(s, "off") )
@@ -804,35 +803,57 @@ void arch_get_xen_caps(xen_capabilities_
 void arch_get_xen_caps(xen_capabilities_info_t info)
 {
     char *p = info;
+    int i = 0;
     int major = xen_major_version();
     int minor = xen_minor_version();
 
 #if defined(CONFIG_X86_32) && !defined(CONFIG_X86_PAE)
 
-    p += sprintf(p, "xen-%d.%d-x86_32 ", major, minor);
+    i = snprintf(p, sizeof(xen_capabilities_info_t),
+                 "xen-%d.%d-x86_32 ", major, minor);
+    p += i;
+    if ( hvm_enabled ) {
+        i = snprintf(p, sizeof(xen_capabilities_info_t) - i,
+		"hvm-%d.%d-x86_32 ", major, minor);
+	p += i;
+    }
+
+#elif defined(CONFIG_X86_32) && defined(CONFIG_X86_PAE)
+
+    i = snprintf(p, sizeof(xen_capabilities_info_t),
+                 "xen-%d.%d-x86_32p ", major, minor);
+    p += i;
     if ( hvm_enabled )
-        p += sprintf(p, "hvm-%d.%d-x86_32 ", major, minor);
-
-#elif defined(CONFIG_X86_32) && defined(CONFIG_X86_PAE)
-
-    p += sprintf(p, "xen-%d.%d-x86_32p ", major, minor);
+    {
+        i = snprintf(p, sizeof(xen_capabilities_info_t),
+                     "hvm-%d.%d-x86_32 ", major, minor);
+        p += i;
+        i = snprintf(p, sizeof(xen_capabilities_info_t) - i,
+                     "hvm-%d.%d-x86_32p ", major, minor);
+        p += i;
+    }
+
+#elif defined(CONFIG_X86_64)
+
+    i = snprintf(p, sizeof(xen_capabilities_info_t),
+                 "xen-%d.%d-x86_64 ", major, minor);
+    p += i;
+#ifdef CONFIG_COMPAT
+    i = snprintf(p, sizeof(xen_capabilities_info_t) - i,
+                "xen-%d.%d-x86_32p ", major, minor);
+    p += i;
+#endif
     if ( hvm_enabled )
     {
-        p += sprintf(p, "hvm-%d.%d-x86_32 ", major, minor);
-        p += sprintf(p, "hvm-%d.%d-x86_32p ", major, minor);
-    }
-
-#elif defined(CONFIG_X86_64)
-
-    p += sprintf(p, "xen-%d.%d-x86_64 ", major, minor);
-#ifdef CONFIG_COMPAT
-    p += sprintf(p, "xen-%d.%d-x86_32p ", major, minor);
-#endif
-    if ( hvm_enabled )
-    {
-        p += sprintf(p, "hvm-%d.%d-x86_32 ", major, minor);
-        p += sprintf(p, "hvm-%d.%d-x86_32p ", major, minor);
-        p += sprintf(p, "hvm-%d.%d-x86_64 ", major, minor);
+        i = snprintf(p, sizeof(xen_capabilities_info_t) - i,
+                     "hvm-%d.%d-x86_32 ", major, minor);
+        p += i;
+        i = snprintf(p, sizeof(xen_capabilities_info_t) - i,
+                     "hvm-%d.%d-x86_32p ", major, minor);
+        p += i;
+        i = snprintf(p, sizeof(xen_capabilities_info_t) - i,
+                     "hvm-%d.%d-x86_64 ", major, minor);
+        p += i;
     }
 
 #else
diff -r f8ddcb758117 xen/arch/x86/time.c
--- a/xen/arch/x86/time.c	Sun Jan 28 19:02:00 2007 +0000
+++ b/xen/arch/x86/time.c	Mon Jan 29 10:36:50 2007 +0100
@@ -274,7 +274,7 @@ static char *freq_string(u64 freq)
     unsigned int x, y;
     y = (unsigned int)do_div(freq, 1000000) / 1000;
     x = (unsigned int)freq;
-    sprintf(s, "%u.%03uMHz", x, y);
+    snprintf(s, sizeof(s), "%u.%03uMHz", x, y);
     return s;
 }
 
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 10:08:43 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 =
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 10:09:01 2007 +0100
@@ -67,7 +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);
+    strlcpy(key_table[key].desc, desc, STR_MAX);
     key_table[key].desc[STR_MAX-1] = '\0';
 }
 
@@ -77,8 +77,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);
 }

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

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

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2007-01-29 13:49 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-01-29 10:10 [PATCH] Use string bounded functions Christoph Egger
2007-01-29 10:52 ` Keir Fraser
2007-01-29 11:10   ` Christoph Egger
2007-01-29 13:41     ` Jimi Xenidis
2007-01-29 13:49       ` Keir Fraser

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.