From patchwork Sun Nov 3 22:59:38 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Kartikeya Dwivedi X-Patchwork-Id: 13860684 X-Patchwork-Delegate: bpf@iogearbox.net Received: from mail-wm1-f68.google.com (mail-wm1-f68.google.com [209.85.128.68]) (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 8E95218B497 for ; Sun, 3 Nov 2024 22:59:45 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.68 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730674787; cv=none; b=tZLJs7kJgRXWxsresS0HsLuRYbH4/p+a8I2oyO+s4awKHHKxWlIHI3HuC9SpKucH9aEKhL4pYjqPobOaSvRGAE7iCM+A1lD+qgUj/2hTbUtYfV+qIbvirzceKkpldGFQ3gnXyeRMFOEqai1PuqEBJt8v7veNxMWD9YGdS+fh74k= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730674787; c=relaxed/simple; bh=hTDEHm7KPUVegNa8ajbrO7dQTDhQriCT+IavL4mnmjU=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=AFW/q9houDIqS0RFh6XrXoI1rQuPmmMHLYyZmusvvf3OK/TosmRyF0NQsicayWJv6m9NY/l3lwE1bRBk8lh4KVBKJXTCwCl4ND9NmjbhxK2C4JoeHNMDZ1EsyY2QRTnjy8y1o+L5xxVRKPWB7rpcHw6u2uujzsf08L2ELtvtUcs= 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=H5T5aBOs; arc=none smtp.client-ip=209.85.128.68 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="H5T5aBOs" Received: by mail-wm1-f68.google.com with SMTP id 5b1f17b1804b1-4314fa33a35so28139865e9.1 for ; Sun, 03 Nov 2024 14:59:45 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1730674783; x=1731279583; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=B0hbH+nV4kSlKadft/+ZPJqCvizFXnBqrXWROlk1T1Y=; b=H5T5aBOsxKGOvEbKw03Zam1Z2aUjP5cSMIh5e0Mt4mhSI4f3H+wq4nDCPbNXuucJ1N LC3QxlgvvdNDk4Z9mhn6oqThYOC7gRAUDttUAqU298ytOTkBQpDCwA17D+gSgbfY34oQ tgFZg1D/CY22Lho90uRMFtInIGNaU76pHKAmlRAHW24UbDVHoBLsrkcvrlj7o0vfr/9U LOM6TEhw5dHZ1I1AST9+cUL0LN3GCPt44DUQAJuyntsFnGJC884OsoYlI6rmcI9ySSq5 Um/6odzEQl3IGjs26FUxYC6jH6B/QZ3rw3jG/GEQB3ehj6nUwBzEo3HXEPqL5mrtc4gg DHWg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730674783; x=1731279583; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=B0hbH+nV4kSlKadft/+ZPJqCvizFXnBqrXWROlk1T1Y=; b=mh6fJSizx9HmpjSWZ3AS9YLV4ig6K1yUkDHIlCFEEQP8GzM0zGb7sUh/PGzQI2TwHR n2nn2iaGwcGoCfjxPUSK/zb8ShrlPsAPFxmL3r+6T6EvsUeiK0P/AayYGdAexM5NKJ1h z1c/wV+EY4VZ/N2UoWJ1en6weSpTBHshUPkgETFR+zJn6NIXrLrWvtCeThL/MgINvFEr IzC1ymyhgiAUOq9YpkV36gXlpOYS8q8slyGwjDip43mlh25spaAMVAkS3Q3K53y/wiAU njJGOnt3WtQmoGN8d/glvK3I7rFozN9VSaIanWyAs+1zisDXmI21AIvIt/wjXjI/5QeH sfwg== X-Gm-Message-State: AOJu0YxLEIL8xli1C497Dw978+3nmQ3EXceAGnU0+Tnh3t9unHHBqYT7 WEEzT1YrxxNUc9o3gsXE2jKPjYwkm7Qqbq/xXSBgtomu4WcYPSuHN6b85xs4d3sQog== X-Google-Smtp-Source: AGHT+IGklWtXAGWxHGsol5wPAxBAz/rL/cimcjC6G4giFmjDkB7DxDYk8uw9BsghQfP4ESw46Sgmnw== X-Received: by 2002:a05:600c:468d:b0:42c:a89e:b0e6 with SMTP id 5b1f17b1804b1-431bb98558cmr156470695e9.11.1730674783429; Sun, 03 Nov 2024 14:59:43 -0800 (PST) Received: from localhost (fwdproxy-cln-033.fbsv.net. [2a03:2880:31ff:21::face:b00c]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-381c113e5e3sm11597146f8f.82.2024.11.03.14.59.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 03 Nov 2024 14:59:42 -0800 (PST) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: kkd@meta.com, Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , Eduard Zingerman , kernel-team@fb.com Subject: [PATCH bpf-next v1 1/3] bpf: Tighten tail call checks for lingering locks, RCU, preempt_disable Date: Sun, 3 Nov 2024 14:59:38 -0800 Message-ID: <20241103225940.1408302-2-memxor@gmail.com> X-Mailer: git-send-email 2.43.5 In-Reply-To: <20241103225940.1408302-1-memxor@gmail.com> References: <20241103225940.1408302-1-memxor@gmail.com> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=1903; h=from:subject; bh=hTDEHm7KPUVegNa8ajbrO7dQTDhQriCT+IavL4mnmjU=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBnJ//pRuopfpCeJSwHmqLvhBuXNwZoWLYSmJlOgkbl bA31Jq2JAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCZyf/6QAKCRBM4MiGSL8RysFYD/ 9N7NMJ4qhPKZFXBf7AT/5QxfF8UkyWlA7jy9GlldXDB0n8ysniGZLwoBAuKqLLJ7dGZMp6THqx5as/ 9Ses9Mb3ks27kX5orpVY2QkXXEZJW/TCUYxBAJ8N2DfpY9nTO8nZrnvpj0h+N0O6air2C9jT23R/sc UlnayH/4vWlVW2IRx5YCm9A8/Pi9YQTUYk3W17q/WWQ2ll6nRGuNRgsanJ8YieiFy4DPHf+hOKc+tf X5973HvnfNYsxX7Ye0TfIj8QgzLEUpMmBABlX1hpcoGBP39vMiqG0EGrshUvCi+7hcPHtgIvE2HVPw lxQHFb83G51FvZLTOhgz66Zrz81f9Z5GGqlANGmAE0vFV8l9YLkqk6rSEm8RUdVXpA+Wi0McMdcpIj o4mI0n4tPuAbtFkxMLel+pMVwIerObvGjjymyh61TxVkXXFDPyryRtoQ404Z0GGSMzOjYOtxhusbYE UEpvp1POphnVpMwCyVEwlj2ddINo2Hv7w2VmLqMiAqyDEISX/qNDlzM61ShTo1ZuwaFcy0qs4/AImK 5LWhO1TwFCopmyl/75jrIF96+8J6CQ71BcscMiAagvCpuw2YqBWG7v17T27MSNhyRXAuDNNHl9NEhI U1U1D5XoOOCv4pedDwAuRrYk3Q0LZJQPXFATfglSAg/vKVSczbd43WQTaYlA== X-Developer-Key: i=memxor@gmail.com; a=openpgp; fpr=4BBE2A7E06ECF9D5823C61114CE0C88648BF11CA X-Patchwork-Delegate: bpf@iogearbox.net There are three situations when a program logically exits and transfers control to the kernel or another program: bpf_throw, BPF_EXIT, and tail calls. The former two check for any lingering locks and references, but tail calls currently do not. Expand the checks to check for spin locks, RCU read sections and preempt disabled sections. Spin locks are indirectly preventing tail calls as function calls are disallowed, but the checks for preemption and RCU are more relaxed, hence ensure tail calls are prevented in their presence. Fixes: 9bb00b2895cb ("bpf: Add kfunc bpf_rcu_read_lock/unlock()") Fixes: fc7566ad0a82 ("bpf: Introduce bpf_preempt_[disable,enable] kfuncs") Signed-off-by: Kumar Kartikeya Dwivedi --- kernel/bpf/verifier.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 797cf3ed32e0..0844b4383ff3 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -10620,11 +10620,26 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn switch (func_id) { case BPF_FUNC_tail_call: + if (env->cur_state->active_lock.ptr) { + verbose(env, "tail_call cannot be used inside bpf_spin_lock-ed region\n"); + return -EINVAL; + } + err = check_reference_leak(env, false); if (err) { verbose(env, "tail_call would lead to reference leak\n"); return err; } + + if (env->cur_state->active_rcu_lock) { + verbose(env, "tail_call cannot be used inside bpf_rcu_read_lock-ed region\n"); + return -EINVAL; + } + + if (env->cur_state->active_preempt_lock) { + verbose(env, "tail_call cannot be used inside bpf_preempt_disable-ed region\n"); + return -EINVAL; + } break; case BPF_FUNC_get_local_storage: /* check that flags argument in get_local_storage(map, flags) is 0, From patchwork Sun Nov 3 22:59:39 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Kartikeya Dwivedi X-Patchwork-Id: 13860685 X-Patchwork-Delegate: bpf@iogearbox.net Received: from mail-wm1-f67.google.com (mail-wm1-f67.google.com [209.85.128.67]) (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 4F19718C341 for ; Sun, 3 Nov 2024 22:59:46 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.67 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730674789; cv=none; b=iogE+Jzi2sDqSqZ5aVFrR7+Rp1qmwtTWbskTpSKqLMv0VY+XtSemoUNq//Tibg+YFjZGbJJ1rY9q/eXYj8ZYPzkw7fpOwH0m7GOCwxSSeRVUTR6M6czv1taHf+1WcafDrTJ/B8bpI6/fTfneQxsyjtkoLMzgpYfxKNfPKYUanLs= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730674789; c=relaxed/simple; bh=T8lHgUfABeZYik4WO7wihDPAjeVYcvUSwtnaBuwuvkY=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=Qrbr9W2o67HVdp2qH99WfTqJLco2uLV8EYgjyMqv4Zd9ReePzcBdOIx9WJKnntVdOWmOR158nd/UtjqHjKk84Q5vZ8RG9yQUHj5JfRxBO7960qLQcMgfVeIp0ZJyATKCtEZZOqiz+7kfwEZRM3mhg5ZptMCupL8B1SLqnM+ONm0= 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=GRpeK69X; arc=none smtp.client-ip=209.85.128.67 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="GRpeK69X" Received: by mail-wm1-f67.google.com with SMTP id 5b1f17b1804b1-4316f3d3c21so28063315e9.3 for ; Sun, 03 Nov 2024 14:59:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1730674785; x=1731279585; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=HqcyiXpwMGVMBRAu6cGRQnZavo2Ny6Ei77oKORDGJWQ=; b=GRpeK69Xd05Rcbm0xMeQ22o3hRs5FTsvgXkxEAGnFuK0EtZA22HaB+UJB2dIhvUIOW 1CDytPBBr/S6Ag57CLh2Uz3AHe4SCLWCZSHxAgVey2yI4pz+bN/M8l+5PjAEX4hSiA0p Ky+htxZ+ipRV45on3WkZ/w691Xf9cz4R0ve3oz3oT8ewXx2aOr1oOTNthLD24V97a98c 2FaHZLgGRbm+rUXR0U/WDJxgKk86OPSbZl8nC1cCC4KWsuW0i1xOVfyDSE9SX7BjWy8k kZw1XunFkRTh/VOdT4LiX91/P1zErk1yvZNbDf5i5ZJ37Rl3mmjZs9OgxTzYxht1eeQj epvA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730674785; x=1731279585; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=HqcyiXpwMGVMBRAu6cGRQnZavo2Ny6Ei77oKORDGJWQ=; b=CNW8T0vxBOFpFPzqTrPlP6mjjSqa724LAAYfm00vNQXZ8pX8enkcqAzsFI/jrFRxei viU02ToBvbhefXSezbXg2HDWTHXhqzVtaU2GG4nYw+hD/7YAHYMVax71twhsDTxOVMOw swOCPAJldgUewNKaVvMtQeukdtkVwISD/LPIqpQFL0gqR07cGDwycCsFGjkRrzUvR0af jr1NZDW1xrOqJFPEelgyv0kAkhA+4tfUbTUIFoAl6duRXApeBQ0PaejAJFw3mBgmUYGF V62scEiwbz0gn6iPFXRLMXZlPEUTeJ8r78b87Joky8JF3g9sL+8x/hciFBMkAAemP96/ 86LA== X-Gm-Message-State: AOJu0Yw8fvCNXi8SY6cMSYkzt3OhYaRdILrdW9xxQiTipp7KjJrqFvZb 6yaEvu5a9ayYAuWpoJjP/bednjfE0NG8ljmkMOtOeyEbzWaKVavrJrmMRx+P/LMQpw== X-Google-Smtp-Source: AGHT+IFsCUeKouQPRplnmvkX5GxamZi3MEwKUIPy/noGYFpqa4Hn/kfwA5nQaJBWU5bDaxu7OeD0iA== X-Received: by 2002:adf:e705:0:b0:37c:ddab:a626 with SMTP id ffacd0b85a97d-381b70572bfmr12322448f8f.7.1730674784870; Sun, 03 Nov 2024 14:59:44 -0800 (PST) Received: from localhost (fwdproxy-cln-024.fbsv.net. [2a03:2880:31ff:18::face:b00c]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-431bd947c26sm167536645e9.26.2024.11.03.14.59.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 03 Nov 2024 14:59:44 -0800 (PST) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: kkd@meta.com, Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , Eduard Zingerman , kernel-team@fb.com Subject: [PATCH bpf-next v1 2/3] bpf: Unify resource leak checks Date: Sun, 3 Nov 2024 14:59:39 -0800 Message-ID: <20241103225940.1408302-3-memxor@gmail.com> X-Mailer: git-send-email 2.43.5 In-Reply-To: <20241103225940.1408302-1-memxor@gmail.com> References: <20241103225940.1408302-1-memxor@gmail.com> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=10344; h=from:subject; bh=T8lHgUfABeZYik4WO7wihDPAjeVYcvUSwtnaBuwuvkY=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBnJ//puIe+O95Dz5NKYSU+hhGCSY6GsyxRxp7J4ylz 92B07MyJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCZyf/6QAKCRBM4MiGSL8RyoS7D/ 9OTnLKLDeeBvC9RwjPDCVOHSi3DRqnutK0NCmJgG5xUaiQQwtlo1LH/B0ewoFYZlJ159J+OtC3nNgk nCra49gmZUj2aE5pr2gkr3bpGWsIP5JJrIDmlOFgYNoovKbcaxz5S7CO/145h4I1NEkJK0iYiAdqFG yLqLaUUn4MnZY9wt6cdM0v+82eADTNtC/E3n3neJVKAd6qw59tEnKYjesIXIBvK9Hqmw4BHTNZaCZH t5W9jTg/9ABrrA2grB0Web4etvgz2TmgDnfnCdEKgeVw8GXf3xAFAC5pfuwnHBQB69oYVb/0CPNGsO sPSfIig3ByRoNVz+6T0NOA9npznaDtC5m+sT9KBIJ+JzTEsTDuEUE8qVQX9JIQHjiZ8Ay5DCglpp2E tvFTRRyBMgkyUC3Gpt8pdDiH+aWUQxuLfPZ0X9R5+HUSUe4ZIOa/84jl/vLBQo9lu0on8ffWhrOer3 AYBfcLpHfHwpb8GIfX4/KWt8B4YLzY8SYQnpx0+wlsgDImBdtOrAcntH6Md7i/Q2JXCBo8CiSW8vzh Fk6I3Ix85Bg/zVENXytfyBqKt9VdX3ZmtOBiVMpABNJgef+n94VjfLPZsBX55Hwk3PEg0PgeUrQD6P 1MqYGeSDBjk3vn9yoYXXNwplB7PV2MrM4jC2Z7g5bT1wWqSrtUtyqEQcGWzQ== X-Developer-Key: i=memxor@gmail.com; a=openpgp; fpr=4BBE2A7E06ECF9D5823C61114CE0C88648BF11CA X-Patchwork-Delegate: bpf@iogearbox.net There are similar checks for covering locks, references, RCU read sections and preempt_disable sections in 3 places in the verifer, i.e. for tail calls, bpf_ld_[abs, ind], and exit path (for BPF_EXIT and bpf_throw). Unify all of these into a common check_resource_leak function to avoid code duplication. Also update the error strings in selftests to the new ones in the same change to ensure clean bisection. Signed-off-by: Kumar Kartikeya Dwivedi --- kernel/bpf/verifier.c | 90 +++++++------------ .../selftests/bpf/progs/exceptions_fail.c | 4 +- .../selftests/bpf/progs/preempt_lock.c | 14 +-- .../bpf/progs/verifier_ref_tracking.c | 4 +- .../selftests/bpf/progs/verifier_spin_lock.c | 2 +- 5 files changed, 46 insertions(+), 68 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 0844b4383ff3..ba800c7611e3 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -10352,6 +10352,34 @@ static int check_reference_leak(struct bpf_verifier_env *env, bool exception_exi return refs_lingering ? -EINVAL : 0; } +static int check_resource_leak(struct bpf_verifier_env *env, bool exception_exit, bool check_lock, const char *prefix) +{ + int err; + + if (check_lock && env->cur_state->active_lock.ptr) { + verbose(env, "%s cannot be used inside bpf_spin_lock-ed region\n", prefix); + return -EINVAL; + } + + err = check_reference_leak(env, exception_exit); + if (err) { + verbose(env, "%s would lead to reference leak\n", prefix); + return err; + } + + if (check_lock && env->cur_state->active_rcu_lock) { + verbose(env, "%s cannot be used inside bpf_rcu_read_lock-ed region\n", prefix); + return -EINVAL; + } + + if (check_lock && env->cur_state->active_preempt_lock) { + verbose(env, "%s cannot be used inside bpf_preempt_disable-ed region\n", prefix); + return -EINVAL; + } + + return 0; +} + static int check_bpf_snprintf_call(struct bpf_verifier_env *env, struct bpf_reg_state *regs) { @@ -10620,26 +10648,9 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn switch (func_id) { case BPF_FUNC_tail_call: - if (env->cur_state->active_lock.ptr) { - verbose(env, "tail_call cannot be used inside bpf_spin_lock-ed region\n"); - return -EINVAL; - } - - err = check_reference_leak(env, false); - if (err) { - verbose(env, "tail_call would lead to reference leak\n"); + err = check_resource_leak(env, false, true, "tail_call"); + if (err) return err; - } - - if (env->cur_state->active_rcu_lock) { - verbose(env, "tail_call cannot be used inside bpf_rcu_read_lock-ed region\n"); - return -EINVAL; - } - - if (env->cur_state->active_preempt_lock) { - verbose(env, "tail_call cannot be used inside bpf_preempt_disable-ed region\n"); - return -EINVAL; - } break; case BPF_FUNC_get_local_storage: /* check that flags argument in get_local_storage(map, flags) is 0, @@ -15801,26 +15812,9 @@ static int check_ld_abs(struct bpf_verifier_env *env, struct bpf_insn *insn) * gen_ld_abs() may terminate the program at runtime, leading to * reference leak. */ - err = check_reference_leak(env, false); - if (err) { - verbose(env, "BPF_LD_[ABS|IND] cannot be mixed with socket references\n"); + err = check_resource_leak(env, false, true, "BPF_LD_[ABS|IND]"); + if (err) return err; - } - - if (env->cur_state->active_lock.ptr) { - verbose(env, "BPF_LD_[ABS|IND] cannot be used inside bpf_spin_lock-ed region\n"); - return -EINVAL; - } - - if (env->cur_state->active_rcu_lock) { - verbose(env, "BPF_LD_[ABS|IND] cannot be used inside bpf_rcu_read_lock-ed region\n"); - return -EINVAL; - } - - if (env->cur_state->active_preempt_lock) { - verbose(env, "BPF_LD_[ABS|IND] cannot be used inside bpf_preempt_disable-ed region\n"); - return -EINVAL; - } if (regs[ctx_reg].type != PTR_TO_CTX) { verbose(env, @@ -18606,30 +18600,14 @@ static int do_check(struct bpf_verifier_env *env) return -EINVAL; } process_bpf_exit_full: - if (env->cur_state->active_lock.ptr && !env->cur_state->curframe) { - verbose(env, "bpf_spin_unlock is missing\n"); - return -EINVAL; - } - - if (env->cur_state->active_rcu_lock && !env->cur_state->curframe) { - verbose(env, "bpf_rcu_read_unlock is missing\n"); - return -EINVAL; - } - - if (env->cur_state->active_preempt_lock && !env->cur_state->curframe) { - verbose(env, "%d bpf_preempt_enable%s missing\n", - env->cur_state->active_preempt_lock, - env->cur_state->active_preempt_lock == 1 ? " is" : "(s) are"); - return -EINVAL; - } - /* We must do check_reference_leak here before * prepare_func_exit to handle the case when * state->curframe > 0, it may be a callback * function, for which reference_state must * match caller reference state when it exits. */ - err = check_reference_leak(env, exception_exit); + err = check_resource_leak(env, exception_exit, !env->cur_state->curframe, + "BPF_EXIT instruction"); if (err) return err; diff --git a/tools/testing/selftests/bpf/progs/exceptions_fail.c b/tools/testing/selftests/bpf/progs/exceptions_fail.c index 9cceb6521143..fe0f3fa5aab6 100644 --- a/tools/testing/selftests/bpf/progs/exceptions_fail.c +++ b/tools/testing/selftests/bpf/progs/exceptions_fail.c @@ -131,7 +131,7 @@ int reject_subprog_with_lock(void *ctx) } SEC("?tc") -__failure __msg("bpf_rcu_read_unlock is missing") +__failure __msg("BPF_EXIT instruction cannot be used inside bpf_rcu_read_lock-ed region") int reject_with_rcu_read_lock(void *ctx) { bpf_rcu_read_lock(); @@ -147,7 +147,7 @@ __noinline static int throwing_subprog(struct __sk_buff *ctx) } SEC("?tc") -__failure __msg("bpf_rcu_read_unlock is missing") +__failure __msg("BPF_EXIT instruction cannot be used inside bpf_rcu_read_lock-ed region") int reject_subprog_with_rcu_read_lock(void *ctx) { bpf_rcu_read_lock(); diff --git a/tools/testing/selftests/bpf/progs/preempt_lock.c b/tools/testing/selftests/bpf/progs/preempt_lock.c index 672fc368d9c4..885377e83607 100644 --- a/tools/testing/selftests/bpf/progs/preempt_lock.c +++ b/tools/testing/selftests/bpf/progs/preempt_lock.c @@ -6,7 +6,7 @@ #include "bpf_experimental.h" SEC("?tc") -__failure __msg("1 bpf_preempt_enable is missing") +__failure __msg("BPF_EXIT instruction cannot be used inside bpf_preempt_disable-ed region") int preempt_lock_missing_1(struct __sk_buff *ctx) { bpf_preempt_disable(); @@ -14,7 +14,7 @@ int preempt_lock_missing_1(struct __sk_buff *ctx) } SEC("?tc") -__failure __msg("2 bpf_preempt_enable(s) are missing") +__failure __msg("BPF_EXIT instruction cannot be used inside bpf_preempt_disable-ed region") int preempt_lock_missing_2(struct __sk_buff *ctx) { bpf_preempt_disable(); @@ -23,7 +23,7 @@ int preempt_lock_missing_2(struct __sk_buff *ctx) } SEC("?tc") -__failure __msg("3 bpf_preempt_enable(s) are missing") +__failure __msg("BPF_EXIT instruction cannot be used inside bpf_preempt_disable-ed region") int preempt_lock_missing_3(struct __sk_buff *ctx) { bpf_preempt_disable(); @@ -33,7 +33,7 @@ int preempt_lock_missing_3(struct __sk_buff *ctx) } SEC("?tc") -__failure __msg("1 bpf_preempt_enable is missing") +__failure __msg("BPF_EXIT instruction cannot be used inside bpf_preempt_disable-ed region") int preempt_lock_missing_3_minus_2(struct __sk_buff *ctx) { bpf_preempt_disable(); @@ -55,7 +55,7 @@ static __noinline void preempt_enable(void) } SEC("?tc") -__failure __msg("1 bpf_preempt_enable is missing") +__failure __msg("BPF_EXIT instruction cannot be used inside bpf_preempt_disable-ed region") int preempt_lock_missing_1_subprog(struct __sk_buff *ctx) { preempt_disable(); @@ -63,7 +63,7 @@ int preempt_lock_missing_1_subprog(struct __sk_buff *ctx) } SEC("?tc") -__failure __msg("2 bpf_preempt_enable(s) are missing") +__failure __msg("BPF_EXIT instruction cannot be used inside bpf_preempt_disable-ed region") int preempt_lock_missing_2_subprog(struct __sk_buff *ctx) { preempt_disable(); @@ -72,7 +72,7 @@ int preempt_lock_missing_2_subprog(struct __sk_buff *ctx) } SEC("?tc") -__failure __msg("1 bpf_preempt_enable is missing") +__failure __msg("BPF_EXIT instruction cannot be used inside bpf_preempt_disable-ed region") int preempt_lock_missing_2_minus_1_subprog(struct __sk_buff *ctx) { preempt_disable(); diff --git a/tools/testing/selftests/bpf/progs/verifier_ref_tracking.c b/tools/testing/selftests/bpf/progs/verifier_ref_tracking.c index c4c6da21265e..683a882b3e6d 100644 --- a/tools/testing/selftests/bpf/progs/verifier_ref_tracking.c +++ b/tools/testing/selftests/bpf/progs/verifier_ref_tracking.c @@ -791,7 +791,7 @@ l0_%=: r0 = *(u8*)skb[0]; \ SEC("tc") __description("reference tracking: forbid LD_ABS while holding reference") -__failure __msg("BPF_LD_[ABS|IND] cannot be mixed with socket references") +__failure __msg("BPF_LD_[ABS|IND] would lead to reference leak") __naked void ld_abs_while_holding_reference(void) { asm volatile (" \ @@ -836,7 +836,7 @@ l0_%=: r7 = 1; \ SEC("tc") __description("reference tracking: forbid LD_IND while holding reference") -__failure __msg("BPF_LD_[ABS|IND] cannot be mixed with socket references") +__failure __msg("BPF_LD_[ABS|IND] would lead to reference leak") __naked void ld_ind_while_holding_reference(void) { asm volatile (" \ diff --git a/tools/testing/selftests/bpf/progs/verifier_spin_lock.c b/tools/testing/selftests/bpf/progs/verifier_spin_lock.c index fb316c080c84..3f679de73229 100644 --- a/tools/testing/selftests/bpf/progs/verifier_spin_lock.c +++ b/tools/testing/selftests/bpf/progs/verifier_spin_lock.c @@ -187,7 +187,7 @@ l0_%=: r6 = r0; \ SEC("cgroup/skb") __description("spin_lock: test6 missing unlock") -__failure __msg("unlock is missing") +__failure __msg("BPF_EXIT instruction cannot be used inside bpf_spin_lock-ed region") __failure_unpriv __msg_unpriv("") __naked void spin_lock_test6_missing_unlock(void) { From patchwork Sun Nov 3 22:59:40 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Kartikeya Dwivedi X-Patchwork-Id: 13860686 X-Patchwork-Delegate: bpf@iogearbox.net Received: from mail-wr1-f67.google.com (mail-wr1-f67.google.com [209.85.221.67]) (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 3EEF018B497 for ; Sun, 3 Nov 2024 22:59:48 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.67 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730674789; cv=none; b=YG7DWuZEziRbRAgRQoynV4znN5Ja6B5KY0zh6PW3du64tld4j20Pe5muUuMhxwpyWAV08dxO2xnyMb36Sp6QbevIbSoGkw6tuaAB4W9GYhDLzvdQjdZFJ9IE/BgHKDnik/Re6KbpbjXA93LJurHy6QCN+8W44cAJ8yCiSE6Se1s= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730674789; c=relaxed/simple; bh=T0ikGcuZLizG+9qdvlh2bCrtYoBDsSk1JaKHw8b29HY=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=KCkMSpBBZWJgA7rzQfpqmBdgKI+ofzx2v2+kEnNfjXFjviCOBNB+j+Z024tsTGR2tEABRFoXiVL4GaJp/RcDEhwOTYXdwKTGexscFb9TcstKjlE9jytI6n5t3kiAGkZf0gT/hMx1VENJr2UxL4Q0S9ammlJw7vNSShTwWgtgbEY= 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=iUfU0ivY; arc=none smtp.client-ip=209.85.221.67 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="iUfU0ivY" Received: by mail-wr1-f67.google.com with SMTP id ffacd0b85a97d-37d50fad249so2650176f8f.1 for ; Sun, 03 Nov 2024 14:59:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1730674786; x=1731279586; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=b8W8p0I3ZORpkSffN3EQ//vJwQVZRFBeB8L+KyUHq4I=; b=iUfU0ivYll+VuPgUOPW63l1y4ob5lvBuny0wpkApJy+lMJwGfcAXpTR+D9MfzJjMs8 uR/DXvHrLFr2AkuouyHwASwF/A1y0Av2q25hWebcQUVLwDeY7lI29Sh4A211iwFfIx2M MYMC0nBEijHzNdnNNWQjZLUbrAPUXJu7rnWZ+EmyvyHdd18p7HK8seW+50N235yYrjNx 6wZ198fGbDrSLoPWRC6v+RqHT3Vq5539qJXNYg+4Uqa/wq+biySU0u/4HTiGurCZbOKe 6raindeOB/Vw9qwJ8fWjPvqUAHLdN24t5lC8qhp1Gm9TVG4YZguQ0iuRny1B0Om3upko hyng== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730674786; x=1731279586; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=b8W8p0I3ZORpkSffN3EQ//vJwQVZRFBeB8L+KyUHq4I=; b=QWy3mUDBfebV4P2cQ899doAFoToxFzXx0sPdLtYqpQ0evJL87C5pk1C1DaQj+Jg0Tw ApLfM4e3kycrusRHIkqznb9px+lD/AZRHnLeRe1ZJiNF/KI3v9obWYXvx8QAgftabKJP LT/HfXFVjwmJyn/cQ0PC9sXNhGf/Lh7vflGEtCykYB/XCjCLj9g6gK8Xe6opsWYy6IgV /Gc84CJsryRY9m4VJv/3oC3s/KvhQioFcSTSnrbQdNVBwL+saSZiAJCzAYg2VDAo2a1w uL/pJ6DNTm7CZcPRVz/gWthR4aDePWOs0ENUpPhkvuyV5il3pHCdpJrQrpwzt1dbyADp L+FA== X-Gm-Message-State: AOJu0YwS873mZeFwiZ3CT58uGNtS7OAUf9S4OQZv/t/l9trQnbnMAwyW Q3RMlqlHdT/fdzaE5/V60vLkDnC4BbbIyvTURt9ToQ/olUKOi4csjwJHNc/wcC8AFA== X-Google-Smtp-Source: AGHT+IHmA9Zf803ad7nVOpbQgsxrXBta39UFPvGP2ucDNAx3MRS2nFqXyVIBqgWj6SBjYx3d4ZcmxQ== X-Received: by 2002:a05:6000:7:b0:37c:d162:8297 with SMTP id ffacd0b85a97d-380611edf92mr21006368f8f.47.1730674786292; Sun, 03 Nov 2024 14:59:46 -0800 (PST) Received: from localhost (fwdproxy-cln-015.fbsv.net. [2a03:2880:31ff:f::face:b00c]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-381c113dd7fsm11642708f8f.70.2024.11.03.14.59.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 03 Nov 2024 14:59:45 -0800 (PST) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org Cc: kkd@meta.com, Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , Eduard Zingerman , kernel-team@fb.com Subject: [PATCH bpf-next v1 3/3] selftests/bpf: Add tests for tail calls with locks and refs Date: Sun, 3 Nov 2024 14:59:40 -0800 Message-ID: <20241103225940.1408302-4-memxor@gmail.com> X-Mailer: git-send-email 2.43.5 In-Reply-To: <20241103225940.1408302-1-memxor@gmail.com> References: <20241103225940.1408302-1-memxor@gmail.com> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=3612; h=from:subject; bh=T0ikGcuZLizG+9qdvlh2bCrtYoBDsSk1JaKHw8b29HY=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBnJ//pkJMSKXM/W9xMBwJFvINFWL8peWEhJIFBcsx1 3nCmdgaJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCZyf/6QAKCRBM4MiGSL8RygFCD/ 4xn1AClGr2Llr1NGXIXjKnV9QDFHsIjRHrFdCxXpR0CyBYP3ikEj/wGH3x1VfnqhPv/Pl+ZySAL0vi AWAibkLPvHEbciEA6DXUw+Lndtho/wKpXBQXurvs4drHfkx1OWuTTg+R/LEYxzu3v70nGivbm01O43 vEgEGzST0wcmPLkh245o7SKqcB872G/M6nGCMEAbG27k4xgCEJvoG7EaKvh6nct0uDHuAXOOEL4ZbE iAQ15t1jCbwye9dR0E7xFF56mBY/Jlo5JOpsXZnNSc6pGiC39d4y5CfuZvzkI+bDBYLlthfr89gEnL l4QIv428VyXd5uOGCd/qctMQshfz9hWTd7yYC3TeVJwXqpJ/a77hsIcHzVtGcfvd19Y+yVJQiu4qLC dRdQgBouMiq/3B1zqsLchMDkOoRPc8/CEWYKIj7GjfuYupUHy8vp/jdCYAx6C0926100fkKZexFUnW Hj5cL5Qlr61mBF0H0XXTW1zmu+hopWTUMGIXyS+Ca/FBGIvqrz6cjWgyyhNGi7F56yusZIjuKfeIT1 5m++tVnolN26lTY6ujanFrliUYAH8uMyRIjGwRXokc7e4LY3grFzbBjhT8DXvroZF3LIjHGLihOjEw Rou8fuP5DqbvnIUo1bhj6p/MRDUAVAA0CDLPGMHSx5kyacQPuHvC5RnsvUaA== X-Developer-Key: i=memxor@gmail.com; a=openpgp; fpr=4BBE2A7E06ECF9D5823C61114CE0C88648BF11CA X-Patchwork-Delegate: bpf@iogearbox.net Add failure tests to ensure bugs don't slip through for tail calls and lingering locks, RCU sections, preemption disabled sections, and references prevent tail calls. Signed-off-by: Kumar Kartikeya Dwivedi --- .../selftests/bpf/prog_tests/tailcalls.c | 8 +++ .../selftests/bpf/progs/tailcall_fail.c | 64 +++++++++++++++++++ 2 files changed, 72 insertions(+) create mode 100644 tools/testing/selftests/bpf/progs/tailcall_fail.c diff --git a/tools/testing/selftests/bpf/prog_tests/tailcalls.c b/tools/testing/selftests/bpf/prog_tests/tailcalls.c index 40f22454cf05..544144620ca6 100644 --- a/tools/testing/selftests/bpf/prog_tests/tailcalls.c +++ b/tools/testing/selftests/bpf/prog_tests/tailcalls.c @@ -7,6 +7,7 @@ #include "tailcall_bpf2bpf_hierarchy3.skel.h" #include "tailcall_freplace.skel.h" #include "tc_bpf2bpf.skel.h" +#include "tailcall_fail.skel.h" /* test_tailcall_1 checks basic functionality by patching multiple locations * in a single program for a single tail call slot with nop->jmp, jmp->nop @@ -1646,6 +1647,11 @@ static void test_tailcall_bpf2bpf_freplace(void) tc_bpf2bpf__destroy(tc_skel); } +static void test_tailcall_failure() +{ + RUN_TESTS(tailcall_fail); +} + void test_tailcalls(void) { if (test__start_subtest("tailcall_1")) @@ -1698,4 +1704,6 @@ void test_tailcalls(void) test_tailcall_freplace(); if (test__start_subtest("tailcall_bpf2bpf_freplace")) test_tailcall_bpf2bpf_freplace(); + if (test__start_subtest("tailcall_failure")) + test_tailcall_failure(); } diff --git a/tools/testing/selftests/bpf/progs/tailcall_fail.c b/tools/testing/selftests/bpf/progs/tailcall_fail.c new file mode 100644 index 000000000000..bc77921d2bb0 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/tailcall_fail.c @@ -0,0 +1,64 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2024 Meta Platforms, Inc. and affiliates. */ +#include +#include +#include +#include + +#include "bpf_misc.h" +#include "bpf_experimental.h" + +extern void bpf_rcu_read_lock(void) __ksym; +extern void bpf_rcu_read_unlock(void) __ksym; + +#define private(name) SEC(".bss." #name) __hidden __attribute__((aligned(8))) + +private(A) struct bpf_spin_lock lock; + +struct { + __uint(type, BPF_MAP_TYPE_PROG_ARRAY); + __uint(max_entries, 3); + __uint(key_size, sizeof(__u32)); + __uint(value_size, sizeof(__u32)); +} jmp_table SEC(".maps"); + +SEC("?tc") +__failure __msg("function calls are not allowed while holding a lock") +int reject_tail_call_spin_lock(struct __sk_buff *ctx) +{ + bpf_spin_lock(&lock); + bpf_tail_call_static(ctx, &jmp_table, 0); + return 0; +} + +SEC("?tc") +__failure __msg("tail_call cannot be used inside bpf_rcu_read_lock-ed region") +int reject_tail_call_rcu_lock(struct __sk_buff *ctx) +{ + bpf_rcu_read_lock(); + bpf_tail_call_static(ctx, &jmp_table, 0); + bpf_rcu_read_unlock(); + return 0; +} + +SEC("?tc") +__failure __msg("tail_call cannot be used inside bpf_preempt_disable-ed region") +int reject_tail_call_preempt_lock(struct __sk_buff *ctx) +{ + bpf_guard_preempt(); + bpf_tail_call_static(ctx, &jmp_table, 0); + return 0; +} + +SEC("?tc") +__failure __msg("tail_call would lead to reference leak") +int reject_tail_call_ref(struct __sk_buff *ctx) +{ + struct foo { int i; } *p; + + p = bpf_obj_new(typeof(*p)); + bpf_tail_call_static(ctx, &jmp_table, 0); + return 0; +} + +char _license[] SEC("license") = "GPL";