[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] Prevent segmentation fault in case of relative
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH] Prevent segmentation fault in case of relative resolve of uri |
Date: |
Thu, 18 Sep 2014 16:54:08 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
Stefan Hajnoczi <address@hidden> writes:
> On Tue, Sep 09, 2014 at 09:45:06AM +0200, 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>
>> ---
>> util/uri.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/util/uri.c b/util/uri.c
>> index e348c17..16c01d0 100644
>> --- a/util/uri.c
>> +++ b/util/uri.c
>> @@ -1985,7 +1985,8 @@ uri_resolve_relative (const char *uri, const char *
>> base)
>> val = g_strdup (uri);
>> goto done;
>> }
>> - if (!strcmp(bas->path, ref->path)) {
>> + if (bas->path != NULL && ref->path != NULL &&
>> + !strcmp(bas->path, ref->path)) {
>> val = g_strdup("");
>> goto done;
>> }
>
> This patch adds more conditions, what's needed is to audit and clean up this
> function. There is dead code and things could be simplified instead:
>
> if (!strcmp(bas->path, ref->path)) {
> val = g_strdup("");
> goto done;
> }
> if (bas->path == NULL) {
> val = g_strdup(ref->path);
> goto done;
> }
> if (ref->path == NULL) {
> ref->path = (char *) "/";
> remove_path = 1;
> }
> <---- bas->path cannot be NULL because we took goto done
> <---- ref->path cannot be NULL because we assigned "/"
>
> /*
> * 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) { <--- dead code!
> if (ref->path != NULL) {
> uptr = ref->path;
> if (*uptr == '/')
> uptr++;
> /* exception characters from uri_to_string */
> val = uri_string_escape(uptr, "/;&=+$,");
> }
> goto done;
> }
> bptr = bas->path;
> if (ref->path == NULL) { <--- dead code!
>
> Also, perhaps the strcmp() you touched should really be moved below the NULL
> checks.
If you simplify this function, please get rid of the silly conditionals
around g_strdup(). See commits 80e0090 c14e984 c64f50d for examples.