From: Andrei Emeltchenko <[email protected]>
It is better not to dereference freed pointer
---
gobex/gobex.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/gobex/gobex.c b/gobex/gobex.c
index 3848884..35e546f 100644
--- a/gobex/gobex.c
+++ b/gobex/gobex.c
@@ -263,8 +263,6 @@ static gboolean req_timeout(gpointer user_data)
g_error_free(err);
pending_pkt_free(p);
- p->timeout_id = 0;
-
return FALSE;
}
--
1.9.1
Hi Andrei,
On Tue, Aug 5, 2014 at 10:56 AM, Andrei Emeltchenko
<[email protected]> wrote:
> ping
>
> On Fri, Aug 01, 2014 at 11:44:34AM +0300, Andrei Emeltchenko wrote:
>> From: Andrei Emeltchenko <[email protected]>
>>
>> transfer_complete() frees transfer pointer.
>> ---
>> gobex/gobex-transfer.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/gobex/gobex-transfer.c b/gobex/gobex-transfer.c
>> index 8498177..6dc7d9f 100644
>> --- a/gobex/gobex-transfer.c
>> +++ b/gobex/gobex-transfer.c
>> @@ -378,6 +378,7 @@ static void transfer_put_req_first(struct transfer *transfer, GObexPacket *req,
>> if (!g_obex_send(transfer->obex, rsp, &err)) {
>> transfer_complete(transfer, err);
>> g_error_free(err);
>> + return;
>> }
>>
>> if (rspcode != G_OBEX_RSP_CONTINUE)
>> --
>> 1.9.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
Pushed.
--
Luiz Augusto von Dentz
ping
On Fri, Aug 01, 2014 at 11:44:34AM +0300, Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <[email protected]>
>
> transfer_complete() frees transfer pointer.
> ---
> gobex/gobex-transfer.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/gobex/gobex-transfer.c b/gobex/gobex-transfer.c
> index 8498177..6dc7d9f 100644
> --- a/gobex/gobex-transfer.c
> +++ b/gobex/gobex-transfer.c
> @@ -378,6 +378,7 @@ static void transfer_put_req_first(struct transfer *transfer, GObexPacket *req,
> if (!g_obex_send(transfer->obex, rsp, &err)) {
> transfer_complete(transfer, err);
> g_error_free(err);
> + return;
> }
>
> if (rspcode != G_OBEX_RSP_CONTINUE)
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Andrei Emeltchenko <[email protected]>
transfer_complete() frees transfer pointer.
---
gobex/gobex-transfer.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/gobex/gobex-transfer.c b/gobex/gobex-transfer.c
index 8498177..6dc7d9f 100644
--- a/gobex/gobex-transfer.c
+++ b/gobex/gobex-transfer.c
@@ -378,6 +378,7 @@ static void transfer_put_req_first(struct transfer *transfer, GObexPacket *req,
if (!g_obex_send(transfer->obex, rsp, &err)) {
transfer_complete(transfer, err);
g_error_free(err);
+ return;
}
if (rspcode != G_OBEX_RSP_CONTINUE)
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
Refactor function transfer_get_req_first() to avoid use after free.
---
gobex/gobex-transfer.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/gobex/gobex-transfer.c b/gobex/gobex-transfer.c
index efae72b..d7707f9 100644
--- a/gobex/gobex-transfer.c
+++ b/gobex/gobex-transfer.c
@@ -553,7 +553,8 @@ static gssize get_get_data(void *buf, gsize len, gpointer user_data)
return ret;
}
-static void transfer_get_req_first(struct transfer *transfer, GObexPacket *rsp)
+static gboolean transfer_get_req_first(struct transfer *transfer,
+ GObexPacket *rsp)
{
GError *err = NULL;
@@ -564,7 +565,10 @@ static void transfer_get_req_first(struct transfer *transfer, GObexPacket *rsp)
if (!g_obex_send(transfer->obex, rsp, &err)) {
transfer_complete(transfer, err);
g_error_free(err);
+ return FALSE;
}
+
+ return TRUE;
}
static void transfer_get_req(GObex *obex, GObexPacket *req, gpointer user_data)
@@ -596,7 +600,8 @@ guint g_obex_get_rsp_pkt(GObex *obex, GObexPacket *rsp,
transfer = transfer_new(obex, G_OBEX_OP_GET, complete_func, user_data);
transfer->data_producer = data_func;
- transfer_get_req_first(transfer, rsp);
+ if (!transfer_get_req_first(transfer, rsp))
+ return 0;
if (!g_slist_find(transfers, transfer))
return 0;
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
---
gobex/gobex-transfer.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/gobex/gobex-transfer.c b/gobex/gobex-transfer.c
index 6dc7d9f..efae72b 100644
--- a/gobex/gobex-transfer.c
+++ b/gobex/gobex-transfer.c
@@ -406,6 +406,7 @@ static void transfer_put_req(GObex *obex, GObexPacket *req, gpointer user_data)
if (!g_obex_send(obex, rsp, &err)) {
transfer_complete(transfer, err);
g_error_free(err);
+ return;
}
done:
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
First remove object from the list and then remove it.
---
profiles/proximity/monitor.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/profiles/proximity/monitor.c b/profiles/proximity/monitor.c
index f2e0739..b05cdd7 100644
--- a/profiles/proximity/monitor.c
+++ b/profiles/proximity/monitor.c
@@ -606,13 +606,13 @@ static void monitor_destroy(gpointer user_data)
{
struct monitor *monitor = user_data;
+ monitors = g_slist_remove(monitors, monitor);
+
btd_device_unref(monitor->device);
g_free(monitor->linklosslevel);
g_free(monitor->immediatelevel);
g_free(monitor->signallevel);
g_free(monitor);
-
- monitors = g_slist_remove(monitors, monitor);
}
static struct monitor *register_monitor(struct btd_device *device)
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
Refactor filter_data_remove_callback so that we do not iterate over
freed pointer.
---
gdbus/watch.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/gdbus/watch.c b/gdbus/watch.c
index 0f99f4f..474d3d4 100644
--- a/gdbus/watch.c
+++ b/gdbus/watch.c
@@ -362,6 +362,7 @@ static void service_data_free(struct service_data *data)
callback->data = NULL;
}
+/* Returns TRUE if data is freed */
static gboolean filter_data_remove_callback(struct filter_data *data,
struct filter_callback *cb)
{
@@ -383,7 +384,7 @@ static gboolean filter_data_remove_callback(struct filter_data *data,
/* Don't remove the filter if other callbacks exist or data is lock
* processing callbacks */
if (data->callbacks || data->lock)
- return TRUE;
+ return FALSE;
if (data->registered && !remove_match(data))
return FALSE;
@@ -405,7 +406,9 @@ static DBusHandlerResult signal_filter(DBusConnection *connection,
if (cb->signal_func && !cb->signal_func(connection, message,
cb->user_data)) {
- filter_data_remove_callback(data, cb);
+ if (filter_data_remove_callback(data, cb))
+ break;
+
continue;
}
@@ -489,7 +492,9 @@ static DBusHandlerResult service_filter(DBusConnection *connection,
/* Only auto remove if it is a bus name watch */
if (data->argument[0] == ':' &&
(cb->conn_func == NULL || cb->disc_func == NULL)) {
- filter_data_remove_callback(data, cb);
+ if (filter_data_remove_callback(data, cb))
+ break;
+
continue;
}
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
It is better not to dereference freed pointer
---
gobex/gobex.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/gobex/gobex.c b/gobex/gobex.c
index 3848884..16a4aae 100644
--- a/gobex/gobex.c
+++ b/gobex/gobex.c
@@ -250,6 +250,7 @@ static gboolean req_timeout(gpointer user_data)
g_assert(p != NULL);
+ p->timeout_id = 0;
obex->pending_req = NULL;
err = g_error_new(G_OBEX_ERROR, G_OBEX_ERROR_TIMEOUT,
@@ -263,8 +264,6 @@ static gboolean req_timeout(gpointer user_data)
g_error_free(err);
pending_pkt_free(p);
- p->timeout_id = 0;
-
return FALSE;
}
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
Refactor filter_data_remove_callback so that we do not iterate over
freed pointer.
---
gdbus/watch.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/gdbus/watch.c b/gdbus/watch.c
index 0f99f4f..e18ca1f 100644
--- a/gdbus/watch.c
+++ b/gdbus/watch.c
@@ -362,6 +362,7 @@ static void service_data_free(struct service_data *data)
callback->data = NULL;
}
+/* Returns TRUE if data is freed */
static gboolean filter_data_remove_callback(struct filter_data *data,
struct filter_callback *cb)
{
@@ -383,7 +384,7 @@ static gboolean filter_data_remove_callback(struct filter_data *data,
/* Don't remove the filter if other callbacks exist or data is lock
* processing callbacks */
if (data->callbacks || data->lock)
- return TRUE;
+ return FALSE;
if (data->registered && !remove_match(data))
return FALSE;
@@ -405,7 +406,9 @@ static DBusHandlerResult signal_filter(DBusConnection *connection,
if (cb->signal_func && !cb->signal_func(connection, message,
cb->user_data)) {
- filter_data_remove_callback(data, cb);
+ if (filter_data_remove_callback(data, cb))
+ break;
+
continue;
}
@@ -489,7 +492,9 @@ static DBusHandlerResult service_filter(DBusConnection *connection,
/* Only auto remove if it is a bus name watch */
if (data->argument[0] == ':' &&
(cb->conn_func == NULL || cb->disc_func == NULL)) {
- filter_data_remove_callback(data, cb);
+ if(filter_data_remove_callback(data, cb))
+ break;
+
continue;
}
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
Refactor function transfer_get_req_first() to avoid use after free.
---
gobex/gobex-transfer.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/gobex/gobex-transfer.c b/gobex/gobex-transfer.c
index efae72b..e47382c 100644
--- a/gobex/gobex-transfer.c
+++ b/gobex/gobex-transfer.c
@@ -553,7 +553,8 @@ static gssize get_get_data(void *buf, gsize len, gpointer user_data)
return ret;
}
-static void transfer_get_req_first(struct transfer *transfer, GObexPacket *rsp)
+static gboolean transfer_get_req_first(struct transfer *transfer,
+ GObexPacket *rsp)
{
GError *err = NULL;
@@ -564,7 +565,10 @@ static void transfer_get_req_first(struct transfer *transfer, GObexPacket *rsp)
if (!g_obex_send(transfer->obex, rsp, &err)) {
transfer_complete(transfer, err);
g_error_free(err);
+ return FALSE;
}
+
+ return TRUE;
}
static void transfer_get_req(GObex *obex, GObexPacket *req, gpointer user_data)
@@ -596,7 +600,8 @@ guint g_obex_get_rsp_pkt(GObex *obex, GObexPacket *rsp,
transfer = transfer_new(obex, G_OBEX_OP_GET, complete_func, user_data);
transfer->data_producer = data_func;
- transfer_get_req_first(transfer, rsp);
+ if(!transfer_get_req_first(transfer, rsp))
+ return 0;
if (!g_slist_find(transfers, transfer))
return 0;
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
It is better not to dereference freed pointer
---
gobex/gobex.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/gobex/gobex.c b/gobex/gobex.c
index 3848884..16a4aae 100644
--- a/gobex/gobex.c
+++ b/gobex/gobex.c
@@ -250,6 +250,7 @@ static gboolean req_timeout(gpointer user_data)
g_assert(p != NULL);
+ p->timeout_id = 0;
obex->pending_req = NULL;
err = g_error_new(G_OBEX_ERROR, G_OBEX_ERROR_TIMEOUT,
@@ -263,8 +264,6 @@ static gboolean req_timeout(gpointer user_data)
g_error_free(err);
pending_pkt_free(p);
- p->timeout_id = 0;
-
return FALSE;
}
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
---
gobex/gobex-transfer.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/gobex/gobex-transfer.c b/gobex/gobex-transfer.c
index 6dc7d9f..efae72b 100644
--- a/gobex/gobex-transfer.c
+++ b/gobex/gobex-transfer.c
@@ -406,6 +406,7 @@ static void transfer_put_req(GObex *obex, GObexPacket *req, gpointer user_data)
if (!g_obex_send(obex, rsp, &err)) {
transfer_complete(transfer, err);
g_error_free(err);
+ return;
}
done:
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
transfer_complete() frees transfer pointer.
---
gobex/gobex-transfer.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/gobex/gobex-transfer.c b/gobex/gobex-transfer.c
index 8498177..6dc7d9f 100644
--- a/gobex/gobex-transfer.c
+++ b/gobex/gobex-transfer.c
@@ -378,6 +378,7 @@ static void transfer_put_req_first(struct transfer *transfer, GObexPacket *req,
if (!g_obex_send(transfer->obex, rsp, &err)) {
transfer_complete(transfer, err);
g_error_free(err);
+ return;
}
if (rspcode != G_OBEX_RSP_CONTINUE)
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
First remove object from the list and then remove it.
---
profiles/proximity/monitor.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/profiles/proximity/monitor.c b/profiles/proximity/monitor.c
index f2e0739..b05cdd7 100644
--- a/profiles/proximity/monitor.c
+++ b/profiles/proximity/monitor.c
@@ -606,13 +606,13 @@ static void monitor_destroy(gpointer user_data)
{
struct monitor *monitor = user_data;
+ monitors = g_slist_remove(monitors, monitor);
+
btd_device_unref(monitor->device);
g_free(monitor->linklosslevel);
g_free(monitor->immediatelevel);
g_free(monitor->signallevel);
g_free(monitor);
-
- monitors = g_slist_remove(monitors, monitor);
}
static struct monitor *register_monitor(struct btd_device *device)
--
1.9.1
Hi Andrei,
On Thu, Jul 31, 2014 at 4:35 PM, Andrei Emeltchenko
<[email protected]> wrote:
> From: Andrei Emeltchenko <[email protected]>
>
> It is better not to dereference freed pointer
> ---
> gobex/gobex.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/gobex/gobex.c b/gobex/gobex.c
> index 3848884..35e546f 100644
> --- a/gobex/gobex.c
> +++ b/gobex/gobex.c
> @@ -263,8 +263,6 @@ static gboolean req_timeout(gpointer user_data)
> g_error_free(err);
> pending_pkt_free(p);
>
> - p->timeout_id = 0;
> -
A more correct fix would be to move this line to the beginning of the
function so we don't call g_source_remove unnecessarily.
--
Luiz Augusto von Dentz