Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   "Ruhl, Michael J" <michael.j.ruhl@intel.com>
To:     Kees Cook <keescook@chromium.org>,
        "Ruhl@www.outflux.net" <Ruhl@www.outflux.net>
CC:     "Brandeburg, Jesse" <jesse.brandeburg@intel.com>,
        "Nguyen, Anthony L" <anthony.l.nguyen@intel.com>,
        "David S. Miller" <davem@davemloft.net>,
        "Eric Dumazet" <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        "intel-wired-lan@lists.osuosl.org" <intel-wired-lan@lists.osuosl.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-hardening@vger.kernel.org" <linux-hardening@vger.kernel.org>
Subject: RE: [PATCH v3 1/2] igb: Do not free q_vector unless new one was
 allocated
Thread-Topic: [PATCH v3 1/2] igb: Do not free q_vector unless new one was
 allocated
Thread-Index: AQHY4tOhXsEwHafDKkupYOAMeXei0K4UEdiQ
Date:   Tue, 18 Oct 2022 12:20:43 +0000
Message-ID: <DM5PR11MB13247434B4473C63898A35A7C1289@DM5PR11MB1324.namprd11.prod.outlook.com>
References: <20221018092340.never.556-kees@kernel.org>
 <20221018092526.4035344-1-keescook@chromium.org>
In-Reply-To: <20221018092526.4035344-1-keescook@chromium.org>
Accept-Language: en-US
Content-Language: en-US
dlp-product: dlpe-windows
dlp-reaction: no-action
dlp-version: 11.6.500.17
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?tsescKqBU+bngQry9lmDVrkjXq9eCFadvWoVo3K29+tt6bZFDnT33UzLlhqt?=
 =?us-ascii?Q?TqidxAkzighLn6tWMZfP1TEpzFc/vltVjUS8NisLNc06GQNebKwURyWoAKXJ?=
 =?us-ascii?Q?sqVtVREPMQS6Rb6G/ATJdXhtix/NHO+eje4MsscebUJW0/tkc05rYTbCy0u/?=
 =?us-ascii?Q?NS3WveSabcuOJq00j/4Xv5MdPfotdWKWFLM61RbY7bh9SP3A6kWFNjmBrQIw?=
 =?us-ascii?Q?g7SJVOwYK9Xkvg0hoqlSyrxM/ipORjdq5HTVyJNYFJkZqs3gTf6D0uDatK0J?=
 =?us-ascii?Q?EoQozcqjxYR05cBN5SbxCcm38A3ImTAYVuR5GdEbM0cZpfrVaRNCUPF9ARQV?=
 =?us-ascii?Q?Pc1H8i7CfKEoqLJeRvIRLAMG/6yAa4+5NKzi1wf/vwscUOHU0uBb6GJFoexj?=
 =?us-ascii?Q?+I2SmbMPk8YCAtVysRUlUN+wD5U+XPo6r8D52iy6WFbLOVl25X+aN9ULLs4I?=
 =?us-ascii?Q?Syn0cqKEAN4gNcwZKEBm65WOFSuoGmJdItiF9YXph/zL3H5Orj/9+LuLIT4R?=
 =?us-ascii?Q?GpPx0Pd2eoB8P9OgmuMN0x0fJICeLPH3XTBnFWXMUkEqwkFYOR76NxNidfQh?=
 =?us-ascii?Q?+nUUMe8KW3MWSez/DRcbQ2oY0KNul1yViCHQn/UMj63HnLHYt6Vuik/Yd/lZ?=
 =?us-ascii?Q?3ecuLv23WDpBJMQHlDlj+x7qQfxLjmUC41fJnUqE/M5Lgl+2kYud8dc6p7Z8?=
 =?us-ascii?Q?Ys0tkQIVGZkKwoPyNEaduRzoTES2yOa9mGkafa7CwrC4+3Bg5ulRYajNdBFb?=
 =?us-ascii?Q?oPZwgOzKS8dqq8zddhYOfFRRSVRoGPFjKPzpT3X7id8/fJF534OElGPR7zEy?=
 =?us-ascii?Q?G9mN/NkFub1d9ZPAkI/C3ZzePd3zF+oEz/jSnrMLgNK/YCeCA/509++NduMR?=
 =?us-ascii?Q?zAdmSM7GvhLAVxPjkf5dM+WBcMGWgspyJ7kuEdJojlDOK8XTUWnlBtqvs/6Z?=
 =?us-ascii?Q?YFRjrZYoQ/w4BBvhBMrLbsGYC02De0H93pnvku40z2+J4/zG6fqoz3BdDDTM?=
 =?us-ascii?Q?RLDrUwe7ZC14ZcbLEailjw9cJFNUCK6BjVqikGYCth0/0Bnq52L3XBXrFr9k?=
 =?us-ascii?Q?l0/8v30bl9auYc49vQj/4Pivb7QUUGN2JJnbadAOfL5WckqUjmNcCQT0AJp8?=
 =?us-ascii?Q?NhRbzMTO4VnDX3QHRRPW6nrs9fSgm9YfIre1e1kdB/LjewTEN7QNN5uPxBvW?=
 =?us-ascii?Q?yC+E6kjSBYRfaCuv13M//nJ+u4ssCDybsi6StL48Y0c3EWfdoM72+/KeWVFu?=
 =?us-ascii?Q?pJ+TzUJQd5td0WpV2a5cyxtHPVPViFYkuvKfly97ArJmUOgrKNYUywSKFx3B?=
 =?us-ascii?Q?kxcDBa/awMKfziqKWQ+20c7lAEydNEvVsTCjYTNkp2CDmJgO5k1sRoXnO4MY?=
 =?us-ascii?Q?g0ucl8hOKH2iALka/p0fG9XqDWC0SzmlvqdkGYKTTxI6GPRoZIcWqUW0rPvK?=
 =?us-ascii?Q?uvk9pJQYIfGxOJCCe+4qlGENmu3LGHa3/3nSbfRJ+TvfDaQV0BgdVtXsBDuX?=
 =?us-ascii?Q?BpMBd82A0vYGbeR2/VhI9MN85ZWqS3R3lvYJxzACw14v4l1k7q+SZZT6R4l3?=
 =?us-ascii?Q?nduRh+p0k5Ub4K5/DSkNRGFVJD6zhJmImW+pg3GR?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DM5PR11MB1324.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 5a3ce4e5-8933-4009-abc7-08dab1032d91
