2015-06-23 08:08:46

by Atul Kumar Rai

[permalink] [raw]
Subject: [PATCH] android/client/if-bt.c: Fix memory leak while using realloc()

While reallocating space to store additional "remote device set" using
realloc, if realloc() fails, the original block is left untouched but
reference to that block is lost as NULL is assigned to remote_devices.
The original block needs to be freed before return.

Signed-off-by: Atul Rai <[email protected]>
---
android/client/if-bt.c | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/android/client/if-bt.c b/android/client/if-bt.c
index 4723024..4f0c2e9 100644
--- a/android/client/if-bt.c
+++ b/android/client/if-bt.c
@@ -94,6 +94,7 @@ static int remote_devices_capacity = 0;
void add_remote_device(const bt_bdaddr_t *addr)
{
int i;
+ bt_bdaddr_t *tmp = NULL;

if (remote_devices == NULL) {
remote_devices = malloc(4 * sizeof(bt_bdaddr_t));
@@ -119,9 +120,17 @@ void add_remote_device(const bt_bdaddr_t *addr)
/* Realloc space if needed */
if (remote_devices_cnt >= remote_devices_capacity) {
remote_devices_capacity *= 2;
+ /*
+ * Save reference to previously allocated memory block so that
+ * it can be freed in case realloc fails.
+ */
+ tmp = remote_devices;
+
remote_devices = realloc(remote_devices, sizeof(bt_bdaddr_t) *
remote_devices_capacity);
if (remote_devices == NULL) {
+ if (NULL != tmp)
+ free(tmp);
remote_devices_capacity = 0;
remote_devices_cnt = 0;
return;
--
2.1.4



2015-07-21 12:22:15

by Szymon Janc

[permalink] [raw]
Subject: Re: [PATCH] android/client/if-bt.c: Fix memory leak while using realloc()

Hi Atul,

On Tuesday 21 of July 2015 17:31:44 Atul Rai wrote:
> ping

I looks like I never got the original patch.

> > ------- Original Message -------
> > Sender : Atul Kumar Rai<[email protected]> Lead Engineer (1)/SRI-Delhi-SWC
> > Group/Samsung Electronics Date : Jun 23, 2015 13:38 (GMT+05:30)
> > Title : [PATCH] android/client/if-bt.c: Fix memory leak while using
> > realloc()

Patch prefix "android/client" should be enough.

> >
> > While reallocating space to store additional "remote device set" using
> > realloc, if realloc() fails, the original block is left untouched but
> > reference to that block is lost as NULL is assigned to remote_devices.
> > The original block needs to be freed before return.
> >
> > Signed-off-by: Atul Rai <[email protected]>

Please remove Signed-off-by as we don't use it for userspace patches.

> > ---
> >
> > android/client/if-bt.c | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/android/client/if-bt.c b/android/client/if-bt.c
> > index 4723024..4f0c2e9 100644
> > --- a/android/client/if-bt.c
> > +++ b/android/client/if-bt.c
> > @@ -94,6 +94,7 @@ static int remote_devices_capacity = 0;
> >
> > void add_remote_device(const bt_bdaddr_t *addr)
> > {
> >
> > int i;
> >
> > + bt_bdaddr_t *tmp = NULL;

No need to initialize it.

> >
> > if (remote_devices == NULL) {
> >
> > remote_devices = malloc(4 * sizeof(bt_bdaddr_t));
> >
> > @@ -119,9 +120,17 @@ void add_remote_device(const bt_bdaddr_t *addr)
> >
> > /* Realloc space if needed */
> > if (remote_devices_cnt >= remote_devices_capacity) {
> >
> > remote_devices_capacity *= 2;
> >
> > + /*
> > + * Save reference to previously allocated memory block so that
> > + * it can be freed in case realloc fails.
> > + */
> > + tmp = remote_devices;
> > +
> >
> > remote_devices = realloc(remote_devices, sizeof(bt_bdaddr_t) *
> >
> > remote_devices_capacity);
> >
> > if (remote_devices == NULL) {
> >
> > + if (NULL != tmp)
> > + free(tmp);

free() already checks for NULL pointer so no need to double check it.

> >
> > remote_devices_capacity = 0;
> > remote_devices_cnt = 0;
> > return;
>
> --
> 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

--
BR
Szymon Janc

2015-07-21 12:01:44

by Atul Kumar Rai

[permalink] [raw]
Subject: Re: [PATCH] android/client/if-bt.c: Fix memory leak while using realloc()

ping

> ------- Original Message -------
> Sender : Atul Kumar Rai<[email protected]> Lead Engineer (1)/SRI-Delhi-SWC Group/Samsung Electronics
> Date : Jun 23, 2015 13:38 (GMT+05:30)
> Title : [PATCH] android/client/if-bt.c: Fix memory leak while using realloc()
>
> While reallocating space to store additional "remote device set" using
> realloc, if realloc() fails, the original block is left untouched but
> reference to that block is lost as NULL is assigned to remote_devices.
> The original block needs to be freed before return.
>
> Signed-off-by: Atul Rai <[email protected]>
> ---
> android/client/if-bt.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/android/client/if-bt.c b/android/client/if-bt.c
> index 4723024..4f0c2e9 100644
> --- a/android/client/if-bt.c
> +++ b/android/client/if-bt.c
> @@ -94,6 +94,7 @@ static int remote_devices_capacity = 0;
> void add_remote_device(const bt_bdaddr_t *addr)
> {
> int i;
> + bt_bdaddr_t *tmp = NULL;
>
> if (remote_devices == NULL) {
> remote_devices = malloc(4 * sizeof(bt_bdaddr_t));
> @@ -119,9 +120,17 @@ void add_remote_device(const bt_bdaddr_t *addr)
> /* Realloc space if needed */
> if (remote_devices_cnt >= remote_devices_capacity) {
> remote_devices_capacity *= 2;
> + /*
> + * Save reference to previously allocated memory block so that
> + * it can be freed in case realloc fails.
> + */
> + tmp = remote_devices;
> +
> remote_devices = realloc(remote_devices, sizeof(bt_bdaddr_t) *
> remote_devices_capacity);
> if (remote_devices == NULL) {
> + if (NULL != tmp)
> + free(tmp);
> remote_devices_capacity = 0;
> remote_devices_cnt = 0;
> return;
> --
> 2.1.4
>