linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Daniel Rosenberg <drosen@google.com>
To: Miklos Szeredi <miklos@szeredi.hu>,
	bpf@vger.kernel.org, Alexei Starovoitov <ast@kernel.org>
Cc: Amir Goldstein <amir73il@gmail.com>,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-unionfs@vger.kernel.org,
	Daniel Borkmann <daniel@iogearbox.net>,
	John Fastabend <john.fastabend@gmail.com>,
	Andrii Nakryiko <andrii@kernel.org>,
	Martin KaFai Lau <martin.lau@linux.dev>,
	Song Liu <song@kernel.org>, Yonghong Song <yhs@fb.com>,
	KP Singh <kpsingh@kernel.org>,
	Stanislav Fomichev <sdf@google.com>, Hao Luo <haoluo@google.com>,
	Jiri Olsa <jolsa@kernel.org>, Shuah Khan <shuah@kernel.org>,
	Jonathan Corbet <corbet@lwn.net>,
	Joanne Koong <joannelkoong@gmail.com>,
	Mykola Lysenko <mykolal@fb.com>,
	kernel-team@android.com, Daniel Rosenberg <drosen@google.com>
Subject: [RFC PATCH v3 02/37] bpf: Allow NULL buffers in bpf_dynptr_slice(_rw)
Date: Mon, 17 Apr 2023 18:40:02 -0700	[thread overview]
Message-ID: <20230418014037.2412394-3-drosen@google.com> (raw)
In-Reply-To: <20230418014037.2412394-1-drosen@google.com>

bpf_dynptr_slice(_rw) uses a user provided buffer if it can not provide
a pointer to a block of contiguous memory. This buffer is unused in the
case of local dynptrs, and may be unused in other cases as well. There
is no need to require the buffer, as the kfunc can just return NULL if
it was needed and not provided.

This adds another kfunc annotation, __opt, which combines with __sz and
__szk to allow the buffer associated with the size to be NULL. If the
buffer is NULL, the verifier does not check that the buffer is of
sufficient size.

Signed-off-by: Daniel Rosenberg <drosen@google.com>
---
 Documentation/bpf/kfuncs.rst | 23 ++++++++++++++++++++++-
 kernel/bpf/helpers.c         | 32 ++++++++++++++++++++------------
 kernel/bpf/verifier.c        | 19 +++++++++++++++++++
 3 files changed, 61 insertions(+), 13 deletions(-)

diff --git a/Documentation/bpf/kfuncs.rst b/Documentation/bpf/kfuncs.rst
index ea2516374d92..7a3d9de5f315 100644
--- a/Documentation/bpf/kfuncs.rst
+++ b/Documentation/bpf/kfuncs.rst
@@ -100,7 +100,7 @@ Hence, whenever a constant scalar argument is accepted by a kfunc which is not a
 size parameter, and the value of the constant matters for program safety, __k
 suffix should be used.
 
-2.2.2 __uninit Annotation
+2.2.3 __uninit Annotation
 -------------------------
 
 This annotation is used to indicate that the argument will be treated as
@@ -117,6 +117,27 @@ Here, the dynptr will be treated as an uninitialized dynptr. Without this
 annotation, the verifier will reject the program if the dynptr passed in is
 not initialized.
 
+2.2.4 __opt Annotation
+-------------------------
+
+This annotation is used to indicate that the buffer associated with an __sz or __szk
+argument may be null. If the function is passed a nullptr in place of the buffer,
+the verifier will not check that length is appropriate for the buffer. The kfunc is
+responsible for checking if this buffer is null before using it.
+
+An example is given below::
+
+        __bpf_kfunc void *bpf_dynptr_slice(..., void *buffer__opt, u32 buffer__szk)
+        {
+        ...
+        }
+
+Here, the buffer may be null. If buffer is not null, it at least of size buffer_szk.
+Either way, the returned buffer is either NULL, or of size buffer_szk. Without this
+annotation, the verifier will reject the program if a null pointer is passed in with
+a nonzero size.
+
+
 .. _BPF_kfunc_nodef:
 
 2.3 Using an existing kernel function
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 00e5fb0682ac..bfb75ecacb76 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -2167,13 +2167,15 @@ __bpf_kfunc struct task_struct *bpf_task_from_pid(s32 pid)
  * bpf_dynptr_slice() - Obtain a read-only pointer to the dynptr data.
  * @ptr: The dynptr whose data slice to retrieve
  * @offset: Offset into the dynptr
