From patchwork Thu Dec 8 00:02:13 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kees Cook X-Patchwork-Id: 31097 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:f944:0:0:0:0:0 with SMTP id q4csp471314wrr; Wed, 7 Dec 2022 16:05:23 -0800 (PST) X-Google-Smtp-Source: AA0mqf6Z+gsluoX7Vp7CFsAi0wJVn/6Nf7zn8u4oKLWOfLWKPpjnM7PJDjLhqGWv9k5lZbAJprng X-Received: by 2002:a17:906:fa98:b0:7c0:a8ff:3380 with SMTP id lt24-20020a170906fa9800b007c0a8ff3380mr25932542ejb.92.1670457923646; Wed, 07 Dec 2022 16:05:23 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1670457923; cv=none; d=google.com; s=arc-20160816; b=neFuSrh3JhlZbjNWxBl2L4PAVjSuX+Yx/Dk8YhHDqLvi5l9zhjQeNH88HLXVP5V3Ne G6gt146ojTm7XFNnj66G+ddaptlODOdkj6pvt1ePg+ZUiXfMEd4R92GQZCOHimCiYcZD lnhrGbLt2DT0XRhpeccrsXhoSdEE2SlvvdhQnhtqdjGj5TYcPkP6p+lqRef/N5gq4kX+ Zcs/aiuJsw/qxbPgcBHHvFsU0tMUEaUUPRK1F50lOL/N2Yz4zTdcd3f6c+dkKmvy1Xu5 BsLbXr3wXfGdqOhOdeItv8gMQL8UItm4crDypek5W0i5BALH+dAc3+S51gQmc8FAFlBY 1tEA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=dk1yAoyQU+RLCYzQPhP3++n5k0AlqyyhOlwfArVtCN8=; b=LaVpztBGKKzYnzxCA8mJcTgBbOYHeXAQuUzb/BdxQa5D0K4Heg1HyPkXIHAV/KWV03 s+o5DT2A2b3VZRHduuSQJziHx0wqLkpoxlq4YIwpEQsLMrypSjnaOLfjyViaWCVN4dt/ 7tuRdj69f3xetCPcmCglg7zjMNySgy4nPZYOWsagKKRKIb+BrZhZ6mbNVhmv+LjQMoOb 8+6VtuatGuz6HsmavZHmIiA2pP9SekS5ItMNeBHZ47hAVMKf0E30jz3XzMg8XAqUOyXb 7YexMKM0kUWc7Uy/xGO11MCnIhjBt8sCZOBqf4ZM8llUZZVks8e3CfyxbQjvzW82zD8E Y0nA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b=CfAuGzp9; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id cs10-20020a170906dc8a00b007c10ac8f9afsi5792928ejc.807.2022.12.07.16.04.36; Wed, 07 Dec 2022 16:05:23 -0800 (PST) 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; Authentication-Results: mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b=CfAuGzp9; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230042AbiLHACV (ORCPT + 99 others); Wed, 7 Dec 2022 19:02:21 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35284 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229915AbiLHACR (ORCPT ); Wed, 7 Dec 2022 19:02:17 -0500 Received: from mail-pj1-x102e.google.com (mail-pj1-x102e.google.com [IPv6:2607:f8b0:4864:20::102e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2AA518BD1E for ; Wed, 7 Dec 2022 16:02:16 -0800 (PST) Received: by mail-pj1-x102e.google.com with SMTP id k88-20020a17090a4ce100b00219d0b857bcso3302220pjh.1 for ; Wed, 07 Dec 2022 16:02:16 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=dk1yAoyQU+RLCYzQPhP3++n5k0AlqyyhOlwfArVtCN8=; b=CfAuGzp9X4iBKtpuQ05m/nLzqNjWLddivhcjBXfwQpOFO0jBdxJx2iBZedSmSZhQkq xUTzYrTLLf23grDjO0hf/+R8GsE4OfFz5vPRSFlyvyiRbq7JAN5B0OhF2O1LGbvKXpc2 dkD25p/EY7gml9MOqEmL3vlNPDCwS8NjoJx1w= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=dk1yAoyQU+RLCYzQPhP3++n5k0AlqyyhOlwfArVtCN8=; b=nfDoSo9VqZCP0IZK/hywZfYwemK89n3cfltuoAtbkIbJTvkvMOu1VrHTcYA19fiHEt unuxKUIOIApUkyL9ragMHO/bxXtrnyXfvFZWm7Dd1BCcyD0KVN4P8Unkp5+d2DpZQ+0d Ub4L/wwxhcE+JIigWeLFxWsnqYj//GUEE2QmcofJclrXhnVPveqmUQLnkF6Yy0fdgetG /GDJ0A2DyFbD5WhpRuJk4b+zj6P59kuBNBtQCl0lF2/c+qnfupckZmSkvaUlYyQWqHRC hhePC8UgA3qTEbv7AllKBH81CcciyRYb+ig+1rMcsG9GA81R9nfLw7nO65dgk3BmVyAV Jbdw== X-Gm-Message-State: ANoB5pnhob7GnQl3lsZ+DyuojAIrD7vvoSitwBPnfMH42oX2HLhEo9iR JdVlXND+dftT9ITGIAjwbRYldQ== X-Received: by 2002:a17:90b:711:b0:210:9858:2b2c with SMTP id s17-20020a17090b071100b0021098582b2cmr102785702pjz.191.1670457735590; Wed, 07 Dec 2022 16:02:15 -0800 (PST) Received: from www.outflux.net (smtp.outflux.net. [198.145.64.163]) by smtp.gmail.com with ESMTPSA id s30-20020a63925e000000b00477def759cbsm5933519pgn.58.2022.12.07.16.02.14 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 07 Dec 2022 16:02:15 -0800 (PST) From: Kees Cook To: Jakub Kicinski Cc: Kees Cook , syzbot+fda18eaa8c12534ccb3b@syzkaller.appspotmail.com, Eric Dumazet , "David S. Miller" , Paolo Abeni , Pavel Begunkov , pepsipu , Vlastimil Babka , kasan-dev , Andrii Nakryiko , ast@kernel.org, bpf , Daniel Borkmann , Hao Luo , Jesper Dangaard Brouer , John Fastabend , jolsa@kernel.org, KP Singh , martin.lau@linux.dev, Stanislav Fomichev , song@kernel.org, Yonghong Song , netdev@vger.kernel.org, LKML , Rasesh Mody , Ariel Elior , Manish Chopra , Menglong Dong , David Ahern , Richard Gobert , David Rientjes , Andrey Konovalov , GR-Linux-NIC-Dev@marvell.com, linux-hardening@vger.kernel.org Subject: [PATCH net-next v2] skbuff: Introduce slab_build_skb() Date: Wed, 7 Dec 2022 16:02:13 -0800 Message-Id: <20221208000209.gonna.368-kees@kernel.org> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=6532; h=from:subject:message-id; bh=I5zpjdf6giyPTGWK3waT0b1aZ9hEwojgcygujraz8dc=; b=owEBbQKS/ZANAwAKAYly9N/cbcAmAcsmYgBjkSmEkOgkOZcWwciYH5GLWV3obftsCrvUcG4e847b Bi5nkbGJAjMEAAEKAB0WIQSlw/aPIp3WD3I+bhOJcvTf3G3AJgUCY5EphAAKCRCJcvTf3G3AJrfZD/ 9J2X/Gw9ncWI4dk8wA/WdzHwO05CFaHzRpUpXr+GMG8xWhch5i+SxqJDXVTzXb0YA7TXlpkO9D+otk 5R29AmizYM7EXsl1yZeyMbGQ5hpAufcuX82qAOIodz/lW3bzrzs21vRcUZLrwoV/Fyw0pxzHY5Pf5h ph+jxiId3DghZjSkowXYtsPGXEcnWxRVtUdTcXIwi4Ry8XvyJpqk0jOBV7ZnM8kEJVJ6pUgNjdt61c Gdhjag0bgthVFgM2mpz/QmavD+S+3QlwLB9a3DoVWUJOKJTSQrHNNzbygYMoN5RqmnPz+OcqVneI4E +XcJ6Dh3/v1qbD+rVNET9u/b3bPLy/x06rWB8dgrKn0P/mb601uzlKCKhmm1TG24z1Z9h8w366AVA7 9dU9EwQINOJTDi6NUYcnGa3GApoKuVGCZ03C9xewFJ6BfCrj5qjwQT7CGTH5Vug9puDgl7rBqlIskS WiKGOdtd2hO+BdD3ggipo0i6+bmO07sYVTvcUDv/6GsZZyUy8jET1YpxQS6KU6pKyabXunytY0XNDN o2/3jZeX+Q1xeLy+ge8aeZvS6ypBvKHk51BiHlDd+HM/Ea0wvyjuZr6d5xnNwg9UqO3Po4lIYkKctJ kXJR6WE6EASTgDFD1Xdp19J2MB65NyB2AMFRRTo2I8Nk6cmA+N6Z+5R+M5Sg== X-Developer-Key: i=keescook@chromium.org; a=openpgp; fpr=A5C3F68F229DD60F723E6E138972F4DFDC6DC026 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS 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: X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1751602087696325581?= X-GMAIL-MSGID: =?utf-8?q?1751602087696325581?= syzkaller reported: BUG: KASAN: slab-out-of-bounds in __build_skb_around+0x235/0x340 net/core/skbuff.c:294 Write of size 32 at addr ffff88802aa172c0 by task syz-executor413/5295 For bpf_prog_test_run_skb(), which uses a kmalloc()ed buffer passed to build_skb(). When build_skb() is passed a frag_size of 0, it means the buffer came from kmalloc. In these cases, ksize() is used to find its actual size, but since the allocation may not have been made to that size, actually perform the krealloc() call so that all the associated buffer size checking will be correctly notified. Split this logic out into a new interface, slab_build_skb(), but leave the original 0 checking for now to catch any stragglers. Reported-by: syzbot+fda18eaa8c12534ccb3b@syzkaller.appspotmail.com Link: https://groups.google.com/g/syzkaller-bugs/c/UnIKxTtU5-0/m/-wbXinkgAQAJ Fixes: 38931d8989b5 ("mm: Make ksize() a reporting-only function") Cc: Jakub Kicinski Cc: Eric Dumazet Cc: "David S. Miller" Cc: Paolo Abeni Cc: Pavel Begunkov Cc: pepsipu Cc: syzbot+fda18eaa8c12534ccb3b@syzkaller.appspotmail.com Cc: Vlastimil Babka Cc: kasan-dev Cc: Andrii Nakryiko Cc: ast@kernel.org Cc: bpf Cc: Daniel Borkmann Cc: Hao Luo Cc: Jesper Dangaard Brouer Cc: John Fastabend Cc: jolsa@kernel.org Cc: KP Singh Cc: martin.lau@linux.dev Cc: Stanislav Fomichev Cc: song@kernel.org Cc: Yonghong Song Cc: netdev@vger.kernel.org Cc: LKML Signed-off-by: Kees Cook --- Is this what you had in mind for this kind of change? v2: introduce separate helper (kuba) v1: https://lore.kernel.org/netdev/20221206231659.never.929-kees@kernel.org/ --- drivers/net/ethernet/broadcom/bnx2.c | 2 +- drivers/net/ethernet/qlogic/qed/qed_ll2.c | 2 +- include/linux/skbuff.h | 1 + net/bpf/test_run.c | 2 +- net/core/skbuff.c | 52 +++++++++++++++++++++-- 5 files changed, 52 insertions(+), 7 deletions(-) diff --git a/drivers/net/ethernet/broadcom/bnx2.c b/drivers/net/ethernet/broadcom/bnx2.c index fec57f1982c8..b2230a4a2086 100644 --- a/drivers/net/ethernet/broadcom/bnx2.c +++ b/drivers/net/ethernet/broadcom/bnx2.c @@ -3045,7 +3045,7 @@ bnx2_rx_skb(struct bnx2 *bp, struct bnx2_rx_ring_info *rxr, u8 *data, dma_unmap_single(&bp->pdev->dev, dma_addr, bp->rx_buf_use_size, DMA_FROM_DEVICE); - skb = build_skb(data, 0); + skb = slab_build_skb(data); if (!skb) { kfree(data); goto error; diff --git a/drivers/net/ethernet/qlogic/qed/qed_ll2.c b/drivers/net/ethernet/qlogic/qed/qed_ll2.c index ed274f033626..e5116a86cfbc 100644 --- a/drivers/net/ethernet/qlogic/qed/qed_ll2.c +++ b/drivers/net/ethernet/qlogic/qed/qed_ll2.c @@ -200,7 +200,7 @@ static void qed_ll2b_complete_rx_packet(void *cxt, dma_unmap_single(&cdev->pdev->dev, buffer->phys_addr, cdev->ll2->rx_size, DMA_FROM_DEVICE); - skb = build_skb(buffer->data, 0); + skb = slab_build_skb(buffer->data); if (!skb) { DP_INFO(cdev, "Failed to build SKB\n"); kfree(buffer->data); diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index 7be5bb4c94b6..0b391b635430 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h @@ -1253,6 +1253,7 @@ struct sk_buff *build_skb_around(struct sk_buff *skb, void skb_attempt_defer_free(struct sk_buff *skb); struct sk_buff *napi_build_skb(void *data, unsigned int frag_size); +struct sk_buff *slab_build_skb(void *data); /** * alloc_skb - allocate a network buffer diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c index 13d578ce2a09..611b1f4082cf 100644 --- a/net/bpf/test_run.c +++ b/net/bpf/test_run.c @@ -1130,7 +1130,7 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr, } sock_init_data(NULL, sk); - skb = build_skb(data, 0); + skb = slab_build_skb(data); if (!skb) { kfree(data); kfree(ctx); diff --git a/net/core/skbuff.c b/net/core/skbuff.c index 1d9719e72f9d..2bff6af6a777 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -269,12 +269,10 @@ static struct sk_buff *napi_skb_cache_get(void) return skb; } -/* Caller must provide SKB that is memset cleared */ -static void __build_skb_around(struct sk_buff *skb, void *data, - unsigned int frag_size) +static inline void __finalize_skb_around(struct sk_buff *skb, void *data, + unsigned int size) { struct skb_shared_info *shinfo; - unsigned int size = frag_size ? : ksize(data); size -= SKB_DATA_ALIGN(sizeof(struct skb_shared_info)); @@ -296,6 +294,52 @@ static void __build_skb_around(struct sk_buff *skb, void *data, skb_set_kcov_handle(skb, kcov_common_handle()); } +static inline void __slab_build_skb(struct sk_buff *skb, void *data, + unsigned int *size) +{ + void *resized; + + *size = ksize(data); + /* krealloc() will immediate return "data" when + * "ksize(data)" is requested: it is the existing upper + * bounds. As a result, GFP_ATOMIC will be ignored. + */ + resized = krealloc(data, *size, GFP_ATOMIC); + WARN_ON_ONCE(resized != data); +} + +struct sk_buff *slab_build_skb(void *data) +{ + struct sk_buff *skb; + unsigned int size; + + skb = kmem_cache_alloc(skbuff_head_cache, GFP_ATOMIC); + if (unlikely(!skb)) + return NULL; + + memset(skb, 0, offsetof(struct sk_buff, tail)); + __slab_build_skb(skb, data, &size); + __finalize_skb_around(skb, data, size); + + return skb; +} +EXPORT_SYMBOL(slab_build_skb); + +/* Caller must provide SKB that is memset cleared */ +static void __build_skb_around(struct sk_buff *skb, void *data, + unsigned int frag_size) +{ + unsigned int size = frag_size; + + /* When frag_size == 0, the buffer came from kmalloc, so we + * must find its true allocation size (and grow it to match). + */ + if (WARN_ONCE(size == 0, "Use slab_build_skb() instead")) + __slab_build_skb(skb, data, &size); + + __finalize_skb_around(skb, data, size); +} + /** * __build_skb - build a network buffer * @data: data buffer provided by caller