* [net-next PATCH] bpf: Output error message to logbuf when loading
@ 2015-10-26 6:36 Wang Nan
2015-10-26 6:48 ` Wangnan (F)
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Wang Nan @ 2015-10-26 6:36 UTC (permalink / raw)
To: acme, ast
Cc: linux-kernel, pi3orama, lizefan, Wang Nan,
Arnaldo Carvalho de Melo, David S. Miller
Many reason can make bpf_prog_load() return EINVAL. This patch utilizes
logbuf passed from user to deliver the actual reason of failure.
Without this patch, people is easy to forget fixing the "version"
section in their BPF objects.
Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David S. Miller <davem@davemloft.net>
---
kernel/bpf/syscall.c | 41 ++++++++++++++++++++++++++++++++++++++---
1 file changed, 38 insertions(+), 3 deletions(-)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 687dd6c..3a0e4e7 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -574,6 +574,32 @@ struct bpf_prog *bpf_prog_get(u32 ufd)
}
EXPORT_SYMBOL_GPL(bpf_prog_get);
+static void
+bpf_prog_load_note(union bpf_attr *attr, const char *fmt, ...)
+{
+ u32 log_level, log_size, log_len;
+ char __user *log_ubuf = NULL;
+ /* 64 chars should be long enough for a one line note. */
+ char log_buf[64];
+ va_list args;
+
+ log_ubuf = (char __user *) (unsigned long) attr->log_buf;
+ log_level = attr->log_level;
+ log_size = sizeof(log_buf);
+ if (attr->log_size < log_size)
+ log_size = attr->log_size;
+
+ if (log_level == 0 || !log_size || !log_ubuf)
+ return;
+
+ va_start(args, fmt);
+ log_len = vscnprintf(log_buf, log_size, fmt, args);
+ va_end(args);
+
+ /* Don't need to care the copying result too much */
+ copy_to_user(log_ubuf, log_buf, log_size);
+}
+
/* last field in 'union bpf_attr' used by this command */
#define BPF_PROG_LOAD_LAST_FIELD kern_version
@@ -597,12 +623,19 @@ static int bpf_prog_load(union bpf_attr *attr)
/* eBPF programs must be GPL compatible to use GPL-ed functions */
is_gpl = license_is_gpl_compatible(license);
- if (attr->insn_cnt >= BPF_MAXINSNS)
+ if (attr->insn_cnt >= BPF_MAXINSNS) {
+ bpf_prog_load_note(attr, "Too many instructions: %d > %d\n",
+ attr->insn_cnt, BPF_MAXINSNS);
return -EINVAL;
+ }
if (type == BPF_PROG_TYPE_KPROBE &&
- attr->kern_version != LINUX_VERSION_CODE)
+ attr->kern_version != LINUX_VERSION_CODE) {
+ bpf_prog_load_note(attr,
+ "Kernel version mismatch: 0x%x != 0x%x\n",
+ attr->kern_version, LINUX_VERSION_CODE);
return -EINVAL;
+ }
if (type != BPF_PROG_TYPE_SOCKET_FILTER && !capable(CAP_SYS_ADMIN))
return -EPERM;
@@ -631,8 +664,10 @@ static int bpf_prog_load(union bpf_attr *attr)
/* find program type: socket_filter vs tracing_filter */
err = find_prog_type(type, prog);
- if (err < 0)
+ if (err < 0) {
+ bpf_prog_load_note(attr, "Invalid program type: %d\n", type);
goto free_prog;
+ }
/* run eBPF verifier */
err = bpf_check(&prog, attr);
--
1.8.3.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [net-next PATCH] bpf: Output error message to logbuf when loading
2015-10-26 6:36 [net-next PATCH] bpf: Output error message to logbuf when loading Wang Nan
@ 2015-10-26 6:48 ` Wangnan (F)
2015-10-26 6:50 ` kbuild test robot
2015-10-26 7:13 ` [net-next PATCHv2] bpf: Output error message to logbuf when loading failure Wang Nan
2 siblings, 0 replies; 6+ messages in thread
From: Wangnan (F) @ 2015-10-26 6:48 UTC (permalink / raw)
To: acme, ast
Cc: linux-kernel, pi3orama, lizefan, Arnaldo Carvalho de Melo,
David S. Miller
On 2015/10/26 14:36, Wang Nan wrote:
> Many reason can make bpf_prog_load() return EINVAL. This patch utilizes
> logbuf passed from user to deliver the actual reason of failure.
>
> Without this patch, people is easy to forget fixing the "version"
> section in their BPF objects.
>
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: David S. Miller <davem@davemloft.net>
> ---
> kernel/bpf/syscall.c | 41 ++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 38 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 687dd6c..3a0e4e7 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -574,6 +574,32 @@ struct bpf_prog *bpf_prog_get(u32 ufd)
> }
> EXPORT_SYMBOL_GPL(bpf_prog_get);
>
> +static void
> +bpf_prog_load_note(union bpf_attr *attr, const char *fmt, ...)
> +{
> + u32 log_level, log_size, log_len;
> + char __user *log_ubuf = NULL;
> + /* 64 chars should be long enough for a one line note. */
> + char log_buf[64];
> + va_list args;
> +
> + log_ubuf = (char __user *) (unsigned long) attr->log_buf;
> + log_level = attr->log_level;
> + log_size = sizeof(log_buf);
> + if (attr->log_size < log_size)
> + log_size = attr->log_size;
> +
> + if (log_level == 0 || !log_size || !log_ubuf)
> + return;
> +
> + va_start(args, fmt);
> + log_len = vscnprintf(log_buf, log_size, fmt, args);
Don't need this log_len actually. Will send a v2.
Thank you.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [net-next PATCH] bpf: Output error message to logbuf when loading
2015-10-26 6:36 [net-next PATCH] bpf: Output error message to logbuf when loading Wang Nan
2015-10-26 6:48 ` Wangnan (F)
@ 2015-10-26 6:50 ` kbuild test robot
2015-10-26 7:13 ` [net-next PATCHv2] bpf: Output error message to logbuf when loading failure Wang Nan
2 siblings, 0 replies; 6+ messages in thread
From: kbuild test robot @ 2015-10-26 6:50 UTC (permalink / raw)
To: Wang Nan
Cc: kbuild-all, acme, ast, linux-kernel, pi3orama, lizefan, Wang Nan,
Arnaldo Carvalho de Melo, David S. Miller
[-- Attachment #1: Type: text/plain, Size: 1745 bytes --]
Hi Wang,
[auto build test WARNING on net-next/master -- if it's inappropriate base, please suggest rules for selecting the more suitable base]
url: https://github.com/0day-ci/linux/commits/Wang-Nan/bpf-Output-error-message-to-logbuf-when-loading/20151026-143920
config: x86_64-randconfig-x019-201543 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
kernel/bpf/syscall.c: In function 'bpf_prog_load_note':
>> kernel/bpf/syscall.c:600:2: warning: ignoring return value of 'copy_to_user', declared with attribute warn_unused_result [-Wunused-result]
copy_to_user(log_ubuf, log_buf, log_size);
^
vim +/copy_to_user +600 kernel/bpf/syscall.c
584 va_list args;
585
586 log_ubuf = (char __user *) (unsigned long) attr->log_buf;
587 log_level = attr->log_level;
588 log_size = sizeof(log_buf);
589 if (attr->log_size < log_size)
590 log_size = attr->log_size;
591
592 if (log_level == 0 || !log_size || !log_ubuf)
593 return;
594
595 va_start(args, fmt);
596 log_len = vscnprintf(log_buf, log_size, fmt, args);
597 va_end(args);
598
599 /* Don't need to care the copying result too much */
> 600 copy_to_user(log_ubuf, log_buf, log_size);
601 }
602
603 /* last field in 'union bpf_attr' used by this command */
604 #define BPF_PROG_LOAD_LAST_FIELD kern_version
605
606 static int bpf_prog_load(union bpf_attr *attr)
607 {
608 enum bpf_prog_type type = attr->prog_type;
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 25729 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* [net-next PATCHv2] bpf: Output error message to logbuf when loading failure
2015-10-26 6:36 [net-next PATCH] bpf: Output error message to logbuf when loading Wang Nan
2015-10-26 6:48 ` Wangnan (F)
2015-10-26 6:50 ` kbuild test robot
@ 2015-10-26 7:13 ` Wang Nan
2015-10-27 3:26 ` Alexei Starovoitov
2 siblings, 1 reply; 6+ messages in thread
From: Wang Nan @ 2015-10-26 7:13 UTC (permalink / raw)
To: acme, ast
Cc: linux-kernel, pi3orama, lizefan, Wang Nan,
Arnaldo Carvalho de Melo, David S. Miller, Wu Fengguang
Many reasons can make bpf_prog_load() return EINVAL. This patch utilizes
logbuf to deliver the actual reason of the failure.
Without this patch, it is very easy for user to pass an object with
"version" section not match the kernel version code, and the problem
is hard to determine from return code (EINVAL).
Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Wu Fengguang <fengguang.wu@intel.com>
---
kernel/bpf/syscall.c | 45 ++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 42 insertions(+), 3 deletions(-)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 687dd6c..719d0cb 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -17,6 +17,7 @@
#include <linux/license.h>
#include <linux/filter.h>
#include <linux/version.h>
+#include <linux/bug.h>
int sysctl_unprivileged_bpf_disabled __read_mostly;
@@ -574,6 +575,35 @@ struct bpf_prog *bpf_prog_get(u32 ufd)
}
EXPORT_SYMBOL_GPL(bpf_prog_get);
+static void
+bpf_prog_load_note(union bpf_attr *attr, const char *fmt, ...)
+{
+ u32 log_level, log_size;
+ char __user *log_ubuf = NULL;
+ /* 64 chars should be long enough for a one line note. */
+ char log_buf[64];
+ va_list args;
+
+ log_ubuf = (char __user *) (unsigned long) attr->log_buf;
+ log_level = attr->log_level;
+ log_size = sizeof(log_buf);
+ if (attr->log_size < log_size)
+ log_size = attr->log_size;
+
+ if (log_level == 0 || !log_size || !log_ubuf)
+ return;
+
+ va_start(args, fmt);
+ vscnprintf(log_buf, log_size, fmt, args);
+ va_end(args);
+ log_buf[sizeof(log_buf) - 1] = '\0';
+
+ /* Don't need care the copying result too much */
+ WARN(copy_to_user(log_ubuf, log_buf, log_size),
+ KERN_WARNING "Failed to copy BPF error note '%s' to log buffer\n",
+ log_buf);
+}
+
/* last field in 'union bpf_attr' used by this command */
#define BPF_PROG_LOAD_LAST_FIELD kern_version
@@ -597,12 +627,19 @@ static int bpf_prog_load(union bpf_attr *attr)
/* eBPF programs must be GPL compatible to use GPL-ed functions */
is_gpl = license_is_gpl_compatible(license);
- if (attr->insn_cnt >= BPF_MAXINSNS)
+ if (attr->insn_cnt >= BPF_MAXINSNS) {
+ bpf_prog_load_note(attr, "Too many instructions: %d > %d\n",
+ attr->insn_cnt, BPF_MAXINSNS);
return -EINVAL;
+ }
if (type == BPF_PROG_TYPE_KPROBE &&
- attr->kern_version != LINUX_VERSION_CODE)
+ attr->kern_version != LINUX_VERSION_CODE) {
+ bpf_prog_load_note(attr,
+ "Kernel version mismatch: 0x%x != 0x%x\n",
+ attr->kern_version, LINUX_VERSION_CODE);
return -EINVAL;
+ }
if (type != BPF_PROG_TYPE_SOCKET_FILTER && !capable(CAP_SYS_ADMIN))
return -EPERM;
@@ -631,8 +668,10 @@ static int bpf_prog_load(union bpf_attr *attr)
/* find program type: socket_filter vs tracing_filter */
err = find_prog_type(type, prog);
- if (err < 0)
+ if (err < 0) {
+ bpf_prog_load_note(attr, "Invalid program type: %d\n", type);
goto free_prog;
+ }
/* run eBPF verifier */
err = bpf_check(&prog, attr);
--
1.8.3.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [net-next PATCHv2] bpf: Output error message to logbuf when loading failure
2015-10-26 7:13 ` [net-next PATCHv2] bpf: Output error message to logbuf when loading failure Wang Nan
@ 2015-10-27 3:26 ` Alexei Starovoitov
2015-10-27 3:42 ` Wangnan (F)
0 siblings, 1 reply; 6+ messages in thread
From: Alexei Starovoitov @ 2015-10-27 3:26 UTC (permalink / raw)
To: Wang Nan
Cc: acme, ast, linux-kernel, pi3orama, lizefan,
Arnaldo Carvalho de Melo, David S. Miller, Wu Fengguang
On Mon, Oct 26, 2015 at 07:13:08AM +0000, Wang Nan wrote:
> Many reasons can make bpf_prog_load() return EINVAL. This patch utilizes
> logbuf to deliver the actual reason of the failure.
>
> Without this patch, it is very easy for user to pass an object with
> "version" section not match the kernel version code, and the problem
> is hard to determine from return code (EINVAL).
>
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Wu Fengguang <fengguang.wu@intel.com>
NACK
for both implementation and design.
> + /* Don't need care the copying result too much */
> + WARN(copy_to_user(log_ubuf, log_buf, log_size),
> + KERN_WARNING "Failed to copy BPF error note '%s' to log buffer\n",
> + log_buf);
unprivilged user will be spamming kernel logs?!
> - if (attr->insn_cnt >= BPF_MAXINSNS)
> + if (attr->insn_cnt >= BPF_MAXINSNS) {
> + bpf_prog_load_note(attr, "Too many instructions: %d > %d\n",
> + attr->insn_cnt, BPF_MAXINSNS);
> return -EINVAL;
if user space did that, it's wrong and can detect it
on its own.
> if (type == BPF_PROG_TYPE_KPROBE &&
> - attr->kern_version != LINUX_VERSION_CODE)
> + attr->kern_version != LINUX_VERSION_CODE) {
> + bpf_prog_load_note(attr,
> + "Kernel version mismatch: 0x%x != 0x%x\n",
> + attr->kern_version, LINUX_VERSION_CODE);
> return -EINVAL;
user space (perf) could have checked that on its own
without kernel changes.
> /* find program type: socket_filter vs tracing_filter */
> err = find_prog_type(type, prog);
> - if (err < 0)
> + if (err < 0) {
> + bpf_prog_load_note(attr, "Invalid program type: %d\n", type);
> goto free_prog;
same here.
In general syscalls muxing different error conditions into EINVAL
is a kernel wide problem and should be solved for all.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [net-next PATCHv2] bpf: Output error message to logbuf when loading failure
2015-10-27 3:26 ` Alexei Starovoitov
@ 2015-10-27 3:42 ` Wangnan (F)
0 siblings, 0 replies; 6+ messages in thread
From: Wangnan (F) @ 2015-10-27 3:42 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: acme, ast, linux-kernel, pi3orama, lizefan,
Arnaldo Carvalho de Melo, David S. Miller, Wu Fengguang
On 2015/10/27 11:26, Alexei Starovoitov wrote:
> On Mon, Oct 26, 2015 at 07:13:08AM +0000, Wang Nan wrote:
>> Many reasons can make bpf_prog_load() return EINVAL. This patch utilizes
>> logbuf to deliver the actual reason of the failure.
>>
>> Without this patch, it is very easy for user to pass an object with
>> "version" section not match the kernel version code, and the problem
>> is hard to determine from return code (EINVAL).
>>
>> Signed-off-by: Wang Nan <wangnan0@huawei.com>
>> Cc: Alexei Starovoitov <ast@kernel.org>
>> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
>> Cc: David S. Miller <davem@davemloft.net>
>> Cc: Wu Fengguang <fengguang.wu@intel.com>
> NACK
> for both implementation and design.
OK. Let perf to report error message.
Thank you.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-10-27 3:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-26 6:36 [net-next PATCH] bpf: Output error message to logbuf when loading Wang Nan
2015-10-26 6:48 ` Wangnan (F)
2015-10-26 6:50 ` kbuild test robot
2015-10-26 7:13 ` [net-next PATCHv2] bpf: Output error message to logbuf when loading failure Wang Nan
2015-10-27 3:26 ` Alexei Starovoitov
2015-10-27 3:42 ` Wangnan (F)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).