From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-oi1-f179.google.com (mail-oi1-f179.google.com [209.85.167.179]) (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 7FA261DDC37 for ; Fri, 29 May 2026 01:03:38 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.167.179 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780016619; cv=none; b=pMRpsTKAyexFkGNC+AwXCQxpunTuR/0EvybtkyG2xrSjbE5czzHKlIW1TJXgts0iXfmpCTbD1GntMFL/M6T5B2NlbU6Y3/vlxvfSv8gKxQZg/hQMJU8skDg4aVOK79AAzRGGrB9tj56h2rtguHPz8XE/7psZ/1bdL3qy/7Z+N5A= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780016619; c=relaxed/simple; bh=/KkzOjBdfKQy2PKOpjoFKv4ZMutKIw6NJW/W8FCK7cU=; h=Mime-Version:Content-Type:Date:Message-Id:To:Cc:Subject:From: References:In-Reply-To; b=hJPzU/94Fi+WqZ/RFKxudZ7QtnkQac+vzSBs2WhMFzfYC/HzPR/ESAv1okL94OeNHLgxlMSSI+KSMQcSZF9bWQrZ5rAxpOJz3pZyK/fKnEQoWwDx4qUfcyBP8aGesL9s7KnKmBm8ALedMhLdhdCGl9MTkdsC76hit/sdhpVVwoo= 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=HsOJLqQ4; arc=none smtp.client-ip=209.85.167.179 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="HsOJLqQ4" Received: by mail-oi1-f179.google.com with SMTP id 5614622812f47-482de4ef03aso8232079b6e.1 for ; Thu, 28 May 2026 18:03:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1780016617; x=1780621417; darn=vger.kernel.org; h=in-reply-to:references:from:subject:cc:to:message-id:date :content-transfer-encoding:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=ylWRg4bGcwe7ZEke+FfWeW8Bfa4iTQbilHYuBByUGKs=; b=HsOJLqQ45GjDHlhXoEXmst8zrRZeU2ZTqniHc3c2ngQNcJ7661n3+wjEkVw6K90y7a zct/b1ZiQwOjdG1MwI/K672tn+EQdS9kD8ej/DSgfdictJ7GToSIF7P9HTKb5sYsSdDK +5UqDMblJj2b6IIDEaD61ILs/lgzRuDLJ7UlC+m+qdg4LtpzdsEAWz2+3BOcQHfEE1R7 RyFphW0QZWL+/lAkgd/VLJk8LFo0oMU7wMuze8fwpVdXp+lFabvCjcrfblNDH/YVA/1x yFMKRGmvgozifFOfvH+A7EavKVFyWqGmtOdP6Y7mmzvlyis7H6qQqdKnpkke+lP2vjQN aNAw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1780016617; x=1780621417; h=in-reply-to:references:from:subject:cc:to:message-id:date :content-transfer-encoding:mime-version:x-gm-gg:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=ylWRg4bGcwe7ZEke+FfWeW8Bfa4iTQbilHYuBByUGKs=; b=IYBk5Dp74Y/cT4vMplaf1C1hYzKnQCz0Kpnq6AU4XCLiRwBT5528zmXYSrl4dbGzbK Yd8GYf41CHBXu1Q/3D763PRW7y4lkfDMFOFGsfQFCtMvJl47F/uz2BQgVdKDyQySWf/v mrWxscBdFyXlTjfZras5WdeqUdZYW/RUQsitf9LJzTJYJe/yjrzbhX08DByPZ5geSQOj 2RjparRkOw6Xy4rxaZG8lc6R5RTmwcxGoYqxPZ8ZQtWQu24FsKkX97orzgInZQJSuqY4 8Ffj7aJ7ywbA1VO9ipZCvkdVQRjU868/2DAmqdxYX8U2OiXwaXfvNfxeusTgjZGod+er n+Gw== X-Forwarded-Encrypted: i=1; AFNElJ9eyfiP6YZ916M5ToP/DQhOqLskBHovX3F4iDXVaEkSUhKa1wkNkaprEtaEfEHlnHFTkd1+hnGgYdhmgpA=@vger.kernel.org X-Gm-Message-State: AOJu0Yy6ZUYs7tyPtkKi0Ar4uBSPITZVhlhbwqdNAjZPEVFxoay9i8mJ tYXcbUe0hT8HbyZ/EtzWbDRjDAdRJbwpvii+UQOeYImgbRFZ3vyUMQJs X-Gm-Gg: Acq92OEBsxiADywjt+6eYeSMn8SehzdIBKlFi2b6OoaIZ6H+OBJGoHNEaFt7MUek65Y miS5d3jKmCwc5f7pbNcATqr61hL1tVSa463SXQA7mXZow4a4z9E1L91PgUtGLTMoOVk9wzUpTjE 0aFElSwuubhyzwzLztBE2ZYj3pZR32BCsj6vS7lyNuYmP6mqSeIHYyHShvM8sjkoblZJHFucRr5 MvdN1SrglRB+n1ARxs0M3ZHrjNyNQ366d9xz//wQY1M6Sfc/VaQ4hItZwIp/UBG/CfXmWY/nRuo Y4t1ntw5vvM5Cxdd5U/tRs6dxOpht+qp9kyGmTDDkV6TiXPpA0iHr6D7eJUsfMeVITXC/FVW9Ep 1kGQdVAPRehVLwi1NSyOh01GP+39O+uM9WTI50tZNg38mqgq678Hvd5WMxdc6qFQoR1tbbWISxd PgT70UiBpn9MsKFeRkGt2jb5gsDixRCoxN6owNVe3nXk1xmeGS0P1IHblIIBEHyOx4w7s2uZJRq 3zmXrfrkPfrTUAIeTCAVuMtUG9Q X-Received: by 2002:a05:6808:1a1d:b0:485:4c7f:8f51 with SMTP id 5614622812f47-485e73f86bemr368030b6e.25.1780016617320; Thu, 28 May 2026 18:03:37 -0700 (PDT) Received: from localhost ([2a03:2880:10ff:72::]) by smtp.gmail.com with ESMTPSA id 46e09a7af769-7e695d17b1asm338219a34.17.2026.05.28.18.03.35 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 28 May 2026 18:03:36 -0700 (PDT) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Thu, 28 May 2026 18:03:34 -0700 Message-Id: To: "Yuyang Huang" Cc: "Daniel Borkmann" , , "Alexei Starovoitov" , "Andrew Lunn" , "Andrii Nakryiko" , "Eduard" , "Eric Dumazet" , "Jakub Kicinski" , "Jiri Olsa" , "John Fastabend" , "Kumar Kartikeya Dwivedi" , "Martin KaFai Lau" , "Nikolay Aleksandrov" , "Paolo Abeni" , "Shuah Khan" , "Simon Horman" , "Song Liu" , "Stanislav Fomichev" , "Yonghong Song" , "bpf" , "LKML" , "open list:KERNEL SELFTEST FRAMEWORK" , "Network Development" , =?utf-8?q?Maciej_=C5=BBenczykowski?= , "Lorenzo Colitti" , "Martin KaFai Lau" , "Chris Mason" , "Ihor Solodrai" Subject: Re: [PATCH bpf-next 1/2] bpf: align syscall writeback behavior with caller-declared size From: "Alexei Starovoitov" X-Mailer: aerc References: <20260515071504.2054786-2-yuyanghuang@google.com> <2e08eb1ca27a9a2f8ad29e1c24f779b579621b0a648589f7044799d91c5e00f5@mail.kernel.org> In-Reply-To: On Mon May 25, 2026 at 12:21 AM PDT, Yuyang Huang wrote: > > For this example, the regression lies in legacy CGROUP queries: > 1. BPF_PROG_QUERY for cgroups is not a new feature; it has been in the > kernel since 2017. > 2. There are existing legacy userspace applications and language > wrappers (like our android net-tests) written years ago with older > structure layouts (passing size =3D 40, ending at > query.prog_attach_flags) that query cgroups. > 3. In June 2025, Commit 120933984460 backported "revision" support to > cgroup queries, introducing an unconditional writeback in > `__cgroup_bpf_query()` at offset 56: > ```c > // In __cgroup_bpf_query() (kernel/bpf/cgroup.c) - Introduced in 12093398= 4460: > if (copy_to_user(&uattr->query.revision, &revision, sizeof(revision))) > return -EFAULT; > ``` Ok. This is fair. 120933984460 is indeed problematic. > Now lets talk about BPF_TCX_EGRESS: > >> bpf_mprog_query() and tcx_prog_query() were only introduced there. >> How come your userspace passes shorter uatter to query newer >> BPF_TCX_EGRESS attachment? > > I understand your point, but two common cases exist where userspace > will legitimately query BPF_TCX_EGRESS with a 40-byte structure: > > First, a generic BPF querying tool (assume it called `./bpf-query`) > compiled in 2020 (when the query buffer size was 40 bytes) might > accept the attach type dynamically from the command line: > ./bpf-query --type 47 --fd 3 # 47 is BPF_TCX_EGRESS Don't buy this at all. This one is clearly a user space bug. > Add gating to the TCX path, will not be able to fix the legacy cgroup > query path. As shown above, the cgroup query path has the exact same > OOB writeback regression affecting legacy userspace. Since we must > plumb uattr_size to cgroup.c to resolve the cgroup regression safely, > applying the same consistent size-gating in bpf_mprog_query() seemed > like the most consistent and robust architectural path. Not really. Your patch adds checks to what looks like "random" copy_to_user= () places. This thread is clear indication that it's not a "robust architectur= al path". True robust path would be to wrap every copy_to_user() into another helper that takes uattr start pointer and size, does extra check, and returns something like ENOSPC (and not EFAULT), but that would be a significant churn. So I prefer a minimal patch that add single check in bpf_prog_query() that checks that user space supplied buffer is large enough for attr->query= . If not -> EFAULT. That would be better than overwritting. One can argue that partial copy_to_user() in __cgroup_bpf_query() should still be allowed, but I'm against partial and inconsistent queries. query.revision might seem benign, but if we do it for all copy_to_user() we better return ENOSPC to differentiate vs EFAULT to tell user space to fix itself.