* [Qemu-devel] [PATCH] Prevent segmentation fault in case of relative resolve of uri
@ 2014-09-09 7:45 mrezanin
2014-09-17 9:26 ` Stefan Hajnoczi
0 siblings, 1 reply; 4+ messages in thread
From: mrezanin @ 2014-09-09 7:45 UTC (permalink / raw)
To: qemu-devel
From: Miroslav Rezanina <mrezanin@redhat.com>
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 <mrezanin@redhat.com>
---
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;
}
--
1.9.3
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH] Prevent segmentation fault in case of relative resolve of uri
2014-09-09 7:45 [Qemu-devel] [PATCH] Prevent segmentation fault in case of relative resolve of uri mrezanin
@ 2014-09-17 9:26 ` Stefan Hajnoczi
2014-09-18 14:54 ` Markus Armbruster
0 siblings, 1 reply; 4+ messages in thread
From: Stefan Hajnoczi @ 2014-09-17 9:26 UTC (permalink / raw)
To: mrezanin; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2115 bytes --]
On Tue, Sep 09, 2014 at 09:45:06AM +0200, mrezanin@redhat.com wrote:
> From: Miroslav Rezanina <mrezanin@redhat.com>
>
> 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 <mrezanin@redhat.com>
> ---
> 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.
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH] Prevent segmentation fault in case of relative resolve of uri
2014-09-17 9:26 ` Stefan Hajnoczi
@ 2014-09-18 14:54 ` Markus Armbruster
2014-09-18 14:57 ` Markus Armbruster
0 siblings, 1 reply; 4+ messages in thread
From: Markus Armbruster @ 2014-09-18 14:54 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: mrezanin, qemu-devel
Stefan Hajnoczi <stefanha@gmail.com> writes:
> On Tue, Sep 09, 2014 at 09:45:06AM +0200, mrezanin@redhat.com wrote:
>> From: Miroslav Rezanina <mrezanin@redhat.com>
>>
>> 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 <mrezanin@redhat.com>
>> ---
>> 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.
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH] Prevent segmentation fault in case of relative resolve of uri
2014-09-18 14:54 ` Markus Armbruster
@ 2014-09-18 14:57 ` Markus Armbruster
0 siblings, 0 replies; 4+ messages in thread
From: Markus Armbruster @ 2014-09-18 14:57 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: mrezanin, qemu-devel
Markus Armbruster <armbru@redhat.com> writes:
> Stefan Hajnoczi <stefanha@gmail.com> writes:
>
>> On Tue, Sep 09, 2014 at 09:45:06AM +0200, mrezanin@redhat.com wrote:
>>> From: Miroslav Rezanina <mrezanin@redhat.com>
>>>
>>> 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 <mrezanin@redhat.com>
>>> ---
>>> 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.
Likewise: g_free(). See commits 9e28840 64641d8 f7047c2 fb7da62 fec0da9
ec15993.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-09-18 14:57 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-09 7:45 [Qemu-devel] [PATCH] Prevent segmentation fault in case of relative resolve of uri mrezanin
2014-09-17 9:26 ` Stefan Hajnoczi
2014-09-18 14:54 ` Markus Armbruster
2014-09-18 14:57 ` Markus Armbruster
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).