bpf: adding get_file_path helper

When people want to identify which file system files are being opened, read, and written to, they can use this helper with file descriptor as input to achieve this goal. Other pseudo filesystems are also supported.

This requirement is mainly discussed here:

https://github.com/iovisor/bcc/issues/237

v9->v10: addressed Andrii's feedback
- send this patch together with the patch selftests as one patch series

v8->v9:
- format helper description

v7->v8: addressed Alexei's feedback
- use fget_raw instead of fdget_raw, as fdget_raw is only used inside fs/
- ensure we're in user context which is safe fot the help to run
- filter unmountable pseudo filesystem, because they don't have real path
- supplement the description of this helper function

v6->v7:
- fix missing signed-off-by line

v5->v6: addressed Andrii's feedback
- avoid unnecessary goto end by having two explicit returns

v4->v5: addressed Andrii and Daniel's feedback
- rename bpf_fd2path to bpf_get_file_path to be consistent with other helper's names
- when fdget_raw fails, set ret to -EBADF instead of -EINVAL
- remove fdput from fdget_raw's error path
- use IS_ERR instead of IS_ERR_OR_NULL as d_path ether returns a pointer into the buffer or an error code if the path was too long
- modify the normal path's return value to return copied string length including NUL
- update this helper description's Return bits.

v3->v4: addressed Daniel's feedback
- fix missing fdput()
- move fd2path from kernel/bpf/trace.c to kernel/trace/bpf_trace.c
- move fd2path's test code to another patch
- add comment to explain why use fdget_raw instead of fdget

v2->v3: addressed Yonghong's feedback
- remove unnecessary LOCKDOWN_BPF_READ
- refactor error handling section for enhanced readability
- provide a test case in tools/testing/selftests/bpf

v1->v2: addressed Daniel's feedback
- fix backward compatibility
- add this helper description
- fix signed-off name

Signed-off-by Wenbo Zhang <[email protected]>

kernel/trace/bpf_trace.c +41 -0