- * @buffer: User-provided buffer to copy contents into
- * @buffer__szk: Size (in bytes) of the buffer. This is the length of the
- *		 requested slice. This must be a constant.
+ * @buffer__opt: User-provided buffer to copy contents into.  May be NULL
+ * @buffer__szk: Size (in bytes) of the buffer if present. This is the
+ *               length of the requested slice. This must be a constant.
  *
  * For non-skb and non-xdp type dynptrs, there is no difference between
  * bpf_dynptr_slice and bpf_dynptr_data.
  *
+ *  If buffer__opt is NULL, the call will fail if buffer_opt was needed.
+ *
  * If the intention is to write to the data slice, please use
  * bpf_dynptr_slice_rdwr.
  *
@@ -2190,7 +2192,7 @@ __bpf_kfunc struct task_struct *bpf_task_from_pid(s32 pid)
  * direct pointer)
  */
 __bpf_kfunc void *bpf_dynptr_slice(const struct bpf_dynptr_kern *ptr, u32 offset,
-				   void *buffer, u32 buffer__szk)
+				   void *buffer__opt, u32 buffer__szk)
 {
 	enum bpf_dynptr_type type;
 	u32 len = buffer__szk;
@@ -2210,15 +2212,19 @@ __bpf_kfunc void *bpf_dynptr_slice(const struct bpf_dynptr_kern *ptr, u32 offset
 	case BPF_DYNPTR_TYPE_RINGBUF:
 		return ptr->data + ptr->offset + offset;
 	case BPF_DYNPTR_TYPE_SKB:
-		return skb_header_pointer(ptr->data, ptr->offset + offset, len, buffer);
+		if (!buffer__opt)
+			return NULL;
+		return skb_header_pointer(ptr->data, ptr->offset + offset, len, buffer__opt);
 	case BPF_DYNPTR_TYPE_XDP:
 	{
 		void *xdp_ptr = bpf_xdp_pointer(ptr->data, ptr->offset + offset, len);
 		if (xdp_ptr)
 			return xdp_ptr;
 
-		bpf_xdp_copy_buf(ptr->data, ptr->offset + offset, buffer, len, false);
-		return buffer;
+		if (!buffer__opt)
+			return NULL;
+		bpf_xdp_copy_buf(ptr->data, ptr->offset + offset, buffer__opt, len, false);
+		return buffer__opt;
 	}
 	default:
 		WARN_ONCE(true, "unknown dynptr type %d\n", type);
@@ -2230,13 +2236,15 @@ __bpf_kfunc void *bpf_dynptr_slice(const struct bpf_dynptr_kern *ptr, u32 offset
  * bpf_dynptr_slice_rdwr() - Obtain a writable pointer to the dynptr data.
  * @ptr: The dynptr whose data slice to retrieve
  * @offset: Offset into the dynptr
- * @buffer: User-provided buffer to copy contents into
- * @buffer__szk: Size (in bytes) of the buffer. This is the length of the
- *		 requested slice. This must be a constant.
+ * @buffer__opt: User-provided buffer to copy contents into. May be NULL
+ * @buffer__szk: Size (in bytes) of the buffer if present. This is the
+ *               length of the requested slice. This must be a constant.
  *
  * For non-skb and non-xdp type dynptrs, there is no difference between
  * bpf_dynptr_slice and bpf_dynptr_data.
  *
+ * If buffer__opt is NULL, the call will fail if buffer_opt was needed.
+ *
  * The returned pointer is writable and may point to either directly the dynptr
  * data at the requested offset or to the buffer if unable to obtain a direct
  * data pointer to (example: the requested slice is to the paged area of an skb
@@ -2267,7 +2275,7 @@ __bpf_kfunc void *bpf_dynptr_slice(const struct bpf_dynptr_kern *ptr, u32 offset
  * direct pointer)
  */
 __bpf_kfunc void *bpf_dynptr_slice_rdwr(const struct bpf_dynptr_kern *ptr, u32 offset,
-					void *buffer, u32 buffer__szk)
+					void *buffer__opt, u32 buffer__szk)
 {
 	if (!ptr->data || bpf_dynptr_is_rdonly(ptr))
 		return NULL;
@@ -2294,7 +2302,7 @@ __bpf_kfunc void *bpf_dynptr_slice_rdwr(const struct bpf_dynptr_kern *ptr, u32 o
 	 * will be copied out into the buffer and the user will need to call
 	 * bpf_dynptr_write() to commit changes.
 	 */
-	return bpf_dynptr_slice(ptr, offset, buffer, buffer__szk);
+	return bpf_dynptr_slice(ptr, offset, buffer__opt, buffer__szk);
 }
 
 __bpf_kfunc void *bpf_cast_to_kern_ctx(void *obj)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index ebc638bfed87..fd959824469d 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -9387,6 +9387,19 @@ static bool is_kfunc_arg_const_mem_size(const struct btf *btf,
 	return __kfunc_param_match_suffix(btf, arg, "__szk");
 }
 
+static bool is_kfunc_arg_optional(const struct btf *btf,
+		  const struct btf_param *arg,
+		  const struct bpf_reg_state *reg)
+{
+	const struct btf_type *t;
+
+	t = btf_type_skip_modifiers(btf, arg->type, NULL);
+	if (!btf_type_is_ptr(t) || reg->type != SCALAR_VALUE || reg->umax_value > 0)
+		return false;
+
+	return __kfunc_param_match_suffix(btf, arg, "__opt");
+}
+
 static bool is_kfunc_arg_constant(const struct btf *btf, const struct btf_param *arg)
 {
 	return __kfunc_param_match_suffix(btf, arg, "__k");
@@ -10453,10 +10466,16 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
 			break;
 		case KF_ARG_PTR_TO_MEM_SIZE:
 		{
+			struct bpf_reg_state *buff_reg = &regs[regno];
+			const struct btf_param *buff_arg = &args[i];
 			struct bpf_reg_state *size_reg = &regs[regno + 1];
 			const struct btf_param *size_arg = &args[i + 1];
 
 			ret = check_kfunc_mem_size_reg(env, size_reg, regno + 1);
+			if (ret < 0 && is_kfunc_arg_optional(meta->btf, buff_arg, buff_reg)) {
+				verbose(env, "error was %d", ret);
+				ret = 0;
+			}
 			if (ret < 0) {
 				verbose(env, "arg#%d arg#%d memory, len pair leads to invalid memory access\n", i, i + 1);
 				return ret;
-- 
2.40.0.634.g4ca3ef3211-goog


  parent reply	other threads:[~2023-04-18  1:41 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-18  1:40 [RFC PATCH bpf-next v3 00/37] FUSE BPF: A Stacked Filesystem Extension for FUSE Daniel Rosenberg
2023-04-18  1:40 ` [RFC PATCH v3 01/37] bpf: verifier: Accept dynptr mem as mem in herlpers Daniel Rosenberg
2023-04-18  1:40 ` Daniel Rosenberg [this message]
2023-04-18  1:40 ` [RFC PATCH v3 03/37] selftests/bpf: Test allowing NULL buffer in dynptr slice Daniel Rosenberg
2023-04-18  1:40 ` [RFC PATCH v3 04/37] fs: Generic function to convert iocb to rw flags Daniel Rosenberg
2023-04-18  1:40 ` [RFC PATCH v3 05/37] fuse-bpf: Update fuse side uapi Daniel Rosenberg
2023-04-18  1:40 ` [RFC PATCH v3 06/37] fuse-bpf: Add data structures for fuse-bpf Daniel Rosenberg
2023-04-18  1:40 ` [RFC PATCH v3 07/37] fuse-bpf: Prepare for fuse-bpf patch Daniel Rosenberg
2023-04-18  1:40 ` [RFC PATCH v3 08/37] fuse: Add fuse-bpf, a stacked fs extension for FUSE Daniel Rosenberg
2023-05-02  3:38   ` Alexei Starovoitov
2023-05-03  3:45     ` Amir Goldstein
2023-04-18  1:40 ` [RFC PATCH v3 09/37] fuse-bpf: Add ioctl interface for /dev/fuse Daniel Rosenberg
2023-04-18  1:40 ` [RFC PATCH v3 10/37] fuse-bpf: Don't support export_operations Daniel Rosenberg
2023-04-18  1:40 ` [RFC PATCH v3 11/37] fuse-bpf: Add support for access Daniel Rosenberg
2023-04-18  1:40 ` [RFC PATCH v3 12/37] fuse-bpf: Partially add mapping support Daniel Rosenberg
2023-04-18  1:40 ` [RFC PATCH v3 13/37] fuse-bpf: Add lseek support Daniel Rosenberg
2023-04-18  1:40 ` [RFC PATCH v3 14/37] fuse-bpf: Add support for fallocate Daniel Rosenberg
2023-04-18  1:40 ` [RFC PATCH v3 15/37] fuse-bpf: Support file/dir open/close Daniel Rosenberg
2023-04-18  1:40 ` [RFC PATCH v3 16/37] fuse-bpf: Support mknod/unlink/mkdir/rmdir Daniel Rosenberg
2023-04-18  1:40 ` [RFC PATCH v3 17/37] fuse-bpf: Add support for read/write iter Daniel Rosenberg
2023-05-16 20:21   ` Amir Goldstein
2023-04-18  1:40 ` [RFC PATCH v3 18/37] fuse-bpf: support readdir Daniel Rosenberg
2023-04-18  1:40 ` [RFC PATCH v3 19/37] fuse-bpf: Add support for sync operations Daniel Rosenberg
2023-04-18  1:40 ` [RFC PATCH v3 20/37] fuse-bpf: Add Rename support Daniel Rosenberg
2023-04-18  1:40 ` [RFC PATCH v3 21/37] fuse-bpf: Add attr support Daniel Rosenberg
2023-04-18  1:40 ` [RFC PATCH v3 22/37] fuse-bpf: Add support for FUSE_COPY_FILE_RANGE Daniel Rosenberg
2023-04-18  1:40 ` [RFC PATCH v3 23/37] fuse-bpf: Add xattr support Daniel Rosenberg
2023-04-18  1:40 ` [RFC PATCH v3 24/37] fuse-bpf: Add symlink/link support Daniel Rosenberg
2023-04-18  1:40 ` [RFC PATCH v3 25/37] fuse-bpf: allow mounting with no userspace daemon Daniel Rosenberg
2023-04-18  1:40 ` [RFC PATCH v3 26/37] bpf: Increase struct_op limits Daniel Rosenberg
2023-04-18  1:40 ` [RFC PATCH v3 27/37] fuse-bpf: Add fuse-bpf constants Daniel Rosenberg
2023-04-18  1:40 ` [RFC PATCH v3 28/37] WIP: bpf: Add fuse_ops struct_op programs Daniel Rosenberg
2023-04-27  4:18   ` Andrii Nakryiko
2023-05-03  1:53     ` Daniel Rosenberg
2023-05-03 18:22       ` Andrii Nakryiko
2023-04-18  1:40 ` [RFC PATCH v3 29/37] fuse-bpf: Export Functions Daniel Rosenberg
2023-04-18  1:40 ` [RFC PATCH v3 30/37] fuse: Provide registration functions for fuse-bpf Daniel Rosenberg
2023-04-18  1:40 ` [RFC PATCH v3 31/37] fuse-bpf: Set fuse_ops at mount or lookup time Daniel Rosenberg
2023-04-18  1:40 ` [RFC PATCH v3 32/37] fuse-bpf: Call bpf for pre/post filters Daniel Rosenberg
2023-04-18  1:40 ` [RFC PATCH v3 33/37] fuse-bpf: Add userspace " Daniel Rosenberg
2023-04-18  1:40 ` [RFC PATCH v3 34/37] WIP: fuse-bpf: add error_out Daniel Rosenberg
2023-04-18  1:40 ` [RFC PATCH v3 35/37] tools: Add FUSE, update bpf includes Daniel Rosenberg
2023-04-27  4:24   ` Andrii Nakryiko
2023-04-28  0:48     ` Daniel Rosenberg
2023-04-18  1:40 ` [RFC PATCH v3 36/37] fuse-bpf: Add selftests Daniel Rosenberg
2023-04-18  1:40 ` [RFC PATCH v3 37/37] fuse: Provide easy way to test fuse struct_op call Daniel Rosenberg
2023-04-18  5:33 ` [RFC PATCH bpf-next v3 00/37] FUSE BPF: A Stacked Filesystem Extension for FUSE Amir Goldstein
2023-04-21  1:41   ` Daniel Rosenberg
2023-04-23 14:50     ` Amir Goldstein
2023-04-24 15:32 ` Miklos Szeredi
2023-05-02  0:07   ` Daniel Rosenberg
2023-05-17  2:50     ` Gao Xiang
2023-05-17  6:51       ` Amir Goldstein
2023-05-17  7:05         ` Gao Xiang
2023-05-17  7:19           ` Gao Xiang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230418014037.2412394-3-drosen@google.com \
    --to=drosen@google.com \
    --cc=amir73il@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=corbet@lwn.net \
    --cc=daniel@iogearbox.net \
    --cc=haoluo@google.com \
    --cc=joannelkoong@gmail.com \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kernel-team@android.com \
    --cc=kpsingh@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-unionfs@vger.kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=miklos@szeredi.hu \
    --cc=mykolal@fb.com \
    --cc=sdf@google.com \
    --cc=shuah@kernel.org \
    --cc=song@kernel.org \
    --cc=yhs@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).