[PATCH v2 1/4] bpf: Add support for reading user pointers

[PATCH v2 1/4] bpf: Add support for reading user pointers

[Joel Fernandes (Google)]

The eBPF based opensnoop tool fails to read the file path string passed
to the do_sys_open function. This is because it is a pointer to
userspace address and causes an -EFAULT when read with
probe_kernel_read. This is not an issue when running the tool on x86 but
is an issue on arm64. This patch adds a new bpf function call based
which calls the recently proposed probe_user_read function [1].
Using this function call from opensnoop fixes the issue on arm64.

[1] https://lore.kernel.org/patchwork/patch/1051588/

Cc: Michal Gregorczyk <michalgr@live.com>
Cc: Adrian Ratiu <adrian.ratiu@collabora.com>
Cc: Mohammad Husain <russoue@gmail.com>
Cc: Qais Yousef <qais.yousef@arm.com>
Cc: Srinivas Ramana <sramana@codeaurora.org>
Cc: duyuchao <yuchao.du@unisoc.com>
Cc: Manjo Raja Rao <linux@manojrajarao.com>
Cc: Karim Yaghmour <karim.yaghmour@opersys.com>
Cc: Tamir Carmeli <carmeli.tamir@gmail.com>
Cc: Yonghong Song <yhs@fb.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Brendan Gregg <brendan.d.gregg@gmail.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Peter Ziljstra <peterz@infradead.org>
Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: kernel-team@android.com
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
Masami, could you carry these patches in the series where are you add
probe_user_read function?

