[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCHv2] Prevent segmentation fault in case of relativ
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCHv2] Prevent segmentation fault in case of relative resolve of uri |
Date: |
Mon, 09 Feb 2015 12:31:18 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 |
On 09/02/2015 11:47, address@hidden wrote:
> From: Miroslav Rezanina <address@hidden>
>
> It was possible to call strcmp with NULL argument, that can cause
> segmentation fault. Properly checking parameters to prevent this
> situation.
>
> Signed-off-by: Miroslav Rezanina <address@hidden>
> ---
> v2:
> - instead of adding NULL checks to strcmp call refactor whole
> NULL checking path. This will remove dead code and make whole checking
> easier to understand.
>
> Relative path generation part is not touched as I'm not fully sure
> of correct behavior and purpose of this patch is to prevent segmentation
> fault.
> ---
> util/uri.c | 55 +++++++++++++++++++++++++------------------------------
> 1 file changed, 25 insertions(+), 30 deletions(-)
>
> diff --git a/util/uri.c b/util/uri.c
> index 918d235..23dbaca 100644
> --- a/util/uri.c
> +++ b/util/uri.c
> @@ -1964,44 +1964,39 @@ uri_resolve_relative (const char *uri, const char *
> base)
> * If the scheme / server on the URI differs from the base,
> * just return the URI
> */
> - if ((ref->scheme != NULL) &&
> - ((bas->scheme == NULL) ||
> - (strcmp (bas->scheme, ref->scheme)) ||
> - (strcmp (bas->server, ref->server)))) {
> - val = g_strdup (uri);
> - goto done;
> +
> + if ((ref->scheme != NULL) &&
> + ((bas->scheme == NULL) || (strcmp (bas->scheme, ref->scheme)))) {
> + val = g_strdup(uri);
> + goto done;
> }
> - if (!strcmp(bas->path, ref->path)) {
> - val = g_strdup("");
> - goto done;
> - }
> - if (bas->path == NULL) {
> - val = g_strdup(ref->path);
> - goto done;
> + if ((ref->server != NULL) &&
> + ((bas->server == NULL) || (strcmp (bas->server, ref->server)))) {
> + val = g_strdup(uri);
> + goto done;
> }
> +
> if (ref->path == NULL) {
> ref->path = (char *) "/";
> - remove_path = 1;
> + remove_path = 1;
> }
>
> - /*
> - * At this point (at last!) we can compare the two paths
> - *
> - * First we take care of the special case where either of the
> - * two path components may be missing (bug 316224)
> - */
> if (bas->path == NULL) {
> - if (ref->path != NULL) {
> - uptr = ref->path;
> - if (*uptr == '/')
> - uptr++;
> - /* exception characters from uri_to_string */
> - val = uri_string_escape(uptr, "/;&=+$,");
> - }
> - goto done;
> + uptr = ref->path;
> + if (*uptr == '/')
> + uptr++;
> + /* exception characters from uri_to_string */
> + val = uri_string_escape(uptr, "/;&=+$,");
> + goto done;
> }
> +
> + if (!strcmp(bas->path, ref->path)) {
> + val = g_strdup("");
> + goto done;
> + }
> +
> bptr = bas->path;
> - if (ref->path == NULL) {
> + if (remove_path == 1) {
> for (ix = 0; bptr[ix] != 0; ix++) {
> if (bptr[ix] == '/')
> nbslash++;
> @@ -2010,7 +2005,7 @@ uri_resolve_relative (const char *uri, const char *
> base)
> len = 1; /* this is for a string terminator only */
> } else {
> /*
> - * Next we compare the two strings and find where they first differ
> + * We compare the two strings and find where they first differ
> */
> if ((ref->path[pos] == '.') && (ref->path[pos+1] == '/'))
> pos += 2;
>
It's the third time a fix for this is submitted. :)
[PATCH 3/3] util/uri: URI member path can be null, compare more carfully
(by Markus)
[PATCH 3/7] uri: avoid NULL arguments to strcmp
(by me).
Markus's patch was accepted. Further cleanups on the code on top of his
patch are welcome, though.
The logic for "compare two possibly-NULL strings" could be replaced by
g_strcmp0, but that's only provided by glib versions 2.16 or newer,
while we support 2.12.
Paolo