Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <9619692082caef649e236eb154129e922c46b547.camel@sipsolutions.net>
Subject: Re: [PATCH] wifi: iwlwifi: dvm: Fix memcpy: detected field-spanning
 write backtrace
From:   Johannes Berg <johannes@sipsolutions.net>
To:     Hans de Goede <hdegoede@redhat.com>,
        Gregory Greenman <gregory.greenman@intel.com>,
        Kalle Valo <kvalo@kernel.org>
Cc:     linux-wireless@vger.kernel.org, Kees Cook <keescook@chromium.org>
Date:   Tue, 18 Apr 2023 16:43:18 +0200
In-Reply-To: <20230418132546.45297-1-hdegoede@redhat.com>
References: <20230418132546.45297-1-hdegoede@redhat.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.46.4 (3.46.4-1.fc37) 
MIME-Version: 1.0
Precedence: bulk

On Tue, 2023-04-18 at 15:25 +0200, Hans de Goede wrote:
> A received TKIP key may be up to 32 bytes because it may contain
> MIC rx/tx keys too. These are not used by iwl and copying these
> over overflows the iwl_keyinfo.key field.

Thanks for doing (and more importantly testing) this :)

> -		memcpy(sta_cmd.key.key, keyconf->key, keyconf->keylen);
> +		/* keyconf may contain MIC rx/tx keys which iwl does not use */
> +		to_copy =3D min_t(size_t, sizeof(sta_cmd.key.key), keyconf->keylen);
> +		memcpy(sta_cmd.key.key, keyconf->key, to_copy);
>=20

I'd kind of argue that just hardcoding 16 is fine here, keylen _looks_
variable so the compiler can't optimize it away, but in reality keylen
always must be 32 and sizeof() must be always 16 :)

But it also really doesn't matter.

Reviewed-by: Johannes Berg <johannes@sipsolutions.net>


Should we Cc stable on this?

johannes