From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 24CA5C83F12 for ; Tue, 29 Aug 2023 09:32:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232106AbjH2JcC (ORCPT ); Tue, 29 Aug 2023 05:32:02 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49470 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234898AbjH2Jbl (ORCPT ); Tue, 29 Aug 2023 05:31:41 -0400 Received: from mail-ot1-x333.google.com (mail-ot1-x333.google.com [IPv6:2607:f8b0:4864:20::333]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 04D4AE66 for ; Tue, 29 Aug 2023 02:31:10 -0700 (PDT) Received: by mail-ot1-x333.google.com with SMTP id 46e09a7af769-6bd8639e7e5so3068042a34.1 for ; Tue, 29 Aug 2023 02:31:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=purestorage.com; s=google2022; t=1693301468; x=1693906268; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=guePXrPopLgOPcdT283gc5sTu7sw+Yj419mnndMktxQ=; b=G46WNf6fPWXXmiaXK3LOGve4BCV8l2KqIxaloMmV3wkqAr3ZO5uSD1EkVsUytFDNd3 ZW4wQc3URjkmfHrKBc1uGO5VYKoynZgFmnk5k16OkThVEiarySxdGyUnYRHng+Yt/fSH NCB07AZkoGuNVEDW5GYDoeYyUd2f5KfLOmyRNRwsctkVhLExnZjkRl3BrldDlq3AP6kd uwdb9K9QgkwThOczV4x6f3yoKSVIYJqxQiS3n/krZDQQ9uZC7lcHl7RdUJiUjBbtpruE uERXMKeg8Q93dAefTNg/TtxWxcUFrwtd/D7UADUeJAwcRqLx26yJ87Kz1LBjcAayiaE3 lj3A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1693301468; x=1693906268; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=guePXrPopLgOPcdT283gc5sTu7sw+Yj419mnndMktxQ=; b=Fia49Y/SFQ6uVSk1VqAOCbyS3IFOwnIWZETb4UvW/8cVvZrCMRiQUCm1wB2RvoCAxR /Lsb3KccaK41kOjGQ/SDP1pMrlhsYn/C9uyH5fVD8r+N62uULdTbLxgutKl1ffUV/Wrv gwbMNy6SbE5FTHvwXquVsd/ewtkkO7PwlBlKymNzAEXS4vMcLvJ8soG5EDCllpcdjFq3 kv1Yp55LXYgRfAy+TXHgKvFnqs+Go4zuNp2arcsYKEz6alBDIqS4JUVwNy0SSVoxsUsB gohKhROQ9vpCR6gmkTQDORoXs935/mT+GEz8/fMiM1+WQgE7waYSFJbeIOynXoH4BWul TP9g== X-Gm-Message-State: AOJu0YybeRJiZSwJGhfqNtqsCogN5r5KfKybB5NRwF2J2a7lD/73G51l TEdeA41/GBuXUddPp2i5PchnKA== X-Google-Smtp-Source: AGHT+IGkPraI08Qww1nC+lwqJvqyudyb6VsEAq0bqdSWvhaRu2JO4pyOm2p9Ozj4qi8c/yS/CCrB5A== X-Received: by 2002:a05:6830:119:b0:6b8:7a79:db37 with SMTP id i25-20020a056830011900b006b87a79db37mr16733428otp.22.1693301467999; Tue, 29 Aug 2023 02:31:07 -0700 (PDT) Received: from medusa.lab.kspace.sh (c-98-207-191-243.hsd1.ca.comcast.net. [98.207.191.243]) by smtp.googlemail.com with ESMTPSA id g7-20020a63ad07000000b005649cee408fsm8741081pgf.0.2023.08.29.02.31.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 29 Aug 2023 02:31:07 -0700 (PDT) Date: Tue, 29 Aug 2023 02:31:05 -0700 From: Mohamed Khalfella To: Eric Dumazet Cc: willemjdebruijn , "David S. Miller" , Jakub Kicinski , Paolo Abeni , Willem de Bruijn , Alexander Duyck , David Howells , Jesper Dangaard Brouer , Kees Cook , "open list:NETWORKING [GENERAL]" , open list , "open list:BPF [MISC]" Subject: Re: [PATCH] skbuff: skb_segment, Update nfrags after calling zero copy functions Message-ID: <20230829093105.GA611013@medusa> References: <20230828233210.36532-1-mkhalfella@purestorage.com> <64ed7188a2745_9cf208e1@penguin.notmuch> <20230829065010.GO4091703@medusa> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2023-08-29 10:07:59 +0200, Eric Dumazet wrote: > On Tue, Aug 29, 2023 at 8:50 AM Mohamed Khalfella > wrote: > > > > On 2023-08-28 21:18:16 -0700, willemjdebruijn wrote: > > > Small point: nfrags is not the only state that needs to be refreshed > > > after a fags realloc, also frag. > > > > I am new to this code. Can you help me understand why frag needs to be > > updated too? My reading of this code is that frag points to frags array > > in shared info. As long as shared info pointer remain the same frag > > pointer should remain valid. > > > > skb_copy_ubufs() could actually call skb_unclone() and thus skb->head > could be re-allocated. > > I guess that if you run your patch (and a repro of the bug ?) with > KASAN enabled kernel, you should see a possible use-after-free ? > > To force the skb_unclone() path, having a tcpdump catching all packets > would be enough I think. > Okay, I see it now. I have not tested this patch with tcpdump capturing packets at the same time. Also, during my testing I have not seen the value of skb->head changnig. Now you are mentioning it it, I will make sure to test with tcpdump running and see skb->head changing. Thank you for pointing that out. For frag, I guess something like frag = &skb_shinfo(list_skb)->frags[i]; should do the job. I have not tested it though. I will need to do more testing before posting updated patch.