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;
DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 91C2160117
From:   Kalle Valo <kvalo@codeaurora.org>
To:     Ganapathi Bhat <gbhat@marvell.com>
Cc:     <linux-wireless@vger.kernel.org>, Cathy Luo <cluo@marvell.com>,
        Zhiyuan Yang <yangzy@marvell.com>,
        James Cao <jcao@marvell.com>,
        Rakesh Parmar <rakeshp@marvell.com>,
        Brian Norris <briannorris@chromium.org>,
        Sharvari Harisangam <sharvari@marvell.com>
Subject: Re: [PATCH 1/2] mwifiex: fix requesting zero memory for firmware dump
References: <1573622132-16181-1-git-send-email-gbhat@marvell.com>
Date:   Thu, 14 Nov 2019 17:18:10 +0200
In-Reply-To: <1573622132-16181-1-git-send-email-gbhat@marvell.com> (Ganapathi
        Bhat's message of "Wed, 13 Nov 2019 10:45:31 +0530")
Message-ID: <87v9rmbi6l.fsf@tynnyri.adurom.net>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

Ganapathi Bhat <gbhat@marvell.com> writes:

> From: Sharvari Harisangam <sharvari@marvell.com>
>
> mwifiex_pcie_fw_dump would read firmware scratch registers, to
> get the size of the dump. It does a vmalloc of memory_size + 1,
> read above, to save the dump. It is possible that the value read
> by  memory_size scratch register be invalid, i.e 0xffffffff. This
> would pass an invalid size(0) to vmalloc. To fix this check for
> invalid scratch register read.
>
> Signed-off-by: Sharvari Harisangam <sharvari@marvell.com>
> Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
> ---
>  drivers/net/wireless/marvell/mwifiex/pcie.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
> index fc1706d..483b521 100644
> --- a/drivers/net/wireless/marvell/mwifiex/pcie.c
> +++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
> @@ -2727,6 +2727,13 @@ static void mwifiex_pcie_fw_dump(struct mwifiex_adapter *adapter)
>  			break;
>  		}
>  
> +		if (memory_size == 0xffffffff) {
> +			mwifiex_dbg(adapter, ERROR,
> +				    "Invalid dump size: 0x%x, for %s\n",
> +				    memory_size, entry->mem_name);
> +			return;
> +		}
> +
>  		mwifiex_dbg(adapter, DUMP,
>  			    "%s_SIZE=0x%x\n", entry->mem_name, memory_size);
>  		entry->mem_ptr = vmalloc(memory_size + 1);

So 0xfffffffe would be a valid length for vmalloc()? I doubt that :) A
proper fix would be to add a reasonable maximum for memory_size and
return if it's anything bigger than the limit. Never trust the firmware.

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