From: Maximilian Luz <luzmaximilian@gmail.com>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Amitkumar Karwar <amitkarwar@gmail.com>,
Ganapathi Bhat <ganapathi.bhat@nxp.com>,
Xinming Hu <huxinming820@gmail.com>,
Kalle Valo <kvalo@codeaurora.org>,
"David S. Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>,
linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Kaloyan Nikolov <konik98@gmail.com>,
Brian Norris <briannorris@chromium.org>
Subject: Re: [PATCH net] mwifiex: Increase AES key storage size to 256 bits
Date: Tue, 25 Aug 2020 22:17:10 +0200 [thread overview]
Message-ID: <54a785f9-d49e-354b-1430-8a854b4d36c5@gmail.com> (raw)
In-Reply-To: <20200825185151.GV5493@kadam>
On 8/25/20 8:51 PM, Dan Carpenter wrote:
> I sort of feel like the code was broken before I added the bounds
> checking but it's also okay if the Fixes tag points to my change as
> well just to make backporting easier.
I'd argue the same. Any critical out-of-bounds access was just never
discovered (at least for 256 bit keys) due to the struct containing the
key being a union member, as Brian observed.
> Another question would be if it would be better to move the bounds
> check after the "if (key_v2->key_param_set.key_type != KEY_TYPE_ID_AES)"
> check? Do we care if the length is invalid on the other paths?
Given that I have pretty much no knowledge of the driver, I
unfortunately can't answer this. But I agree that this should be looked
at. I think this has the potential to work now (as long as the maximum
key size is 256 bit) but break in the future if the maximum key size
ever gets larger and the check excludes some key types that would be
valid in this context (if there are even any?).
Regards,
Max
next prev parent reply other threads:[~2020-08-25 20:17 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-25 15:38 Maximilian Luz
2020-08-25 18:51 ` Dan Carpenter
2020-08-25 20:17 ` Maximilian Luz [this message]
2020-08-25 19:30 ` Brian Norris
2020-08-25 20:18 ` Maximilian Luz
2020-08-27 8:02 ` Kalle Valo
2020-08-27 13:16 ` [net] " Kalle Valo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=54a785f9-d49e-354b-1430-8a854b4d36c5@gmail.com \
--to=luzmaximilian@gmail.com \
--cc=amitkarwar@gmail.com \
--cc=briannorris@chromium.org \
--cc=dan.carpenter@oracle.com \
--cc=davem@davemloft.net \
--cc=ganapathi.bhat@nxp.com \
--cc=huxinming820@gmail.com \
--cc=konik98@gmail.com \
--cc=kuba@kernel.org \
--cc=kvalo@codeaurora.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®