Re: [PATCH ] profiles: Fix crash due to NULL pointer access

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Bharat,

On Thu, Jun 05, 2014, bharat panda wrote:
> >> NULL pointer check is added after memory allocation
> >> to prevent core dump due to NULL pointer access.
> >>
> >> Signed-off-by: Bharat Panda <bharat.panda@xxxxxxxxxxx>
> >> ---
> >> profiles/audio/a2dp.c  |    8 ++++++++
> >> profiles/audio/avctp.c |    4 ++++
> >> profiles/audio/avdtp.c |   16 ++++++++++++++++
> >> profiles/health/hdp.c  |    4 +++-
> >> profiles/health/mcap.c |    2 ++
> >> 5 files changed, 33 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
> >> index c9dac9a..580cb60 100644
> >> --- a/profiles/audio/a2dp.c
> >> +++ b/profiles/audio/a2dp.c
> >> @@ -523,6 +523,8 @@ static gboolean endpoint_getcap_ind(struct avdtp *session,
> >>                                                       a2dp_sep->user_data);
> >>
> >>       codec_caps = g_malloc0(sizeof(*codec_caps) + length);
> >> +     if(!codec_caps)
> >> +             return -ENOMEM;
> >
> > the only way this can return NULL is when the size argument is 0. In all other cases it will abort the program.
> >
> In one of our a2dp connection test, we found it restarted bluetoothd,
> and failed to store the capabilities because of NULL pointer abort.
> Just to avoid same issue in other cases, I have added this check to
> ignore the abort.

It might be ok to add such checks to allocations where it's possible
that we pass a size 0 to g_malloc, however any other allocation GLib
will never give you NULL but simply call abort() if something fails.
Adding a NULL check doesn't help at all then since the program has
already aborted before g_malloc returns.

However, even for the cases where we might pass 0 usually a better fix
is to have a zero-check somewhere earlier in the function (e.g. if
you're allocating "length * sizeof(foo)" maybe length value 0 is an
invalid value to be passed to the function and should make it fail much
earlier.

Either way you'll need to fix up both of the patches you sent to follow
the above principles.

Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux