RE: [PATCH v2 06/16] igb: Proactively round up to kmalloc bucket size
From: Ruhl, Michael J <hidden>
Date: 2022-09-26 16:55:38
Also in:
dri-devel, intel-wired-lan, linux-btrfs, linux-fsdevel, linux-hardening, linux-media, linux-mm, lkml, llvm
quoted hunk ↗ jump to hunk
-----Original Message----- From: Kees Cook <redacted> Sent: Friday, September 23, 2022 4:28 PM To: Vlastimil Babka <redacted> Cc: Kees Cook <redacted>; Brandeburg, Jesse [off-list ref]; Nguyen, Anthony L [off-list ref]; David S. Miller [off-list ref]; Eric Dumazet [off-list ref]; Jakub Kicinski [off-list ref]; Paolo Abeni [off-list ref]; intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; Ruhl, Michael J [off-list ref]; Hyeonggon Yoo [off-list ref]; Christoph Lameter [off-list ref]; Pekka Enberg [off-list ref]; David Rientjes [off-list ref]; Joonsoo Kim [off-list ref]; Andrew Morton [off-list ref]; Greg Kroah-Hartman [off-list ref]; Nick Desaulniers [off-list ref]; Alex Elder [off-list ref]; Josef Bacik [off-list ref]; David Sterba [off-list ref]; Sumit Semwal [off-list ref]; Christian König [off-list ref]; Daniel Micay [off-list ref]; Yonghong Song [off-list ref]; Marco Elver [off-list ref]; Miguel Ojeda [off-list ref]; linux- kernel@vger.kernel.org; linux-mm@kvack.org; linux-btrfs@vger.kernel.org; linux-media@vger.kernel.org; dri-devel@lists.freedesktop.org; linaro-mm- sig@lists.linaro.org; linux-fsdevel@vger.kernel.org; dev@openvswitch.org; x86@kernel.org; llvm@lists.linux.dev; linux-hardening@vger.kernel.org Subject: [PATCH v2 06/16] igb: Proactively round up to kmalloc bucket size In preparation for removing the "silently change allocation size" users of ksize(), explicitly round up all q_vector allocations so that allocations can be correctly compared to ksize(). Additionally fix potential use-after-free in the case of new allocation failure: only free memory if the replacement allocation succeeds. Cc: Jesse Brandeburg <redacted> 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 <redacted> --- drivers/net/ethernet/intel/igb/igb_main.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)diff --git a/drivers/net/ethernet/intel/igb/igb_main.cb/drivers/net/ethernet/intel/igb/igb_main.c index 2796e81d2726..eb51e531c096 100644--- a/drivers/net/ethernet/intel/igb/igb_main.c +++ b/drivers/net/ethernet/intel/igb/igb_main.c@@ -1195,15 +1195,16 @@ static int igb_alloc_q_vector(struct igb_adapter*adapter, return -ENOMEM; ring_count = txr_count + rxr_count; - size = struct_size(q_vector, ring, ring_count); + size = kmalloc_size_roundup(struct_size(q_vector, ring, ring_count));
This looks good to me...
/* allocate q_vector and rings */
q_vector = adapter->q_vector[v_idx];
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);
+ if (q_vector)
+ kfree_rcu(q_vector, rcu);Even though this is in the ksize part, this seems like an unrelated change? Should this be in a different patch? Also, the kfree_rcu will free q_vector after the RCU grace period? Is that what you want to do? How does rcu distinguish between the original q_vector, and the newly kzalloced one? Thanks, Mike
} else {
memset(q_vector, 0, size);
}
--
2.34.1