From: Brian Norris briannorris@chromium.org Sent: Friday, December 15, 2023 2:52 AM To: David Lin yu-hao.lin@nxp.com Cc: Francesco Dolcini francesco@dolcini.it; linux-wireless@vger.kernel.org; linux-kernel@vger.kernel.org; kvalo@kernel.org; Pete Hsieh tsung-hsien.hsieh@nxp.com; stable@vger.kernel.org Subject: Re: [EXT] Re: [PATCH v2] wifi: mwifiex: fix STA cannot connect to AP
Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button
On Thu, Dec 14, 2023 at 3:38 AM David Lin yu-hao.lin@nxp.com wrote:
From: Francesco Dolcini francesco@dolcini.it
On Thu, Dec 14, 2023 at 02:22:57AM +0000, David Lin wrote:
From: Brian Norris briannorris@chromium.org It probably wouldn't hurt to significantly write much of this driver, but at a minimum, we could probably use a few checks like this:
cmd_size += sizeof(struct host_cmd_tlv_mac_addr); if (cmd_size > MWIFIEX_SIZE_OF_CMD_BUFFER) return -1; // Only touch tlv *after* the bounds check.
That doesn't need to block this patch, of course.
Brian
I will modify the code for next patch.
I would suggest not modify this in this patch, we should fix all the code that is subjected to this potential issue.
I would personally do a follow-up patch just to add the check to avoid overflowing the cmd buffer everywhere it is used.
Right, there's tons of code that could potentially be affected, and this is definitely a separate patch. (Your feature only adds on to the existing issue, so these are separate logical changes.)
O.K. I will only change commit message. In fact, this TLV command is added
as the first one command.
Well, it doesn't really matter than your TLV is "first" -- if there's an overflow, there's an overflow. Maybe the 8 bytes you're adding here are the necessary tipping point. I don't know without doing some kind of informal mathematics/proof.
Brian
Understood. Thanks.