From: "Christoph Egger" <Christoph.Egger@amd.com>
To: xen-devel@lists.xensource.com
Subject: [PATCH] Use string bounded functions
Date: Mon, 29 Jan 2007 11:10:29 +0100 [thread overview]
Message-ID: <200701291110.29420.Christoph.Egger@amd.com> (raw)
[-- 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
next reply other threads:[~2007-01-29 10:10 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-01-29 10:10 Christoph Egger [this message]
2007-01-29 10:52 ` [PATCH] Use string bounded functions Keir Fraser
2007-01-29 11:10 ` Christoph Egger
2007-01-29 13:41 ` Jimi Xenidis
2007-01-29 13:49 ` Keir Fraser
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=200701291110.29420.Christoph.Egger@amd.com \
--to=christoph.egger@amd.com \
--cc=xen-devel@lists.xensource.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.