xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Ian Jackson <ian.jackson@eu.citrix.com>
To: xen-devel@lists.xensource.com
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
Subject: [PATCH 6/6] xl: Rewrite trim()
Date: Tue, 7 Jul 2015 17:13:25 +0100	[thread overview]
Message-ID: <1436285605-18411-7-git-send-email-ian.jackson@eu.citrix.com> (raw)
In-Reply-To: <1436285605-18411-1-git-send-email-ian.jackson@eu.citrix.com>

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

  parent reply	other threads:[~2015-07-07 16:13 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 ` Ian Jackson [this message]
2015-07-07 16:21 ` [PATCH 0/6] libxl: config file string handling cleanups Ian Jackson
2015-07-07 16:30   ` Wei Liu

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=1436285605-18411-7-git-send-email-ian.jackson@eu.citrix.com \
    --to=ian.jackson@eu.citrix.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 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).