2012-05-18 07:54:03

by Mikel Astiz

Subject: [PATCH obexd v0 1/2] client: Fix NULL dereference in case of error

From: Mikel Astiz <[email protected]>

obc_session_queue assumes that the given transfer is not NULL, so this
must be checked explicitly.
---
client/manager.c | 16 ++++++++++++++--
1 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/client/manager.c b/client/manager.c
index 9f26cb0..53f0d0f 100644
--- a/client/manager.c
+++ b/client/manager.c
@@ -294,13 +294,18 @@ static void pull_obc_session_callback(struct obc_session *session,
}

pull = obc_transfer_get("text/x-vcard", NULL, data->filename, &gerr);
+ if (pull == NULL) {
+ reply = g_dbus_create_error(data->message,
+ "org.openobex.Error.Failed",
+ "%s", gerr->message);
+ goto fail;
+ }

if (!obc_session_queue(session, pull, pull_complete_callback, data,
&gerr)) {
reply = g_dbus_create_error(data->message,
"org.openobex.Error.Failed",
"%s", gerr->message);
- g_error_free(gerr);
goto fail;
}

@@ -311,6 +316,7 @@ fail:
shutdown_session(session);
dbus_message_unref(data->message);
dbus_connection_unref(data->connection);
+ g_clear_error(&gerr);
g_free(data->filename);
g_free(data->sender);
g_free(data);
@@ -516,13 +522,18 @@ static void capability_obc_session_callback(struct obc_session *session,

pull = obc_transfer_get("x-obex/capability", NULL, data->filename,
&gerr);
+ if (pull == NULL) {
+ reply = g_dbus_create_error(data->message,
+ "org.openobex.Error.Failed",
+ "%s", gerr->message);
+ goto fail;
+ }

if (!obc_session_queue(session, pull, capabilities_complete_callback,
data, &gerr)) {
reply = g_dbus_create_error(data->message,
"org.openobex.Error.Failed",
"%s", gerr->message);
- g_error_free(gerr);
goto fail;
}

@@ -533,6 +544,7 @@ fail:
shutdown_session(session);
dbus_message_unref(data->message);
dbus_connection_unref(data->connection);
+ g_clear_error(&gerr);
g_free(data->sender);
g_free(data);
}
--
1.7.7.6

2012-05-18 19:25:29

by Johan Hedberg

Subject: Re: [PATCH obexd v0 1/2] client: Fix NULL dereference in case of error

Hi Mikel,

On Fri, May 18, 2012, Mikel Astiz wrote:
> From: Mikel Astiz <[email protected]>
>
> obc_session_queue assumes that the given transfer is not NULL, so this
> must be checked explicitly.
> ---
> client/manager.c | 16 ++++++++++++++--
> 1 files changed, 14 insertions(+), 2 deletions(-)

Both patches have been applied. Thanks.

Johan

2012-05-18 07:54:04

by Mikel Astiz

Subject: [PATCH obexd v0 2/2] client: Simplify error-handling code

From: Mikel Astiz <[email protected]>

Refactor error-handling code to avoid duplicated code.
---
client/manager.c | 48 ++++++++++++++++++------------------------------
1 files changed, 18 insertions(+), 30 deletions(-)

diff --git a/client/manager.c b/client/manager.c
index 53f0d0f..b6d4afc 100644
--- a/client/manager.c
+++ b/client/manager.c
@@ -286,32 +286,26 @@ static void pull_obc_session_callback(struct obc_session *session,
DBusMessage *reply;
GError *gerr = NULL;

- if (err != NULL) {
- reply = g_dbus_create_error(data->message,
- "org.openobex.Error.Failed",
- "%s", err->message);
+ if (err != NULL)
goto fail;
- }

pull = obc_transfer_get("text/x-vcard", NULL, data->filename, &gerr);
- if (pull == NULL) {
- reply = g_dbus_create_error(data->message,
- "org.openobex.Error.Failed",
- "%s", gerr->message);
+ if (pull == NULL)
goto fail;
- }

if (!obc_session_queue(session, pull, pull_complete_callback, data,
- &gerr)) {
- reply = g_dbus_create_error(data->message,
- "org.openobex.Error.Failed",
- "%s", gerr->message);
+ &gerr))
goto fail;
- }

return;

fail:
+ if (err == NULL)
+ err = gerr;
+
+ reply = g_dbus_create_error(data->message,
+ "org.openobex.Error.Failed",
+ "%s", err->message);
g_dbus_send_message(data->connection, reply);
shutdown_session(session);
dbus_message_unref(data->message);
@@ -513,33 +507,27 @@ static void capability_obc_session_callback(struct obc_session *session,
DBusMessage *reply;
GError *gerr = NULL;

- if (err != NULL) {
- reply = g_dbus_create_error(data->message,
- "org.openobex.Error.Failed",
- "%s", err->message);
+ if (err != NULL)
goto fail;
- }

pull = obc_transfer_get("x-obex/capability", NULL, data->filename,
&gerr);
- if (pull == NULL) {
- reply = g_dbus_create_error(data->message,
- "org.openobex.Error.Failed",
- "%s", gerr->message);
+ if (pull == NULL)
goto fail;
- }

if (!obc_session_queue(session, pull, capabilities_complete_callback,
- data, &gerr)) {
- reply = g_dbus_create_error(data->message,
- "org.openobex.Error.Failed",
- "%s", gerr->message);
+ data, &gerr))
goto fail;
- }

return;

fail:
+ if (err == NULL)
+ err = gerr;
+
+ reply = g_dbus_create_error(data->message,
+ "org.openobex.Error.Failed",
+ "%s", err->message);
g_dbus_send_message(data->connection, reply);
shutdown_session(session);
dbus_message_unref(data->message);
--
1.7.7.6