Received-SPF: pass (google.com: best guess record for domain of linux-wireless-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Content-Type: text/plain;
        charset=gb2312
Mime-Version: 1.0 (Mac OS X Mail 11.3 \(3445.6.18\))
Subject: Re: [EXT] [PATCH] mwifiex: Fix heap overflow in
 mmwifiex_process_tdls_action_frame()
From:   qize wang <wangqize888888888@gmail.com>
In-Reply-To: <1ED930FE-1360-4E0F-B770-244D02778411@gmail.com>
Date:   Tue, 26 Nov 2019 00:04:32 +0800
Cc:     Ganapathi Bhat <gbhat@marvell.com>,
        "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
        amitkarwar <amitkarwar@gmail.com>,
        nishants <nishants@marvell.com>,
        huxinming820 <huxinming820@gmail.com>,
        kvalo <kvalo@codeaurora.org>, Greg KH <greg@kroah.com>,
        security <security@kernel.org>,
        linux-distros <linux-distros@vs.openwall.org>,
        Solar Designer <solar@openwall.com>
Content-Transfer-Encoding: quoted-printable
Message-Id: <4C693FCA-3AFF-47C1-97CD-563C9219F4D0@gmail.com>
References: <E40E893E-D9B4-4C63-8139-1DD5E1C2CECB@gmail.com>
 <20191122111339.GH617@kadam>
 <MN2PR18MB26373BC0C0D63A7C3CE09FC4A0490@MN2PR18MB2637.namprd18.prod.outlook.com>
 <20191122123739.GJ617@kadam> <1ED930FE-1360-4E0F-B770-244D02778411@gmail.com>
To:     Dan Carpenter <dan.carpenter@oracle.com>
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

This fix addresses CVE-2019-14901

> =D4=DA 2019=C4=EA11=D4=C222=C8=D5=A3=AC=CF=C2=CE=E710:27=A3=ACqize =
wang <wangqize888888888@gmail.com> =D0=B4=B5=C0=A3=BA
>=20
> Hi, dan
>=20
> I forget to explain  the reason of changing "pos" to "pos + 2=A1=B1 in =
the commit message.
> Thank you for your suggestion.
> IE is TLV struct, but ht_cap and  ht_oper aren=A1=AFt TLV struct,the =
origin marvell driver code is wrong.=20
> we fix the bug by changing pos(the address of IE) to  pos+2 ( the =
address of IE=A1=AFs value ).
>=20
> regards,
> qize wang
>=20
>> =D4=DA 2019=C4=EA11=D4=C222=C8=D5=A3=AC=CF=C2=CE=E78:37=A3=ACDan =
Carpenter <dan.carpenter@oracle.com> =D0=B4=B5=C0=A3=BA
>>=20
>> On Fri, Nov 22, 2019 at 11:40:40AM +0000, Ganapathi Bhat wrote:
>>> Hi Dan,
>>>=20
>>>>> +			/* copy the ie's value into ht_capb*/
>>>>> +			memcpy((u8 *)&sta_ptr->tdls_cap.ht_capb, pos + =
2,
>>>>                                                                =
^^^^^^^
>>>>=20
>>>> I don't understand why we changed "pos" to "pos + 2".  Presumably =
there is
>>>> a reason, but it needs to explained in the commit message.
>>>=20
>>> I think, we were doing wrong in the original code. We are supposed =
to use 'pos + 2' itself, instead of just 'pos'. This is because, 'pos' =
is pointing to 'ieee_types_header', followed by the actual data and the =
destination do not start with (i.e. it do not contain) =
'ieee_types_header'(ex: 'sta_ptr->tdls_cap.ht_oper').
>>>=20
>>> Also, there are few places were the destination starts with =
'ieee_types_header'(ex: 'sta_ptr->tdls_cap.extcap'), which need just =
'pos'.
>>=20
>> I assumed it was something like this but it needs to be explained in
>> the commit message.
>>=20
>> regards,
>> dan carpenter
>>=20
>=20