qemu-devel
[Top][All Lists]
Advanced

[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.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]