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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 20A05C48297 for ; Sat, 10 Feb 2024 02:33:14 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 597C16B0072; Fri, 9 Feb 2024 21:33:14 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 547586B0074; Fri, 9 Feb 2024 21:33:14 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 40EFE6B0075; Fri, 9 Feb 2024 21:33:14 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 323CB6B0072 for ; Fri, 9 Feb 2024 21:33:14 -0500 (EST) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id CBFC0A050F for ; Sat, 10 Feb 2024 02:33:13 +0000 (UTC) X-FDA: 81774322266.13.D380879 Received: from mail-wr1-f43.google.com (mail-wr1-f43.google.com [209.85.221.43]) by imf21.hostedemail.com (Postfix) with ESMTP id 050791C0007 for ; Sat, 10 Feb 2024 02:33:11 +0000 (UTC) Authentication-Results: imf21.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=E+j5GRDB; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf21.hostedemail.com: domain of alexei.starovoitov@gmail.com designates 209.85.221.43 as permitted sender) smtp.mailfrom=alexei.starovoitov@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1707532392; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=9MpCdF8ON0eMIc46JXDNd8gADv5BuQCYb+ck4K+GqhA=; b=XIib3I/Z5BTOlElKRaHDeI0pS0t+Htc7s47gxxNmygmOdjeACumfolxkOupe81OIrynVZ+ /dRZmTIZ2PV3jjZP4VokKQwRdoo4rA3Mhe1rSEd0gXdWwkE3uuUSuvE5XXwGbsmjlowT+G K9a1SrI4C9VPOErMi5zYpUnzy9TIQSk= ARC-Authentication-Results: i=1; imf21.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=E+j5GRDB; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf21.hostedemail.com: domain of alexei.starovoitov@gmail.com designates 209.85.221.43 as permitted sender) smtp.mailfrom=alexei.starovoitov@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1707532392; a=rsa-sha256; cv=none; b=UKtIFrZtUt2SXe5IdNV63ovfLJR6XcOMNyIEXRGKzehImCv7mZrkwv3zeQWDMJp1oApk+3 Nn24eQ9CIKio36+w2wPDNX/9KA8Rl3J/bcDu3V3Npkr7w5p4l/JehYqWjQdwP8ju22Vhzq +B5+JDfwcw6yjR+0kiiAny8JR+uKlrY= Received: by mail-wr1-f43.google.com with SMTP id ffacd0b85a97d-33b1d7f736bso922049f8f.3 for ; Fri, 09 Feb 2024 18:33:11 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1707532390; x=1708137190; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=9MpCdF8ON0eMIc46JXDNd8gADv5BuQCYb+ck4K+GqhA=; b=E+j5GRDBQ4qW8G8OuTxPKq48w6/6Jt9kbUu+kozyKYOSASx2g9vMmyFtBgVJ7UkIhg u176k6MxCFlwP3r5pl6Z4kxeBah07FHeXIyAn4IEAzgDgFQvSmmntVrJf+K14NMkHVro QMjXVXWrUzGPGCfyqtwIjr35h/ZVF3HIGC2ANST1hFz84wfYo5lJydzrJtZ8wmbToKQ0 NM3HhG/nqL3rqey+RcwGkCXWcSIdLS6WMg4EF+rIJkCH0P0dHkNfVemX2Kte0bkfi9yy T8Itqaowu+fMYmwbpWUj7jRzDN9C7sj66/9opHB11eEkcHJWIteCf/mf5ZYNZCnoYVIf RXLA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1707532390; x=1708137190; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=9MpCdF8ON0eMIc46JXDNd8gADv5BuQCYb+ck4K+GqhA=; b=bqWcfJ+TorHqK0mZlfRobgIhhgDH6E6iGBft9DHW/j8J3UEQJCQdEFWFLz9Dx6AIS1 CNVJrmzzgH8VvkzCT13cAYdbJDchXdr4J8vvNIP1hpCjoHGlbH6tkfXYB8gEHJdboSL9 n9oFAXS/UPxhYKKQTVOlhbOhzMN2fC+rkwKdZOoIbMbwlrZbgS8xifbCt2dL70We3oxC hAQPZOdNdO1xnmt9HvKhDEA9wZdssZvYn5kf0jXVviPSMAOLdxt464rBqiAT2K2dpY/b 6NCloD1jINH/NFiTG3VRZX4GkYKKSLIll+076Gs31ADje0/wm4I7ktCjprDF51PrqO2k 0I3w== X-Forwarded-Encrypted: i=1; AJvYcCWxcO+WRbpPvxWtkB6dJfZmW2Fg1g7/hhbyMqMFmVLg81/3Mx+gPPjXOPCZDVHXnhAHC3Z3kYdYP3dbXb9TwTuQL0k= X-Gm-Message-State: AOJu0Yz/oip6/dKlgsMqqZCbs6MZAJffFrBIIQHjiCP2Rn8tmpCp+ULf nF924dfRO5scam/FBcaLWtH1T8Ma6OrU6Faz1cLDJPBAUxCqz/3vyY/kpHTGaI/3vu+I2MDWpXU LmmFSTb8nBgSlWgc9WVlrYEZQCbI= X-Google-Smtp-Source: AGHT+IE5cO6RKNBNRcqQWQPrlfKFYo3kbxatiPndfb5TLuqs6saYM8SdMxskcym57ocMTdMny/xyEs0xTUiLbsm3D3E= X-Received: by 2002:adf:f9ce:0:b0:33b:1894:c497 with SMTP id w14-20020adff9ce000000b0033b1894c497mr401431wrr.35.1707532390065; Fri, 09 Feb 2024 18:33:10 -0800 (PST) MIME-Version: 1.0 References: <20240206220441.38311-1-alexei.starovoitov@gmail.com> <20240206220441.38311-2-alexei.starovoitov@gmail.com> In-Reply-To: From: Alexei Starovoitov Date: Fri, 9 Feb 2024 18:32:58 -0800 Message-ID: Subject: Re: [PATCH bpf-next 01/16] bpf: Allow kfuncs return 'void *' To: Andrii Nakryiko Cc: bpf , Daniel Borkmann , Andrii Nakryiko , Martin KaFai Lau , Kumar Kartikeya Dwivedi , Eddy Z , Tejun Heo , Barret Rhoden , Johannes Weiner , linux-mm , Kernel Team Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Stat-Signature: hienp8hhg7ncp5u8uhapkd3u368ft9pb X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 050791C0007 X-HE-Tag: 1707532391-179668 X-HE-Meta: U2FsdGVkX1/R7RdXwTC9ewwMR41XkRCgnvxo6n7kYlP9Wlz/VLiCb+ZOs+B76+RiYFctRxV0fgL50FyzEW7ja77pxJKM6BCg+qAfbBSL5G1x86TtD/PYE2ExS0xep9z/ajC1FsV770LR6KF5KGzSAbQ5KvgeqQ87OqRF0gYOFCp8TbDsoejVDJT4Z4h77HJHpyxWT0SvLkJlFKdbz+86gGspFGJKRWwKIg9qx0DZtPTzlgAjS40OzyVAh+8Ftl+8jZ0uE9qNojZtIeVbjIuNG6UgnV0I2VBLvDMLc/fFrnub0dBRX0dCcaizs1B6UdcSLKnYIUbXkEz++7Dtm2Qwtkism2feTupFzSorbs0lrjHTqpLgWjTTpMIk1UEf23HsiJpBMYpQoyC0Xu5MLMFsOR2GYZd7s6VFHwNDIzbR78xuePUGHIA+05gt9EmU54MUHWrBL0zc4z40bsl0cj+uvnW+23XBrHs12ZT+uz041SL8nRybnBVoizIBwicPvJjpDzleTSsDYGXokwAu3aGGMTbWP/RHWdQkpk5Iykt8aPFykMbdqTnAmiz0/tyiTmu+wQAoLRvgM0ovAhGE+nHjcci4T/vrH1OUuiabQrTJzejJxPeE+pxGak+6zkZxwbmw7chty9QWPyxLBssggH2F369yeXEXfDT8yuQeGu0xlKFSyGS4MjFM8RtMib3rrzKt7N8YLmO5huKNVm0/WUgtoEAGwGpnwHBc9iLQ3wNCh+Zb4XPZ5V8gWBtOGl8DfdctxFwnv60//9YWmamtaloACBmMPWC4LoyQhyshhDxEm/q5MzfYTxXFrIEXG0gd2hgKznLAtxCqYeA3pMXLnRtL8yJTxMP4ZMUaZpD9PIdK/WuzdIjM0Xurq4zldQWEIxWWRTfbM9hAlm5rjFXKlmdUHZOMOip4IsB7zZocB7XaOBOrVmRKUL6Y0aNRJppnuvzWIf8Q5nENI2SuZbBFjbM yEvrGXX4 SghoWZENzgKbxR/vHcP4S+1i1J/I7za2tKYbZX7n6wKEA1tTR7JzRhOl40Y88EuKjLCUybxGQz+9/a5+U/E8PFhagJEFD5YIw9YxTWXmMPtepTW7ZQJzpQ6h5wDLnUc/SfRTCksJavSV6snBc0HXe4iqQrUxl9RkVv01h7FSOHNMfcItlRfa2FuGA5CVYXPA2Twe9UIpFQ9Nbq2zjZoBD9H61HzAMmycNMhi+873IMC2VntIAPXD0JDDx/+QV1caQB7J1VDy4Sssqj9Fzn6EjlkaNZVnsqhN+DMJB7rrPfMjRaWQYNnKAksnn+2g0PNxw043l8rPTCNq2sSuAMETsrUkki6z/i5iIo4++GP4ZmPpg+rODO2qSrT5egzQLrnfdmlMMu9mwM0bEGdHXVjGAuuPkgqFe7CsyQqjj5HSAvTron74/5Hfbb021tyT2p6Bl/zlSOA1PxHPQ5dx7ReLqBvYQMkW+h8cf8iH2KtXsSKQV/PeR96sBmR2m7vvCWomiN7FOlaI5u1c7TAiLsj9IRvoReGvSOGc13j7z X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Fri, Feb 9, 2024 at 11:09=E2=80=AFAM Andrii Nakryiko wrote: > > On Thu, Feb 8, 2024 at 4:09=E2=80=AFPM Alexei Starovoitov > wrote: > > > > On Thu, Feb 8, 2024 at 11:40=E2=80=AFAM Andrii Nakryiko > > wrote: > > > > > > On Tue, Feb 6, 2024 at 2:04=E2=80=AFPM Alexei Starovoitov > > > wrote: > > > > > > > > From: Alexei Starovoitov > > > > > > > > Recognize return of 'void *' from kfunc as returning unknown scalar= . > > > > > > > > Signed-off-by: Alexei Starovoitov > > > > --- > > > > kernel/bpf/verifier.c | 3 +++ > > > > 1 file changed, 3 insertions(+) > > > > > > > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > > > > index ddaf09db1175..d9c2dbb3939f 100644 > > > > --- a/kernel/bpf/verifier.c > > > > +++ b/kernel/bpf/verifier.c > > > > @@ -12353,6 +12353,9 @@ static int check_kfunc_call(struct bpf_veri= fier_env *env, struct bpf_insn *insn, > > > > meta.func_name); > > > > return -EFAULT; > > > > } > > > > + } else if (btf_type_is_void(ptr_type)) { > > > > + /* kfunc returning 'void *' is equivalent t= o returning scalar */ > > > > + mark_reg_unknown(env, regs, BPF_REG_0); > > > > > > Acked-by: Andrii Nakryiko > > > > > > I think we should do a similar extension when passing `void *` into > > > global funcs. It's best to treat it as SCALAR instead of rejecting it > > > because we can't calculate the size. Currently users in practice just > > > have to define it as `uintptr_t` and then cast (or create static > > > wrappers doing the casting). Anyways, my point is that it makes sense > > > to treat `void *` as non-pointer. > > > > Makes sense. Will add it to my todo list. > > > > On that note I've been thinking how to get rid of __arg_arena > > that I'm adding in this series. > > > > How about the following algorithm? > > do_check_main() sees that scalar or ptr_to_arena is passed > > into global subprog that has BTF 'struct foo *' > > and today would require ptr_to_mem. > > Instead of rejecting the prog the verifier would override > > (only once and in one direction) > > that arg of that global func from ptr_to_mem into scalar. > > And will proceed as usual. > > do_check_common() of that global subprog will pick up scalar > > for that arg, since args are cached. > > And verification will proceed successfully without special __arg_arena > > . > > Can we pass PTR_TO_MEM (e.g., map value pointer) to something that is > expecting PTR_TO_ARENA? Because there are few problems with the above > algorithm, I think. The patch 10 allows only ptr_to_arena and scalar to be passed in, but passing ptr_to_mem is safe too. It won't crash the kernel, but it won't do what user might expect. Hence it's disabled. > First, this check won't be just in do_check_main(), the same global > function can be called from another function. that shouldn't matter. > And second, what if you have the first few calls that pass PTR_TO_MEM. > Verifier sees that, allows it, assumes global func will take > PTR_TO_MEM. Then we get to a call that passes PTR_TO_ARENA or scalar, > we change the argument expectation to be __arg_arena-like and > subsequent checks will assume arena stuff. But the first few calls > already assumed correctness based on PTR_TO_MEM. I think that would the issue only if we call that global func with ptr_to_mem and then went at processed the body of it with ptr_to_mem and later discovered another call site that passes scalar. Such bug can be accounted for. > In short, it seems like this introduces more subtleness and > potentially unexpected interactions. I don't really see explicit > __arg_arena as a bad thing, I find that explicit annotations for > "special things" help in practice as they bring specialness into > attention. And also allow people to ask/google more specific > questions. In general I agree that __arg is a useful indication. I've been writing arena enabled bpf progs and found this __arg_arena quite annoying to add when converting static to global func. I know that globals are verified differently, but it feels we can do better with arena pointers. Anyway I'll proceed with the existing __arg_arean approach and put this "smart" detection of arena pointers on back burner for now.