+BPF_CALL_3(bpf_get_file_path, char *, dst, u32, size, int, fd)
+{
+	struct file *f;
+	char *p;
+	int ret = -EBADF;
+
+	/* Ensure we're in user context which is safe for the helper to
+	 * run. This helper has no business in a kthread.
+	 */
+	if (unlikely(in_interrupt() ||
+		     current->flags & (PF_KTHREAD | PF_EXITING)))
+		return -EPERM;
+
+	/* Use fget_raw instead of fget to support O_PATH, and it doesn't
+	 * have any sleepable code, so it's ok to be here.
+	 */
+	f = fget_raw(fd);
+	if (!f)
+		goto error;
+
+	/* For unmountable pseudo filesystem, it seems to have no meaning
+	 * to get their fake paths as they don't have path, and to be no
+	 * way to validate this function pointer can be always safe to call
+	 * in the current context.
+	 */
+	if (f->f_path.dentry->d_op && f->f_path.dentry->d_op->d_dname)
+		return -EINVAL;
+
+	/* After filter unmountable pseudo filesytem, d_path won't call
+	 * dentry->d_op->d_name(), the normally path doesn't have any
+	 * sleepable code, and despite it uses the current macro to get
+	 * fs_struct (current->fs), we've already ensured we're in user
+	 * context, so it's ok to be here.
+	 */
+	p = d_path(&f->f_path, dst, size);
+	if (IS_ERR(p)) {
+		ret = PTR_ERR(p);
+		fput(f);
+		goto error;
+	}
+

Quoted 13 lines

kernel/trace/bpf_trace.c +13 -0

+	/* After filter unmountable pseudo filesytem, d_path won't call
+	 * dentry->d_op->d_name(), the normally path doesn't have any
+	 * sleepable code, and despite it uses the current macro to get
+	 * fs_struct (current->fs), we've already ensured we're in user
+	 * context, so it's ok to be here.
+	 */
+	p = d_path(&f->f_path, dst, size);
+	if (IS_ERR(p)) {
+		ret = PTR_ERR(p);
+		fput(f);
+		goto error;
+	}
+

kernel/trace/bpf_trace.c static const struct bpf_func_proto bpf_send_signal_proto = { · line 815

Quoted 10 lines

kernel/trace/bpf_trace.c +10 -0

+	ret = strlen(p);
+	memmove(dst, p, ret);
+	dst[ret++] = '\0';
+	fput(f);
+	return ret;
+
+error:
+	memset(dst, '0', size);
+	return ret;
+}

Al,

could you please review about code whether it's doing enough checks to be called safely from preempt_disabled region?

kernel/trace/bpf_trace.c static const struct bpf_func_proto bpf_send_signal_proto = { · line 791

Quoted 11 lines

kernel/trace/bpf_trace.c +11 -0

+	f = fget_raw(fd);
+	if (!f)
+		goto error;
+
+	/* For unmountable pseudo filesystem, it seems to have no meaning
+	 * to get their fake paths as they don't have path, and to be no
+	 * way to validate this function pointer can be always safe to call
+	 * in the current context.
+	 */
+	if (f->f_path.dentry->d_op && f->f_path.dentry->d_op->d_dname)
+		return -EINVAL;

Quoted 11 lines · Alexei Starovoitov

kernel/trace/bpf_trace.c static const struct bpf_func_proto bpf_send_signal_proto = { · line 791

Quoted 11 lines

kernel/trace/bpf_trace.c +11 -0

+	f = fget_raw(fd);
+	if (!f)
+		goto error;
+
+	/* For unmountable pseudo filesystem, it seems to have no meaning
+	 * to get their fake paths as they don't have path, and to be no
+	 * way to validate this function pointer can be always safe to call
+	 * in the current context.
+	 */
+	if (f->f_path.dentry->d_op && f->f_path.dentry->d_op->d_dname)
+		return -EINVAL;

An obvious leak here, BTW.

Depends. Which context is it running in? In particular, which locks might be already held?

Anyway, what could that be used for? I mean, if you want to check something about syscall arguments, that's an unfixably racy way to go. Descriptor table can be a shared data structure, and two consequent fdget() on the same number can bloody well yield completely unrelated struct file references.

IOW, anything that does descriptor -> struct file * translation more than once is an instant TOCTOU suspect. In this particular case, the function will produce a pathname of something that was once reachable via descriptor with this number; quite possibly never before that function had been called _and_ not once after it has returned.

hard to tell. It will be run out of bpf prog that attaches to kprobe or tracepoint. What is the concern about locking?
d_path() doesn't take any locks and doesn't depend on any locks. Above 'if' checks that plain d_path() is used and not some specilized callback with unknown logic.

Quoted 2 lines

with this number; quite possibly never before that function had been called _and_ not once after it has returned.

Right. TOCTOU is not a concern here. It's tracing. It's ok for full path to be 'one time deal'.

Quoted 5 lines · Alexei Starovoitov

hard to tell. It will be run out of bpf prog that attaches to kprobe or tracepoint. What is the concern about locking?
d_path() doesn't take any locks and doesn't depend on any locks. Above 'if' checks that plain d_path() is used and not some specilized callback with unknown logic.

It sure as hell does. It might end up taking rename_lock and/or mount_lock spinlock components. It'll try not to, but if the first pass ends up with seqlock mismatch, it will just grab the spinlock the second time around.

Quoted 5 lines

Quoted 2 lines

with this number; quite possibly never before that function had been called _and_ not once after it has returned.

Right. TOCTOU is not a concern here. It's tracing. It's ok for full path to be 'one time deal'.

It might very well be a full path of something completely unrelated to what the syscall ends up operating upon. It's not that the file might've been moved; it might be a different file. IOW, results of that tracing might be misleading.

Quoted 11 lines · Al Viro

Quoted 5 lines · Alexei Starovoitov

hard to tell. It will be run out of bpf prog that attaches to kprobe or tracepoint. What is the concern about locking?
d_path() doesn't take any locks and doesn't depend on any locks. Above 'if' checks that plain d_path() is used and not some specilized callback with unknown logic.

It sure as hell does. It might end up taking rename_lock and/or mount_lock spinlock components. It'll try not to, but if the first pass ends up with seqlock mismatch, it will just grab the spinlock the second time around.

ohh. got it. I missed _or_lock() part in there.
The need_seqretry() logic is tricky. afaics there is no way for the checks outside of prepend_path() to prevent spin_lock to happen. And adding a flag to prepend_path() to return early if retry is needed is too ugly. So this helper won't be safe to be run out of kprobe. But if we allow it for tracepoints only it should be ok. I think. There are no tracepoints in inner guts of vfs and I don't think they will ever be. So running in tracepoint->bpf_prog->d_path we will be sure that rename_lock+mount_lock can be safely spinlocked. Am I missing something?

Quoted 7 lines

Quoted 6 lines

Quoted 3 lines

with this number; quite possibly never before that function had been called _and_ not once after it has returned.

Right. TOCTOU is not a concern here. It's tracing. It's ok for full path to be 'one time deal'.

Quoted 4 lines

It might very well be a full path of something completely unrelated to what the syscall ends up operating upon. It's not that the file might've been moved; it might be a different file. IOW, results of that tracing might be misleading.

That is correct. Tracing is fine with such limitation. Still better than probe_read.

kernel/trace/bpf_trace.c +30 -0

+BPF_CALL_3(bpf_get_file_path, char *, dst, u32, size, int, fd)
+{
+	struct file *f;
+	char *p;
+	int ret = -EBADF;
+
+	/* Ensure we're in user context which is safe for the helper to
+	 * run. This helper has no business in a kthread.
+	 */
+	if (unlikely(in_interrupt() ||
+		     current->flags & (PF_KTHREAD | PF_EXITING))) {
+		ret = -EPERM;
+		goto error;
+	}
+
+	/* Use fget_raw instead of fget to support O_PATH, and it doesn't
+	 * have any sleepable code, so it's ok to be here.
+	 */
+	f = fget_raw(fd);
+	if (!f)
+		goto error;
+
+	/* For unmountable pseudo filesystem, it seems to have no meaning
+	 * to get their fake paths as they don't have path, and to be no
+	 * way to validate this function pointer can be always safe to call
+	 * in the current context.
+	 */
+	if (f->f_path.dentry->d_op && f->f_path.dentry->d_op->d_dname) {
+		ret = -EINVAL;
+		fput(f);

Quoted 11 lines · Alexei Starovoitov

Quoted 6 lines

hard to tell. It will be run out of bpf prog that attaches to kprobe or tracepoint. What is the concern about locking?
d_path() doesn't take any locks and doesn't depend on any locks. Above 'if' checks that plain d_path() is used and not some specilized callback with unknown logic.

It sure as hell does. It might end up taking rename_lock and/or mount_lock spinlock components. It'll try not to, but if the first pass ends up with seqlock mismatch, it will just grab the spinlock the second time around.

ohh. got it. I missed _or_lock() part in there.
The need_seqretry() logic is tricky. afaics there is no way for the checks outside of prepend_path() to prevent spin_lock to happen. And adding a flag to prepend_path() to return early if retry is needed is too ugly. So this helper won't be safe to be run out of kprobe. But if we allow it for tracepoints only it should be ok. I think. There are no tracepoints in inner guts of vfs and I don't think they will ever be. So running in tracepoint->bpf_prog->d_path we will be sure that rename_lock+mount_lock can be safely spinlocked. Am I missing something?

Above 'if's are not enough to make sure that it won't dead lock. Allowing it in tracing_func_proto() means that it's available to kprobe too. Hence deadlock is possible. Please see previous email thread. This helper is safe in tracepoint+bpf only.

Quoted 31 lines · Wenbo Zhang

kernel/trace/bpf_trace.c +30 -0

+BPF_CALL_3(bpf_get_file_path, char *, dst, u32, size, int, fd)
+{
+     struct file *f;
+     char *p;
+     int ret = -EBADF;
+
+     /* Ensure we're in user context which is safe for the helper to
+      * run. This helper has no business in a kthread.
+      */
+     if (unlikely(in_interrupt() ||
+                  current->flags & (PF_KTHREAD | PF_EXITING))) {
+             ret = -EPERM;
+             goto error;
+     }
+
+     /* Use fget_raw instead of fget to support O_PATH, and it doesn't
+      * have any sleepable code, so it's ok to be here.
+      */
+     f = fget_raw(fd);
+     if (!f)
+             goto error;
+
+     /* For unmountable pseudo filesystem, it seems to have no meaning
+      * to get their fake paths as they don't have path, and to be no
+      * way to validate this function pointer can be always safe to call
+      * in the current context.
+      */
+     if (f->f_path.dentry->d_op && f->f_path.dentry->d_op->d_dname) {
+             ret = -EINVAL;
+             fput(f);

kernel/trace/bpf_trace.c static const struct bpf_func_proto bpf_send_signal_proto = { · line 804

Quoted 10 lines

kernel/trace/bpf_trace.c +10 -0

+             goto error;
+     }
+
+     /* After filter unmountable pseudo filesytem, d_path won't call
+      * dentry->d_op->d_name(), the normally path doesn't have any
+      * sleepable code, and despite it uses the current macro to get
+      * fs_struct (current->fs), we've already ensured we're in user
+      * context, so it's ok to be here.
+      */
+     p = d_path(&f->f_path, dst, size);

Above 'if's are not enough to make sure that it won't dead lock. Allowing it in tracing_func_proto() means that it's available to kprobe too. Hence deadlock is possible. Please see previous email thread. This helper is safe in tracepoint+bpf only.

When people want to identify which file system files are being opened, read, and written to, they can use this helper with file descriptor as input to achieve this goal. Other pseudo filesystems are also supported.

This requirement is mainly discussed here:

   https://github.com/iovisor/bcc/issues/237

v11->v12: addressed Alexei's feedback
- only allow tracepoints to make sure it won't dead lock

v10->v11: addressed Al and Alexei's feedback
- fix missing fput()

v9->v10: addressed Andrii's feedback
- send this patch together with the patch selftests as one patch series

v8->v9:
- format helper description

v7->v8: addressed Alexei's feedback
- use fget_raw instead of fdget_raw, as fdget_raw is only used inside fs/
- ensure we're in user context which is safe fot the help to run
- filter unmountable pseudo filesystem, because they don't have real path
- supplement the description of this helper function

v6->v7:
- fix missing signed-off-by line

v5->v6: addressed Andrii's feedback
- avoid unnecessary goto end by having two explicit returns

v4->v5: addressed Andrii and Daniel's feedback
- rename bpf_fd2path to bpf_get_file_path to be consistent with other helper's names
- when fdget_raw fails, set ret to -EBADF instead of -EINVAL
- remove fdput from fdget_raw's error path
- use IS_ERR instead of IS_ERR_OR_NULL as d_path ether returns a pointer into the buffer or an error code if the path was too long
- modify the normal path's return value to return copied string length including NUL
- update this helper description's Return bits.

v3->v4: addressed Daniel's feedback
- fix missing fdput()
- move fd2path from kernel/bpf/trace.c to kernel/trace/bpf_trace.c
- move fd2path's test code to another patch
- add comment to explain why use fdget_raw instead of fdget

v2->v3: addressed Yonghong's feedback
- remove unnecessary LOCKDOWN_BPF_READ
- refactor error handling section for enhanced readability
- provide a test case in tools/testing/selftests/bpf

v1->v2: addressed Daniel's feedback
- fix backward compatibility
- add this helper description
- fix signed-off name

Signed-off-by Wenbo Zhang <[email protected]>

---
  include/uapi/linux/bpf.h       | 29 +++++++++++++-
  kernel/trace/bpf_trace.c       | 70 ++++++++++++++++++++++++++++++++++
  tools/include/uapi/linux/bpf.h | 29 +++++++++++++-
  3 files changed, 126 insertions(+), 2 deletions(-)

include/uapi/linux/bpf.h +2 -0

index dbbcf0b02970..71d9705df120 100644
@@ -2821,6 +2821,32 @@ union bpf_attr {
   * 	Return
   * 		On success, the strictly positive length of the string,	including
   * 		the trailing NUL character. On error, a negative value.
+ *
+ * int bpf_get_file_path(char *path, u32 size, int fd)

include/uapi/linux/bpf.h +12 -1

+ *
+ *		On failure, the returned value is one of the following:
+ *
+ *		**-EPERM** if no permission to get the path (eg: in irq ctx).
+ *
+ *		**-EBADF** if *fd* is invalid.
+ *
+ *		**-EINVAL** if *fd* corresponds to a unmountable pseudo fs
+ *
+ *		**-ENAMETOOLONG** if full path is longer than *size*
   */
  #define __BPF_FUNC_MAPPER(FN)		\
  	FN(unspec),			\

@@ -2938,7 +2964,8 @@ union bpf_attr {
  	FN(probe_read_user),		\
  	FN(probe_read_kernel),		\
  	FN(probe_read_user_str),	\
-	FN(probe_read_kernel_str),
+	FN(probe_read_kernel_str),	\
+	FN(get_file_path),
 
  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
   * function eBPF program intends to call

kernel/trace/bpf_trace.c +39 -0

index e5ef4ae9edb5..db9c0ec46a5d 100644
@@ -762,6 +762,72 @@ static const struct bpf_func_proto bpf_send_signal_proto = {
  	.arg1_type	= ARG_ANYTHING,
  };
 
+BPF_CALL_3(bpf_get_file_path, char *, dst, u32, size, int, fd)
+{
+	struct file *f;
+	char *p;
+	int ret = -EBADF;
+
+	/* Ensure we're in user context which is safe for the helper to
+	 * run. This helper has no business in a kthread.
+	 */
+	if (unlikely(in_interrupt() ||
+		     current->flags & (PF_KTHREAD | PF_EXITING))) {
+		ret = -EPERM;
+		goto error;
+	}
+
+	/* Use fget_raw instead of fget to support O_PATH, and it doesn't
+	 * have any sleepable code, so it's ok to be here.
+	 */
+	f = fget_raw(fd);
+	if (!f)
+		goto error;
+
+	/* For unmountable pseudo filesystem, it seems to have no meaning
+	 * to get their fake paths as they don't have path, and to be no
+	 * way to validate this function pointer can be always safe to call
+	 * in the current context.
+	 */
+	if (f->f_path.dentry->d_op && f->f_path.dentry->d_op->d_dname) {
+		ret = -EINVAL;
+		fput(f);
+		goto error;
+	}
+
+	/* After filter unmountable pseudo filesytem, d_path won't call
+	 * dentry->d_op->d_name(), the normally path doesn't have any
+	 * sleepable code, and despite it uses the current macro to get
+	 * fs_struct (current->fs), we've already ensured we're in user
+	 * context, so it's ok to be here.
+	 */

kernel/trace/bpf_trace.c +19 -0

+
+error:
+	memset(dst, '0', size);
+	return ret;
+}
+
+static const struct bpf_func_proto bpf_get_file_path_proto = {
+	.func       = bpf_get_file_path,
+	.gpl_only   = true,
+	.ret_type   = RET_INTEGER,
+	.arg1_type  = ARG_PTR_TO_UNINIT_MEM,
+	.arg2_type  = ARG_CONST_SIZE,
+	.arg3_type  = ARG_ANYTHING,
+};
+
  static const struct bpf_func_proto *
  tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
  {

@@ -953,6 +1019,8 @@ tp_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
  		return &bpf_get_stackid_proto_tp;
  	case BPF_FUNC_get_stack:
  		return &bpf_get_stack_proto_tp;
+	case BPF_FUNC_get_file_path:
+		return &bpf_get_file_path_proto;
  	default:
  		return tracing_func_proto(func_id, prog);
  	}

@@ -1146,6 +1214,8 @@ raw_tp_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
  		return &bpf_get_stackid_proto_raw_tp;
  	case BPF_FUNC_get_stack:
  		return &bpf_get_stack_proto_raw_tp;
+	case BPF_FUNC_get_file_path:
+		return &bpf_get_file_path_proto;
  	default:
  		return tracing_func_proto(func_id, prog);
  	}

tools/include/uapi/linux/bpf.h +2 -0

index dbbcf0b02970..71d9705df120 100644
@@ -2821,6 +2821,32 @@ union bpf_attr {
   * 	Return
   * 		On success, the strictly positive length of the string,	including
   * 		the trailing NUL character. On error, a negative value.
+ *
+ * int bpf_get_file_path(char *path, u32 size, int fd)

tools/include/uapi/linux/bpf.h +12 -1

+ *
+ *		On failure, the returned value is one of the following:
+ *
+ *		**-EPERM** if no permission to get the path (eg: in irq ctx).
+ *
+ *		**-EBADF** if *fd* is invalid.
+ *
+ *		**-EINVAL** if *fd* corresponds to a unmountable pseudo fs
+ *
+ *		**-ENAMETOOLONG** if full path is longer than *size*
   */
  #define __BPF_FUNC_MAPPER(FN)		\
  	FN(unspec),			\

@@ -2938,7 +2964,8 @@ union bpf_attr {
  	FN(probe_read_user),		\
  	FN(probe_read_kernel),		\
  	FN(probe_read_user_str),	\
-	FN(probe_read_kernel_str),
+	FN(probe_read_kernel_str),	\
+	FN(get_file_path),
 
  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
   * function eBPF program intends to call

Quoted 2 lines

+ * - a regular full path (include mountable fs eg: /proc, /sys)
+ * - a regular full path with "(deleted)" at the end.

Let us say with " (deleted)" is appended to be consistent with comments in d_path() and is more clear to user what the format will looks like.

kernel/trace/bpf_trace.c static const struct bpf_func_proto bpf_send_signal_proto = { · line 813

Quoted 3 lines

kernel/trace/bpf_trace.c +3 -0

+     ret = strlen(p);
+     memmove(dst, p, ret);
+     dst[ret++] = '\0';

nit: you could do memmove(dst, p, ret + 1)?

Review

Quoted 2 lines

(quoted diff) +2 -0

+     fput(f);
+     return ret;

The description says the return value length including trailing '\0'. The above 'ret' does not include trailing '\0'.

Quoted 76 lines · Wenbo Zhang

When people want to identify which file system files are being opened, read, and written to, they can use this helper with file descriptor as input to achieve this goal. Other pseudo filesystems are also supported.

This requirement is mainly discussed here:

   https://github.com/iovisor/bcc/issues/237

v11->v12: addressed Alexei's feedback
- only allow tracepoints to make sure it won't dead lock

v10->v11: addressed Al and Alexei's feedback
- fix missing fput()

v9->v10: addressed Andrii's feedback
- send this patch together with the patch selftests as one patch series

v8->v9:
- format helper description

v7->v8: addressed Alexei's feedback
- use fget_raw instead of fdget_raw, as fdget_raw is only used inside fs/
- ensure we're in user context which is safe fot the help to run
- filter unmountable pseudo filesystem, because they don't have real path
- supplement the description of this helper function

v6->v7:
- fix missing signed-off-by line

v5->v6: addressed Andrii's feedback
- avoid unnecessary goto end by having two explicit returns

v4->v5: addressed Andrii and Daniel's feedback
- rename bpf_fd2path to bpf_get_file_path to be consistent with other helper's names
- when fdget_raw fails, set ret to -EBADF instead of -EINVAL
- remove fdput from fdget_raw's error path
- use IS_ERR instead of IS_ERR_OR_NULL as d_path ether returns a pointer into the buffer or an error code if the path was too long
- modify the normal path's return value to return copied string length including NUL
- update this helper description's Return bits.

v3->v4: addressed Daniel's feedback
- fix missing fdput()
- move fd2path from kernel/bpf/trace.c to kernel/trace/bpf_trace.c
- move fd2path's test code to another patch
- add comment to explain why use fdget_raw instead of fdget

v2->v3: addressed Yonghong's feedback
- remove unnecessary LOCKDOWN_BPF_READ
- refactor error handling section for enhanced readability
- provide a test case in tools/testing/selftests/bpf

v1->v2: addressed Daniel's feedback
- fix backward compatibility
- add this helper description
- fix signed-off name

Signed-off-by Wenbo Zhang <[email protected]>

---
  include/uapi/linux/bpf.h       | 29 +++++++++++++-
  kernel/trace/bpf_trace.c       | 70 ++++++++++++++++++++++++++++++++++
  tools/include/uapi/linux/bpf.h | 29 +++++++++++++-
  3 files changed, 126 insertions(+), 2 deletions(-)

include/uapi/linux/bpf.h +2 -0

index dbbcf0b02970..71d9705df120 100644
@@ -2821,6 +2821,32 @@ union bpf_attr {
   *  Return
   *          On success, the strictly positive length of the string, including
   *          the trailing NUL character. On error, a negative value.
+ *
+ * int bpf_get_file_path(char *path, u32 size, int fd)

include/uapi/linux/bpf.h union bpf_attr { · line 2835

Quoted 10 lines

include/uapi/linux/bpf.h +10 -0

+ *   Description
+ *           Get **file** atrribute from the current task by *fd*, then call
+ *           **d_path** to get it's absolute path and copy it as string into
+ *           *path* of *size*. Notice the **path** don't support unmountable
+ *           pseudo filesystems as they don't have path (eg: SOCKFS, PIPEFS).
+ *           The *size* must be strictly positive. On success, the helper
+ *           makes sure that the *path* is NUL-terminated, and the buffer
+ *           could be:
+ *           - a regular full path (include mountable fs eg: /proc, /sys)
+ *           - a regular full path with "(deleted)" at the end.

Let us say with " (deleted)" is appended to be consistent with comments in d_path() and is more clear to user what the format will looks like.

include/uapi/linux/bpf.h union bpf_attr { · line 2839

Quoted 4 lines

include/uapi/linux/bpf.h +4 -0

+ *           On failure, it is filled with zeroes.
+ *   Return
+ *           On success, returns the length of the copied string INCLUDING
+ *           the trailing NUL.

trailing '\0'.

Quoted 70 lines

include/uapi/linux/bpf.h +12 -1

+ *
+ *           On failure, the returned value is one of the following:
+ *
+ *           **-EPERM** if no permission to get the path (eg: in irq ctx).
+ *
+ *           **-EBADF** if *fd* is invalid.
+ *
+ *           **-EINVAL** if *fd* corresponds to a unmountable pseudo fs
+ *
+ *           **-ENAMETOOLONG** if full path is longer than *size*
   */
  #define __BPF_FUNC_MAPPER(FN)               \
      FN(unspec),                     \

@@ -2938,7 +2964,8 @@ union bpf_attr {
      FN(probe_read_user),            \
      FN(probe_read_kernel),          \
      FN(probe_read_user_str),        \
-     FN(probe_read_kernel_str),
+     FN(probe_read_kernel_str),      \
+     FN(get_file_path),
 
  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
   * function eBPF program intends to call

kernel/trace/bpf_trace.c +39 -0

index e5ef4ae9edb5..db9c0ec46a5d 100644
@@ -762,6 +762,72 @@ static const struct bpf_func_proto bpf_send_signal_proto = {
      .arg1_type      = ARG_ANYTHING,
  };
 
+BPF_CALL_3(bpf_get_file_path, char *, dst, u32, size, int, fd)
+{
+     struct file *f;
+     char *p;
+     int ret = -EBADF;
+
+     /* Ensure we're in user context which is safe for the helper to
+      * run. This helper has no business in a kthread.
+      */
+     if (unlikely(in_interrupt() ||
+                  current->flags & (PF_KTHREAD | PF_EXITING))) {
+             ret = -EPERM;
+             goto error;
+     }
+
+     /* Use fget_raw instead of fget to support O_PATH, and it doesn't
+      * have any sleepable code, so it's ok to be here.
+      */
+     f = fget_raw(fd);
+     if (!f)
+             goto error;
+
+     /* For unmountable pseudo filesystem, it seems to have no meaning
+      * to get their fake paths as they don't have path, and to be no
+      * way to validate this function pointer can be always safe to call
+      * in the current context.
+      */
+     if (f->f_path.dentry->d_op && f->f_path.dentry->d_op->d_dname) {
+             ret = -EINVAL;
+             fput(f);
+             goto error;
+     }
+
+     /* After filter unmountable pseudo filesytem, d_path won't call
+      * dentry->d_op->d_name(), the normally path doesn't have any
+      * sleepable code, and despite it uses the current macro to get
+      * fs_struct (current->fs), we've already ensured we're in user
+      * context, so it's ok to be here.
+      */

kernel/trace/bpf_trace.c static const struct bpf_func_proto bpf_send_signal_proto = { · line 813

Quoted 10 lines

kernel/trace/bpf_trace.c +10 -0

+     p = d_path(&f->f_path, dst, size);
+     if (IS_ERR(p)) {
+             ret = PTR_ERR(p);
+             fput(f);
+             goto error;
+     }
+
+     ret = strlen(p);
+     memmove(dst, p, ret);
+     dst[ret++] = '\0';

nit: you could do memmove(dst, p, ret + 1)?

kernel/trace/bpf_trace.c static const struct bpf_func_proto bpf_send_signal_proto = { · line 815

Quoted 2 lines

kernel/trace/bpf_trace.c +2 -0

+     fput(f);
+     return ret;

The description says the return value length including trailing '\0'. The above 'ret' does not include trailing '\0'.

Quoted 46 lines

kernel/trace/bpf_trace.c +19 -0

+
+error:
+     memset(dst, '0', size);
+     return ret;
+}
+
+static const struct bpf_func_proto bpf_get_file_path_proto = {
+     .func       = bpf_get_file_path,
+     .gpl_only   = true,
+     .ret_type   = RET_INTEGER,
+     .arg1_type  = ARG_PTR_TO_UNINIT_MEM,
+     .arg2_type  = ARG_CONST_SIZE,
+     .arg3_type  = ARG_ANYTHING,
+};
+
  static const struct bpf_func_proto *
  tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
  {

@@ -953,6 +1019,8 @@ tp_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
              return &bpf_get_stackid_proto_tp;
      case BPF_FUNC_get_stack:
              return &bpf_get_stack_proto_tp;
+     case BPF_FUNC_get_file_path:
+             return &bpf_get_file_path_proto;
      default:
              return tracing_func_proto(func_id, prog);
      }

@@ -1146,6 +1214,8 @@ raw_tp_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
              return &bpf_get_stackid_proto_raw_tp;
      case BPF_FUNC_get_stack:
              return &bpf_get_stack_proto_raw_tp;
+     case BPF_FUNC_get_file_path:
+             return &bpf_get_file_path_proto;
      default:
              return tracing_func_proto(func_id, prog);
      }

tools/include/uapi/linux/bpf.h +2 -0

index dbbcf0b02970..71d9705df120 100644
@@ -2821,6 +2821,32 @@ union bpf_attr {
   *  Return
   *          On success, the strictly positive length of the string, including
   *          the trailing NUL character. On error, a negative value.
+ *
+ * int bpf_get_file_path(char *path, u32 size, int fd)

tools/include/uapi/linux/bpf.h union bpf_attr { · line 2835

Quoted 10 lines

tools/include/uapi/linux/bpf.h +10 -0

+ *   Description
+ *           Get **file** atrribute from the current task by *fd*, then call
+ *           **d_path** to get it's absolute path and copy it as string into
+ *           *path* of *size*. Notice the **path** don't support unmountable
+ *           pseudo filesystems as they don't have path (eg: SOCKFS, PIPEFS).
+ *           The *size* must be strictly positive. On success, the helper
+ *           makes sure that the *path* is NUL-terminated, and the buffer
+ *           could be:
+ *           - a regular full path (include mountable fs eg: /proc, /sys)
+ *           - a regular full path with "(deleted)" at the end.

ditto

tools/include/uapi/linux/bpf.h union bpf_attr { · line 2839

Quoted 4 lines

tools/include/uapi/linux/bpf.h +4 -0

+ *           On failure, it is filled with zeroes.
+ *   Return
+ *           On success, returns the length of the copied string INCLUDING
+ *           the trailing NUL.

ditto

Quoted 24 lines

tools/include/uapi/linux/bpf.h +12 -1

+ *
+ *           On failure, the returned value is one of the following:
+ *
+ *           **-EPERM** if no permission to get the path (eg: in irq ctx).
+ *
+ *           **-EBADF** if *fd* is invalid.
+ *
+ *           **-EINVAL** if *fd* corresponds to a unmountable pseudo fs
+ *
+ *           **-ENAMETOOLONG** if full path is longer than *size*
   */
  #define __BPF_FUNC_MAPPER(FN)               \
      FN(unspec),                     \

@@ -2938,7 +2964,8 @@ union bpf_attr {
      FN(probe_read_user),            \
      FN(probe_read_kernel),          \
      FN(probe_read_user_str),        \
-     FN(probe_read_kernel_str),
+     FN(probe_read_kernel_str),      \
+     FN(get_file_path),
 
  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
   * function eBPF program intends to call

Quoted 5 lines

Quoted 2 lines

+ * - a regular full path (include mountable fs eg: /proc, /sys)
+ * - a regular full path with "(deleted)" at the end.

Let us say with " (deleted)" is appended to be consistent with comments in d_path() and is more clear to user what the format will looks like.

Thank you, I'll fix this.

Quoted 5 lines

kernel/trace/bpf_trace.c static const struct bpf_func_proto bpf_send_signal_proto = { · line 813

Quoted 3 lines

kernel/trace/bpf_trace.c +3 -0

+     ret = strlen(p);
+     memmove(dst, p, ret);
+     dst[ret++] = '\0';

nit: you could do memmove(dst, p, ret + 1)?

I did with dst[ret++]='\0'; to return value length including trailing '\0'. as you mentioned below:

Quoted 5 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+     fput(f);
+     return ret;

The description says the return value length including trailing '\0'. The above 'ret' does not include trailing '\0'.

It seems [ret++] not very clear to read and '\0' can be done by memmove. I think I'll refactor to

```
ret = strlen(p) + 1;
memmove(dst, p, ret);
fput(f);
return ret;
```

Is this better?

When people want to identify which file system files are being opened, read, and written to, they can use this helper with file descriptor as input to achieve this goal. Other pseudo filesystems are also supported.

This requirement is mainly discussed here:

   https://github.com/iovisor/bcc/issues/237

v11->v12: addressed Alexei's feedback
- only allow tracepoints to make sure it won't dead lock

v10->v11: addressed Al and Alexei's feedback
- fix missing fput()

v9->v10: addressed Andrii's feedback
- send this patch together with the patch selftests as one patch series

v8->v9:
- format helper description

v7->v8: addressed Alexei's feedback
- use fget_raw instead of fdget_raw, as fdget_raw is only used inside fs/
- ensure we're in user context which is safe fot the help to run
- filter unmountable pseudo filesystem, because they don't have real path
- supplement the description of this helper function

v6->v7:
- fix missing signed-off-by line

v5->v6: addressed Andrii's feedback
- avoid unnecessary goto end by having two explicit returns

v4->v5: addressed Andrii and Daniel's feedback
- rename bpf_fd2path to bpf_get_file_path to be consistent with other helper's names
- when fdget_raw fails, set ret to -EBADF instead of -EINVAL
- remove fdput from fdget_raw's error path
- use IS_ERR instead of IS_ERR_OR_NULL as d_path ether returns a pointer into the buffer or an error code if the path was too long
- modify the normal path's return value to return copied string length including NUL
- update this helper description's Return bits.

v3->v4: addressed Daniel's feedback
- fix missing fdput()
- move fd2path from kernel/bpf/trace.c to kernel/trace/bpf_trace.c
- move fd2path's test code to another patch
- add comment to explain why use fdget_raw instead of fdget

v2->v3: addressed Yonghong's feedback
- remove unnecessary LOCKDOWN_BPF_READ
- refactor error handling section for enhanced readability
- provide a test case in tools/testing/selftests/bpf

v1->v2: addressed Daniel's feedback
- fix backward compatibility
- add this helper description
- fix signed-off name

Signed-off-by Wenbo Zhang <[email protected]>

---
  include/uapi/linux/bpf.h       | 29 +++++++++++++-
  kernel/trace/bpf_trace.c       | 70 ++++++++++++++++++++++++++++++++++
  tools/include/uapi/linux/bpf.h | 29 +++++++++++++-
  3 files changed, 126 insertions(+), 2 deletions(-)

include/uapi/linux/bpf.h +28 -1

index dbbcf0b02970..71d9705df120 100644
@@ -2821,6 +2821,32 @@ union bpf_attr {
   * 	Return
   * 		On success, the strictly positive length of the string,	including
   * 		the trailing NUL character. On error, a negative value.
+ *
+ * int bpf_get_file_path(char *path, u32 size, int fd)
+ *	Description
+ *		Get **file** atrribute from the current task by *fd*, then call
+ *		**d_path** to get it's absolute path and copy it as string into
+ *		*path* of *size*. Notice the **path** don't support unmountable
+ *		pseudo filesystems as they don't have path (eg: SOCKFS, PIPEFS).
+ *		The *size* must be strictly positive. On success, the helper
+ *		makes sure that the *path* is NUL-terminated, and the buffer
+ *		could be:
+ *		- a regular full path (include mountable fs eg: /proc, /sys)
+ *		- a regular full path with "(deleted)" at the end.
+ *		On failure, it is filled with zeroes.
+ *	Return
+ *		On success, returns the length of the copied string INCLUDING
+ *		the trailing NUL.
+ *
+ *		On failure, the returned value is one of the following:
+ *
+ *		**-EPERM** if no permission to get the path (eg: in irq ctx).
+ *
+ *		**-EBADF** if *fd* is invalid.
+ *
+ *		**-EINVAL** if *fd* corresponds to a unmountable pseudo fs
+ *
+ *		**-ENAMETOOLONG** if full path is longer than *size*
   */
  #define __BPF_FUNC_MAPPER(FN)		\
  	FN(unspec),			\

@@ -2938,7 +2964,8 @@ union bpf_attr {
  	FN(probe_read_user),		\
  	FN(probe_read_kernel),		\
  	FN(probe_read_user_str),	\
-	FN(probe_read_kernel_str),
+	FN(probe_read_kernel_str),	\
+	FN(get_file_path),
 
  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
   * function eBPF program intends to call

kernel/trace/bpf_trace.c +0 -0

index e5ef4ae9edb5..db9c0ec46a5d 100644

kernel/trace/bpf_trace.c +52 -0

+
+	/* Ensure we're in user context which is safe for the helper to
+	 * run. This helper has no business in a kthread.
+	 */
+	if (unlikely(in_interrupt() ||
+		     current->flags & (PF_KTHREAD | PF_EXITING))) {
+		ret = -EPERM;
+		goto error;
+	}
+
+	/* Use fget_raw instead of fget to support O_PATH, and it doesn't
+	 * have any sleepable code, so it's ok to be here.
+	 */
+	f = fget_raw(fd);
+	if (!f)
+		goto error;
+
+	/* For unmountable pseudo filesystem, it seems to have no meaning
+	 * to get their fake paths as they don't have path, and to be no
+	 * way to validate this function pointer can be always safe to call
+	 * in the current context.
+	 */
+	if (f->f_path.dentry->d_op && f->f_path.dentry->d_op->d_dname) {
+		ret = -EINVAL;
+		fput(f);
+		goto error;
+	}
+
+	/* After filter unmountable pseudo filesytem, d_path won't call
+	 * dentry->d_op->d_name(), the normally path doesn't have any
+	 * sleepable code, and despite it uses the current macro to get
+	 * fs_struct (current->fs), we've already ensured we're in user
+	 * context, so it's ok to be here.
+	 */
+	p = d_path(&f->f_path, dst, size);
+	if (IS_ERR(p)) {
+		ret = PTR_ERR(p);
+		fput(f);
+		goto error;
+	}
+
+	ret = strlen(p);
+	memmove(dst, p, ret);
+	dst[ret++] = '\0';
+	fput(f);
+	return ret;
+
+error:
+	memset(dst, '0', size);
+	return ret;
+}
+

kernel/trace/bpf_trace.c static const struct bpf_func_proto bpf_send_signal_proto = { · line 769

Quoted 5 lines

kernel/trace/bpf_trace.c +5 -0

+BPF_CALL_3(bpf_get_file_path, char *, dst, u32, size, int, fd)
+{
+     struct file *f;
+     char *p;
+     int ret = -EBADF;

please try to use reverse Christmas tree for declarations.

Quoted 118 lines · Wenbo Zhang

When people want to identify which file system files are being opened, read, and written to, they can use this helper with file descriptor as input to achieve this goal. Other pseudo filesystems are also supported.

This requirement is mainly discussed here:

   https://github.com/iovisor/bcc/issues/237

v11->v12: addressed Alexei's feedback
- only allow tracepoints to make sure it won't dead lock

v10->v11: addressed Al and Alexei's feedback
- fix missing fput()

v9->v10: addressed Andrii's feedback
- send this patch together with the patch selftests as one patch series

v8->v9:
- format helper description

v7->v8: addressed Alexei's feedback
- use fget_raw instead of fdget_raw, as fdget_raw is only used inside fs/
- ensure we're in user context which is safe fot the help to run
- filter unmountable pseudo filesystem, because they don't have real path
- supplement the description of this helper function

v6->v7:
- fix missing signed-off-by line

v5->v6: addressed Andrii's feedback
- avoid unnecessary goto end by having two explicit returns

v4->v5: addressed Andrii and Daniel's feedback
- rename bpf_fd2path to bpf_get_file_path to be consistent with other helper's names
- when fdget_raw fails, set ret to -EBADF instead of -EINVAL
- remove fdput from fdget_raw's error path
- use IS_ERR instead of IS_ERR_OR_NULL as d_path ether returns a pointer into the buffer or an error code if the path was too long
- modify the normal path's return value to return copied string length including NUL
- update this helper description's Return bits.

v3->v4: addressed Daniel's feedback
- fix missing fdput()
- move fd2path from kernel/bpf/trace.c to kernel/trace/bpf_trace.c
- move fd2path's test code to another patch
- add comment to explain why use fdget_raw instead of fdget

v2->v3: addressed Yonghong's feedback
- remove unnecessary LOCKDOWN_BPF_READ
- refactor error handling section for enhanced readability
- provide a test case in tools/testing/selftests/bpf

v1->v2: addressed Daniel's feedback
- fix backward compatibility
- add this helper description
- fix signed-off name

Signed-off-by Wenbo Zhang <[email protected]>

---
  include/uapi/linux/bpf.h       | 29 +++++++++++++-
  kernel/trace/bpf_trace.c       | 70 ++++++++++++++++++++++++++++++++++
  tools/include/uapi/linux/bpf.h | 29 +++++++++++++-
  3 files changed, 126 insertions(+), 2 deletions(-)

include/uapi/linux/bpf.h +28 -1

index dbbcf0b02970..71d9705df120 100644
@@ -2821,6 +2821,32 @@ union bpf_attr {
   *  Return
   *          On success, the strictly positive length of the string, including
   *          the trailing NUL character. On error, a negative value.
+ *
+ * int bpf_get_file_path(char *path, u32 size, int fd)
+ *   Description
+ *           Get **file** atrribute from the current task by *fd*, then call
+ *           **d_path** to get it's absolute path and copy it as string into
+ *           *path* of *size*. Notice the **path** don't support unmountable
+ *           pseudo filesystems as they don't have path (eg: SOCKFS, PIPEFS).
+ *           The *size* must be strictly positive. On success, the helper
+ *           makes sure that the *path* is NUL-terminated, and the buffer
+ *           could be:
+ *           - a regular full path (include mountable fs eg: /proc, /sys)
+ *           - a regular full path with "(deleted)" at the end.
+ *           On failure, it is filled with zeroes.
+ *   Return
+ *           On success, returns the length of the copied string INCLUDING
+ *           the trailing NUL.
+ *
+ *           On failure, the returned value is one of the following:
+ *
+ *           **-EPERM** if no permission to get the path (eg: in irq ctx).
+ *
+ *           **-EBADF** if *fd* is invalid.
+ *
+ *           **-EINVAL** if *fd* corresponds to a unmountable pseudo fs
+ *
+ *           **-ENAMETOOLONG** if full path is longer than *size*
   */
  #define __BPF_FUNC_MAPPER(FN)               \
      FN(unspec),                     \

@@ -2938,7 +2964,8 @@ union bpf_attr {
      FN(probe_read_user),            \
      FN(probe_read_kernel),          \
      FN(probe_read_user_str),        \
-     FN(probe_read_kernel_str),
+     FN(probe_read_kernel_str),      \
+     FN(get_file_path),
 
  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
   * function eBPF program intends to call

kernel/trace/bpf_trace.c +0 -0

index e5ef4ae9edb5..db9c0ec46a5d 100644

kernel/trace/bpf_trace.c static const struct bpf_func_proto bpf_send_signal_proto = { · line 769

Quoted 8 lines

kernel/trace/bpf_trace.c +5 -0

@@ -762,6 +762,72 @@ static const struct bpf_func_proto bpf_send_signal_proto = {
      .arg1_type      = ARG_ANYTHING,
  };
 
+BPF_CALL_3(bpf_get_file_path, char *, dst, u32, size, int, fd)
+{
+     struct file *f;
+     char *p;
+     int ret = -EBADF;

please try to use reverse Christmas tree for declarations.

Quoted 52 lines

kernel/trace/bpf_trace.c +52 -0

+
+     /* Ensure we're in user context which is safe for the helper to
+      * run. This helper has no business in a kthread.
+      */
+     if (unlikely(in_interrupt() ||
+                  current->flags & (PF_KTHREAD | PF_EXITING))) {
+             ret = -EPERM;
+             goto error;
+     }
+
+     /* Use fget_raw instead of fget to support O_PATH, and it doesn't
+      * have any sleepable code, so it's ok to be here.
+      */
+     f = fget_raw(fd);
+     if (!f)
+             goto error;
+
+     /* For unmountable pseudo filesystem, it seems to have no meaning
+      * to get their fake paths as they don't have path, and to be no
+      * way to validate this function pointer can be always safe to call
+      * in the current context.
+      */
+     if (f->f_path.dentry->d_op && f->f_path.dentry->d_op->d_dname) {
+             ret = -EINVAL;
+             fput(f);
+             goto error;
+     }
+
+     /* After filter unmountable pseudo filesytem, d_path won't call
+      * dentry->d_op->d_name(), the normally path doesn't have any
+      * sleepable code, and despite it uses the current macro to get
+      * fs_struct (current->fs), we've already ensured we're in user
+      * context, so it's ok to be here.
+      */
+     p = d_path(&f->f_path, dst, size);
+     if (IS_ERR(p)) {
+             ret = PTR_ERR(p);
+             fput(f);
+             goto error;
+     }
+
+     ret = strlen(p);
+     memmove(dst, p, ret);
+     dst[ret++] = '\0';
+     fput(f);
+     return ret;
+
+error:
+     memset(dst, '0', size);
+     return ret;
+}
+

[...]

When people want to identify which file system files are being opened, read, and written to, they can use this helper with file descriptor as input to achieve this goal. Other pseudo filesystems are also supported.

This requirement is mainly discussed here:

https://github.com/iovisor/bcc/issues/237

v11->v12: addressed Alexei's feedback
- only allow tracepoints to make sure it won't dead lock

v10->v11: addressed Al and Alexei's feedback
- fix missing fput()

v9->v10: addressed Andrii's feedback
- send this patch together with the patch selftests as one patch series

v8->v9:
- format helper description

v7->v8: addressed Alexei's feedback
- use fget_raw instead of fdget_raw, as fdget_raw is only used inside fs/
- ensure we're in user context which is safe fot the help to run
- filter unmountable pseudo filesystem, because they don't have real path
- supplement the description of this helper function

v6->v7:
- fix missing signed-off-by line

v5->v6: addressed Andrii's feedback
- avoid unnecessary goto end by having two explicit returns

v4->v5: addressed Andrii and Daniel's feedback
- rename bpf_fd2path to bpf_get_file_path to be consistent with other helper's names
- when fdget_raw fails, set ret to -EBADF instead of -EINVAL
- remove fdput from fdget_raw's error path
- use IS_ERR instead of IS_ERR_OR_NULL as d_path ether returns a pointer into the buffer or an error code if the path was too long
- modify the normal path's return value to return copied string length including NUL
- update this helper description's Return bits.

v3->v4: addressed Daniel's feedback
- fix missing fdput()
- move fd2path from kernel/bpf/trace.c to kernel/trace/bpf_trace.c
- move fd2path's test code to another patch
- add comment to explain why use fdget_raw instead of fdget

v2->v3: addressed Yonghong's feedback
- remove unnecessary LOCKDOWN_BPF_READ
- refactor error handling section for enhanced readability
- provide a test case in tools/testing/selftests/bpf

v1->v2: addressed Daniel's feedback
- fix backward compatibility
- add this helper description
- fix signed-off name

Signed-off-by Wenbo Zhang <[email protected]>

---
 include/uapi/linux/bpf.h       | 29 +++++++++++++-
 kernel/trace/bpf_trace.c       | 70 ++++++++++++++++++++++++++++++++++
 tools/include/uapi/linux/bpf.h | 29 +++++++++++++-
 3 files changed, 126 insertions(+), 2 deletions(-)

include/uapi/linux/bpf.h +28 -1

index dbbcf0b02970..71d9705df120 100644
@@ -2821,6 +2821,32 @@ union bpf_attr {
  *     Return
  *             On success, the strictly positive length of the string, including
  *             the trailing NUL character. On error, a negative value.
+ *
+ * int bpf_get_file_path(char *path, u32 size, int fd)
+ *     Description
+ *             Get **file** atrribute from the current task by *fd*, then call
+ *             **d_path** to get it's absolute path and copy it as string into
+ *             *path* of *size*. Notice the **path** don't support unmountable
+ *             pseudo filesystems as they don't have path (eg: SOCKFS, PIPEFS).
+ *             The *size* must be strictly positive. On success, the helper
+ *             makes sure that the *path* is NUL-terminated, and the buffer
+ *             could be:
+ *             - a regular full path (include mountable fs eg: /proc, /sys)
+ *             - a regular full path with "(deleted)" at the end.
+ *             On failure, it is filled with zeroes.
+ *     Return
+ *             On success, returns the length of the copied string INCLUDING
+ *             the trailing NUL.
+ *
+ *             On failure, the returned value is one of the following:
+ *
+ *             **-EPERM** if no permission to get the path (eg: in irq ctx).
+ *
+ *             **-EBADF** if *fd* is invalid.
+ *
+ *             **-EINVAL** if *fd* corresponds to a unmountable pseudo fs
+ *
+ *             **-ENAMETOOLONG** if full path is longer than *size*
  */
 #define __BPF_FUNC_MAPPER(FN)          \
        FN(unspec),                     \

@@ -2938,7 +2964,8 @@ union bpf_attr {
        FN(probe_read_user),            \
        FN(probe_read_kernel),          \
        FN(probe_read_user_str),        \
-       FN(probe_read_kernel_str),
+       FN(probe_read_kernel_str),      \
+       FN(get_file_path),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call

kernel/trace/bpf_trace.c +70 -0

index e5ef4ae9edb5..db9c0ec46a5d 100644
@@ -762,6 +762,72 @@ static const struct bpf_func_proto bpf_send_signal_proto = {
        .arg1_type      = ARG_ANYTHING,
 };
 
+BPF_CALL_3(bpf_get_file_path, char *, dst, u32, size, int, fd)
+{
+       struct file *f;
+       char *p;
+       int ret = -EBADF;
+
+       /* Ensure we're in user context which is safe for the helper to
+        * run. This helper has no business in a kthread.
+        */
+       if (unlikely(in_interrupt() ||
+                    current->flags & (PF_KTHREAD | PF_EXITING))) {
+               ret = -EPERM;
+               goto error;
+       }
+
+       /* Use fget_raw instead of fget to support O_PATH, and it doesn't
+        * have any sleepable code, so it's ok to be here.
+        */
+       f = fget_raw(fd);
+       if (!f)
+               goto error;
+
+       /* For unmountable pseudo filesystem, it seems to have no meaning
+        * to get their fake paths as they don't have path, and to be no
+        * way to validate this function pointer can be always safe to call
+        * in the current context.
+        */
+       if (f->f_path.dentry->d_op && f->f_path.dentry->d_op->d_dname) {
+               ret = -EINVAL;
+               fput(f);
+               goto error;
+       }
+
+       /* After filter unmountable pseudo filesytem, d_path won't call
+        * dentry->d_op->d_name(), the normally path doesn't have any
+        * sleepable code, and despite it uses the current macro to get
+        * fs_struct (current->fs), we've already ensured we're in user
+        * context, so it's ok to be here.
+        */
+       p = d_path(&f->f_path, dst, size);
+       if (IS_ERR(p)) {
+               ret = PTR_ERR(p);
+               fput(f);
+               goto error;
+       }
+
+       ret = strlen(p);
+       memmove(dst, p, ret);
+       dst[ret++] = '\0';
+       fput(f);
+       return ret;
+
+error:
+       memset(dst, '0', size);
+       return ret;
+}
+
+static const struct bpf_func_proto bpf_get_file_path_proto = {
+       .func       = bpf_get_file_path,
+       .gpl_only   = true,
+       .ret_type   = RET_INTEGER,
+       .arg1_type  = ARG_PTR_TO_UNINIT_MEM,
+       .arg2_type  = ARG_CONST_SIZE,
+       .arg3_type  = ARG_ANYTHING,
+};
+
 static const struct bpf_func_proto *
 tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 {

@@ -953,6 +1019,8 @@ tp_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
                return &bpf_get_stackid_proto_tp;
        case BPF_FUNC_get_stack:
                return &bpf_get_stack_proto_tp;
+       case BPF_FUNC_get_file_path:
+               return &bpf_get_file_path_proto;
        default:
                return tracing_func_proto(func_id, prog);
        }

@@ -1146,6 +1214,8 @@ raw_tp_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
                return &bpf_get_stackid_proto_raw_tp;
        case BPF_FUNC_get_stack:
                return &bpf_get_stack_proto_raw_tp;
+       case BPF_FUNC_get_file_path:
+               return &bpf_get_file_path_proto;
        default:
                return tracing_func_proto(func_id, prog);
        }

tools/include/uapi/linux/bpf.h +26 -0

index dbbcf0b02970..71d9705df120 100644
@@ -2821,6 +2821,32 @@ union bpf_attr {
  *     Return
  *             On success, the strictly positive length of the string, including
  *             the trailing NUL character. On error, a negative value.
+ *
+ * int bpf_get_file_path(char *path, u32 size, int fd)
+ *     Description
+ *             Get **file** atrribute from the current task by *fd*, then call
+ *             **d_path** to get it's absolute path and copy it as string into
+ *             *path* of *size*. Notice the **path** don't support unmountable
+ *             pseudo filesystems as they don't have path (eg: SOCKFS, PIPEFS).
+ *             The *size* must be strictly positive. On success, the helper
+ *             makes sure that the *path* is NUL-terminated, and the buffer
+ *             could be:
+ *             - a regular full path (include mountable fs eg: /proc, /sys)
+ *             - a regular full path with "(deleted)" at the end.
+ *             On failure, it is filled with zeroes.
+ *     Return
+ *             On success, returns the length of the copied string INCLUDING
+ *             the trailing NUL.
+ *
+ *             On failure, the returned value is one of the following:
+ *
+ *             **-EPERM** if no permission to get the path (eg: in irq ctx).
+ *
+ *             **-EBADF** if *fd* is invalid.
+ *
+ *             **-EINVAL** if *fd* corresponds to a unmountable pseudo fs
+ *
+ *             **-ENAMETOOLONG** if full path is longer than *size*
  */
 #define __BPF_FUNC_MAPPER(FN)          \
        FN(unspec),                     \

I just realized that among my tools that want the path, the input is either:

A) syscall tracepoints: int fd
B) kprobes: struct file *

This serves (A). If we ever add a different helper for (B), we might think that this helper was misnamed. Should it be called get_fd_path instead? That leaves get_file_path available for a later "struct file *" -> pathname helper.

Quoted 247 lines · Wenbo Zhang

When people want to identify which file system files are being opened, read, and written to, they can use this helper with file descriptor as input to achieve this goal. Other pseudo filesystems are also supported.

This requirement is mainly discussed here:

https://github.com/iovisor/bcc/issues/237

v11->v12: addressed Alexei's feedback
- only allow tracepoints to make sure it won't dead lock

v10->v11: addressed Al and Alexei's feedback
- fix missing fput()

v9->v10: addressed Andrii's feedback
- send this patch together with the patch selftests as one patch series

v8->v9:
- format helper description

v7->v8: addressed Alexei's feedback
- use fget_raw instead of fdget_raw, as fdget_raw is only used inside fs/
- ensure we're in user context which is safe fot the help to run
- filter unmountable pseudo filesystem, because they don't have real path
- supplement the description of this helper function

v6->v7:
- fix missing signed-off-by line

v5->v6: addressed Andrii's feedback
- avoid unnecessary goto end by having two explicit returns

v4->v5: addressed Andrii and Daniel's feedback
- rename bpf_fd2path to bpf_get_file_path to be consistent with other helper's names
- when fdget_raw fails, set ret to -EBADF instead of -EINVAL
- remove fdput from fdget_raw's error path
- use IS_ERR instead of IS_ERR_OR_NULL as d_path ether returns a pointer into the buffer or an error code if the path was too long
- modify the normal path's return value to return copied string length including NUL
- update this helper description's Return bits.

v3->v4: addressed Daniel's feedback
- fix missing fdput()
- move fd2path from kernel/bpf/trace.c to kernel/trace/bpf_trace.c
- move fd2path's test code to another patch
- add comment to explain why use fdget_raw instead of fdget

v2->v3: addressed Yonghong's feedback
- remove unnecessary LOCKDOWN_BPF_READ
- refactor error handling section for enhanced readability
- provide a test case in tools/testing/selftests/bpf

v1->v2: addressed Daniel's feedback
- fix backward compatibility
- add this helper description
- fix signed-off name

Signed-off-by Wenbo Zhang <[email protected]>

---
 include/uapi/linux/bpf.h       | 29 +++++++++++++-
 kernel/trace/bpf_trace.c       | 70 ++++++++++++++++++++++++++++++++++
 tools/include/uapi/linux/bpf.h | 29 +++++++++++++-
 3 files changed, 126 insertions(+), 2 deletions(-)

include/uapi/linux/bpf.h +28 -1

index dbbcf0b02970..71d9705df120 100644
@@ -2821,6 +2821,32 @@ union bpf_attr {
  *     Return
  *             On success, the strictly positive length of the string, including
  *             the trailing NUL character. On error, a negative value.
+ *
+ * int bpf_get_file_path(char *path, u32 size, int fd)
+ *     Description
+ *             Get **file** atrribute from the current task by *fd*, then call
+ *             **d_path** to get it's absolute path and copy it as string into
+ *             *path* of *size*. Notice the **path** don't support unmountable
+ *             pseudo filesystems as they don't have path (eg: SOCKFS, PIPEFS).
+ *             The *size* must be strictly positive. On success, the helper
+ *             makes sure that the *path* is NUL-terminated, and the buffer
+ *             could be:
+ *             - a regular full path (include mountable fs eg: /proc, /sys)
+ *             - a regular full path with "(deleted)" at the end.
+ *             On failure, it is filled with zeroes.
+ *     Return
+ *             On success, returns the length of the copied string INCLUDING
+ *             the trailing NUL.
+ *
+ *             On failure, the returned value is one of the following:
+ *
+ *             **-EPERM** if no permission to get the path (eg: in irq ctx).
+ *
+ *             **-EBADF** if *fd* is invalid.
+ *
+ *             **-EINVAL** if *fd* corresponds to a unmountable pseudo fs
+ *
+ *             **-ENAMETOOLONG** if full path is longer than *size*
  */
 #define __BPF_FUNC_MAPPER(FN)          \
        FN(unspec),                     \

@@ -2938,7 +2964,8 @@ union bpf_attr {
        FN(probe_read_user),            \
        FN(probe_read_kernel),          \
        FN(probe_read_user_str),        \
-       FN(probe_read_kernel_str),
+       FN(probe_read_kernel_str),      \
+       FN(get_file_path),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call

kernel/trace/bpf_trace.c +70 -0

index e5ef4ae9edb5..db9c0ec46a5d 100644
@@ -762,6 +762,72 @@ static const struct bpf_func_proto bpf_send_signal_proto = {
        .arg1_type      = ARG_ANYTHING,
 };
 
+BPF_CALL_3(bpf_get_file_path, char *, dst, u32, size, int, fd)
+{
+       struct file *f;
+       char *p;
+       int ret = -EBADF;
+
+       /* Ensure we're in user context which is safe for the helper to
+        * run. This helper has no business in a kthread.
+        */
+       if (unlikely(in_interrupt() ||
+                    current->flags & (PF_KTHREAD | PF_EXITING))) {
+               ret = -EPERM;
+               goto error;
+       }
+
+       /* Use fget_raw instead of fget to support O_PATH, and it doesn't
+        * have any sleepable code, so it's ok to be here.
+        */
+       f = fget_raw(fd);
+       if (!f)
+               goto error;
+
+       /* For unmountable pseudo filesystem, it seems to have no meaning
+        * to get their fake paths as they don't have path, and to be no
+        * way to validate this function pointer can be always safe to call
+        * in the current context.
+        */
+       if (f->f_path.dentry->d_op && f->f_path.dentry->d_op->d_dname) {
+               ret = -EINVAL;
+               fput(f);
+               goto error;
+       }
+
+       /* After filter unmountable pseudo filesytem, d_path won't call
+        * dentry->d_op->d_name(), the normally path doesn't have any
+        * sleepable code, and despite it uses the current macro to get
+        * fs_struct (current->fs), we've already ensured we're in user
+        * context, so it's ok to be here.
+        */
+       p = d_path(&f->f_path, dst, size);
+       if (IS_ERR(p)) {
+               ret = PTR_ERR(p);
+               fput(f);
+               goto error;
+       }
+
+       ret = strlen(p);
+       memmove(dst, p, ret);
+       dst[ret++] = '\0';
+       fput(f);
+       return ret;
+
+error:
+       memset(dst, '0', size);
+       return ret;
+}
+
+static const struct bpf_func_proto bpf_get_file_path_proto = {
+       .func       = bpf_get_file_path,
+       .gpl_only   = true,
+       .ret_type   = RET_INTEGER,
+       .arg1_type  = ARG_PTR_TO_UNINIT_MEM,
+       .arg2_type  = ARG_CONST_SIZE,
+       .arg3_type  = ARG_ANYTHING,
+};
+
 static const struct bpf_func_proto *
 tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 {

@@ -953,6 +1019,8 @@ tp_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
                return &bpf_get_stackid_proto_tp;
        case BPF_FUNC_get_stack:
                return &bpf_get_stack_proto_tp;
+       case BPF_FUNC_get_file_path:
+               return &bpf_get_file_path_proto;
        default:
                return tracing_func_proto(func_id, prog);
        }

@@ -1146,6 +1214,8 @@ raw_tp_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
                return &bpf_get_stackid_proto_raw_tp;
        case BPF_FUNC_get_stack:
                return &bpf_get_stack_proto_raw_tp;
+       case BPF_FUNC_get_file_path:
+               return &bpf_get_file_path_proto;
        default:
                return tracing_func_proto(func_id, prog);
        }

tools/include/uapi/linux/bpf.h +26 -0

index dbbcf0b02970..71d9705df120 100644
@@ -2821,6 +2821,32 @@ union bpf_attr {
  *     Return
  *             On success, the strictly positive length of the string, including
  *             the trailing NUL character. On error, a negative value.
+ *
+ * int bpf_get_file_path(char *path, u32 size, int fd)
+ *     Description
+ *             Get **file** atrribute from the current task by *fd*, then call
+ *             **d_path** to get it's absolute path and copy it as string into
+ *             *path* of *size*. Notice the **path** don't support unmountable
+ *             pseudo filesystems as they don't have path (eg: SOCKFS, PIPEFS).
+ *             The *size* must be strictly positive. On success, the helper
+ *             makes sure that the *path* is NUL-terminated, and the buffer
+ *             could be:
+ *             - a regular full path (include mountable fs eg: /proc, /sys)
+ *             - a regular full path with "(deleted)" at the end.
+ *             On failure, it is filled with zeroes.
+ *     Return
+ *             On success, returns the length of the copied string INCLUDING
+ *             the trailing NUL.
+ *
+ *             On failure, the returned value is one of the following:
+ *
+ *             **-EPERM** if no permission to get the path (eg: in irq ctx).
+ *
+ *             **-EBADF** if *fd* is invalid.
+ *
+ *             **-EINVAL** if *fd* corresponds to a unmountable pseudo fs
+ *
+ *             **-ENAMETOOLONG** if full path is longer than *size*
  */
 #define __BPF_FUNC_MAPPER(FN)          \
        FN(unspec),                     \

tools/include/uapi/linux/bpf.h union bpf_attr { · line 2968

Quoted 6 lines

tools/include/uapi/linux/bpf.h +2 -1

@@ -2938,7 +2964,8 @@ union bpf_attr {
        FN(probe_read_user),            \
        FN(probe_read_kernel),          \
        FN(probe_read_user_str),        \
-       FN(probe_read_kernel_str),
+       FN(probe_read_kernel_str),      \
+       FN(get_file_path),

I just realized that among my tools that want the path, the input is either:

A) syscall tracepoints: int fd
B) kprobes: struct file *

This serves (A). If we ever add a different helper for (B), we might think that this helper was misnamed. Should it be called get_fd_path instead? That leaves get_file_path available for a later "struct file *" -> pathname helper.

Brendan

--
Brendan Gregg, Senior Performance Architect, Netflix

When people want to identify which file system files are being opened, read, and written to, they can use this helper with file descriptor as input to achieve this goal. Other pseudo filesystems are also supported.

This requirement is mainly discussed here:

   https://github.com/iovisor/bcc/issues/237

v12->v13: addressed Gregg and Yonghong's feedback
- rename to get_fd_path
- refactor code & comment to be clearer and more compliant

v11->v12: addressed Alexei's feedback
- only allow tracepoints to make sure it won't dead lock

v10->v11: addressed Al and Alexei's feedback
- fix missing fput()

v9->v10: addressed Andrii's feedback
- send this patch together with the patch selftests as one patch series

v8->v9:
- format helper description

v7->v8: addressed Alexei's feedback
- use fget_raw instead of fdget_raw, as fdget_raw is only used inside fs/
- ensure we're in user context which is safe fot the help to run
- filter unmountable pseudo filesystem, because they don't have real path
- supplement the description of this helper function

v6->v7:
- fix missing signed-off-by line

v5->v6: addressed Andrii's feedback
- avoid unnecessary goto end by having two explicit returns

v4->v5: addressed Andrii and Daniel's feedback
- rename bpf_fd2path to bpf_get_file_path to be consistent with other helper's names
- when fdget_raw fails, set ret to -EBADF instead of -EINVAL
- remove fdput from fdget_raw's error path
- use IS_ERR instead of IS_ERR_OR_NULL as d_path ether returns a pointer into the buffer or an error code if the path was too long
- modify the normal path's return value to return copied string length including NUL
- update this helper description's Return bits.

v3->v4: addressed Daniel's feedback
- fix missing fdput()
- move fd2path from kernel/bpf/trace.c to kernel/trace/bpf_trace.c
- move fd2path's test code to another patch
- add comment to explain why use fdget_raw instead of fdget

v2->v3: addressed Yonghong's feedback
- remove unnecessary LOCKDOWN_BPF_READ
- refactor error handling section for enhanced readability
- provide a test case in tools/testing/selftests/bpf

v1->v2: addressed Daniel's feedback
- fix backward compatibility
- add this helper description
- fix signed-off name

Signed-off-by Wenbo Zhang <[email protected]>

---
  include/uapi/linux/bpf.h       | 29 +++++++++++++-
  kernel/trace/bpf_trace.c       | 69 ++++++++++++++++++++++++++++++++++
  tools/include/uapi/linux/bpf.h | 29 +++++++++++++-
  3 files changed, 125 insertions(+), 2 deletions(-)

include/uapi/linux/bpf.h +2 -0

index dbbcf0b02970..c1e4fd286614 100644
@@ -2821,6 +2821,32 @@ union bpf_attr {
   * 	Return
   * 		On success, the strictly positive length of the string,	including
   * 		the trailing NUL character. On error, a negative value.
+ *
+ * int bpf_get_fd_path(char *path, u32 size, int fd)

include/uapi/linux/bpf.h +16 -1

+ *		On failure, it is filled with zeroes.
+ *	Return
+ *		On success, returns the length of the copied string INCLUDING
+ *		the trailing '\0'.
+ *
+ *		On failure, the returned value is one of the following:
+ *
+ *		**-EPERM** if no permission to get the path (eg: in irq ctx).
+ *
+ *		**-EBADF** if *fd* is invalid.
+ *
+ *		**-EINVAL** if *fd* corresponds to a unmountable pseudo fs
+ *
+ *		**-ENAMETOOLONG** if full path is longer than *size*
   */
  #define __BPF_FUNC_MAPPER(FN)		\
  	FN(unspec),			\

@@ -2938,7 +2964,8 @@ union bpf_attr {
  	FN(probe_read_user),		\
  	FN(probe_read_kernel),		\
  	FN(probe_read_user_str),	\
-	FN(probe_read_kernel_str),
+	FN(probe_read_kernel_str),	\
+	FN(get_fd_path),
 
  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
   * function eBPF program intends to call

kernel/trace/bpf_trace.c +69 -0

index e5ef4ae9edb5..43a6aa6ad967 100644
@@ -762,6 +762,71 @@ static const struct bpf_func_proto bpf_send_signal_proto = {
  	.arg1_type	= ARG_ANYTHING,
  };
 
+BPF_CALL_3(bpf_get_fd_path, char *, dst, u32, size, int, fd)
+{
+	int ret = -EBADF;
+	struct file *f;
+	char *p;
+
+	/* Ensure we're in user context which is safe for the helper to
+	 * run. This helper has no business in a kthread.
+	 */
+	if (unlikely(in_interrupt() ||
+		     current->flags & (PF_KTHREAD | PF_EXITING))) {
+		ret = -EPERM;
+		goto error;
+	}
+
+	/* Use fget_raw instead of fget to support O_PATH, and it doesn't
+	 * have any sleepable code, so it's ok to be here.
+	 */
+	f = fget_raw(fd);
+	if (!f)
+		goto error;
+
+	/* For unmountable pseudo filesystem, it seems to have no meaning
+	 * to get their fake paths as they don't have path, and to be no
+	 * way to validate this function pointer can be always safe to call
+	 * in the current context.
+	 */
+	if (f->f_path.dentry->d_op && f->f_path.dentry->d_op->d_dname) {
+		ret = -EINVAL;
+		fput(f);
+		goto error;
+	}
+
+	/* After filter unmountable pseudo filesytem, d_path won't call
+	 * dentry->d_op->d_name(), the normally path doesn't have any
+	 * sleepable code, and despite it uses the current macro to get
+	 * fs_struct (current->fs), we've already ensured we're in user
+	 * context, so it's ok to be here.
+	 */
+	p = d_path(&f->f_path, dst, size);
+	if (IS_ERR(p)) {
+		ret = PTR_ERR(p);
+		fput(f);
+		goto error;
+	}
+
+	ret = strlen(p) + 1;
+	memmove(dst, p, ret);
+	fput(f);
+	return ret;
+
+error:
+	memset(dst, '0', size);
+	return ret;
+}
+
+static const struct bpf_func_proto bpf_get_fd_path_proto = {
+	.func       = bpf_get_fd_path,
+	.gpl_only   = true,
+	.ret_type   = RET_INTEGER,
+	.arg1_type  = ARG_PTR_TO_UNINIT_MEM,
+	.arg2_type  = ARG_CONST_SIZE,
+	.arg3_type  = ARG_ANYTHING,
+};
+
  static const struct bpf_func_proto *
  tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
  {

@@ -953,6 +1018,8 @@ tp_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
  		return &bpf_get_stackid_proto_tp;
  	case BPF_FUNC_get_stack:
  		return &bpf_get_stack_proto_tp;
+	case BPF_FUNC_get_fd_path:
+		return &bpf_get_fd_path_proto;
  	default:
  		return tracing_func_proto(func_id, prog);
  	}

@@ -1146,6 +1213,8 @@ raw_tp_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
  		return &bpf_get_stackid_proto_raw_tp;
  	case BPF_FUNC_get_stack:
  		return &bpf_get_stack_proto_raw_tp;
+	case BPF_FUNC_get_fd_path:
+		return &bpf_get_fd_path_proto;
  	default:
  		return tracing_func_proto(func_id, prog);
  	}

tools/include/uapi/linux/bpf.h +28 -1

index dbbcf0b02970..c1e4fd286614 100644
@@ -2821,6 +2821,32 @@ union bpf_attr {
   * 	Return
   * 		On success, the strictly positive length of the string,	including
   * 		the trailing NUL character. On error, a negative value.
+ *
+ * int bpf_get_fd_path(char *path, u32 size, int fd)
+ *	Description
+ *		Get **file** atrribute from the current task by *fd*, then call
+ *		**d_path** to get it's absolute path and copy it as string into
+ *		*path* of *size*. Notice the **path** don't support unmountable
+ *		pseudo filesystems as they don't have path (eg: SOCKFS, PIPEFS).
+ *		The *size* must be strictly positive. On success, the helper
+ *		makes sure that the *path* is NUL-terminated, and the buffer
+ *		could be:
+ *		- a regular full path (include mountable fs eg: /proc, /sys)
+ *		- a regular full path with "(deleted)" is appended.
+ *		On failure, it is filled with zeroes.
+ *	Return
+ *		On success, returns the length of the copied string INCLUDING
+ *		the trailing '\0'.
+ *
+ *		On failure, the returned value is one of the following:
+ *
+ *		**-EPERM** if no permission to get the path (eg: in irq ctx).
+ *
+ *		**-EBADF** if *fd* is invalid.
+ *
+ *		**-EINVAL** if *fd* corresponds to a unmountable pseudo fs
+ *
+ *		**-ENAMETOOLONG** if full path is longer than *size*
   */
  #define __BPF_FUNC_MAPPER(FN)		\
  	FN(unspec),			\

@@ -2938,7 +2964,8 @@ union bpf_attr {
  	FN(probe_read_user),		\
  	FN(probe_read_kernel),		\
  	FN(probe_read_user_str),	\
-	FN(probe_read_kernel_str),
+	FN(probe_read_kernel_str),	\
+	FN(get_fd_path),
 
  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
   * function eBPF program intends to call

On 12/17/19 1:47 AM, Wenbo Zhang wrote:

Quoted 1 line

+ * On failure, it is filled with zeroes.

Quoted 66 lines

    */
   #define __BPF_FUNC_MAPPER(FN)		\
   	FN(unspec),			\

include/uapi/linux/bpf.h +2 -1

@@ -2938,7 +2964,8 @@ union bpf_attr {
   	FN(probe_read_user),		\
   	FN(probe_read_kernel),		\
   	FN(probe_read_user_str),	\
-	FN(probe_read_kernel_str),
+	FN(probe_read_kernel_str),	\
+	FN(get_fd_path),
 
   /* integer value in 'imm' field of BPF_CALL instruction selects which helper
    * function eBPF program intends to call

kernel/trace/bpf_trace.c +45 -0

index e5ef4ae9edb5..43a6aa6ad967 100644
@@ -762,6 +762,71 @@ static const struct bpf_func_proto bpf_send_signal_proto = {
   	.arg1_type	= ARG_ANYTHING,
   };
 
+BPF_CALL_3(bpf_get_fd_path, char *, dst, u32, size, int, fd)
+{
+	int ret = -EBADF;
+	struct file *f;
+	char *p;
+
+	/* Ensure we're in user context which is safe for the helper to
+	 * run. This helper has no business in a kthread.
+	 */
+	if (unlikely(in_interrupt() ||
+		     current->flags & (PF_KTHREAD | PF_EXITING))) {
+		ret = -EPERM;
+		goto error;
+	}
+
+	/* Use fget_raw instead of fget to support O_PATH, and it doesn't
+	 * have any sleepable code, so it's ok to be here.
+	 */
+	f = fget_raw(fd);
+	if (!f)
+		goto error;
+
+	/* For unmountable pseudo filesystem, it seems to have no meaning
+	 * to get their fake paths as they don't have path, and to be no
+	 * way to validate this function pointer can be always safe to call
+	 * in the current context.
+	 */
+	if (f->f_path.dentry->d_op && f->f_path.dentry->d_op->d_dname) {
+		ret = -EINVAL;
+		fput(f);
+		goto error;
+	}
+
+	/* After filter unmountable pseudo filesytem, d_path won't call
+	 * dentry->d_op->d_name(), the normally path doesn't have any
+	 * sleepable code, and despite it uses the current macro to get
+	 * fs_struct (current->fs), we've already ensured we're in user
+	 * context, so it's ok to be here.
+	 */
+	p = d_path(&f->f_path, dst, size);
+	if (IS_ERR(p)) {
+		ret = PTR_ERR(p);
+		fput(f);
+		goto error;
+	}

kernel/trace/bpf_trace.c static const struct bpf_func_proto bpf_send_signal_proto = { · line 817

Quoted 8 lines

kernel/trace/bpf_trace.c +8 -0

+
+	ret = strlen(p) + 1;
+	memmove(dst, p, ret);
+	fput(f);
+	return ret;
+
+error:
+	memset(dst, '0', size);

Quoted 2 lines

kernel/trace/bpf_trace.c +2 -0

+	return ret;
+}

[...]

Quoted 1 line

Quoted 1 line

+ * On failure, it is filled with zeroes.

[...]
You fill it with 0x30's ...

Quoted 1 line · Yonghong Song

On 12/17/19 1:47 AM, Wenbo Zhang wrote:

[...]

Quoted 1 line

Quoted 1 line

+ * On failure, it is filled with zeroes.

[...]

Quoted 66 lines

Quoted 66 lines

    */
   #define __BPF_FUNC_MAPPER(FN)             \
     FN(unspec),                     \

include/uapi/linux/bpf.h +2 -1

@@ -2938,7 +2964,8 @@ union bpf_attr {
     FN(probe_read_user),            \
     FN(probe_read_kernel),          \
     FN(probe_read_user_str),        \
-    FN(probe_read_kernel_str),
+    FN(probe_read_kernel_str),      \
+    FN(get_fd_path),
 
   /* integer value in 'imm' field of BPF_CALL instruction selects which helper
    * function eBPF program intends to call

kernel/trace/bpf_trace.c +45 -0

index e5ef4ae9edb5..43a6aa6ad967 100644
@@ -762,6 +762,71 @@ static const struct bpf_func_proto bpf_send_signal_proto = {
     .arg1_type      = ARG_ANYTHING,
   };
 
+BPF_CALL_3(bpf_get_fd_path, char *, dst, u32, size, int, fd)
+{
+    int ret = -EBADF;
+    struct file *f;
+    char *p;
+
+    /* Ensure we're in user context which is safe for the helper to
+     * run. This helper has no business in a kthread.
+     */
+    if (unlikely(in_interrupt() ||
+                 current->flags & (PF_KTHREAD | PF_EXITING))) {
+            ret = -EPERM;
+            goto error;
+    }
+
+    /* Use fget_raw instead of fget to support O_PATH, and it doesn't
+     * have any sleepable code, so it's ok to be here.
+     */
+    f = fget_raw(fd);
+    if (!f)
+            goto error;
+
+    /* For unmountable pseudo filesystem, it seems to have no meaning
+     * to get their fake paths as they don't have path, and to be no
+     * way to validate this function pointer can be always safe to call
+     * in the current context.
+     */
+    if (f->f_path.dentry->d_op && f->f_path.dentry->d_op->d_dname) {
+            ret = -EINVAL;
+            fput(f);
+            goto error;
+    }
+
+    /* After filter unmountable pseudo filesytem, d_path won't call
+     * dentry->d_op->d_name(), the normally path doesn't have any
+     * sleepable code, and despite it uses the current macro to get
+     * fs_struct (current->fs), we've already ensured we're in user
+     * context, so it's ok to be here.
+     */
+    p = d_path(&f->f_path, dst, size);
+    if (IS_ERR(p)) {
+            ret = PTR_ERR(p);
+            fput(f);
+            goto error;
+    }

Quoted 8 lines

kernel/trace/bpf_trace.c static const struct bpf_func_proto bpf_send_signal_proto = { · line 817

Quoted 8 lines

kernel/trace/bpf_trace.c +8 -0

+
+    ret = strlen(p) + 1;
+    memmove(dst, p, ret);
+    fput(f);
+    return ret;
+
+error:
+    memset(dst, '0', size);

You fill it with 0x30's ...

Quoted 2 lines

Quoted 2 lines

kernel/trace/bpf_trace.c +2 -0

+    return ret;
+}

Sorry about a little pedantic. In d_path() function comments, we have:
* Convert a dentry into an ASCII path name. If the entry has been deleted
* the string " (deleted)" is appended. Note that this is ambiguous.

Note that there is a space before "(deleted)". I would like to the above changed to
- a regular full path with " (deleted)" is appended.

Quoted 58 lines · Wenbo Zhang

When people want to identify which file system files are being opened, read, and written to, they can use this helper with file descriptor as input to achieve this goal. Other pseudo filesystems are also supported.

This requirement is mainly discussed here:

   https://github.com/iovisor/bcc/issues/237

v12->v13: addressed Gregg and Yonghong's feedback
- rename to get_fd_path
- refactor code & comment to be clearer and more compliant

v11->v12: addressed Alexei's feedback
- only allow tracepoints to make sure it won't dead lock

v10->v11: addressed Al and Alexei's feedback
- fix missing fput()

v9->v10: addressed Andrii's feedback
- send this patch together with the patch selftests as one patch series

v8->v9:
- format helper description

v7->v8: addressed Alexei's feedback
- use fget_raw instead of fdget_raw, as fdget_raw is only used inside fs/
- ensure we're in user context which is safe fot the help to run
- filter unmountable pseudo filesystem, because they don't have real path
- supplement the description of this helper function

v6->v7:
- fix missing signed-off-by line

v5->v6: addressed Andrii's feedback
- avoid unnecessary goto end by having two explicit returns

v4->v5: addressed Andrii and Daniel's feedback
- rename bpf_fd2path to bpf_get_file_path to be consistent with other helper's names
- when fdget_raw fails, set ret to -EBADF instead of -EINVAL
- remove fdput from fdget_raw's error path
- use IS_ERR instead of IS_ERR_OR_NULL as d_path ether returns a pointer into the buffer or an error code if the path was too long
- modify the normal path's return value to return copied string length including NUL
- update this helper description's Return bits.

v3->v4: addressed Daniel's feedback
- fix missing fdput()
- move fd2path from kernel/bpf/trace.c to kernel/trace/bpf_trace.c
- move fd2path's test code to another patch
- add comment to explain why use fdget_raw instead of fdget

v2->v3: addressed Yonghong's feedback
- remove unnecessary LOCKDOWN_BPF_READ
- refactor error handling section for enhanced readability
- provide a test case in tools/testing/selftests/bpf

Quoted 6 lines

v1->v2: addressed Daniel's feedback
- fix backward compatibility
- add this helper description
- fix signed-off name

Signed-off-by Wenbo Zhang <[email protected]>

Ack with still the minor issue below, not sure whether another revision will be needed or not or the maintainer can just fix up before merging.

Acked-by Yonghong Song <[email protected]>

Quoted 16 lines

---
  include/uapi/linux/bpf.h       | 29 +++++++++++++-
  kernel/trace/bpf_trace.c       | 69 ++++++++++++++++++++++++++++++++++
  tools/include/uapi/linux/bpf.h | 29 +++++++++++++-
  3 files changed, 125 insertions(+), 2 deletions(-)

include/uapi/linux/bpf.h +2 -0

index dbbcf0b02970..c1e4fd286614 100644
@@ -2821,6 +2821,32 @@ union bpf_attr {
   *  Return
   *          On success, the strictly positive length of the string, including
   *          the trailing NUL character. On error, a negative value.
+ *
+ * int bpf_get_fd_path(char *path, u32 size, int fd)

include/uapi/linux/bpf.h union bpf_attr { · line 2835

Quoted 10 lines

include/uapi/linux/bpf.h +10 -0

+ *   Description
+ *           Get **file** atrribute from the current task by *fd*, then call
+ *           **d_path** to get it's absolute path and copy it as string into
+ *           *path* of *size*. Notice the **path** don't support unmountable
+ *           pseudo filesystems as they don't have path (eg: SOCKFS, PIPEFS).
+ *           The *size* must be strictly positive. On success, the helper
+ *           makes sure that the *path* is NUL-terminated, and the buffer
+ *           could be:
+ *           - a regular full path (include mountable fs eg: /proc, /sys)
+ *           - a regular full path with "(deleted)" is appended.

Sorry about a little pedantic. In d_path() function comments, we have:
* Convert a dentry into an ASCII path name. If the entry has been deleted
* the string " (deleted)" is appended. Note that this is ambiguous.

Note that there is a space before "(deleted)". I would like to the above changed to
- a regular full path with " (deleted)" is appended.

Quoted 169 lines

include/uapi/linux/bpf.h +16 -1

+ *           On failure, it is filled with zeroes.
+ *   Return
+ *           On success, returns the length of the copied string INCLUDING
+ *           the trailing '\0'.
+ *
+ *           On failure, the returned value is one of the following:
+ *
+ *           **-EPERM** if no permission to get the path (eg: in irq ctx).
+ *
+ *           **-EBADF** if *fd* is invalid.
+ *
+ *           **-EINVAL** if *fd* corresponds to a unmountable pseudo fs
+ *
+ *           **-ENAMETOOLONG** if full path is longer than *size*
   */
  #define __BPF_FUNC_MAPPER(FN)               \
      FN(unspec),                     \

@@ -2938,7 +2964,8 @@ union bpf_attr {
      FN(probe_read_user),            \
      FN(probe_read_kernel),          \
      FN(probe_read_user_str),        \
-     FN(probe_read_kernel_str),
+     FN(probe_read_kernel_str),      \
+     FN(get_fd_path),
 
  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
   * function eBPF program intends to call

kernel/trace/bpf_trace.c +69 -0

index e5ef4ae9edb5..43a6aa6ad967 100644
@@ -762,6 +762,71 @@ static const struct bpf_func_proto bpf_send_signal_proto = {
      .arg1_type      = ARG_ANYTHING,
  };
 
+BPF_CALL_3(bpf_get_fd_path, char *, dst, u32, size, int, fd)
+{
+     int ret = -EBADF;
+     struct file *f;
+     char *p;
+
+     /* Ensure we're in user context which is safe for the helper to
+      * run. This helper has no business in a kthread.
+      */
+     if (unlikely(in_interrupt() ||
+                  current->flags & (PF_KTHREAD | PF_EXITING))) {
+             ret = -EPERM;
+             goto error;
+     }
+
+     /* Use fget_raw instead of fget to support O_PATH, and it doesn't
+      * have any sleepable code, so it's ok to be here.
+      */
+     f = fget_raw(fd);
+     if (!f)
+             goto error;
+
+     /* For unmountable pseudo filesystem, it seems to have no meaning
+      * to get their fake paths as they don't have path, and to be no
+      * way to validate this function pointer can be always safe to call
+      * in the current context.
+      */
+     if (f->f_path.dentry->d_op && f->f_path.dentry->d_op->d_dname) {
+             ret = -EINVAL;
+             fput(f);
+             goto error;
+     }
+
+     /* After filter unmountable pseudo filesytem, d_path won't call
+      * dentry->d_op->d_name(), the normally path doesn't have any
+      * sleepable code, and despite it uses the current macro to get
+      * fs_struct (current->fs), we've already ensured we're in user
+      * context, so it's ok to be here.
+      */
+     p = d_path(&f->f_path, dst, size);
+     if (IS_ERR(p)) {
+             ret = PTR_ERR(p);
+             fput(f);
+             goto error;
+     }
+
+     ret = strlen(p) + 1;
+     memmove(dst, p, ret);
+     fput(f);
+     return ret;
+
+error:
+     memset(dst, '0', size);
+     return ret;
+}
+
+static const struct bpf_func_proto bpf_get_fd_path_proto = {
+     .func       = bpf_get_fd_path,
+     .gpl_only   = true,
+     .ret_type   = RET_INTEGER,
+     .arg1_type  = ARG_PTR_TO_UNINIT_MEM,
+     .arg2_type  = ARG_CONST_SIZE,
+     .arg3_type  = ARG_ANYTHING,
+};
+
  static const struct bpf_func_proto *
  tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
  {

@@ -953,6 +1018,8 @@ tp_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
              return &bpf_get_stackid_proto_tp;
      case BPF_FUNC_get_stack:
              return &bpf_get_stack_proto_tp;
+     case BPF_FUNC_get_fd_path:
+             return &bpf_get_fd_path_proto;
      default:
              return tracing_func_proto(func_id, prog);
      }

@@ -1146,6 +1213,8 @@ raw_tp_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
              return &bpf_get_stackid_proto_raw_tp;
      case BPF_FUNC_get_stack:
              return &bpf_get_stack_proto_raw_tp;
+     case BPF_FUNC_get_fd_path:
+             return &bpf_get_fd_path_proto;
      default:
              return tracing_func_proto(func_id, prog);
      }

tools/include/uapi/linux/bpf.h +28 -1

index dbbcf0b02970..c1e4fd286614 100644
@@ -2821,6 +2821,32 @@ union bpf_attr {
   *  Return
   *          On success, the strictly positive length of the string, including
   *          the trailing NUL character. On error, a negative value.
+ *
+ * int bpf_get_fd_path(char *path, u32 size, int fd)
+ *   Description
+ *           Get **file** atrribute from the current task by *fd*, then call
+ *           **d_path** to get it's absolute path and copy it as string into
+ *           *path* of *size*. Notice the **path** don't support unmountable
+ *           pseudo filesystems as they don't have path (eg: SOCKFS, PIPEFS).
+ *           The *size* must be strictly positive. On success, the helper
+ *           makes sure that the *path* is NUL-terminated, and the buffer
+ *           could be:
+ *           - a regular full path (include mountable fs eg: /proc, /sys)
+ *           - a regular full path with "(deleted)" is appended.
+ *           On failure, it is filled with zeroes.
+ *   Return
+ *           On success, returns the length of the copied string INCLUDING
+ *           the trailing '\0'.
+ *
+ *           On failure, the returned value is one of the following:
+ *
+ *           **-EPERM** if no permission to get the path (eg: in irq ctx).
+ *
+ *           **-EBADF** if *fd* is invalid.
+ *
+ *           **-EINVAL** if *fd* corresponds to a unmountable pseudo fs
+ *
+ *           **-ENAMETOOLONG** if full path is longer than *size*
   */
  #define __BPF_FUNC_MAPPER(FN)               \
      FN(unspec),                     \

@@ -2938,7 +2964,8 @@ union bpf_attr {
      FN(probe_read_user),            \
      FN(probe_read_kernel),          \
      FN(probe_read_user_str),        \
-     FN(probe_read_kernel_str),
+     FN(probe_read_kernel_str),      \
+     FN(get_fd_path),
 
  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
   * function eBPF program intends to call

When people want to identify which file system files are being opened, read, and written to, they can use this helper with file descriptor as input to achieve this goal. Other pseudo filesystems are also supported.

This requirement is mainly discussed here:

   https://github.com/iovisor/bcc/issues/237

v13->v14: addressed Yonghong and Daniel's feedback
- fix this helper's description to be consistent with comments in d_path
- fix error handling logic fill zeroes not '0's

v12->v13: addressed Brendan and Yonghong's feedback
- rename to get_fd_path
- refactor code & comment to be clearer and more compliant

v11->v12: addressed Alexei's feedback
- only allow tracepoints to make sure it won't dead lock

v10->v11: addressed Al and Alexei's feedback
- fix missing fput()

v9->v10: addressed Andrii's feedback
- send this patch together with the patch selftests as one patch series

v8->v9:
- format helper description

v7->v8: addressed Alexei's feedback
- use fget_raw instead of fdget_raw, as fdget_raw is only used inside fs/
- ensure we're in user context which is safe fot the help to run
- filter unmountable pseudo filesystem, because they don't have real path
- supplement the description of this helper function

v6->v7:
- fix missing signed-off-by line

v5->v6: addressed Andrii's feedback
- avoid unnecessary goto end by having two explicit returns

v4->v5: addressed Andrii and Daniel's feedback
- rename bpf_fd2path to bpf_get_file_path to be consistent with other helper's names
- when fdget_raw fails, set ret to -EBADF instead of -EINVAL
- remove fdput from fdget_raw's error path
- use IS_ERR instead of IS_ERR_OR_NULL as d_path ether returns a pointer into the buffer or an error code if the path was too long
- modify the normal path's return value to return copied string length including NUL
- update this helper description's Return bits.

v3->v4: addressed Daniel's feedback
- fix missing fdput()
- move fd2path from kernel/bpf/trace.c to kernel/trace/bpf_trace.c
- move fd2path's test code to another patch
- add comment to explain why use fdget_raw instead of fdget

v2->v3: addressed Yonghong's feedback
- remove unnecessary LOCKDOWN_BPF_READ
- refactor error handling section for enhanced readability
- provide a test case in tools/testing/selftests/bpf

v1->v2: addressed Daniel's feedback
- fix backward compatibility
- add this helper description
- fix signed-off name

Signed-off-by Wenbo Zhang <[email protected]>

When people want to identify which file system files are being opened, read, and written to, they can use this helper with file descriptor as input to achieve this goal. Other pseudo filesystems are also supported.

This requirement is mainly discussed here:

   https://github.com/iovisor/bcc/issues/237

v13->v14: addressed Yonghong and Daniel's feedback
- fix this helper's description to be consistent with comments in d_path
- fix error handling logic fill zeroes not '0's

v12->v13: addressed Brendan and Yonghong's feedback
- rename to get_fd_path
- refactor code & comment to be clearer and more compliant

v11->v12: addressed Alexei's feedback
- only allow tracepoints to make sure it won't dead lock

v10->v11: addressed Al and Alexei's feedback
- fix missing fput()

v9->v10: addressed Andrii's feedback
- send this patch together with the patch selftests as one patch series

v8->v9:
- format helper description

v7->v8: addressed Alexei's feedback
- use fget_raw instead of fdget_raw, as fdget_raw is only used inside fs/
- ensure we're in user context which is safe fot the help to run
- filter unmountable pseudo filesystem, because they don't have real path
- supplement the description of this helper function

v6->v7:
- fix missing signed-off-by line

v5->v6: addressed Andrii's feedback
- avoid unnecessary goto end by having two explicit returns

v4->v5: addressed Andrii and Daniel's feedback
- rename bpf_fd2path to bpf_get_file_path to be consistent with other helper's names
- when fdget_raw fails, set ret to -EBADF instead of -EINVAL
- remove fdput from fdget_raw's error path
- use IS_ERR instead of IS_ERR_OR_NULL as d_path ether returns a pointer into the buffer or an error code if the path was too long
- modify the normal path's return value to return copied string length including NUL
- update this helper description's Return bits.

v3->v4: addressed Daniel's feedback
- fix missing fdput()
- move fd2path from kernel/bpf/trace.c to kernel/trace/bpf_trace.c
- move fd2path's test code to another patch
- add comment to explain why use fdget_raw instead of fdget

v2->v3: addressed Yonghong's feedback
- remove unnecessary LOCKDOWN_BPF_READ
- refactor error handling section for enhanced readability
- provide a test case in tools/testing/selftests/bpf

v1->v2: addressed Daniel's feedback
- fix backward compatibility
- add this helper description
- fix signed-off name

Signed-off-by Wenbo Zhang <[email protected]>

---
  include/uapi/linux/bpf.h       | 29 +++++++++++++-
  kernel/trace/bpf_trace.c       | 69 ++++++++++++++++++++++++++++++++++
  tools/include/uapi/linux/bpf.h | 29 +++++++++++++-
  3 files changed, 125 insertions(+), 2 deletions(-)

include/uapi/linux/bpf.h +28 -1

index dbbcf0b02970..4534ce49f838 100644
@@ -2821,6 +2821,32 @@ union bpf_attr {
   * 	Return
   * 		On success, the strictly positive length of the string,	including
   * 		the trailing NUL character. On error, a negative value.
+ *
+ * int bpf_get_fd_path(char *path, u32 size, int fd)
+ *	Description
+ *		Get **file** atrribute from the current task by *fd*, then call
+ *		**d_path** to get it's absolute path and copy it as string into
+ *		*path* of *size*. Notice the **path** don't support unmountable
+ *		pseudo filesystems as they don't have path (eg: SOCKFS, PIPEFS).
+ *		The *size* must be strictly positive. On success, the helper
+ *		makes sure that the *path* is NUL-terminated, and the buffer
+ *		could be:
+ *		- a regular full path (include mountable fs eg: /proc, /sys)
+ *		- a regular full path with " (deleted)" is appended.
+ *		On failure, it is filled with zeroes.
+ *	Return
+ *		On success, returns the length of the copied string INCLUDING
+ *		the trailing '\0'.
+ *
+ *		On failure, the returned value is one of the following:
+ *
+ *		**-EPERM** if no permission to get the path (eg: in irq ctx).
+ *
+ *		**-EBADF** if *fd* is invalid.
+ *
+ *		**-EINVAL** if *fd* corresponds to a unmountable pseudo fs
+ *
+ *		**-ENAMETOOLONG** if full path is longer than *size*
   */
  #define __BPF_FUNC_MAPPER(FN)		\
  	FN(unspec),			\

@@ -2938,7 +2964,8 @@ union bpf_attr {
  	FN(probe_read_user),		\
  	FN(probe_read_kernel),		\
  	FN(probe_read_user_str),	\
-	FN(probe_read_kernel_str),
+	FN(probe_read_kernel_str),	\
+	FN(get_fd_path),
 
  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
   * function eBPF program intends to call

kernel/trace/bpf_trace.c +69 -0

index e5ef4ae9edb5..a2c18b193141 100644
@@ -762,6 +762,71 @@ static const struct bpf_func_proto bpf_send_signal_proto = {
  	.arg1_type	= ARG_ANYTHING,
  };
 
+BPF_CALL_3(bpf_get_fd_path, char *, dst, u32, size, int, fd)
+{
+	int ret = -EBADF;
+	struct file *f;
+	char *p;
+
+	/* Ensure we're in user context which is safe for the helper to
+	 * run. This helper has no business in a kthread.
+	 */
+	if (unlikely(in_interrupt() ||
+		     current->flags & (PF_KTHREAD | PF_EXITING))) {
+		ret = -EPERM;
+		goto error;
+	}
+
+	/* Use fget_raw instead of fget to support O_PATH, and it doesn't
+	 * have any sleepable code, so it's ok to be here.
+	 */
+	f = fget_raw(fd);
+	if (!f)
+		goto error;
+
+	/* For unmountable pseudo filesystem, it seems to have no meaning
+	 * to get their fake paths as they don't have path, and to be no
+	 * way to validate this function pointer can be always safe to call
+	 * in the current context.
+	 */
+	if (f->f_path.dentry->d_op && f->f_path.dentry->d_op->d_dname) {
+		ret = -EINVAL;
+		fput(f);
+		goto error;
+	}
+
+	/* After filter unmountable pseudo filesytem, d_path won't call
+	 * dentry->d_op->d_name(), the normally path doesn't have any
+	 * sleepable code, and despite it uses the current macro to get
+	 * fs_struct (current->fs), we've already ensured we're in user
+	 * context, so it's ok to be here.
+	 */
+	p = d_path(&f->f_path, dst, size);
+	if (IS_ERR(p)) {
+		ret = PTR_ERR(p);
+		fput(f);
+		goto error;
+	}
+
+	ret = strlen(p) + 1;
+	memmove(dst, p, ret);
+	fput(f);
+	return ret;
+
+error:
+	memset(dst, 0, size);
+	return ret;
+}
+
+static const struct bpf_func_proto bpf_get_fd_path_proto = {
+	.func       = bpf_get_fd_path,
+	.gpl_only   = true,
+	.ret_type   = RET_INTEGER,
+	.arg1_type  = ARG_PTR_TO_UNINIT_MEM,
+	.arg2_type  = ARG_CONST_SIZE,
+	.arg3_type  = ARG_ANYTHING,
+};
+
  static const struct bpf_func_proto *
  tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
  {

@@ -953,6 +1018,8 @@ tp_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
  		return &bpf_get_stackid_proto_tp;
  	case BPF_FUNC_get_stack:
  		return &bpf_get_stack_proto_tp;
+	case BPF_FUNC_get_fd_path:
+		return &bpf_get_fd_path_proto;
  	default:
  		return tracing_func_proto(func_id, prog);
  	}

@@ -1146,6 +1213,8 @@ raw_tp_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
  		return &bpf_get_stackid_proto_raw_tp;
  	case BPF_FUNC_get_stack:
  		return &bpf_get_stack_proto_raw_tp;
+	case BPF_FUNC_get_fd_path:
+		return &bpf_get_fd_path_proto;
  	default:
  		return tracing_func_proto(func_id, prog);
  	}

tools/include/uapi/linux/bpf.h +28 -1

index dbbcf0b02970..4534ce49f838 100644
@@ -2821,6 +2821,32 @@ union bpf_attr {
   * 	Return
   * 		On success, the strictly positive length of the string,	including
   * 		the trailing NUL character. On error, a negative value.
+ *
+ * int bpf_get_fd_path(char *path, u32 size, int fd)
+ *	Description
+ *		Get **file** atrribute from the current task by *fd*, then call
+ *		**d_path** to get it's absolute path and copy it as string into
+ *		*path* of *size*. Notice the **path** don't support unmountable
+ *		pseudo filesystems as they don't have path (eg: SOCKFS, PIPEFS).
+ *		The *size* must be strictly positive. On success, the helper
+ *		makes sure that the *path* is NUL-terminated, and the buffer
+ *		could be:
+ *		- a regular full path (include mountable fs eg: /proc, /sys)
+ *		- a regular full path with " (deleted)" is appended.
+ *		On failure, it is filled with zeroes.
+ *	Return
+ *		On success, returns the length of the copied string INCLUDING
+ *		the trailing '\0'.
+ *
+ *		On failure, the returned value is one of the following:
+ *
+ *		**-EPERM** if no permission to get the path (eg: in irq ctx).
+ *
+ *		**-EBADF** if *fd* is invalid.
+ *
+ *		**-EINVAL** if *fd* corresponds to a unmountable pseudo fs
+ *
+ *		**-ENAMETOOLONG** if full path is longer than *size*
   */
  #define __BPF_FUNC_MAPPER(FN)		\
  	FN(unspec),			\

@@ -2938,7 +2964,8 @@ union bpf_attr {
  	FN(probe_read_user),		\
  	FN(probe_read_kernel),		\
  	FN(probe_read_user_str),	\
-	FN(probe_read_kernel_str),
+	FN(probe_read_kernel_str),	\
+	FN(get_fd_path),
 
  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
   * function eBPF program intends to call

[ Wenbo, please keep also Al (added here) in the loop since he was providing
feedback on prior submissions as well wrt vfs bits. ]

[ Wenbo, please keep also Al (added here) in the loop since he was providing
feedback on prior submissions as well wrt vfs bits. ]

Quoted 263 lines · Wenbo Zhang

When people want to identify which file system files are being opened, read, and written to, they can use this helper with file descriptor as input to achieve this goal. Other pseudo filesystems are also supported.

This requirement is mainly discussed here:

   https://github.com/iovisor/bcc/issues/237

v13->v14: addressed Yonghong and Daniel's feedback
- fix this helper's description to be consistent with comments in d_path
- fix error handling logic fill zeroes not '0's

v12->v13: addressed Brendan and Yonghong's feedback
- rename to get_fd_path
- refactor code & comment to be clearer and more compliant

v11->v12: addressed Alexei's feedback
- only allow tracepoints to make sure it won't dead lock

v10->v11: addressed Al and Alexei's feedback
- fix missing fput()

v9->v10: addressed Andrii's feedback
- send this patch together with the patch selftests as one patch series

v8->v9:
- format helper description

v7->v8: addressed Alexei's feedback
- use fget_raw instead of fdget_raw, as fdget_raw is only used inside fs/
- ensure we're in user context which is safe fot the help to run
- filter unmountable pseudo filesystem, because they don't have real path
- supplement the description of this helper function

v6->v7:
- fix missing signed-off-by line

v5->v6: addressed Andrii's feedback
- avoid unnecessary goto end by having two explicit returns

v4->v5: addressed Andrii and Daniel's feedback
- rename bpf_fd2path to bpf_get_file_path to be consistent with other helper's names
- when fdget_raw fails, set ret to -EBADF instead of -EINVAL
- remove fdput from fdget_raw's error path
- use IS_ERR instead of IS_ERR_OR_NULL as d_path ether returns a pointer into the buffer or an error code if the path was too long
- modify the normal path's return value to return copied string length including NUL
- update this helper description's Return bits.

v3->v4: addressed Daniel's feedback
- fix missing fdput()
- move fd2path from kernel/bpf/trace.c to kernel/trace/bpf_trace.c
- move fd2path's test code to another patch
- add comment to explain why use fdget_raw instead of fdget

v2->v3: addressed Yonghong's feedback
- remove unnecessary LOCKDOWN_BPF_READ
- refactor error handling section for enhanced readability
- provide a test case in tools/testing/selftests/bpf

v1->v2: addressed Daniel's feedback
- fix backward compatibility
- add this helper description
- fix signed-off name

Signed-off-by Wenbo Zhang <[email protected]>

---
  include/uapi/linux/bpf.h       | 29 +++++++++++++-
  kernel/trace/bpf_trace.c       | 69 ++++++++++++++++++++++++++++++++++
  tools/include/uapi/linux/bpf.h | 29 +++++++++++++-
  3 files changed, 125 insertions(+), 2 deletions(-)

include/uapi/linux/bpf.h +28 -1

index dbbcf0b02970..4534ce49f838 100644
@@ -2821,6 +2821,32 @@ union bpf_attr {
   *  Return
   *          On success, the strictly positive length of the string, including
   *          the trailing NUL character. On error, a negative value.
+ *
+ * int bpf_get_fd_path(char *path, u32 size, int fd)
+ *   Description
+ *           Get **file** atrribute from the current task by *fd*, then call
+ *           **d_path** to get it's absolute path and copy it as string into
+ *           *path* of *size*. Notice the **path** don't support unmountable
+ *           pseudo filesystems as they don't have path (eg: SOCKFS, PIPEFS).
+ *           The *size* must be strictly positive. On success, the helper
+ *           makes sure that the *path* is NUL-terminated, and the buffer
+ *           could be:
+ *           - a regular full path (include mountable fs eg: /proc, /sys)
+ *           - a regular full path with " (deleted)" is appended.
+ *           On failure, it is filled with zeroes.
+ *   Return
+ *           On success, returns the length of the copied string INCLUDING
+ *           the trailing '\0'.
+ *
+ *           On failure, the returned value is one of the following:
+ *
+ *           **-EPERM** if no permission to get the path (eg: in irq ctx).
+ *
+ *           **-EBADF** if *fd* is invalid.
+ *
+ *           **-EINVAL** if *fd* corresponds to a unmountable pseudo fs
+ *
+ *           **-ENAMETOOLONG** if full path is longer than *size*
   */
  #define __BPF_FUNC_MAPPER(FN)               \
      FN(unspec),                     \

@@ -2938,7 +2964,8 @@ union bpf_attr {
      FN(probe_read_user),            \
      FN(probe_read_kernel),          \
      FN(probe_read_user_str),        \
-     FN(probe_read_kernel_str),
+     FN(probe_read_kernel_str),      \
+     FN(get_fd_path),
 
  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
   * function eBPF program intends to call

kernel/trace/bpf_trace.c +69 -0

index e5ef4ae9edb5..a2c18b193141 100644
@@ -762,6 +762,71 @@ static const struct bpf_func_proto bpf_send_signal_proto = {
      .arg1_type      = ARG_ANYTHING,
  };
 
+BPF_CALL_3(bpf_get_fd_path, char *, dst, u32, size, int, fd)
+{
+     int ret = -EBADF;
+     struct file *f;
+     char *p;
+
+     /* Ensure we're in user context which is safe for the helper to
+      * run. This helper has no business in a kthread.
+      */
+     if (unlikely(in_interrupt() ||
+                  current->flags & (PF_KTHREAD | PF_EXITING))) {
+             ret = -EPERM;
+             goto error;
+     }
+
+     /* Use fget_raw instead of fget to support O_PATH, and it doesn't
+      * have any sleepable code, so it's ok to be here.
+      */
+     f = fget_raw(fd);
+     if (!f)
+             goto error;
+
+     /* For unmountable pseudo filesystem, it seems to have no meaning
+      * to get their fake paths as they don't have path, and to be no
+      * way to validate this function pointer can be always safe to call
+      * in the current context.
+      */
+     if (f->f_path.dentry->d_op && f->f_path.dentry->d_op->d_dname) {
+             ret = -EINVAL;
+             fput(f);
+             goto error;
+     }
+
+     /* After filter unmountable pseudo filesytem, d_path won't call
+      * dentry->d_op->d_name(), the normally path doesn't have any
+      * sleepable code, and despite it uses the current macro to get
+      * fs_struct (current->fs), we've already ensured we're in user
+      * context, so it's ok to be here.
+      */
+     p = d_path(&f->f_path, dst, size);
+     if (IS_ERR(p)) {
+             ret = PTR_ERR(p);
+             fput(f);
+             goto error;
+     }
+
+     ret = strlen(p) + 1;
+     memmove(dst, p, ret);
+     fput(f);
+     return ret;
+
+error:
+     memset(dst, 0, size);
+     return ret;
+}
+
+static const struct bpf_func_proto bpf_get_fd_path_proto = {
+     .func       = bpf_get_fd_path,
+     .gpl_only   = true,
+     .ret_type   = RET_INTEGER,
+     .arg1_type  = ARG_PTR_TO_UNINIT_MEM,
+     .arg2_type  = ARG_CONST_SIZE,
+     .arg3_type  = ARG_ANYTHING,
+};
+
  static const struct bpf_func_proto *
  tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
  {

@@ -953,6 +1018,8 @@ tp_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
              return &bpf_get_stackid_proto_tp;
      case BPF_FUNC_get_stack:
              return &bpf_get_stack_proto_tp;
+     case BPF_FUNC_get_fd_path:
+             return &bpf_get_fd_path_proto;
      default:
              return tracing_func_proto(func_id, prog);
      }

@@ -1146,6 +1213,8 @@ raw_tp_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
              return &bpf_get_stackid_proto_raw_tp;
      case BPF_FUNC_get_stack:
              return &bpf_get_stack_proto_raw_tp;
+     case BPF_FUNC_get_fd_path:
+             return &bpf_get_fd_path_proto;
      default:
              return tracing_func_proto(func_id, prog);
      }

tools/include/uapi/linux/bpf.h +28 -1

index dbbcf0b02970..4534ce49f838 100644
@@ -2821,6 +2821,32 @@ union bpf_attr {
   *  Return
   *          On success, the strictly positive length of the string, including
   *          the trailing NUL character. On error, a negative value.
+ *
+ * int bpf_get_fd_path(char *path, u32 size, int fd)
+ *   Description
+ *           Get **file** atrribute from the current task by *fd*, then call
+ *           **d_path** to get it's absolute path and copy it as string into
+ *           *path* of *size*. Notice the **path** don't support unmountable
+ *           pseudo filesystems as they don't have path (eg: SOCKFS, PIPEFS).
+ *           The *size* must be strictly positive. On success, the helper
+ *           makes sure that the *path* is NUL-terminated, and the buffer
+ *           could be:
+ *           - a regular full path (include mountable fs eg: /proc, /sys)
+ *           - a regular full path with " (deleted)" is appended.
+ *           On failure, it is filled with zeroes.
+ *   Return
+ *           On success, returns the length of the copied string INCLUDING
+ *           the trailing '\0'.
+ *
+ *           On failure, the returned value is one of the following:
+ *
+ *           **-EPERM** if no permission to get the path (eg: in irq ctx).
+ *
+ *           **-EBADF** if *fd* is invalid.
+ *
+ *           **-EINVAL** if *fd* corresponds to a unmountable pseudo fs
+ *
+ *           **-ENAMETOOLONG** if full path is longer than *size*
   */
  #define __BPF_FUNC_MAPPER(FN)               \
      FN(unspec),                     \

@@ -2938,7 +2964,8 @@ union bpf_attr {
      FN(probe_read_user),            \
      FN(probe_read_kernel),          \
      FN(probe_read_user_str),        \
-     FN(probe_read_kernel_str),
+     FN(probe_read_kernel_str),      \
+     FN(get_fd_path),
 
  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
   * function eBPF program intends to call