Return-Path: MIME-Version: 1.0 In-Reply-To: <449781348.237751431668266873.JavaMail.weblogic@ep2mlwas05d> References: <449781348.237751431668266873.JavaMail.weblogic@ep2mlwas05d> Date: Tue, 2 Jun 2015 13:58:18 +0530 Message-ID: Subject: Re: Re: [PATCH] Bluetooth: Fix potential NULL dereference From: Jaganath K To: Marcel Holtmann Cc: "linux-bluetooth@vger.kernel.org" Content-Type: multipart/alternative; boundary=f46d04428884758aff051784b99f List-ID: --f46d04428884758aff051784b99f Content-Type: text/plain; charset=UTF-8 Ping. On Fri, May 15, 2015 at 11:07 AM, JAGANATH KANAKKASSERY < jaganath.k@samsung.com> wrote: > Hi Marcel, > > >>>> addr can be NULL and it should not be dereferenced before NULL > checking. > >>>> > >>>> Signed-off-by: Jaganath Kanakkassery > >>>> --- > >> > > >> >if we start changing things here, then we better change the code into > something that all the other socket handling code is doing anyway>y. So do > the min comparison and copy the data into a local copy of the sockaddr_rc. > >>> > >>> And on a side note, I wonder if addr can actually be NULL. It might be > interesting to check the generic socket code if this really can happe>n if > you provide no address structure to the bind() system call or if this gets > filtered out by the core socket code. > >> > >> I checked generic socket code and it looks like addr will never be NULL > when user space calls bind. > >> But this can be called from kernel_bind() also which I think will never > be called for RFCOMM. > >> So this patch is not required? > > > >that is what I thought. However converting it to the same handling using > min and copying into local storage might be a good idea. The more pieces in > HCI, L2CAP, SCO and RFCOMM sockets that are similar, the better. > > I have raised v1 with the changes you suggested, Plz check it. > > Thanks, > Jaganath --f46d04428884758aff051784b99f Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable

Ping.

On Fri, May 15, 2015 at= 11:07 AM, JAGANATH KANAKKASSERY <jaganath.k@samsung.com> wrote:

Hi Marcel,

>>>> addr can be NULL and it should not be dereferenced before = NULL checking.
>>>>
>>>> Signed-off-by: Jaganath Kanakkassery <jaganath.k@samsung.com>
>>>> ---
>> >
>> >if we start changing things here, then we better change= the code into something that all the other socket handling code is doing a= nyway>y. So do the min comparison and copy the data into a local copy of= the sockaddr_rc.
>>>
>>> And on a side note, I wonder if addr can actually be NULL. It = might be interesting to check the generic socket code if this really can ha= ppe>n if you provide no address structure to the bind() system call or i= f this gets filtered out by the core socket code.
>>
>> I checked generic socket code and it looks like addr will never be= NULL when user space calls bind.
>> But this can be called from kernel_bind() also which I think will = never be called for RFCOMM.
>> So this patch is not required?
>
>that is what I thought. However converting it to the same handling usin= g min and copying into local storage might be a good idea. The more pieces = in HCI, L2CAP, SCO and RFCOMM sockets that are similar, the better.

I have raised v1 with the changes you suggested, Plz check it.

Thanks,
Jaganath

--f46d04428884758aff051784b99f--