2015-05-29 11:48:10

by Bharat Bhusan Panda

[permalink] [raw]
Subject: [PATCH ] unit: Use g_memdup replacing g_malloc0+memcpy

Replace use of g_malloc0+memcpy with g_memdup.
---
unit/test-gattrib.c | 3 +--
unit/test-sdp.c | 4 +---
2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/unit/test-gattrib.c b/unit/test-gattrib.c
index 416e596..d9c52d7 100644
--- a/unit/test-gattrib.c
+++ b/unit/test-gattrib.c
@@ -224,8 +224,7 @@ static void result_canary(guint8 status, const guint8 *pdu, guint16 len,
struct result_data *result = data;

result->status = status;
- result->pdu = g_malloc0(len);
- memcpy(result->pdu, pdu, len);
+ result->pdu = g_memdup(pdu, len);
result->len = len;

if (g_test_verbose())
diff --git a/unit/test-sdp.c b/unit/test-sdp.c
index b4ef4d1..58d90f8 100644
--- a/unit/test-sdp.c
+++ b/unit/test-sdp.c
@@ -189,9 +189,7 @@ static gboolean send_pdu(gpointer user_data)

pdu_len = req_pdu->raw_size + context->cont_size;

- buf = g_malloc0(pdu_len);
-
- memcpy(buf, req_pdu->raw_data, req_pdu->raw_size);
+ buf = g_memdup(req_pdu->raw_data, req_pdu->raw_size);

if (context->cont_size > 0)
memcpy(buf + req_pdu->raw_size, context->cont_data,
--
1.9.1



2015-05-29 15:25:56

by Szymon Janc

[permalink] [raw]
Subject: Re: [PATCH ] unit: Use g_memdup replacing g_malloc0+memcpy

Hi Bharat,

On Friday 29 of May 2015 20:26:40 Bharat Bhusan Panda wrote:
> Hi Szymon,
>
> > -----Original Message-----
> > From: [email protected] [mailto:linux-bluetooth-
> > [email protected]] On Behalf Of Szymon Janc
> > Sent: Friday, May 29, 2015 8:10 PM
> > To: Bharat Panda
> > Cc: [email protected]; [email protected]
> > Subject: Re: [PATCH ] unit: Use g_memdup replacing g_malloc0+memcpy
> >
> > Hi Bharat,
> >
> > On Friday 29 of May 2015 17:18:10 Bharat Panda wrote:
> > > Replace use of g_malloc0+memcpy with g_memdup.
> > > ---
> > >
> > > unit/test-gattrib.c | 3 +--
> > > unit/test-sdp.c | 4 +---
> > > 2 files changed, 2 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/unit/test-gattrib.c b/unit/test-gattrib.c index
> > > 416e596..d9c52d7 100644
> > > --- a/unit/test-gattrib.c
> > > +++ b/unit/test-gattrib.c
> > > @@ -224,8 +224,7 @@ static void result_canary(guint8 status, const
> > > guint8 *pdu, guint16 len, struct result_data *result = data;
> > >
> > > result->status = status;
> > >
> > > - result->pdu = g_malloc0(len);
> > > - memcpy(result->pdu, pdu, len);
> > > + result->pdu = g_memdup(pdu, len);
> > >
> > > result->len = len;
> > >
> > > if (g_test_verbose())
> > >
> > > diff --git a/unit/test-sdp.c b/unit/test-sdp.c index b4ef4d1..58d90f8
> > > 100644
> > > --- a/unit/test-sdp.c
> > > +++ b/unit/test-sdp.c
> > > @@ -189,9 +189,7 @@ static gboolean send_pdu(gpointer user_data)
> > >
> > > pdu_len = req_pdu->raw_size + context->cont_size;
> > >
> > > - buf = g_malloc0(pdu_len);
> > > -
> > > - memcpy(buf, req_pdu->raw_data, req_pdu->raw_size);
> > > + buf = g_memdup(req_pdu->raw_data, req_pdu->raw_size);
> >
> > This is not correct. If context->cont_size > 0 then you write after
>
> allocated
>
> > memory (see line below).
>
> Below line is to copy the context->cont_data after req_pdu->raw_size length
> of req_pdu->raw_data.
> But in above lines, it was allocating pdu_len size then doing memcpy of
> req_pdu->raw_data, which was replaced by the g_memdup.

Yes. So instead of allocating pdu_len bytes you allocate only
req_pdu->raw_size bytes. This is not enough if context->cont_size is greater
than zero. You have second memcpy just line below.

> > > if (context->cont_size > 0)
> > >
> > > memcpy(buf + req_pdu->raw_size, context->cont_data,

This will write after allocated memory.

>
> --
> Bharat
>
> --
> 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-05-29 14:56:40

by Bharat Bhusan Panda

[permalink] [raw]
Subject: RE: [PATCH ] unit: Use g_memdup replacing g_malloc0+memcpy

Hi Szymon,

> -----Original Message-----
> From: [email protected] [mailto:linux-bluetooth-
> [email protected]] On Behalf Of Szymon Janc
> Sent: Friday, May 29, 2015 8:10 PM
> To: Bharat Panda
> Cc: [email protected]; [email protected]
> Subject: Re: [PATCH ] unit: Use g_memdup replacing g_malloc0+memcpy
>
> Hi Bharat,
>
> On Friday 29 of May 2015 17:18:10 Bharat Panda wrote:
> > Replace use of g_malloc0+memcpy with g_memdup.
> > ---
> > unit/test-gattrib.c | 3 +--
> > unit/test-sdp.c | 4 +---
> > 2 files changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/unit/test-gattrib.c b/unit/test-gattrib.c index
> > 416e596..d9c52d7 100644
> > --- a/unit/test-gattrib.c
> > +++ b/unit/test-gattrib.c
> > @@ -224,8 +224,7 @@ static void result_canary(guint8 status, const
> > guint8 *pdu, guint16 len, struct result_data *result = data;
> >
> > result->status = status;
> > - result->pdu = g_malloc0(len);
> > - memcpy(result->pdu, pdu, len);
> > + result->pdu = g_memdup(pdu, len);
> > result->len = len;
> >
> > if (g_test_verbose())
> > diff --git a/unit/test-sdp.c b/unit/test-sdp.c index b4ef4d1..58d90f8
> > 100644
> > --- a/unit/test-sdp.c
> > +++ b/unit/test-sdp.c
> > @@ -189,9 +189,7 @@ static gboolean send_pdu(gpointer user_data)
> >
> > pdu_len = req_pdu->raw_size + context->cont_size;
> >
> > - buf = g_malloc0(pdu_len);
> > -
> > - memcpy(buf, req_pdu->raw_data, req_pdu->raw_size);
> > + buf = g_memdup(req_pdu->raw_data, req_pdu->raw_size);
>
> This is not correct. If context->cont_size > 0 then you write after
allocated
> memory (see line below).
Below line is to copy the context->cont_data after req_pdu->raw_size length
of req_pdu->raw_data.
But in above lines, it was allocating pdu_len size then doing memcpy of
req_pdu->raw_data, which was replaced by the g_memdup.
>
> > if (context->cont_size > 0)
> > memcpy(buf + req_pdu->raw_size, context->cont_data,
>
--
Bharat


2015-05-29 14:40:23

by Szymon Janc

[permalink] [raw]
Subject: Re: [PATCH ] unit: Use g_memdup replacing g_malloc0+memcpy

Hi Bharat,

On Friday 29 of May 2015 17:18:10 Bharat Panda wrote:
> Replace use of g_malloc0+memcpy with g_memdup.
> ---
> unit/test-gattrib.c | 3 +--
> unit/test-sdp.c | 4 +---
> 2 files changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/unit/test-gattrib.c b/unit/test-gattrib.c
> index 416e596..d9c52d7 100644
> --- a/unit/test-gattrib.c
> +++ b/unit/test-gattrib.c
> @@ -224,8 +224,7 @@ static void result_canary(guint8 status, const guint8
> *pdu, guint16 len, struct result_data *result = data;
>
> result->status = status;
> - result->pdu = g_malloc0(len);
> - memcpy(result->pdu, pdu, len);
> + result->pdu = g_memdup(pdu, len);
> result->len = len;
>
> if (g_test_verbose())
> diff --git a/unit/test-sdp.c b/unit/test-sdp.c
> index b4ef4d1..58d90f8 100644
> --- a/unit/test-sdp.c
> +++ b/unit/test-sdp.c
> @@ -189,9 +189,7 @@ static gboolean send_pdu(gpointer user_data)
>
> pdu_len = req_pdu->raw_size + context->cont_size;
>
> - buf = g_malloc0(pdu_len);
> -
> - memcpy(buf, req_pdu->raw_data, req_pdu->raw_size);
> + buf = g_memdup(req_pdu->raw_data, req_pdu->raw_size);

This is not correct. If context->cont_size > 0 then you write after allocated
memory (see line below).

> if (context->cont_size > 0)
> memcpy(buf + req_pdu->raw_size, context->cont_data,

--
BR
Szymon Janc

2015-06-01 10:39:00

by Bharat Bhusan Panda

[permalink] [raw]
Subject: RE: [PATCH ] unit: Use g_memdup replacing g_malloc0+memcpy

Hi Szymon,


> -----Original Message-----
> From: [email protected] [mailto:linux-bluetooth-
> [email protected]] On Behalf Of Szymon Janc
> Sent: Friday, May 29, 2015 8:56 PM
> To: Bharat Bhusan Panda
> Cc: [email protected]; [email protected]
> Subject: Re: [PATCH ] unit: Use g_memdup replacing g_malloc0+memcpy
>
> Hi Bharat,
>
> On Friday 29 of May 2015 20:26:40 Bharat Bhusan Panda wrote:
> > Hi Szymon,
> >
> > > -----Original Message-----
> > > From: [email protected] [mailto:linux-bluetooth-
> > > [email protected]] On Behalf Of Szymon Janc
> > > Sent: Friday, May 29, 2015 8:10 PM
> > > To: Bharat Panda
> > > Cc: [email protected]; [email protected]
> > > Subject: Re: [PATCH ] unit: Use g_memdup replacing g_malloc0+memcpy
> > >
> > > Hi Bharat,
> > >
> > > On Friday 29 of May 2015 17:18:10 Bharat Panda wrote:
> > > > Replace use of g_malloc0+memcpy with g_memdup.
> > > > ---
> > > >
> > > > unit/test-gattrib.c | 3 +--
> > > > unit/test-sdp.c | 4 +---
> > > > 2 files changed, 2 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/unit/test-gattrib.c b/unit/test-gattrib.c index
> > > > 416e596..d9c52d7 100644
> > > > --- a/unit/test-gattrib.c
> > > > +++ b/unit/test-gattrib.c
> > > > @@ -224,8 +224,7 @@ static void result_canary(guint8 status, const
> > > > guint8 *pdu, guint16 len, struct result_data *result = data;
> > > >
> > > > result->status = status;
> > > >
> > > > - result->pdu = g_malloc0(len);
> > > > - memcpy(result->pdu, pdu, len);
> > > > + result->pdu = g_memdup(pdu, len);
> > > >
> > > > result->len = len;
> > > >
> > > > if (g_test_verbose())
> > > >
> > > > diff --git a/unit/test-sdp.c b/unit/test-sdp.c index
> > > > b4ef4d1..58d90f8
> > > > 100644
> > > > --- a/unit/test-sdp.c
> > > > +++ b/unit/test-sdp.c
> > > > @@ -189,9 +189,7 @@ static gboolean send_pdu(gpointer user_data)
> > > >
> > > > pdu_len = req_pdu->raw_size + context->cont_size;
> > > >
> > > > - buf = g_malloc0(pdu_len);
> > > > -
> > > > - memcpy(buf, req_pdu->raw_data, req_pdu->raw_size);
> > > > + buf = g_memdup(req_pdu->raw_data, req_pdu->raw_size);
> > >
> > > This is not correct. If context->cont_size > 0 then you write after
> >
> > allocated
> >
> > > memory (see line below).
> >
> > Below line is to copy the context->cont_data after req_pdu->raw_size
> > length of req_pdu->raw_data.
> > But in above lines, it was allocating pdu_len size then doing memcpy
> > of req_pdu->raw_data, which was replaced by the g_memdup.
>
> Yes. So instead of allocating pdu_len bytes you allocate only req_pdu-
> >raw_size bytes. This is not enough if context->cont_size is greater than
> zero. You have second memcpy just line below.
>
> > > > if (context->cont_size > 0)
> > > >
> > > > memcpy(buf + req_pdu->raw_size, context->cont_data,
>
> This will write after allocated memory.

Yes correct. I have submitted v2 incorporating your comments.

Thanks,
Best Regards
Bharat