Linux-Wireless Archive on lore.kernel.org
 help / color / Atom feed
* Re: [PATCH v2] mwifiex: Fix heap overflow in mmwifiex_process_tdls_action_frame()
       [not found] <CAGftXBHnkYt2KR=kqJfDhEqEuW52ckbepCmTnQQcDyDcVG0WZg@mail.gmail.com>
@ 2019-11-29  8:34 ` Greg KH
  2019-11-29  8:39   ` Kalle Valo
  0 siblings, 1 reply; 2+ messages in thread
From: Greg KH @ 2019-11-29  8:34 UTC (permalink / raw)
  To: qize wang
  Cc: linux-wireless, amitkarwar, nishants, gbhat, huxinming820, kvalo,
	security, linux-distros, dan.carpenter, Solar Designer

Some minor problems with your patch:

On Fri, Nov 29, 2019 at 04:18:21PM +0800, qize wang wrote:
> mwifiex_process_tdls_action_frame() without checking
> the incoming tdls infomation element's vality before use it,
> this may cause multi heap buffer overflows.
> 
> Fix them by putting vality check before use it.
> 
> IE is TLV struct, but ht_cap and  ht_oper aren’t TLV struct.
> the origin marvell driver code is wrong:
> 
> memcpy(&sta_ptr->tdls_cap.ht_oper, pos,....
> memcpy((u8 *)&sta_ptr->tdls_cap.ht_capb, pos,...
> 
> Fix the bug by changing pos(the address of IE) to
> pos+2 ( the address of IE’s value ).
> 
> Signed-off-by: wangqize <540263207@qq.com>

This has to match the name on the From: line.

> ---
> v2: change commit log
>  drivers/net/wireless/marvell/mwifiex/tdls.c | 70
> ++++++++++++++++++++++++++---
>  1 file changed, 64 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/wireless/marvell/mwifiex/tdls.c
> b/drivers/net/wireless/marvell/mwifiex/tdls.c
> index 09313047beed..7caf1d26124a 100644
> --- a/drivers/net/wireless/marvell/mwifiex/tdls.c
> +++ b/drivers/net/wireless/marvell/mwifiex/tdls.c
> @@ -953,59 +953,117 @@ void mwifiex_process_tdls_action_frame(struct
> mwifiex_private *priv,
> 
>   switch (*pos) {
>   case WLAN_EID_SUPP_RATES:
> + if (pos[1] > 32)
> + return;

All of your whitespace is totally damaged here, making this patch
impossible to apply :(

Please fix up your email client to not do that (you can just use 'git
send-email' directly) and resend a v3.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] mwifiex: Fix heap overflow in mmwifiex_process_tdls_action_frame()
  2019-11-29  8:34 ` [PATCH v2] mwifiex: Fix heap overflow in mmwifiex_process_tdls_action_frame() Greg KH
@ 2019-11-29  8:39   ` Kalle Valo
  0 siblings, 0 replies; 2+ messages in thread
From: Kalle Valo @ 2019-11-29  8:39 UTC (permalink / raw)
  To: Greg KH
  Cc: qize wang, linux-wireless, amitkarwar, nishants, gbhat,
	huxinming820, dan.carpenter, Solar Designer

(dropping security lists)

Greg KH <gregkh@linuxfoundation.org> writes:

> Some minor problems with your patch:
>
> On Fri, Nov 29, 2019 at 04:18:21PM +0800, qize wang wrote:
>> mwifiex_process_tdls_action_frame() without checking
>> the incoming tdls infomation element's vality before use it,
>> this may cause multi heap buffer overflows.
>> 
>> Fix them by putting vality check before use it.
>> 
>> IE is TLV struct, but ht_cap and  ht_oper aren’t TLV struct.
>> the origin marvell driver code is wrong:
>> 
>> memcpy(&sta_ptr->tdls_cap.ht_oper, pos,....
>> memcpy((u8 *)&sta_ptr->tdls_cap.ht_capb, pos,...
>> 
>> Fix the bug by changing pos(the address of IE) to
>> pos+2 ( the address of IE’s value ).
>> 
>> Signed-off-by: wangqize <540263207@qq.com>
>
> This has to match the name on the From: line.
>
>> ---
>> v2: change commit log
>>  drivers/net/wireless/marvell/mwifiex/tdls.c | 70
>> ++++++++++++++++++++++++++---
>>  1 file changed, 64 insertions(+), 6 deletions(-)
>> 
>> diff --git a/drivers/net/wireless/marvell/mwifiex/tdls.c
>> b/drivers/net/wireless/marvell/mwifiex/tdls.c
>> index 09313047beed..7caf1d26124a 100644
>> --- a/drivers/net/wireless/marvell/mwifiex/tdls.c
>> +++ b/drivers/net/wireless/marvell/mwifiex/tdls.c
>> @@ -953,59 +953,117 @@ void mwifiex_process_tdls_action_frame(struct
>> mwifiex_private *priv,
>> 
>>   switch (*pos) {
>>   case WLAN_EID_SUPP_RATES:
>> + if (pos[1] > 32)
>> + return;
>
> All of your whitespace is totally damaged here, making this patch
> impossible to apply :(

And even worse, it was using HTML :)

> Please fix up your email client to not do that (you can just use 'git
> send-email' directly) and resend a v3.

Yes, please. And even better if you try sending the patch to yourself
and then applying with git-am. That way you should notice any problems
with the mail settings.

More info in the link below, read it very carefully.

-- 
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, back to index

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CAGftXBHnkYt2KR=kqJfDhEqEuW52ckbepCmTnQQcDyDcVG0WZg@mail.gmail.com>
2019-11-29  8:34 ` [PATCH v2] mwifiex: Fix heap overflow in mmwifiex_process_tdls_action_frame() Greg KH
2019-11-29  8:39   ` Kalle Valo

Linux-Wireless Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-wireless/0 linux-wireless/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-wireless linux-wireless/ https://lore.kernel.org/linux-wireless \
		linux-wireless@vger.kernel.org
	public-inbox-index linux-wireless

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-wireless


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git