From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ed1-f47.google.com (mail-ed1-f47.google.com [209.85.208.47]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B05D53FD970 for ; Tue, 26 May 2026 14:50:55 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.208.47 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779807057; cv=none; b=QNxR9UR28/L5KMKAb//PITclI/GZhN54V7oDCddCD9WpnglpynZeQUI4RQbPTbqKk4J17/GiNkd8C0DO4UCvuER/ttgj8KDWKj6rJ3uSjJir1qLXTq4JonW0tKUkYfynR+q27GMsInN0fmeBETKrmNNKDCesmgg0cvkgxqrZOCg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779807057; c=relaxed/simple; bh=F5YMabiE8O+ywnrU305U0Znz0rgWJB8UF35MQVPj76k=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=qsQGxIgoNkEbi9BDbU4h9SFPS8x1VjOF8n5kwcVfD4da9KfRSWsVthNsSicAIm2GS+aHNqkWFPO5L9XvtgbRB5d8MVt8Z0u6Rt4AkTq0ql20Vg1qU28LwoMwYr61UFPHscNDex3Wu+pz38qb/GQZJN/TmUSgFwIleq7nbF3KYEg= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=sDxCOmCz; arc=none smtp.client-ip=209.85.208.47 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="sDxCOmCz" Received: by mail-ed1-f47.google.com with SMTP id 4fb4d7f45d1cf-6870ad8072eso3036004a12.0 for ; Tue, 26 May 2026 07:50:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1779807054; x=1780411854; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=AjQZn0mYVYssmLLf8Ym16szY/Tz1EKrMNloBKoyB+kw=; b=sDxCOmCzfmsYyApJVRTicHNKTJncmplELdx5V2OOUIMNYtSkx29uAHcwJxPe/Viq/z CSAxuit3weSUfGUTy4cdvH4E7+wSj9nKEwyGwp4k5k9FpNMzTvzy42BWN+NDJ2UrT29J JKfik4TN8gJnB7SP1GpHNrKRXujJrubP9QEYXLvkDWhuHj5m9iwfOhrI/KBMR7CqSv2f HO2ND/+Yf0JbPabbju5QDM1RQi0Q/ocCWoSQCrt6NwcOS7X5S6OJc49jdNshfWYnS8OF vwpIR960FbmKFRRqhky+htJSP+BYHJIJfMRYr2/bWxhphbZqywKgUrCr7B4s+CzDaec8 7ZSA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1779807054; x=1780411854; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=AjQZn0mYVYssmLLf8Ym16szY/Tz1EKrMNloBKoyB+kw=; b=bkux0zhaDnZhJUgpm0SlZmQLLKqP2VzUGY3q2j4gmrjiY6Enuc30MuFO0wfAuPTkk3 pxV49Sl+u5qCKoYWYiQbuh1n+zLTdmL7g9W6W4FxtX5SM2mSzJ5RgiEM9xFdFsN3/m9M WIwiSZm8eFkqOtinHFUoofZAlBoO8Xoyu8qK2oTehFAYo2Yj+fVaM3Bs4dhJj8hJB8C6 bQIbRNdB37IqWfnQLvdsne9SY3fwz1gDO5lb+RX3hK52bCTfTm42wwdYLcZG0qVAAu4y oaqZIMBpnWqWiXb1AwYeYipixiD6dsTIOA0awvEveUmsUmU3rVjiLjJJnAd8ObCWjhhm kPmw== X-Forwarded-Encrypted: i=1; AFNElJ+U0s8lwKjHO+d2r8W1pI+LFUMadPEiec6FyHi3vepz7Nx4XsAHb8M6iZe2ojz998DI/Ja8odltdrfn418=@vger.kernel.org X-Gm-Message-State: AOJu0YyAB0joYTLT5Aolv1N1te3PjzwBlsTUwKjRRfH1IoJPXE0x0Nx7 Qtbt66JwKxolBcygPOYGwtFz/9rOaWiB75GQnb2Fx/IyOoWzXvyqAxn9 X-Gm-Gg: Acq92OHvlcZq/ae1F88fsIXjwaLjV6Y6Kh2iXMBngFHeDoaTWCxcapUC8qc1B+R2SdS YrSUIeLDZ6ceX3DFqO2X0NHnPLW31IyrI+xXnqt8vY/6GKkb1rrZwiq82ul3F4FQpku4LiXRbVR x3ZMLjPpl8FcAX3ogBDrO5UD6sXxfWPTLYdK53OkWf7i1CtPZLdDuTS3325njLHpeSJr3s1R2tc qKVQDArtbp1ywgogjt/xyfTa0lk/l1w2+UqTsfGJvnhLvH/u3kAFdIPUtfj0JzabWUhS+XG1oK4 Go44i3EX41OBU2Kwxqd23sk8pyifbi8prfuavKMYjxmsvvjydmNKXbmc7OcMD8LSd78m9Qfjguj JRXytvI+OJ4fInZns35++E7Xgc9d5smI4MVIw8Eh1sClAl5ytEYvt5ZvCyLbD/6zrp+WOiisKwn kydq+t6GOuvShp5OVsTh+Wy0zUEotRTVyX2CXfVpB3FPho/8KxYm8V2v8PBDRQ//oau9ymg2C3x ferGPRf4AIoNlgFSthh3pmt554xif3CWk6DImVW0uYgpeErebJQRirivop8SKM= X-Received: by 2002:aa7:d859:0:b0:688:9b94:65e2 with SMTP id 4fb4d7f45d1cf-6889b94691bmr6352673a12.7.1779807053827; Tue, 26 May 2026 07:50:53 -0700 (PDT) Received: from [10.109.92.22] (82-132-212-62.dab.02.net. [82.132.212.62]) by smtp.gmail.com with ESMTPSA id 4fb4d7f45d1cf-688b9b6d287sm5221421a12.6.2026.05.26.07.50.50 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 26 May 2026 07:50:52 -0700 (PDT) Message-ID: Date: Tue, 26 May 2026 15:50:47 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH net] net: skbuff: fix missing zerocopy reference in pskb_carve helpers To: Willem de Bruijn , lazyming , netdev@vger.kernel.org Cc: davem@davemloft.net, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com, horms@kernel.org, w@1wt.eu, security@kernel.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org, achender@kernel.org, mst@redhat.com, jasowang@redhat.com References: <20260521121628.309924-1-minhnguyen.080505@gmail.com> Content-Language: en-US From: Pavel Begunkov In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 5/25/26 16:31, Willem de Bruijn wrote: > Willem de Bruijn wrote: >> Willem de Bruijn wrote: >>> Willem de Bruijn wrote: >>>> lazyming wrote: >>>>> pskb_carve_inside_header() and pskb_carve_inside_nonlinear() both copy >>>>> the old skb_shared_info header into a new buffer via memcpy(), which >>>>> includes the destructor_arg pointer (uarg) for MSG_ZEROCOPY skbs. >>>> >>>> These functions are not supposed to maintain zerocopy frags. >>>> >>>> Both call skb_orphan_frags. >>>> >>>> I think what may need to happen is to invert the order of that call >>>> and the memcpy. Current code: >>>> >>>> memcpy((struct skb_shared_info *)(data + size), >>>> skb_shinfo(skb), offsetof(struct skb_shared_info, frags[0])); >>>> if (skb_orphan_frags(skb, gfp_mask)) { >>>> skb_kfree_head(data); >>>> return -ENOMEM; >>>> } >>> >>> Never mind. This actually corresponds to the first Sashiko report you >>> mentioned: if zerocopy skbs are converted, then the memcpy prior to >>> that call will have stale state. >>> >>> For skbs where skb_orphan_frags does not do a deep copy, we do need to >>> take this extra reference. >>> >>> Reviewed-by: Willem de Bruijn >> >> Not sure the potential preexisting issue is reachable. >> >> Vhost-net and other zerocopy that predates MSG_ZEROCOPY does not >> refcount ubuf_info. Instead it calls skb_copy_ubufs on skb_clone. >> >> So if such an skb reaches pskb_expand_head, it should be guaranteed to >> not be a clone. Same for the carve methods added later. >> >> But, the commit that added zerocopy, commit a6686f2f382b >> ("skbuff: skb supports zero-copy buffers"), included this >> pksb_expand_head call to skb_copy_ubufs from the start. That implies >> that was expected to be reachable. I just don't see how yet. >> >> If it is reachable, then all that is needed is to clear shinfo->flags. >> Or more neatly, >> >> skb_shinfo(skb)->flags &= ~SKBFL_ALL_ZEROCOPY; > > Also, I'm not the expert on more recent managed frags > (SKBFL_MANAGED_FRAG_REFS). For that one, pages are guaranteed to be alive as long as the ubuf_info is not destroyed, hence we don't hold per shinfo refs. IOW, the lifetime of the pages is bound to the ubuf_info. > That calls skb_zcopy_downgrade_managed in pskb_expand_head, but not in > the two other functions with memcpy before skb_copy_ubufs: > pskb_carve_inside_header and pskb_carve_inside_nonlinear. > > I assume because those shorten the skb, so no risk of getting mixed > mode refcounted and non-refcounted frags? From a quick glance, if reachable, they should "downgrade", otherwise they leak pages. The new data inherits SKBFL_MANAGED_FRAG_REFS and ubuf_info but takes additional references with skb_frag_ref(). I'll take a closer look. > In general zerocopy can be split in refcounted and non-refcounted. > > Refcounted zerocopy will not downgrade in these cases, so will not > modify shinfo->flags after memcpy. > > Non-refcounted should always get converted to copy in skb_clone, > so will not enter the skb_cloned() branch here. > > If in doubt maybe warrants a rare WARN_ON_ONCE patch. -- Pavel Begunkov