Previous submissions is here:
https://lore.kernel.org/patchwork/patch/1069552/
v1->v2: split tools uapi sync into separate commit, added deprecation
warning for old bpf_probe_read function.

 include/uapi/linux/bpf.h |  9 ++++++++-
 kernel/trace/bpf_trace.c | 22 ++++++++++++++++++++++
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 929c8e537a14..8146784b9fe3 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2431,6 +2431,12 @@ union bpf_attr {
  *	Return
  *		A **struct bpf_sock** pointer on success, or **NULL** in
  *		case of failure.
+ *
+ * int bpf_probe_read_user(void *dst, int size, void *src)
+ *     Description
+ *             Read a userspace pointer safely.
+ *     Return
+ *             0 on success or negative error
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -2531,7 +2537,8 @@ union bpf_attr {
 	FN(sk_fullsock),		\
 	FN(tcp_sock),			\
 	FN(skb_ecn_set_ce),		\
-	FN(get_listener_sock),
+	FN(get_listener_sock),		\
+	FN(probe_read_user),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index d64c00afceb5..7485deb0777f 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -153,6 +153,26 @@ static const struct bpf_func_proto bpf_probe_read_proto = {
 	.arg3_type	= ARG_ANYTHING,
 };
 
+BPF_CALL_3(bpf_probe_read_user, void *, dst, u32, size, const void *, unsafe_ptr)
+{
+	int ret;
+
+	ret = probe_user_read(dst, unsafe_ptr, size);
+	if (unlikely(ret < 0))
+		memset(dst, 0, size);
+
+	return ret;
+}
+
+static const struct bpf_func_proto bpf_probe_read_user_proto = {
+	.func		= bpf_probe_read_user,
+	.gpl_only	= true,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_PTR_TO_UNINIT_MEM,
+	.arg2_type	= ARG_CONST_SIZE_OR_ZERO,
+	.arg3_type	= ARG_ANYTHING,
+};
+
 BPF_CALL_3(bpf_probe_write_user, void *, unsafe_ptr, const void *, src,
 	   u32, size)
 {
@@ -571,6 +591,8 @@ tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_map_delete_elem_proto;
 	case BPF_FUNC_probe_read:
 		return &bpf_probe_read_proto;
+	case BPF_FUNC_probe_read_user:
+		return &bpf_probe_read_user_proto;
 	case BPF_FUNC_ktime_get_ns:
 		return &bpf_ktime_get_ns_proto;
 	case BPF_FUNC_tail_call:
-- 
2.21.0.1020.gf2820cf01a-goog

[PATCH v2 2/4] bpf: Add support for reading kernel pointers

[Joel Fernandes (Google)]

The bpf_probe_read function is ambiguous in whether the pointer being
read is a kernel or user one. Add a specific function for kernel pointer
in this patch. Previous patches add it for userspace pointers.

Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
 include/uapi/linux/bpf.h |  9 ++++++++-
 kernel/trace/bpf_trace.c | 22 ++++++++++++++++++++++
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 8146784b9fe3..05af4e1151d3 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2437,6 +2437,12 @@ union bpf_attr {
  *             Read a userspace pointer safely.
  *     Return
  *             0 on success or negative error
+ *
+ * int bpf_probe_read_kernel(void *dst, int size, void *src)
+ *     Description
+ *             Read a kernel pointer safely.
+ *     Return
+ *             0 on success or negative error
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -2538,7 +2544,8 @@ union bpf_attr {
 	FN(tcp_sock),			\
 	FN(skb_ecn_set_ce),		\
 	FN(get_listener_sock),		\
-	FN(probe_read_user),
+	FN(probe_read_user),		\
+	FN(probe_read_kernel),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 7485deb0777f..99dc354fd62b 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -153,6 +153,26 @@ static const struct bpf_func_proto bpf_probe_read_proto = {
 	.arg3_type	= ARG_ANYTHING,
 };
 
+BPF_CALL_3(bpf_probe_read_kernel, void *, dst, u32, size, const void *, unsafe_ptr)
+{
+	int ret;
+
+	ret = probe_kernel_read(dst, unsafe_ptr, size);
+	if (unlikely(ret < 0))
+		memset(dst, 0, size);
+
+	return ret;
+}
+
+static const struct bpf_func_proto bpf_probe_read_kernel_proto = {
+	.func		= bpf_probe_read_kernel,
+	.gpl_only	= true,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_PTR_TO_UNINIT_MEM,
+	.arg2_type	= ARG_CONST_SIZE_OR_ZERO,
+	.arg3_type	= ARG_ANYTHING,
+};
+
 BPF_CALL_3(bpf_probe_read_user, void *, dst, u32, size, const void *, unsafe_ptr)
 {
 	int ret;
@@ -593,6 +613,8 @@ tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_probe_read_proto;
 	case BPF_FUNC_probe_read_user:
 		return &bpf_probe_read_user_proto;
+	case BPF_FUNC_probe_read_kernel:
+		return &bpf_probe_read_kernel_proto;
 	case BPF_FUNC_ktime_get_ns:
 		return &bpf_ktime_get_ns_proto;
 	case BPF_FUNC_tail_call:
-- 
2.21.0.1020.gf2820cf01a-goog

[PATCH v2 3/4] bpf: Add warning when program uses deprecated bpf_probe_read

[Joel Fernandes (Google)]

bpf_probe_read is deprecated and ambiguous. Add a warning if programs
still use it, so that they may be moved to not use it. After sufficient
time, the warning can be removed.

Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
 kernel/bpf/verifier.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 09d5d972c9ff..f8cc77e85b48 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -7656,6 +7656,10 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)
 		if (insn->src_reg == BPF_PSEUDO_CALL)
 			continue;
 
+		if (insn->imm == BPF_FUNC_probe_read)
+			pr_warn_once("bpf_probe_read is deprecated, please use "
+				     "bpf_probe_read_{kernel,user} in eBPF programs.\n");
+
 		if (insn->imm == BPF_FUNC_get_route_realm)
 			prog->dst_needed = 1;
 		if (insn->imm == BPF_FUNC_get_prandom_u32)
-- 
2.21.0.1020.gf2820cf01a-goog

[PATCH v2 4/4] tools: Sync uapi headers with new bpf function calls

[Joel Fernandes (Google)]

The uapi in tools/ needs an update after support for new bpf function
calls were added. This commit does the same.

Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
 tools/include/uapi/linux/bpf.h | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 929c8e537a14..05af4e1151d3 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -2431,6 +2431,18 @@ union bpf_attr {
  *	Return
  *		A **struct bpf_sock** pointer on success, or **NULL** in
  *		case of failure.
+ *
+ * int bpf_probe_read_user(void *dst, int size, void *src)
+ *     Description
+ *             Read a userspace pointer safely.
+ *     Return
+ *             0 on success or negative error
+ *
+ * int bpf_probe_read_kernel(void *dst, int size, void *src)
+ *     Description
+ *             Read a kernel pointer safely.
+ *     Return
+ *             0 on success or negative error
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -2531,7 +2543,9 @@ union bpf_attr {
 	FN(sk_fullsock),		\
 	FN(tcp_sock),			\
 	FN(skb_ecn_set_ce),		\
-	FN(get_listener_sock),
+	FN(get_listener_sock),		\
+	FN(probe_read_user),		\
+	FN(probe_read_kernel),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
-- 
2.21.0.1020.gf2820cf01a-goog

Re: [PATCH v2 1/4] bpf: Add support for reading user pointers

[Daniel Borkmann]

On 05/06/2019 08:31 PM, Joel Fernandes (Google) wrote:
> The eBPF based opensnoop tool fails to read the file path string passed
> to the do_sys_open function. This is because it is a pointer to
> userspace address and causes an -EFAULT when read with
> probe_kernel_read. This is not an issue when running the tool on x86 but
> is an issue on arm64. This patch adds a new bpf function call based
> which calls the recently proposed probe_user_read function [1].
> Using this function call from opensnoop fixes the issue on arm64.
> 
> [1] https://lore.kernel.org/patchwork/patch/1051588/
> 
> Cc: Michal Gregorczyk <michalgr@live.com>
> Cc: Adrian Ratiu <adrian.ratiu@collabora.com>
> Cc: Mohammad Husain <russoue@gmail.com>
> Cc: Qais Yousef <qais.yousef@arm.com>
> Cc: Srinivas Ramana <sramana@codeaurora.org>
> Cc: duyuchao <yuchao.du@unisoc.com>
> Cc: Manjo Raja Rao <linux@manojrajarao.com>
> Cc: Karim Yaghmour <karim.yaghmour@opersys.com>
> Cc: Tamir Carmeli <carmeli.tamir@gmail.com>
> Cc: Yonghong Song <yhs@fb.com>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Brendan Gregg <brendan.d.gregg@gmail.com>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Peter Ziljstra <peterz@infradead.org>
> Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: kernel-team@android.com
> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> ---
> Masami, could you carry these patches in the series where are you add
> probe_user_read function?
> 
> Previous submissions is here:
> https://lore.kernel.org/patchwork/patch/1069552/
> v1->v2: split tools uapi sync into separate commit, added deprecation
> warning for old bpf_probe_read function.

Please properly submit this series to bpf tree once the base
infrastructure from Masami is upstream. This series here should
also fix up all current probe read usage under samples/bpf/ and
tools/testing/selftests/bpf/.

Thanks,
Daniel

Re: [PATCH v2 1/4] bpf: Add support for reading user pointers

[Joel Fernandes]

On Mon, May 06, 2019 at 09:11:19PM +0200, Daniel Borkmann wrote:
> On 05/06/2019 08:31 PM, Joel Fernandes (Google) wrote:
> > The eBPF based opensnoop tool fails to read the file path string passed
> > to the do_sys_open function. This is because it is a pointer to
> > userspace address and causes an -EFAULT when read with
> > probe_kernel_read. This is not an issue when running the tool on x86 but
> > is an issue on arm64. This patch adds a new bpf function call based
> > which calls the recently proposed probe_user_read function [1].
> > Using this function call from opensnoop fixes the issue on arm64.
> > 
> > [1] https://lore.kernel.org/patchwork/patch/1051588/
> > 
> > Cc: Michal Gregorczyk <michalgr@live.com>
> > Cc: Adrian Ratiu <adrian.ratiu@collabora.com>
> > Cc: Mohammad Husain <russoue@gmail.com>
> > Cc: Qais Yousef <qais.yousef@arm.com>
> > Cc: Srinivas Ramana <sramana@codeaurora.org>
> > Cc: duyuchao <yuchao.du@unisoc.com>
> > Cc: Manjo Raja Rao <linux@manojrajarao.com>
> > Cc: Karim Yaghmour <karim.yaghmour@opersys.com>
> > Cc: Tamir Carmeli <carmeli.tamir@gmail.com>
> > Cc: Yonghong Song <yhs@fb.com>
> > Cc: Alexei Starovoitov <ast@kernel.org>
> > Cc: Brendan Gregg <brendan.d.gregg@gmail.com>
> > Cc: Masami Hiramatsu <mhiramat@kernel.org>
> > Cc: Peter Ziljstra <peterz@infradead.org>
> > Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
> > Cc: Steven Rostedt <rostedt@goodmis.org>
> > Cc: Kees Cook <keescook@chromium.org>
> > Cc: kernel-team@android.com
> > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> > ---
> > Masami, could you carry these patches in the series where are you add
> > probe_user_read function?
> > 
> > Previous submissions is here:
> > https://lore.kernel.org/patchwork/patch/1069552/
> > v1->v2: split tools uapi sync into separate commit, added deprecation
> > warning for old bpf_probe_read function.
> 
> Please properly submit this series to bpf tree once the base
> infrastructure from Masami is upstream.

Could you clarify what do you mean by "properly submit this series to bpf
tree" mean? bpf@vger.kernel.org is CC'd.

> This series here should
> also fix up all current probe read usage under samples/bpf/ and
> tools/testing/selftests/bpf/.

Ok. Agreed, will do that.

thanks,

- Joel

Re: [PATCH v2 1/4] bpf: Add support for reading user pointers

[Daniel Borkmann]

On 05/06/2019 09:57 PM, Joel Fernandes wrote:
> On Mon, May 06, 2019 at 09:11:19PM +0200, Daniel Borkmann wrote:
>> On 05/06/2019 08:31 PM, Joel Fernandes (Google) wrote:
>>> The eBPF based opensnoop tool fails to read the file path string passed
>>> to the do_sys_open function. This is because it is a pointer to
>>> userspace address and causes an -EFAULT when read with
>>> probe_kernel_read. This is not an issue when running the tool on x86 but
>>> is an issue on arm64. This patch adds a new bpf function call based
>>> which calls the recently proposed probe_user_read function [1].
>>> Using this function call from opensnoop fixes the issue on arm64.
>>>
>>> [1] https://lore.kernel.org/patchwork/patch/1051588/
>>>
>>> Cc: Michal Gregorczyk <michalgr@live.com>
>>> Cc: Adrian Ratiu <adrian.ratiu@collabora.com>
>>> Cc: Mohammad Husain <russoue@gmail.com>
>>> Cc: Qais Yousef <qais.yousef@arm.com>
>>> Cc: Srinivas Ramana <sramana@codeaurora.org>
>>> Cc: duyuchao <yuchao.du@unisoc.com>
>>> Cc: Manjo Raja Rao <linux@manojrajarao.com>
>>> Cc: Karim Yaghmour <karim.yaghmour@opersys.com>
>>> Cc: Tamir Carmeli <carmeli.tamir@gmail.com>
>>> Cc: Yonghong Song <yhs@fb.com>
>>> Cc: Alexei Starovoitov <ast@kernel.org>
>>> Cc: Brendan Gregg <brendan.d.gregg@gmail.com>
>>> Cc: Masami Hiramatsu <mhiramat@kernel.org>
>>> Cc: Peter Ziljstra <peterz@infradead.org>
>>> Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
>>> Cc: Steven Rostedt <rostedt@goodmis.org>
>>> Cc: Kees Cook <keescook@chromium.org>
>>> Cc: kernel-team@android.com
>>> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
>>> ---
>>> Masami, could you carry these patches in the series where are you add
>>> probe_user_read function?
>>>
>>> Previous submissions is here:
>>> https://lore.kernel.org/patchwork/patch/1069552/
>>> v1->v2: split tools uapi sync into separate commit, added deprecation
>>> warning for old bpf_probe_read function.
>>
>> Please properly submit this series to bpf tree once the base
>> infrastructure from Masami is upstream.
> 
> Could you clarify what do you mean by "properly submit this series to bpf
> tree" mean? bpf@vger.kernel.org is CC'd.

Yeah, send the BPF series to bpf@vger.kernel.org once Masami's patches have
hit mainline, and we'll then route yours as fixes the usual path through
bpf tree.

>> This series here should
>> also fix up all current probe read usage under samples/bpf/ and
>> tools/testing/selftests/bpf/.
> 
> Ok. Agreed, will do that.

Great, thanks!
Daniel

Re: [PATCH v2 1/4] bpf: Add support for reading user pointers

[Joel Fernandes]

On Tue, May 07, 2019 at 01:10:45AM +0200, Daniel Borkmann wrote:
> On 05/06/2019 09:57 PM, Joel Fernandes wrote:
> > On Mon, May 06, 2019 at 09:11:19PM +0200, Daniel Borkmann wrote:
> >> On 05/06/2019 08:31 PM, Joel Fernandes (Google) wrote:
> >>> The eBPF based opensnoop tool fails to read the file path string passed
> >>> to the do_sys_open function. This is because it is a pointer to
> >>> userspace address and causes an -EFAULT when read with
> >>> probe_kernel_read. This is not an issue when running the tool on x86 but
> >>> is an issue on arm64. This patch adds a new bpf function call based
> >>> which calls the recently proposed probe_user_read function [1].
> >>> Using this function call from opensnoop fixes the issue on arm64.
> >>>
> >>> [1] https://lore.kernel.org/patchwork/patch/1051588/
> >>>
> >>> Cc: Michal Gregorczyk <michalgr@live.com>
> >>> Cc: Adrian Ratiu <adrian.ratiu@collabora.com>
> >>> Cc: Mohammad Husain <russoue@gmail.com>
> >>> Cc: Qais Yousef <qais.yousef@arm.com>
> >>> Cc: Srinivas Ramana <sramana@codeaurora.org>
> >>> Cc: duyuchao <yuchao.du@unisoc.com>
> >>> Cc: Manjo Raja Rao <linux@manojrajarao.com>
> >>> Cc: Karim Yaghmour <karim.yaghmour@opersys.com>
> >>> Cc: Tamir Carmeli <carmeli.tamir@gmail.com>
> >>> Cc: Yonghong Song <yhs@fb.com>
> >>> Cc: Alexei Starovoitov <ast@kernel.org>
> >>> Cc: Brendan Gregg <brendan.d.gregg@gmail.com>
> >>> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> >>> Cc: Peter Ziljstra <peterz@infradead.org>
> >>> Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
> >>> Cc: Steven Rostedt <rostedt@goodmis.org>
> >>> Cc: Kees Cook <keescook@chromium.org>
> >>> Cc: kernel-team@android.com
> >>> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> >>> ---
> >>> Masami, could you carry these patches in the series where are you add
> >>> probe_user_read function?
> >>>
> >>> Previous submissions is here:
> >>> https://lore.kernel.org/patchwork/patch/1069552/
> >>> v1->v2: split tools uapi sync into separate commit, added deprecation
> >>> warning for old bpf_probe_read function.
> >>
> >> Please properly submit this series to bpf tree once the base
> >> infrastructure from Masami is upstream.
> > 
> > Could you clarify what do you mean by "properly submit this series to bpf
> > tree" mean? bpf@vger.kernel.org is CC'd.
> 
> Yeah, send the BPF series to bpf@vger.kernel.org once Masami's patches have
> hit mainline, and we'll then route yours as fixes the usual path through
> bpf tree.

Sounds great to me, thanks!

 - Joel

Re: [PATCH v2 1/4] bpf: Add support for reading user pointers

[Masami Hiramatsu]

On Tue, 7 May 2019 01:10:45 +0200
Daniel Borkmann <daniel@iogearbox.net> wrote:

> On 05/06/2019 09:57 PM, Joel Fernandes wrote:
> > On Mon, May 06, 2019 at 09:11:19PM +0200, Daniel Borkmann wrote:
> >> On 05/06/2019 08:31 PM, Joel Fernandes (Google) wrote:
> >>> The eBPF based opensnoop tool fails to read the file path string passed
> >>> to the do_sys_open function. This is because it is a pointer to
> >>> userspace address and causes an -EFAULT when read with
> >>> probe_kernel_read. This is not an issue when running the tool on x86 but
> >>> is an issue on arm64. This patch adds a new bpf function call based
> >>> which calls the recently proposed probe_user_read function [1].
> >>> Using this function call from opensnoop fixes the issue on arm64.
> >>>
> >>> [1] https://lore.kernel.org/patchwork/patch/1051588/
> >>>
> >>> Cc: Michal Gregorczyk <michalgr@live.com>
> >>> Cc: Adrian Ratiu <adrian.ratiu@collabora.com>
> >>> Cc: Mohammad Husain <russoue@gmail.com>
> >>> Cc: Qais Yousef <qais.yousef@arm.com>
> >>> Cc: Srinivas Ramana <sramana@codeaurora.org>
> >>> Cc: duyuchao <yuchao.du@unisoc.com>
> >>> Cc: Manjo Raja Rao <linux@manojrajarao.com>
> >>> Cc: Karim Yaghmour <karim.yaghmour@opersys.com>
> >>> Cc: Tamir Carmeli <carmeli.tamir@gmail.com>
> >>> Cc: Yonghong Song <yhs@fb.com>
> >>> Cc: Alexei Starovoitov <ast@kernel.org>
> >>> Cc: Brendan Gregg <brendan.d.gregg@gmail.com>
> >>> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> >>> Cc: Peter Ziljstra <peterz@infradead.org>
> >>> Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
> >>> Cc: Steven Rostedt <rostedt@goodmis.org>
> >>> Cc: Kees Cook <keescook@chromium.org>
> >>> Cc: kernel-team@android.com
> >>> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> >>> ---
> >>> Masami, could you carry these patches in the series where are you add
> >>> probe_user_read function?
> >>>
> >>> Previous submissions is here:
> >>> https://lore.kernel.org/patchwork/patch/1069552/
> >>> v1->v2: split tools uapi sync into separate commit, added deprecation
> >>> warning for old bpf_probe_read function.
> >>
> >> Please properly submit this series to bpf tree once the base
> >> infrastructure from Masami is upstream.
> > 
> > Could you clarify what do you mean by "properly submit this series to bpf
> > tree" mean? bpf@vger.kernel.org is CC'd.
> 
> Yeah, send the BPF series to bpf@vger.kernel.org once Masami's patches have
> hit mainline, and we'll then route yours as fixes the usual path through
> bpf tree.

OK, then I focus on my series. Keep this series separated.
Thank you!

> 
> >> This series here should
> >> also fix up all current probe read usage under samples/bpf/ and
> >> tools/testing/selftests/bpf/.
> > 
> > Ok. Agreed, will do that.
> 
> Great, thanks!
> Daniel


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH v2 1/4] bpf: Add support for reading user pointers

[Qais Yousef]

On 05/06/19 14:31, Joel Fernandes (Google) wrote:
> The eBPF based opensnoop tool fails to read the file path string passed
> to the do_sys_open function. This is because it is a pointer to
> userspace address and causes an -EFAULT when read with
> probe_kernel_read. This is not an issue when running the tool on x86 but
> is an issue on arm64. This patch adds a new bpf function call based
> which calls the recently proposed probe_user_read function [1].
> Using this function call from opensnoop fixes the issue on arm64.

You haven't updated the commit message as agreed. Please add more explanation
on how arm64 fails or drop the reference. Anyone reads this as-is would
think it always fails on arm64 but it does under some circumstances which
should be explained properly.

I tried opensnoop on 5.1-rc7 and 4.9.173 stable on juno-r2 using the in-tree
defconfig and opensnoop returned the correct results on both cases.

Thanks

--
Qais Yousef