X-MS-Exchange-CrossTenant-originalarrivaltime: 18 Oct 2022 12:20:43.3729
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: rf6sRZxBY4dwng8txisCZGHbqueKr/eDdXgLQxoG2mQTSN0qQ+NG/B3lGHzvIKVEitEFq204oSHDFEwpkUMdlx7cG93sSyXpyd5ghiEPbao=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB7181
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.7 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

>-----Original Message-----
>From: Kees Cook <keescook@chromium.org>
>Sent: Tuesday, October 18, 2022 5:25 AM
>To: Ruhl@www.outflux.net; Ruhl, Michael J <michael.j.ruhl@intel.com>
>Cc: Kees Cook <keescook@chromium.org>; Brandeburg, Jesse
><jesse.brandeburg@intel.com>; Nguyen, Anthony L
><anthony.l.nguyen@intel.com>; David S. Miller <davem@davemloft.net>;
>Eric Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>;
>Paolo Abeni <pabeni@redhat.com>; intel-wired-lan@lists.osuosl.org;
>netdev@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
>hardening@vger.kernel.org
>Subject: [PATCH v3 1/2] igb: Do not free q_vector unless new one was
>allocated
>
>Avoid potential use-after-free condition under memory pressure. If the
>kzalloc() fails, q_vector will be freed but left in the original
>adapter->q_vector[v_idx] array position.
>
>Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
>Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
>Cc: "David S. Miller" <davem@davemloft.net>
>Cc: Eric Dumazet <edumazet@google.com>
>Cc: Jakub Kicinski <kuba@kernel.org>
>Cc: Paolo Abeni <pabeni@redhat.com>
>Cc: intel-wired-lan@lists.osuosl.org
>Cc: netdev@vger.kernel.org
>Signed-off-by: Kees Cook <keescook@chromium.org>
>---
> drivers/net/ethernet/intel/igb/igb_main.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/net/ethernet/intel/igb/igb_main.c
>b/drivers/net/ethernet/intel/igb/igb_main.c
>index f8e32833226c..6256855d0f62 100644
>--- a/drivers/net/ethernet/intel/igb/igb_main.c
>+++ b/drivers/net/ethernet/intel/igb/igb_main.c
>@@ -1202,8 +1202,12 @@ static int igb_alloc_q_vector(struct igb_adapter
>*adapter,
> 	if (!q_vector) {
> 		q_vector =3D kzalloc(size, GFP_KERNEL);
> 	} else if (size > ksize(q_vector)) {
>-		kfree_rcu(q_vector, rcu);
>-		q_vector =3D kzalloc(size, GFP_KERNEL);
>+		struct igb_q_vector *new_q_vector;
>+
>+		new_q_vector =3D kzalloc(size, GFP_KERNEL);
>+		if (new_q_vector)
>+			kfree_rcu(q_vector, rcu);
>+		q_vector =3D new_q_vector;

Ok, that makes more sense to me.

Reviewed-by: Michael J. Ruhl <michael.j.ruhl@intel.com>

Mike


> 	} else {
> 		memset(q_vector, 0, size);
> 	}
>--
>2.34.1