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 <[email protected]>
Cc: Tony Nguyen <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Eric Dumazet <[email protected]>
Cc: Jakub Kicinski <[email protected]>
Cc: Paolo Abeni <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Kees Cook <[email protected]>
---
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 = kzalloc(size, GFP_KERNEL);
} else if (size > ksize(q_vector)) {
- kfree_rcu(q_vector, rcu);
- q_vector = kzalloc(size, GFP_KERNEL);
+ struct igb_q_vector *new_q_vector;
+
+ new_q_vector = kzalloc(size, GFP_KERNEL);
+ if (new_q_vector)
+ kfree_rcu(q_vector, rcu);
+ q_vector = new_q_vector;
} else {
memset(q_vector, 0, size);
}
--
2.34.1
>-----Original Message-----
>From: Kees Cook <[email protected]>
>Sent: Tuesday, October 18, 2022 5:25 AM
>To: Ruhl@http://www.outflux.net; Ruhl, Michael J <[email protected]>
>Cc: Kees Cook <[email protected]>; Brandeburg, Jesse
><[email protected]>; Nguyen, Anthony L
><[email protected]>; David S. Miller <[email protected]>;
>Eric Dumazet <[email protected]>; Jakub Kicinski <[email protected]>;
>Paolo Abeni <[email protected]>; [email protected];
>[email protected]; [email protected]; linux-
>[email protected]
>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 <[email protected]>
>Cc: Tony Nguyen <[email protected]>
>Cc: "David S. Miller" <[email protected]>
>Cc: Eric Dumazet <[email protected]>
>Cc: Jakub Kicinski <[email protected]>
>Cc: Paolo Abeni <[email protected]>
>Cc: [email protected]
>Cc: [email protected]
>Signed-off-by: Kees Cook <[email protected]>
>---
> 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 = kzalloc(size, GFP_KERNEL);
> } else if (size > ksize(q_vector)) {
>- kfree_rcu(q_vector, rcu);
>- q_vector = kzalloc(size, GFP_KERNEL);
>+ struct igb_q_vector *new_q_vector;
>+
>+ new_q_vector = kzalloc(size, GFP_KERNEL);
>+ if (new_q_vector)
>+ kfree_rcu(q_vector, rcu);
>+ q_vector = new_q_vector;
Ok, that makes more sense to me.
Reviewed-by: Michael J. Ruhl <[email protected]>
Mike
> } else {
> memset(q_vector, 0, size);
> }
>--
>2.34.1
> -----Original Message-----
> From: Intel-wired-lan <[email protected]> On Behalf Of
> Kees Cook
> Sent: Tuesday, October 18, 2022 2:55 PM
> To: Ruhl@http://www.outflux.net; Ruhl, Michael J <[email protected]>
> Cc: Kees Cook <[email protected]>; [email protected];
> [email protected]; Eric Dumazet <[email protected]>;
> [email protected]; [email protected]; Jakub Kicinski
> <[email protected]>; Paolo Abeni <[email protected]>; David S. Miller
> <[email protected]>
> Subject: [Intel-wired-lan] [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 <[email protected]>
> Cc: Tony Nguyen <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Cc: Eric Dumazet <[email protected]>
> Cc: Jakub Kicinski <[email protected]>
> Cc: Paolo Abeni <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Kees Cook <[email protected]>
> ---
> drivers/net/ethernet/intel/igb/igb_main.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
Tested-by: Gurucharan <[email protected]> (A Contingent worker at Intel)