* [PATCH 0/6] libxl: config file string handling cleanups
@ 2015-07-07 16:13 Ian Jackson
2015-07-07 16:13 ` [PATCH 1/6] xl: Do not ignore unparseable PCI BDFs Ian Jackson
` (6 more replies)
0 siblings, 7 replies; 12+ messages in thread
From: Ian Jackson @ 2015-07-07 16:13 UTC (permalink / raw)
To: xen-devel
These 6 followup patches were developed during a review of the xl
string handling code, prompted by XSA-137. They were embargoed until
today.
I have reviewed the whole of xl's string handling for other bugs.
My search terms included:
realloc sn?printf str \bstr \bstrcpy \bstrn \bstrcat \bmemcpy
\bmemchr \bp\b
Surprisingly, I found not too much untoward.
Of these only two are even backport candidates:
xl: Do not ignore unparseable PCI BDFs
xl: Rewrite trim()
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/6] xl: Do not ignore unparseable PCI BDFs
2015-07-07 16:13 [PATCH 0/6] libxl: config file string handling cleanups Ian Jackson
@ 2015-07-07 16:13 ` Ian Jackson
2015-07-07 16:18 ` Andrew Cooper
2015-07-07 16:13 ` [PATCH 2/6] xl: Use ARRAY_EXTEND_INIT for vtpms and nics Ian Jackson
` (5 subsequent siblings)
6 siblings, 1 reply; 12+ messages in thread
From: Ian Jackson @ 2015-07-07 16:13 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
If xlu_pci_parse_bdf fails, abandon the domain creation, rather than
blundering on.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
tools/libxl/xl_cmdimpl.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 08484e4..31d8260 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1942,8 +1942,12 @@ skip_vfb:
pcidev->power_mgmt = pci_power_mgmt;
pcidev->permissive = pci_permissive;
pcidev->seize = pci_seize;
- if (!xlu_pci_parse_bdf(config, pcidev, buf))
- d_config->num_pcidevs++;
+ e = xlu_pci_parse_bdf(config, pcidev, buf);
+ if (e) {
+ fprintf(stderr, "unable to parse PCI BDF for passthrough\n");
+ exit(-e);
+ }
+ d_config->num_pcidevs++;
}
if (d_config->num_pcidevs && c_info->type == LIBXL_DOMAIN_TYPE_PV)
libxl_defbool_set(&b_info->u.pv.e820_host, true);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/6] xl: Use ARRAY_EXTEND_INIT for vtpms and nics
2015-07-07 16:13 [PATCH 0/6] libxl: config file string handling cleanups Ian Jackson
2015-07-07 16:13 ` [PATCH 1/6] xl: Do not ignore unparseable PCI BDFs Ian Jackson
@ 2015-07-07 16:13 ` Ian Jackson
2015-07-07 16:13 ` [PATCH 3/6] xl: Provide and use ARRAY_EXTEND_INIT_NODEVID for disks, pcidevs and dtdevs Ian Jackson
` (4 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Ian Jackson @ 2015-07-07 16:13 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
This removes two open-coded reallocs.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
tools/libxl/xl_cmdimpl.c | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 31d8260..6ea4e6b 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1701,11 +1701,9 @@ static void parse_config_data(const char *config_source,
char *p, *p2;
bool got_backend = false;
- d_config->vtpms = (libxl_device_vtpm *) realloc(d_config->vtpms,
- sizeof(libxl_device_vtpm) * (d_config->num_vtpms+1));
- vtpm = d_config->vtpms + d_config->num_vtpms;
- libxl_device_vtpm_init(vtpm);
- vtpm->devid = d_config->num_vtpms;
+ vtpm = ARRAY_EXTEND_INIT(d_config->vtpms,
+ d_config->num_vtpms,
+ libxl_device_vtpm_init);
p = strtok(buf2, ",");
if(p) {
@@ -1735,7 +1733,6 @@ static void parse_config_data(const char *config_source,
exit(1);
}
free(buf2);
- d_config->num_vtpms++;
}
}
@@ -1825,10 +1822,9 @@ static void parse_config_data(const char *config_source,
char *buf2 = strdup(buf);
char *p;
- d_config->nics = (libxl_device_nic *) realloc(d_config->nics, sizeof (libxl_device_nic) * (d_config->num_nics+1));
- nic = d_config->nics + d_config->num_nics;
- libxl_device_nic_init(nic);
- nic->devid = d_config->num_nics;
+ nic = ARRAY_EXTEND_INIT(d_config->nics,
+ d_config->num_nics,
+ libxl_device_nic_init);
set_default_nic_values(nic);
p = strtok(buf2, ",");
@@ -1841,7 +1837,6 @@ static void parse_config_data(const char *config_source,
} while ((p = strtok(NULL, ",")) != NULL);
skip_nic:
free(buf2);
- d_config->num_nics++;
}
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/6] xl: Provide and use ARRAY_EXTEND_INIT_NODEVID for disks, pcidevs and dtdevs
2015-07-07 16:13 [PATCH 0/6] libxl: config file string handling cleanups Ian Jackson
2015-07-07 16:13 ` [PATCH 1/6] xl: Do not ignore unparseable PCI BDFs Ian Jackson
2015-07-07 16:13 ` [PATCH 2/6] xl: Use ARRAY_EXTEND_INIT for vtpms and nics Ian Jackson
@ 2015-07-07 16:13 ` Ian Jackson
2015-07-07 16:13 ` [PATCH 4/6] xl: Provide and use xvasprintf and xasprintf internally Ian Jackson
` (3 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Ian Jackson @ 2015-07-07 16:13 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
This replaces 3 sets of open-coded reallocs etc.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
tools/libxl/xl_cmdimpl.c | 34 +++++++++++++++++++---------------
1 file changed, 19 insertions(+), 15 deletions(-)
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 6ea4e6b..568d7d2 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -323,16 +323,24 @@ static char *xstrdup(const char *x)
return r;
}
-#define ARRAY_EXTEND_INIT(array,count,initfn) \
+#define ARRAY_EXTEND_INIT__CORE(array,count,initfn,more) \
({ \
typeof((count)) array_extend_old_count = (count); \
(count)++; \
(array) = xrealloc((array), sizeof(*array) * (count)); \
(initfn)(&(array)[array_extend_old_count]); \
- (array)[array_extend_old_count].devid = array_extend_old_count; \
+ more; \
&(array)[array_extend_old_count]; \
})
+#define ARRAY_EXTEND_INIT(array,count,initfn) \
+ ARRAY_EXTEND_INIT__CORE((array),(count),(initfn), ({ \
+ (array)[array_extend_old_count].devid = array_extend_old_count; \
+ }))
+
+#define ARRAY_EXTEND_INIT_NODEVID(array,count,initfn) \
+ ARRAY_EXTEND_INIT__CORE((array),(count),(initfn), /* nothing */ )
+
#define LOG(_f, _a...) dolog(__FILE__, __LINE__, __func__, _f "\n", ##_a)
static void dolog(const char *file, int line, const char *func, char *fmt, ...)
@@ -1683,12 +1691,12 @@ static void parse_config_data(const char *config_source,
libxl_device_disk *disk;
char *buf2 = strdup(buf);
- d_config->disks = (libxl_device_disk *) realloc(d_config->disks, sizeof (libxl_device_disk) * (d_config->num_disks + 1));
- disk = d_config->disks + d_config->num_disks;
+ disk = ARRAY_EXTEND_INIT_NODEVID(d_config->disks,
+ d_config->num_disks,
+ libxl_device_disk_init);
parse_disk_config(&config, buf2, disk);
free(buf2);
- d_config->num_disks++;
}
}
@@ -1929,10 +1937,9 @@ skip_vfb:
for(i = 0; (buf = xlu_cfg_get_listitem (pcis, i)) != NULL; i++) {
libxl_device_pci *pcidev;
- d_config->pcidevs = (libxl_device_pci *) realloc(d_config->pcidevs, sizeof (libxl_device_pci) * (d_config->num_pcidevs + 1));
- pcidev = d_config->pcidevs + d_config->num_pcidevs;
- libxl_device_pci_init(pcidev);
-
+ pcidev = ARRAY_EXTEND_INIT_NODEVID(d_config->pcidevs,
+ d_config->num_pcidevs,
+ libxl_device_pci_init);
pcidev->msitranslate = pci_msitranslate;
pcidev->power_mgmt = pci_power_mgmt;
pcidev->permissive = pci_permissive;
@@ -1942,7 +1949,6 @@ skip_vfb:
fprintf(stderr, "unable to parse PCI BDF for passthrough\n");
exit(-e);
}
- d_config->num_pcidevs++;
}
if (d_config->num_pcidevs && c_info->type == LIBXL_DOMAIN_TYPE_PV)
libxl_defbool_set(&b_info->u.pv.e820_host, true);
@@ -1954,17 +1960,15 @@ skip_vfb:
for (i = 0; (buf = xlu_cfg_get_listitem(dtdevs, i)) != NULL; i++) {
libxl_device_dtdev *dtdev;
- d_config->dtdevs = xrealloc(d_config->dtdevs,
- sizeof (libxl_device_dtdev) * (i + 1));
- dtdev = d_config->dtdevs + d_config->num_dtdevs;
- libxl_device_dtdev_init(dtdev);
+ dtdev = ARRAY_EXTEND_INIT_NODEVID(d_config->dtdevs,
+ d_config->num_dtdevs,
+ libxl_device_dtdev_init);
dtdev->path = strdup(buf);
if (dtdev->path == NULL) {
fprintf(stderr, "unable to duplicate string for dtdevs\n");
exit(-1);
}
- d_config->num_dtdevs++;
}
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 4/6] xl: Provide and use xvasprintf and xasprintf internally
2015-07-07 16:13 [PATCH 0/6] libxl: config file string handling cleanups Ian Jackson
` (2 preceding siblings ...)
2015-07-07 16:13 ` [PATCH 3/6] xl: Provide and use ARRAY_EXTEND_INIT_NODEVID for disks, pcidevs and dtdevs Ian Jackson
@ 2015-07-07 16:13 ` Ian Jackson
2015-07-07 16:13 ` [PATCH 5/6] xl: Use xasprintf for cpupoolnumsplit names Ian Jackson
` (2 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Ian Jackson @ 2015-07-07 16:13 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
Replace all calls to [v]asprintf with this new function. This removes
a fair amount of bespoke error handling.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
tools/libxl/xl_cmdimpl.c | 96 +++++++++++++++++++++++-----------------------
1 file changed, 48 insertions(+), 48 deletions(-)
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 568d7d2..c876d3e 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -362,6 +362,27 @@ static void dolog(const char *file, int line, const char *func, char *fmt, ...)
free(s);
}
+static void xvasprintf(char **strp, const char *fmt, va_list ap)
+ __attribute__((format(printf,2,0)));
+static void xvasprintf(char **strp, const char *fmt, va_list ap)
+{
+ int r = vasprintf(strp, fmt, ap);
+ if (r == -1) {
+ perror("asprintf failed");
+ exit(-ERROR_FAIL);
+ }
+}
+
+static void xasprintf(char **strp, const char *fmt, ...)
+ __attribute__((format(printf,2,3)));
+static void xasprintf(char **strp, const char *fmt, ...)
+{
+ va_list ap;
+ va_start(ap, fmt);
+ xvasprintf(strp, fmt, ap);
+ va_end(ap);
+}
+
static yajl_gen_status printf_info_one_json(yajl_gen hand, int domid,
libxl_domain_config *d_config)
{
@@ -840,11 +861,9 @@ static char *parse_cmdline(XLU_Config *config)
"in favour of cmdline=\n");
} else {
if (root && extra) {
- if (asprintf(&cmdline, "root=%s %s", root, extra) == -1)
- cmdline = NULL;
+ xasprintf(&cmdline, "root=%s %s", root, extra);
} else if (root) {
- if (asprintf(&cmdline, "root=%s", root) == -1)
- cmdline = NULL;
+ xasprintf(&cmdline, "root=%s", root);
} else if (extra) {
cmdline = strdup(extra);
}
@@ -2335,14 +2354,11 @@ static int handle_domain_death(uint32_t *r_domid,
char *corefile;
int rc;
- if (asprintf(&corefile, XEN_DUMP_DIR "/%s", d_config->c_info.name) < 0) {
- LOG("failed to construct core dump path");
- } else {
- LOG("dumping core to %s", corefile);
- rc=libxl_domain_core_dump(ctx, *r_domid, corefile, NULL);
- if (rc) LOG("core dump failed (rc=%d).", rc);
- free(corefile);
- }
+ xasprintf(&corefile, XEN_DUMP_DIR "/%s", d_config->c_info.name);
+ LOG("dumping core to %s", corefile);
+ rc = libxl_domain_core_dump(ctx, *r_domid, corefile, NULL);
+ if (rc) LOG("core dump failed (rc=%d).", rc);
+ free(corefile);
/* No point crying over spilled milk, continue on failure. */
if (action == LIBXL_ACTION_ON_SHUTDOWN_COREDUMP_DESTROY)
@@ -2689,11 +2705,7 @@ static uint32_t create_domain(struct domain_create *dom_info)
common_domname = d_config.c_info.name;
d_config.c_info.name = 0; /* steals allocation from config */
- if (asprintf(&d_config.c_info.name,
- "%s--incoming", common_domname) < 0) {
- fprintf(stderr, "Failed to allocate memory in asprintf\n");
- exit(1);
- }
+ xasprintf(&d_config.c_info.name, "%s--incoming", common_domname);
*dom_info->migration_domname_r = strdup(d_config.c_info.name);
}
}
@@ -2777,10 +2789,7 @@ start:
if (need_daemon) {
char *name;
- if (asprintf(&name, "xl-%s", d_config.c_info.name) < 0) {
- LOG("Failed to allocate memory in asprintf");
- exit(1);
- }
+ xasprintf(&name, "xl-%s", d_config.c_info.name);
ret = do_daemonize(name);
free(name);
if (ret) {
@@ -3173,11 +3182,8 @@ static int cd_insert(uint32_t domid, const char *virtdev, char *phys)
struct stat b;
int rc = 0;
- if (asprintf(&buf, "vdev=%s,access=r,devtype=cdrom,target=%s",
- virtdev, phys ? phys : "") < 0) {
- fprintf(stderr, "out of memory\n");
- return 1;
- }
+ xasprintf(&buf, "vdev=%s,access=r,devtype=cdrom,target=%s",
+ virtdev, phys ? phys : "");
parse_disk_config(&config, buf, &disk);
@@ -4178,8 +4184,7 @@ static void migrate_domain(uint32_t domid, const char *rune, int debug,
fprintf(stderr, "migration sender: Target has acknowledged transfer.\n");
if (common_domname) {
- if (asprintf(&away_domname, "%s--migratedaway", common_domname) < 0)
- goto failed_resume;
+ xasprintf(&away_domname, "%s--migratedaway", common_domname);
rc = libxl_domain_rename(ctx, domid, common_domname, away_domname);
if (rc) goto failed_resume;
}
@@ -4587,13 +4592,12 @@ int main_migrate(int argc, char **argv)
} else {
verbose_len = (minmsglevel_default - minmsglevel) + 2;
}
- if (asprintf(&rune, "exec %s %s xl%s%.*s migrate-receive%s%s",
- ssh_command, host,
- pass_tty_arg ? " -t" : "",
- verbose_len, verbose_buf,
- daemonize ? "" : " -e",
- debug ? " -d" : "") < 0)
- return 1;
+ xasprintf(&rune, "exec %s %s xl%s%.*s migrate-receive%s%s",
+ ssh_command, host,
+ pass_tty_arg ? " -t" : "",
+ verbose_len, verbose_buf,
+ daemonize ? "" : " -e",
+ debug ? " -d" : "");
}
migrate_domain(domid, rune, debug, config_filename);
@@ -6862,7 +6866,6 @@ static char *uptime_to_string(unsigned long uptime, int short_mode)
{
int sec, min, hour, day;
char *time_string;
- int ret;
day = (int)(uptime / 86400);
uptime -= (day * 86400);
@@ -6874,21 +6877,19 @@ static char *uptime_to_string(unsigned long uptime, int short_mode)
if (short_mode)
if (day > 1)
- ret = asprintf(&time_string, "%d days, %2d:%02d", day, hour, min);
+ xasprintf(&time_string, "%d days, %2d:%02d", day, hour, min);
else if (day == 1)
- ret = asprintf(&time_string, "%d day, %2d:%02d", day, hour, min);
+ xasprintf(&time_string, "%d day, %2d:%02d", day, hour, min);
else
- ret = asprintf(&time_string, "%2d:%02d", hour, min);
+ xasprintf(&time_string, "%2d:%02d", hour, min);
else
if (day > 1)
- ret = asprintf(&time_string, "%d days, %2d:%02d:%02d", day, hour, min, sec);
+ xasprintf(&time_string, "%d days, %2d:%02d:%02d", day, hour, min, sec);
else if (day == 1)
- ret = asprintf(&time_string, "%d day, %2d:%02d:%02d", day, hour, min, sec);
+ xasprintf(&time_string, "%d day, %2d:%02d:%02d", day, hour, min, sec);
else
- ret = asprintf(&time_string, "%2d:%02d:%02d", hour, min, sec);
+ xasprintf(&time_string, "%2d:%02d:%02d", hour, min, sec);
- if (ret < 0)
- return NULL;
return time_string;
}
@@ -8040,10 +8041,9 @@ int main_remus(int argc, char **argv)
if (!ssh_command[0]) {
rune = host;
} else {
- if (asprintf(&rune, "exec %s %s xl migrate-receive -r %s",
- ssh_command, host,
- daemonize ? "" : " -e") < 0)
- return 1;
+ xasprintf(&rune, "exec %s %s xl migrate-receive -r %s",
+ ssh_command, host,
+ daemonize ? "" : " -e");
}
save_domain_core_begin(domid, NULL, &config_data, &config_len);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 5/6] xl: Use xasprintf for cpupoolnumsplit names
2015-07-07 16:13 [PATCH 0/6] libxl: config file string handling cleanups Ian Jackson
` (3 preceding siblings ...)
2015-07-07 16:13 ` [PATCH 4/6] xl: Provide and use xvasprintf and xasprintf internally Ian Jackson
@ 2015-07-07 16:13 ` Ian Jackson
2015-07-07 16:13 ` [PATCH 6/6] xl: Rewrite trim() Ian Jackson
2015-07-07 16:21 ` [PATCH 0/6] libxl: config file string handling cleanups Ian Jackson
6 siblings, 0 replies; 12+ messages in thread
From: Ian Jackson @ 2015-07-07 16:13 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
Otherwise we have to do complicated reasoning about the length that %d
might produce.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
tools/libxl/xl_cmdimpl.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index c876d3e..4396095 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -7735,7 +7735,7 @@ int main_cpupoolnumasplit(int argc, char **argv)
int n_pools;
int node;
int n_cpus;
- char name[16];
+ char *name = NULL;
libxl_uuid uuid;
libxl_bitmap cpumap;
libxl_cpupoolinfo *poolinfo;
@@ -7783,7 +7783,7 @@ int main_cpupoolnumasplit(int argc, char **argv)
goto out;
}
- snprintf(name, 15, "Pool-node%d", node);
+ xasprintf(&name, "Pool-node%d", node);
if (libxl_cpupool_rename(ctx, name, 0)) {
fprintf(stderr, "error on renaming Pool 0\n");
goto out;
@@ -7828,7 +7828,8 @@ int main_cpupoolnumasplit(int argc, char **argv)
goto out;
}
- snprintf(name, 15, "Pool-node%d", node);
+ free(name);
+ xasprintf(&name, "Pool-node%d", node);
libxl_uuid_generate(&uuid);
poolid = 0;
if (libxl_cpupool_create(ctx, name, sched, cpumap, &uuid, &poolid)) {
@@ -7853,6 +7854,7 @@ int main_cpupoolnumasplit(int argc, char **argv)
out:
libxl_cputopology_list_free(topology, n_cpus);
libxl_bitmap_dispose(&cpumap);
+ free(name);
return rc;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 6/6] xl: Rewrite trim()
2015-07-07 16:13 [PATCH 0/6] libxl: config file string handling cleanups Ian Jackson
` (4 preceding siblings ...)
2015-07-07 16:13 ` [PATCH 5/6] xl: Use xasprintf for cpupoolnumsplit names Ian Jackson
@ 2015-07-07 16:13 ` Ian Jackson
2015-07-07 16:21 ` [PATCH 0/6] libxl: config file string handling cleanups Ian Jackson
6 siblings, 0 replies; 12+ messages in thread
From: Ian Jackson @ 2015-07-07 16:13 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson
This function would produce a NULL output pointer if the input was an
empty string, leading to a crash.
I don't think this is likely to be a security problem, as the two call
sites involve configuration options which callers are unlikely to
expose to other-than-fully-trusted input.
Also, the function would needlessly copy the input string (which I
care about not for performance reasons but because it makes the memory
handling more confusing), and would mishandle strings which contained
only predicate-true characters.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
tools/libxl/xl_cmdimpl.c | 35 ++++++++++++++++-------------------
1 file changed, 16 insertions(+), 19 deletions(-)
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 4396095..1966316 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -647,26 +647,23 @@ typedef int (*char_predicate_t)(const int c);
static void trim(char_predicate_t predicate, const char *input, char **output)
{
- char *p, *q, *tmp;
+ const char *first, *after;
- *output = NULL;
- if (*input == '\000')
- return;
- /* Input has length >= 1 */
-
- p = tmp = xstrdup(input);
- /* Skip past the characters for which predicate is true */
- while ((*p != '\000') && (predicate((unsigned char)*p)))
- p ++;
- q = p + strlen(p) - 1;
- /* q points to the last non-NULL character */
- while ((q > p) && (predicate((unsigned char)*q)))
- q --;
- /* q points to the last character we want */
- q ++;
- *q = '\000';
- *output = xstrdup(p);
- free(tmp);
+ for (first = input;
+ *first && predicate((unsigned char)first[0]);
+ first++)
+ ;
+
+ for (after = first + strlen(first);
+ after > first && predicate((unsigned char)after[-1]);
+ after--)
+ ;
+
+ size_t len_nonnull = after - first;
+
+ *output = xmalloc(len_nonnull + 1);
+ memcpy(output, first, len_nonnull);
+ output[len_nonnull] = 0;
}
static int split_string_into_pair(const char *str,
--
1.7.10.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/6] xl: Do not ignore unparseable PCI BDFs
2015-07-07 16:13 ` [PATCH 1/6] xl: Do not ignore unparseable PCI BDFs Ian Jackson
@ 2015-07-07 16:18 ` Andrew Cooper
2015-07-16 15:47 ` [PATCH 1/6 v2] " Ian Jackson
0 siblings, 1 reply; 12+ messages in thread
From: Andrew Cooper @ 2015-07-07 16:18 UTC (permalink / raw)
To: Ian Jackson, xen-devel
On 07/07/15 17:13, Ian Jackson wrote:
> If xlu_pci_parse_bdf fails, abandon the domain creation, rather than
> blundering on.
>
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> ---
> tools/libxl/xl_cmdimpl.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 08484e4..31d8260 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -1942,8 +1942,12 @@ skip_vfb:
> pcidev->power_mgmt = pci_power_mgmt;
> pcidev->permissive = pci_permissive;
> pcidev->seize = pci_seize;
> - if (!xlu_pci_parse_bdf(config, pcidev, buf))
> - d_config->num_pcidevs++;
> + e = xlu_pci_parse_bdf(config, pcidev, buf);
> + if (e) {
> + fprintf(stderr, "unable to parse PCI BDF for passthrough\n");
'%s' including the offending buf ?
~Andrew
> + exit(-e);
> + }
> + d_config->num_pcidevs++;
> }
> if (d_config->num_pcidevs && c_info->type == LIBXL_DOMAIN_TYPE_PV)
> libxl_defbool_set(&b_info->u.pv.e820_host, true);
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/6] libxl: config file string handling cleanups
2015-07-07 16:13 [PATCH 0/6] libxl: config file string handling cleanups Ian Jackson
` (5 preceding siblings ...)
2015-07-07 16:13 ` [PATCH 6/6] xl: Rewrite trim() Ian Jackson
@ 2015-07-07 16:21 ` Ian Jackson
2015-07-07 16:30 ` Wei Liu
6 siblings, 1 reply; 12+ messages in thread
From: Ian Jackson @ 2015-07-07 16:21 UTC (permalink / raw)
To: xen-devel; +Cc: Wei Liu, Ian Campbell
Ian Jackson writes ("[PATCH 0/6] libxl: config file string handling cleanups"):
> These 6 followup patches were developed during a review of the xl
> string handling code, prompted by XSA-137. They were embargoed until
> today.
>
> I have reviewed the whole of xl's string handling for other bugs.
> My search terms included:
> realloc sn?printf str \bstr \bstrcpy \bstrn \bstrcat \bmemcpy
> \bmemchr \bp\b
>
> Surprisingly, I found not too much untoward.
>
> Of these only two are even backport candidates:
>
> xl: Do not ignore unparseable PCI BDFs
> xl: Rewrite trim()
FYI, these were acked by Ian Campbell with his security hat on. But
they obviously need to be posted publicly.
If there are no objections I will commit them later this week.
Ian.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/6] libxl: config file string handling cleanups
2015-07-07 16:21 ` [PATCH 0/6] libxl: config file string handling cleanups Ian Jackson
@ 2015-07-07 16:30 ` Wei Liu
0 siblings, 0 replies; 12+ messages in thread
From: Wei Liu @ 2015-07-07 16:30 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel, Wei Liu, Ian Campbell
On Tue, Jul 07, 2015 at 05:21:07PM +0100, Ian Jackson wrote:
> Ian Jackson writes ("[PATCH 0/6] libxl: config file string handling cleanups"):
> > These 6 followup patches were developed during a review of the xl
> > string handling code, prompted by XSA-137. They were embargoed until
> > today.
> >
> > I have reviewed the whole of xl's string handling for other bugs.
> > My search terms included:
> > realloc sn?printf str \bstr \bstrcpy \bstrn \bstrcat \bmemcpy
> > \bmemchr \bp\b
> >
> > Surprisingly, I found not too much untoward.
> >
> > Of these only two are even backport candidates:
> >
> > xl: Do not ignore unparseable PCI BDFs
> > xl: Rewrite trim()
>
> FYI, these were acked by Ian Campbell with his security hat on. But
> they obviously need to be posted publicly.
>
> If there are no objections I will commit them later this week.
>
All patches look good to me:
Acked-by: Wei Liu <wei.liu2@citrix.com>
> Ian.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/6 v2] xl: Do not ignore unparseable PCI BDFs
2015-07-07 16:18 ` Andrew Cooper
@ 2015-07-16 15:47 ` Ian Jackson
2015-07-16 15:52 ` Wei Liu
0 siblings, 1 reply; 12+ messages in thread
From: Ian Jackson @ 2015-07-16 15:47 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Wei Liu, Ian Campbell, Andrew Cooper
[ resending just 1/6 rather than the others as well ]
If xlu_pci_parse_bdf fails, abandon the domain creation, rather than
blundering on.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
---
v2: Print the offending supposed-BDF too.
---
tools/libxl/xl_cmdimpl.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index a08c264..5ab4e16 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1945,8 +1945,14 @@ skip_vfb:
pcidev->power_mgmt = pci_power_mgmt;
pcidev->permissive = pci_permissive;
pcidev->seize = pci_seize;
- if (!xlu_pci_parse_bdf(config, pcidev, buf))
- d_config->num_pcidevs++;
+ e = xlu_pci_parse_bdf(config, pcidev, buf);
+ if (e) {
+ fprintf(stderr,
+ "unable to parse PCI BDF `%s' for passthrough\n",
+ buf);
+ exit(-e);
+ }
+ d_config->num_pcidevs++;
}
if (d_config->num_pcidevs && c_info->type == LIBXL_DOMAIN_TYPE_PV)
libxl_defbool_set(&b_info->u.pv.e820_host, true);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/6 v2] xl: Do not ignore unparseable PCI BDFs
2015-07-16 15:47 ` [PATCH 1/6 v2] " Ian Jackson
@ 2015-07-16 15:52 ` Wei Liu
0 siblings, 0 replies; 12+ messages in thread
From: Wei Liu @ 2015-07-16 15:52 UTC (permalink / raw)
To: Ian Jackson; +Cc: Andrew Cooper, xen-devel, Wei Liu, Ian Campbell
On Thu, Jul 16, 2015 at 04:47:55PM +0100, Ian Jackson wrote:
> [ resending just 1/6 rather than the others as well ]
>
> If xlu_pci_parse_bdf fails, abandon the domain creation, rather than
> blundering on.
>
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
> ---
> v2: Print the offending supposed-BDF too.
> ---
> tools/libxl/xl_cmdimpl.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index a08c264..5ab4e16 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -1945,8 +1945,14 @@ skip_vfb:
> pcidev->power_mgmt = pci_power_mgmt;
> pcidev->permissive = pci_permissive;
> pcidev->seize = pci_seize;
> - if (!xlu_pci_parse_bdf(config, pcidev, buf))
> - d_config->num_pcidevs++;
> + e = xlu_pci_parse_bdf(config, pcidev, buf);
> + if (e) {
> + fprintf(stderr,
> + "unable to parse PCI BDF `%s' for passthrough\n",
> + buf);
> + exit(-e);
> + }
> + d_config->num_pcidevs++;
> }
> if (d_config->num_pcidevs && c_info->type == LIBXL_DOMAIN_TYPE_PV)
> libxl_defbool_set(&b_info->u.pv.e820_host, true);
> --
> 1.7.10.4
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-07-16 15:52 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-07 16:13 [PATCH 0/6] libxl: config file string handling cleanups Ian Jackson
2015-07-07 16:13 ` [PATCH 1/6] xl: Do not ignore unparseable PCI BDFs Ian Jackson
2015-07-07 16:18 ` Andrew Cooper
2015-07-16 15:47 ` [PATCH 1/6 v2] " Ian Jackson
2015-07-16 15:52 ` Wei Liu
2015-07-07 16:13 ` [PATCH 2/6] xl: Use ARRAY_EXTEND_INIT for vtpms and nics Ian Jackson
2015-07-07 16:13 ` [PATCH 3/6] xl: Provide and use ARRAY_EXTEND_INIT_NODEVID for disks, pcidevs and dtdevs Ian Jackson
2015-07-07 16:13 ` [PATCH 4/6] xl: Provide and use xvasprintf and xasprintf internally Ian Jackson
2015-07-07 16:13 ` [PATCH 5/6] xl: Use xasprintf for cpupoolnumsplit names Ian Jackson
2015-07-07 16:13 ` [PATCH 6/6] xl: Rewrite trim() Ian Jackson
2015-07-07 16:21 ` [PATCH 0/6] libxl: config file string handling cleanups Ian Jackson
2015-07-07 16:30 ` Wei Liu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).