Re: [PATCH v2] tools/sdptool: Fix NULL pointer dereference

[Szymon Janc <szymon.janc@tieto.com>]

Hi,

On Tuesday 28 of July 2015 10:40:56 Johan Hedberg wrote:
> Hi Atul,
> 
> On Tue, Jul 28, 2015, Atul Rai wrote:
> > This patch fixes NULL pointer dereferences in case malloc fails
> > and returns NULL.
> > ---
> > 
> >  tools/sdptool.c | 37 +++++++++++++++++++++++++++++++++----
> >  1 file changed, 33 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tools/sdptool.c b/tools/sdptool.c
> > index 257964d..02e7f23 100644
> > --- a/tools/sdptool.c
> > +++ b/tools/sdptool.c
> > @@ -902,9 +902,9 @@ static int set_attribseq(sdp_session_t *session,
> > uint32_t handle, uint16_t attri> 
> >  	uint32_t range = 0x0000ffff;
> >  	sdp_record_t *rec;
> >  	sdp_data_t *pSequenceHolder = NULL;
> > 
> > -	void **dtdArray;
> > -	void **valueArray;
> > -	void **allocArray;
> > +	void **dtdArray = NULL;
> > +	void **valueArray = NULL;
> > +	void **allocArray = NULL;
> 
> This doesn't seem to be related to fixing missing malloc failure checks.
> It's also unnecessary since all of these either way get unconditionally
> assigned to before reading the values.

Those are due to 'goto cleanup' where all pointers are freed.
But we could make this code a bit simpler with:

foo = malloc();
bar = malloc();
if (!foo || !bar)
  goto cleanup;

Then initialization is not needed.

> 
> >  	/* Create arrays */
> >  	dtdArray = (void **)malloc(argc * sizeof(void *));
> 
> While you're at it could you (in a separate patch) fix all of these
> unnecessary typecasts of malloc return values?
> 
> Johan
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth"
> in the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
BR
Szymon Janc