Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH bpf-next 1/3] libbpf: capture value in BTF type info for BTF-defined map defs
From: Andrii Nakryiko @ 2019-06-27 20:19 UTC (permalink / raw)
  To: Song Liu
  Cc: Andrii Nakryiko, Alexei Starovoitov, daniel@iogearbox.net,
	Kernel Team, bpf@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <079A7D73-697C-4CFD-97F3-7CFB741BE4C3@fb.com>

On Thu, Jun 27, 2019 at 10:56 AM Song Liu <songliubraving@fb.com> wrote:
>
>
>
> > On Jun 27, 2019, at 10:47 AM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> >
> > On Thu, Jun 27, 2019 at 10:27 AM Song Liu <songliubraving@fb.com> wrote:
> >>
> >>
> >>
> >>> On Jun 26, 2019, at 4:21 PM, Andrii Nakryiko <andriin@fb.com> wrote:
> >>>
> >>> Change BTF-defined map definitions to capture compile-time integer
> >>> values as part of BTF type definition, to avoid split of key/value type
> >>> information and actual type/size/flags initialization for maps.
> >>
> >> If I have an old bpf program and compiled it with new llvm, will it
> >> work with new libbpf?
> >
> > You mean BPF programs that used previous incarnation of BTF-defined
> > maps? No, they won't work. But we never released them, so I think it's
> > ok to change them. Nothing should be using that except for selftests,
> > which I fixed.
>
> I see. This makes sense.
>
> >
> >>
> >>
> >>>
> >>> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> >>> ---
> >
> > <snip>
> >
> >>> diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
> >>> index 1a5b1accf091..aa5ddf58c088 100644
> >>> --- a/tools/testing/selftests/bpf/bpf_helpers.h
> >>> +++ b/tools/testing/selftests/bpf/bpf_helpers.h
> >>> @@ -8,6 +8,9 @@
> >>> */
> >>> #define SEC(NAME) __attribute__((section(NAME), used))
> >>>
> >>> +#define __int(name, val) int (*name)[val]
> >>> +#define __type(name, val) val *name
> >>> +
> >>
> >> I think we need these two in libbpf.
> >
> > Yes, but it's another story for another set of patches. We'll need to
> > provide bpf_helpers as part of libbpf for inclusion into BPF programs,
> > but there are a bunch of problems right now with existing
> > bpf_heplers.h that prevents us from just copying it into libbpf. We'll
> > need to resolve those first.
> >
> > But then again, there is no use of __int and __type for user-space
> > programs, so for now it's ok.
>
> OK. How about we put these two lines in an separate patch?

Sure, no problem.

>
> Thanks,
> Song
>

^ permalink raw reply

* [PATCH v2 bpf-next 4/4] bpftool: use libbpf_[enable|disable]_sys_bpf()
From: Song Liu @ 2019-06-27 20:19 UTC (permalink / raw)
  To: netdev, bpf; +Cc: ast, daniel, kernel-team, lmb, jannh, gregkh, Song Liu
In-Reply-To: <20190627201923.2589391-1-songliubraving@fb.com>

This patch calls libbpf_[enable|disable]_sys_bpf() from bpftool. This
allows users with access to /dev/bpf to perform operations like root.

Signed-off-by: Song Liu <songliubraving@fb.com>
---
 tools/bpf/bpftool/feature.c | 2 +-
 tools/bpf/bpftool/main.c    | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
index d672d9086fff..772c9f445d34 100644
--- a/tools/bpf/bpftool/feature.c
+++ b/tools/bpf/bpftool/feature.c
@@ -583,7 +583,7 @@ static int do_probe(int argc, char **argv)
 	/* Detection assumes user has sufficient privileges (CAP_SYS_ADMIN).
 	 * Let's approximate, and restrict usage to root user only.
 	 */
-	if (geteuid()) {
+	if (!libbpf_enable_sys_bpf()) {
 		p_err("please run this command as root user");
 		return -1;
 	}
diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
index 4879f6395c7e..56959c5ac552 100644
--- a/tools/bpf/bpftool/main.c
+++ b/tools/bpf/bpftool/main.c
@@ -390,6 +390,10 @@ int main(int argc, char **argv)
 	if (argc < 0)
 		usage();
 
+	if (!libbpf_enable_sys_bpf()) {
+		p_err("cannot enable access to sys_bpf()");
+		usage();
+	}
 	ret = cmd_select(cmds, argc, argv, do_help);
 
 	if (json_output)
@@ -400,5 +404,6 @@ int main(int argc, char **argv)
 		delete_pinned_obj_table(&map_table);
 	}
 
+	libbpf_disable_sys_bpf();
 	return ret;
 }
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2 bpf-next 3/4] libbpf: add libbpf_[enable|disable]_sys_bpf()
From: Song Liu @ 2019-06-27 20:19 UTC (permalink / raw)
  To: netdev, bpf; +Cc: ast, daniel, kernel-team, lmb, jannh, gregkh, Song Liu
In-Reply-To: <20190627201923.2589391-1-songliubraving@fb.com>

This patch adds two more API to libbpf: libbpf_enable_sys_bpf() and
libbpf_disable_sys_bpf().

For root, these two APIs are no-op.

Signed-off-by: Song Liu <songliubraving@fb.com>
---
 tools/lib/bpf/libbpf.c   | 54 ++++++++++++++++++++++++++++++++++++++++
 tools/lib/bpf/libbpf.h   |  7 ++++++
 tools/lib/bpf/libbpf.map |  2 ++
 3 files changed, 63 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 5186b7710430..449764840c5a 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -35,6 +35,7 @@
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/vfs.h>
+#include <sys/ioctl.h>
 #include <tools/libc_compat.h>
 #include <libelf.h>
 #include <gelf.h>
@@ -4286,3 +4287,56 @@ int libbpf_num_possible_cpus(void)
 	}
 	return cpus;
 }
+
+LIBBPF_API bool libbpf_enable_sys_bpf(void)
+{
+	char *cp, errmsg[STRERR_BUFSIZE];
+	int fd, ret;
+
+	if (geteuid() == 0)
+		return true;
+
+	fd = open(LIBBPF_DEV_BPF, O_WRONLY);
+	if (fd < 0) {
+		cp = libbpf_strerror_r(errno, errmsg, sizeof(errmsg));
+		pr_warning("failed to open %s: %s\n", LIBBPF_DEV_BPF, cp);
+		return false;
+	}
+
+	ret = ioctl(fd, BPF_DEV_IOCTL_ENABLE_SYS_BPF);
+
+	if (ret) {
+		cp = libbpf_strerror_r(errno, errmsg, sizeof(errmsg));
+		pr_warning("failed to enable access to sys_bpf(): %s\n", cp);
+		close(fd);
+		return false;
+	}
+	close(fd);
+	pr_debug("enabled access to sys_bpf() for non-privileged user\n");
+	return true;
+}
+
+LIBBPF_API void libbpf_disable_sys_bpf(void)
+{
+	char *cp, errmsg[STRERR_BUFSIZE];
+	int fd, ret;
+
+	if (geteuid() == 0)
+		return;
+
+	fd = open(LIBBPF_DEV_BPF, O_WRONLY);
+	if (fd < 0) {
+		cp = libbpf_strerror_r(errno, errmsg, sizeof(errmsg));
+		pr_warning("failed to open %s: %s\n", LIBBPF_DEV_BPF, cp);
+		return;
+	}
+
+	ret = ioctl(fd, BPF_DEV_IOCTL_DISABLE_SYS_BPF);
+	if (ret) {
+		cp = libbpf_strerror_r(errno, errmsg, sizeof(errmsg));
+		pr_warning("failed to disable access to sys_bpf(): %s\n", cp);
+		close(fd);
+	}
+	close(fd);
+	pr_debug("disabled access to sys_bpf() for non-privileged user\n");
+}
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index d639f47e3110..0e3b9c0f1cdf 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -470,6 +470,13 @@ bpf_program__bpil_offs_to_addr(struct bpf_prog_info_linear *info_linear);
  */
 LIBBPF_API int libbpf_num_possible_cpus(void);
 
+#define LIBBPF_DEV_BPF "/dev/bpf"
+
+/* (For non-root user) get permission to access bpf() syscall */
+LIBBPF_API bool libbpf_enable_sys_bpf(void);
+/* (For non-root user) put permission to access bpf() syscall */
+LIBBPF_API void libbpf_disable_sys_bpf(void);
+
 #ifdef __cplusplus
 } /* extern "C" */
 #endif
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 2c6d835620d2..c5951315a3a5 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -173,4 +173,6 @@ LIBBPF_0.0.4 {
 		btf__parse_elf;
 		bpf_object__load_xattr;
 		libbpf_num_possible_cpus;
+		libbpf_enable_sys_bpf;
+		libbpf_disable_sys_bpf;
 } LIBBPF_0.0.3;
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2 bpf-next 2/4] bpf: sync tools/include/uapi/linux/bpf.h
From: Song Liu @ 2019-06-27 20:19 UTC (permalink / raw)
  To: netdev, bpf; +Cc: ast, daniel, kernel-team, lmb, jannh, gregkh, Song Liu
In-Reply-To: <20190627201923.2589391-1-songliubraving@fb.com>

Sync changes for bpf_dev_ioctl.

Signed-off-by: Song Liu <songliubraving@fb.com>
---
 tools/include/uapi/linux/bpf.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index b077507efa3f..13e148bd6c7c 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -3541,4 +3541,10 @@ struct bpf_sysctl {
 				 */
 };
 
+#define BPF_IOCTL	0xBF
+
+/* enable/disable sys_bpf() for current */
+#define BPF_DEV_IOCTL_ENABLE_SYS_BPF	_IO(BPF_IOCTL, 0x01)
+#define BPF_DEV_IOCTL_DISABLE_SYS_BPF	_IO(BPF_IOCTL, 0x02)
+
 #endif /* _UAPI__LINUX_BPF_H__ */
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2 bpf-next 1/4] bpf: unprivileged BPF access via /dev/bpf
From: Song Liu @ 2019-06-27 20:19 UTC (permalink / raw)
  To: netdev, bpf; +Cc: ast, daniel, kernel-team, lmb, jannh, gregkh, Song Liu
In-Reply-To: <20190627201923.2589391-1-songliubraving@fb.com>

This patch introduce unprivileged BPF access. The access control is
achieved via device /dev/bpf. Users with write access to /dev/bpf are able
to call sys_bpf().

Two ioctl command are added to /dev/bpf:

The two commands enable/disable permission to call sys_bpf() for current
task. This permission is noted by bpf_permitted in task_struct. This
permission is inherited during clone(CLONE_THREAD).

Helper function bpf_capable() is added to check whether the task has got
permission via /dev/bpf.

Signed-off-by: Song Liu <songliubraving@fb.com>
---
 Documentation/ioctl/ioctl-number.txt |  1 +
 include/linux/bpf.h                  | 11 +++++
 include/linux/sched.h                |  3 ++
 include/uapi/linux/bpf.h             |  6 +++
 kernel/bpf/arraymap.c                |  2 +-
 kernel/bpf/cgroup.c                  |  2 +-
 kernel/bpf/core.c                    |  4 +-
 kernel/bpf/cpumap.c                  |  2 +-
 kernel/bpf/devmap.c                  |  2 +-
 kernel/bpf/hashtab.c                 |  4 +-
 kernel/bpf/lpm_trie.c                |  2 +-
 kernel/bpf/offload.c                 |  2 +-
 kernel/bpf/queue_stack_maps.c        |  2 +-
 kernel/bpf/reuseport_array.c         |  2 +-
 kernel/bpf/stackmap.c                |  2 +-
 kernel/bpf/syscall.c                 | 71 +++++++++++++++++++++-------
 kernel/bpf/verifier.c                |  2 +-
 kernel/bpf/xskmap.c                  |  2 +-
 kernel/fork.c                        |  5 ++
 net/core/filter.c                    |  6 +--
 20 files changed, 99 insertions(+), 34 deletions(-)

diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt
index c9558146ac58..19998b99d603 100644
--- a/Documentation/ioctl/ioctl-number.txt
+++ b/Documentation/ioctl/ioctl-number.txt
@@ -327,6 +327,7 @@ Code  Seq#(hex)	Include File		Comments
 0xB4	00-0F	linux/gpio.h		<mailto:linux-gpio@vger.kernel.org>
 0xB5	00-0F	uapi/linux/rpmsg.h	<mailto:linux-remoteproc@vger.kernel.org>
 0xB6	all	linux/fpga-dfl.h
+0xBP	01-02	uapi/linux/bpf.h	<mailto:bpf@vger.kernel.org>
 0xC0	00-0F	linux/usb/iowarrior.h
 0xCA	00-0F	uapi/misc/cxl.h
 0xCA	10-2F	uapi/misc/ocxl.h
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index a62e7889b0b6..d17d57dff467 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -14,6 +14,10 @@
 #include <linux/numa.h>
 #include <linux/wait.h>
 #include <linux/u64_stats_sync.h>
+#include <linux/sched.h>
+#include <linux/capability.h>
+
+#include <asm/current.h>
 
 struct bpf_verifier_env;
 struct perf_event;
@@ -742,6 +746,11 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
 int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog,
 				     const union bpf_attr *kattr,
 				     union bpf_attr __user *uattr);
+
+static inline bool bpf_capable(int cap)
+{
+	return current->bpf_permitted || capable(cap);
+}
 #else /* !CONFIG_BPF_SYSCALL */
 static inline struct bpf_prog *bpf_prog_get(u32 ufd)
 {
@@ -874,6 +883,8 @@ static inline int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog,
 {
 	return -ENOTSUPP;
 }
+
+#define bpf_capable(cap) capable((cap))
 #endif /* CONFIG_BPF_SYSCALL */
 
 static inline struct bpf_prog *bpf_prog_get_type(u32 ufd,
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 11837410690f..e03ee4779c9e 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -732,6 +732,9 @@ struct task_struct {
 	/* to be used once the psi infrastructure lands upstream. */
 	unsigned			use_memdelay:1;
 #endif
+#ifdef CONFIG_BPF_SYSCALL
+	unsigned			bpf_permitted:1;
+#endif
 
 	unsigned long			atomic_flags; /* Flags requiring atomic access. */
 
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index b077507efa3f..13e148bd6c7c 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -3541,4 +3541,10 @@ struct bpf_sysctl {
 				 */
 };
 
+#define BPF_IOCTL	0xBF
+
+/* enable/disable sys_bpf() for current */
+#define BPF_DEV_IOCTL_ENABLE_SYS_BPF	_IO(BPF_IOCTL, 0x01)
+#define BPF_DEV_IOCTL_DISABLE_SYS_BPF	_IO(BPF_IOCTL, 0x02)
+
 #endif /* _UAPI__LINUX_BPF_H__ */
diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
index 1c65ce0098a9..9ae668fa9185 100644
--- a/kernel/bpf/arraymap.c
+++ b/kernel/bpf/arraymap.c
@@ -73,7 +73,7 @@ static struct bpf_map *array_map_alloc(union bpf_attr *attr)
 	bool percpu = attr->map_type == BPF_MAP_TYPE_PERCPU_ARRAY;
 	int ret, numa_node = bpf_map_attr_numa_node(attr);
 	u32 elem_size, index_mask, max_entries;
-	bool unpriv = !capable(CAP_SYS_ADMIN);
+	bool unpriv = !bpf_capable(CAP_SYS_ADMIN);
 	u64 cost, array_size, mask64;
 	struct bpf_map_memory mem;
 	struct bpf_array *array;
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 077ed3a19848..cf9c6a95e4d2 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -794,7 +794,7 @@ cgroup_base_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 	case BPF_FUNC_get_current_cgroup_id:
 		return &bpf_get_current_cgroup_id_proto;
 	case BPF_FUNC_trace_printk:
-		if (capable(CAP_SYS_ADMIN))
+		if (bpf_capable(CAP_SYS_ADMIN))
 			return bpf_get_trace_printk_proto();
 		/* fall through */
 	default:
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 561ed07d3007..762a855404d2 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -646,7 +646,7 @@ static bool bpf_prog_kallsyms_verify_off(const struct bpf_prog *fp)
 void bpf_prog_kallsyms_add(struct bpf_prog *fp)
 {
 	if (!bpf_prog_kallsyms_candidate(fp) ||
-	    !capable(CAP_SYS_ADMIN))
+	    !bpf_capable(CAP_SYS_ADMIN))
 		return;
 
 	spin_lock_bh(&bpf_lock);
@@ -768,7 +768,7 @@ static int bpf_jit_charge_modmem(u32 pages)
 {
 	if (atomic_long_add_return(pages, &bpf_jit_current) >
 	    (bpf_jit_limit >> PAGE_SHIFT)) {
-		if (!capable(CAP_SYS_ADMIN)) {
+		if (!bpf_capable(CAP_SYS_ADMIN)) {
 			atomic_long_sub(pages, &bpf_jit_current);
 			return -EPERM;
 		}
diff --git a/kernel/bpf/cpumap.c b/kernel/bpf/cpumap.c
index 8dff08768087..4c6054626b4f 100644
--- a/kernel/bpf/cpumap.c
+++ b/kernel/bpf/cpumap.c
@@ -83,7 +83,7 @@ static struct bpf_map *cpu_map_alloc(union bpf_attr *attr)
 	u64 cost;
 	int ret;
 
-	if (!capable(CAP_SYS_ADMIN))
+	if (!bpf_capable(CAP_SYS_ADMIN))
 		return ERR_PTR(-EPERM);
 
 	/* check sanity of attributes */
diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
index 40e86a7e0ef0..b7c3785be289 100644
--- a/kernel/bpf/devmap.c
+++ b/kernel/bpf/devmap.c
@@ -83,7 +83,7 @@ static struct bpf_map *dev_map_alloc(union bpf_attr *attr)
 	u64 cost;
 	int err;
 
-	if (!capable(CAP_NET_ADMIN))
+	if (!bpf_capable(CAP_NET_ADMIN))
 		return ERR_PTR(-EPERM);
 
 	/* check sanity of attributes */
diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index 22066a62c8c9..461a75c311a4 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -244,13 +244,13 @@ static int htab_map_alloc_check(union bpf_attr *attr)
 	BUILD_BUG_ON(offsetof(struct htab_elem, fnode.next) !=
 		     offsetof(struct htab_elem, hash_node.pprev));
 
-	if (lru && !capable(CAP_SYS_ADMIN))
+	if (lru && !bpf_capable(CAP_SYS_ADMIN))
 		/* LRU implementation is much complicated than other
 		 * maps.  Hence, limit to CAP_SYS_ADMIN for now.
 		 */
 		return -EPERM;
 
-	if (zero_seed && !capable(CAP_SYS_ADMIN))
+	if (zero_seed && !bpf_capable(CAP_SYS_ADMIN))
 		/* Guard against local DoS, and discourage production use. */
 		return -EPERM;
 
diff --git a/kernel/bpf/lpm_trie.c b/kernel/bpf/lpm_trie.c
index 56e6c75d354d..571962022fdf 100644
--- a/kernel/bpf/lpm_trie.c
+++ b/kernel/bpf/lpm_trie.c
@@ -543,7 +543,7 @@ static struct bpf_map *trie_alloc(union bpf_attr *attr)
 	u64 cost = sizeof(*trie), cost_per_node;
 	int ret;
 
-	if (!capable(CAP_SYS_ADMIN))
+	if (!bpf_capable(CAP_SYS_ADMIN))
 		return ERR_PTR(-EPERM);
 
 	/* check sanity of attributes */
diff --git a/kernel/bpf/offload.c b/kernel/bpf/offload.c
index ba635209ae9a..d3e5378c5a15 100644
--- a/kernel/bpf/offload.c
+++ b/kernel/bpf/offload.c
@@ -366,7 +366,7 @@ struct bpf_map *bpf_map_offload_map_alloc(union bpf_attr *attr)
 	struct bpf_offloaded_map *offmap;
 	int err;
 
-	if (!capable(CAP_SYS_ADMIN))
+	if (!bpf_capable(CAP_SYS_ADMIN))
 		return ERR_PTR(-EPERM);
 	if (attr->map_type != BPF_MAP_TYPE_ARRAY &&
 	    attr->map_type != BPF_MAP_TYPE_HASH)
diff --git a/kernel/bpf/queue_stack_maps.c b/kernel/bpf/queue_stack_maps.c
index f697647ceb54..01d848f1a783 100644
--- a/kernel/bpf/queue_stack_maps.c
+++ b/kernel/bpf/queue_stack_maps.c
@@ -45,7 +45,7 @@ static bool queue_stack_map_is_full(struct bpf_queue_stack *qs)
 /* Called from syscall */
 static int queue_stack_map_alloc_check(union bpf_attr *attr)
 {
-	if (!capable(CAP_SYS_ADMIN))
+	if (!bpf_capable(CAP_SYS_ADMIN))
 		return -EPERM;
 
 	/* check sanity of attributes */
diff --git a/kernel/bpf/reuseport_array.c b/kernel/bpf/reuseport_array.c
index 50c083ba978c..840f38a58c7d 100644
--- a/kernel/bpf/reuseport_array.c
+++ b/kernel/bpf/reuseport_array.c
@@ -154,7 +154,7 @@ static struct bpf_map *reuseport_array_alloc(union bpf_attr *attr)
 	struct bpf_map_memory mem;
 	u64 array_size;
 
-	if (!capable(CAP_SYS_ADMIN))
+	if (!bpf_capable(CAP_SYS_ADMIN))
 		return ERR_PTR(-EPERM);
 
 	array_size = sizeof(*array);
diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
index 052580c33d26..1eab27b0bc17 100644
--- a/kernel/bpf/stackmap.c
+++ b/kernel/bpf/stackmap.c
@@ -90,7 +90,7 @@ static struct bpf_map *stack_map_alloc(union bpf_attr *attr)
 	u64 cost, n_buckets;
 	int err;
 
-	if (!capable(CAP_SYS_ADMIN))
+	if (!bpf_capable(CAP_SYS_ADMIN))
 		return ERR_PTR(-EPERM);
 
 	if (attr->map_flags & ~STACK_CREATE_FLAG_MASK)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 7713cf39795a..13a3ba59d509 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -23,6 +23,8 @@
 #include <linux/timekeeping.h>
 #include <linux/ctype.h>
 #include <linux/nospec.h>
+#include <linux/miscdevice.h>
+#include <linux/resource.h>
 
 #define IS_FD_ARRAY(map) ((map)->map_type == BPF_MAP_TYPE_PROG_ARRAY || \
 			   (map)->map_type == BPF_MAP_TYPE_PERF_EVENT_ARRAY || \
@@ -1166,7 +1168,7 @@ static int map_freeze(const union bpf_attr *attr)
 		err = -EBUSY;
 		goto err_put;
 	}
-	if (!capable(CAP_SYS_ADMIN)) {
+	if (!bpf_capable(CAP_SYS_ADMIN)) {
 		err = -EPERM;
 		goto err_put;
 	}
@@ -1616,7 +1618,7 @@ static int bpf_prog_load(union bpf_attr *attr, union bpf_attr __user *uattr)
 
 	if (!IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) &&
 	    (attr->prog_flags & BPF_F_ANY_ALIGNMENT) &&
-	    !capable(CAP_SYS_ADMIN))
+	    !bpf_capable(CAP_SYS_ADMIN))
 		return -EPERM;
 
 	/* copy eBPF program license from user space */
@@ -1629,11 +1631,12 @@ static int bpf_prog_load(union bpf_attr *attr, union bpf_attr __user *uattr)
 	is_gpl = license_is_gpl_compatible(license);
 
 	if (attr->insn_cnt == 0 ||
-	    attr->insn_cnt > (capable(CAP_SYS_ADMIN) ? BPF_COMPLEXITY_LIMIT_INSNS : BPF_MAXINSNS))
+	    attr->insn_cnt > (bpf_capable(CAP_SYS_ADMIN) ?
+			      BPF_COMPLEXITY_LIMIT_INSNS : BPF_MAXINSNS))
 		return -E2BIG;
 	if (type != BPF_PROG_TYPE_SOCKET_FILTER &&
 	    type != BPF_PROG_TYPE_CGROUP_SKB &&
-	    !capable(CAP_SYS_ADMIN))
+	    !bpf_capable(CAP_SYS_ADMIN))
 		return -EPERM;
 
 	bpf_prog_load_fixup_attach_type(attr);
@@ -1861,7 +1864,7 @@ static int bpf_prog_attach(const union bpf_attr *attr)
 	struct bpf_prog *prog;
 	int ret;
 
-	if (!capable(CAP_NET_ADMIN))
+	if (!bpf_capable(CAP_NET_ADMIN))
 		return -EPERM;
 
 	if (CHECK_ATTR(BPF_PROG_ATTACH))
@@ -1951,7 +1954,7 @@ static int bpf_prog_detach(const union bpf_attr *attr)
 {
 	enum bpf_prog_type ptype;
 
-	if (!capable(CAP_NET_ADMIN))
+	if (!bpf_capable(CAP_NET_ADMIN))
 		return -EPERM;
 
 	if (CHECK_ATTR(BPF_PROG_DETACH))
@@ -2007,7 +2010,7 @@ static int bpf_prog_detach(const union bpf_attr *attr)
 static int bpf_prog_query(const union bpf_attr *attr,
 			  union bpf_attr __user *uattr)
 {
-	if (!capable(CAP_NET_ADMIN))
+	if (!bpf_capable(CAP_NET_ADMIN))
 		return -EPERM;
 	if (CHECK_ATTR(BPF_PROG_QUERY))
 		return -EINVAL;
@@ -2051,7 +2054,7 @@ static int bpf_prog_test_run(const union bpf_attr *attr,
 	struct bpf_prog *prog;
 	int ret = -ENOTSUPP;
 
-	if (!capable(CAP_SYS_ADMIN))
+	if (!bpf_capable(CAP_SYS_ADMIN))
 		return -EPERM;
 	if (CHECK_ATTR(BPF_PROG_TEST_RUN))
 		return -EINVAL;
@@ -2088,7 +2091,7 @@ static int bpf_obj_get_next_id(const union bpf_attr *attr,
 	if (CHECK_ATTR(BPF_OBJ_GET_NEXT_ID) || next_id >= INT_MAX)
 		return -EINVAL;
 
-	if (!capable(CAP_SYS_ADMIN))
+	if (!bpf_capable(CAP_SYS_ADMIN))
 		return -EPERM;
 
 	next_id++;
@@ -2114,7 +2117,7 @@ static int bpf_prog_get_fd_by_id(const union bpf_attr *attr)
 	if (CHECK_ATTR(BPF_PROG_GET_FD_BY_ID))
 		return -EINVAL;
 
-	if (!capable(CAP_SYS_ADMIN))
+	if (!bpf_capable(CAP_SYS_ADMIN))
 		return -EPERM;
 
 	spin_lock_bh(&prog_idr_lock);
@@ -2148,7 +2151,7 @@ static int bpf_map_get_fd_by_id(const union bpf_attr *attr)
 	    attr->open_flags & ~BPF_OBJ_FLAG_MASK)
 		return -EINVAL;
 
-	if (!capable(CAP_SYS_ADMIN))
+	if (!bpf_capable(CAP_SYS_ADMIN))
 		return -EPERM;
 
 	f_flags = bpf_get_file_flag(attr->open_flags);
@@ -2323,7 +2326,7 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
 	info.run_time_ns = stats.nsecs;
 	info.run_cnt = stats.cnt;
 
-	if (!capable(CAP_SYS_ADMIN)) {
+	if (!bpf_capable(CAP_SYS_ADMIN)) {
 		info.jited_prog_len = 0;
 		info.xlated_prog_len = 0;
 		info.nr_jited_ksyms = 0;
@@ -2641,7 +2644,7 @@ static int bpf_btf_load(const union bpf_attr *attr)
 	if (CHECK_ATTR(BPF_BTF_LOAD))
 		return -EINVAL;
 
-	if (!capable(CAP_SYS_ADMIN))
+	if (!bpf_capable(CAP_SYS_ADMIN))
 		return -EPERM;
 
 	return btf_new_fd(attr);
@@ -2654,7 +2657,7 @@ static int bpf_btf_get_fd_by_id(const union bpf_attr *attr)
 	if (CHECK_ATTR(BPF_BTF_GET_FD_BY_ID))
 		return -EINVAL;
 
-	if (!capable(CAP_SYS_ADMIN))
+	if (!bpf_capable(CAP_SYS_ADMIN))
 		return -EPERM;
 
 	return btf_get_fd_by_id(attr->btf_id);
@@ -2723,7 +2726,7 @@ static int bpf_task_fd_query(const union bpf_attr *attr,
 	if (CHECK_ATTR(BPF_TASK_FD_QUERY))
 		return -EINVAL;
 
-	if (!capable(CAP_SYS_ADMIN))
+	if (!bpf_capable(CAP_SYS_ADMIN))
 		return -EPERM;
 
 	if (attr->task_fd_query.flags != 0)
@@ -2791,7 +2794,7 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz
 	union bpf_attr attr = {};
 	int err;
 
-	if (sysctl_unprivileged_bpf_disabled && !capable(CAP_SYS_ADMIN))
+	if (sysctl_unprivileged_bpf_disabled && !bpf_capable(CAP_SYS_ADMIN))
 		return -EPERM;
 
 	err = bpf_check_uarg_tail_zero(uattr, sizeof(attr), size);
@@ -2886,3 +2889,39 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz
 
 	return err;
 }
+
+static long bpf_dev_ioctl(struct file *filp,
+			  unsigned int ioctl, unsigned long arg)
+{
+	switch (ioctl) {
+	case BPF_DEV_IOCTL_ENABLE_SYS_BPF:
+		current->bpf_permitted = 1;
+		break;
+	case BPF_DEV_IOCTL_DISABLE_SYS_BPF:
+		current->bpf_permitted = 0;
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static const struct file_operations bpf_chardev_ops = {
+	.unlocked_ioctl = bpf_dev_ioctl,
+};
+
+static struct miscdevice bpf_dev = {
+	.minor		= MISC_DYNAMIC_MINOR,
+	.name		= "bpf",
+	.fops		= &bpf_chardev_ops,
+	.mode		= 0220,
+};
+
+static int __init bpf_dev_init(void)
+{
+	if (misc_register(&bpf_dev))
+		pr_warn("BPF: Failed to create /dev/bpf. Continue without it...\n");
+
+	return 0;
+}
+device_initcall(bpf_dev_init);
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 0e079b2298f8..79dc4d641cf3 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -9134,7 +9134,7 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr,
 		env->insn_aux_data[i].orig_idx = i;
 	env->prog = *prog;
 	env->ops = bpf_verifier_ops[env->prog->type];
-	is_priv = capable(CAP_SYS_ADMIN);
+	is_priv = bpf_capable(CAP_SYS_ADMIN);
 
 	/* grab the mutex to protect few globals used by verifier */
 	if (!is_priv)
diff --git a/kernel/bpf/xskmap.c b/kernel/bpf/xskmap.c
index ef7338cebd18..06063679c27a 100644
--- a/kernel/bpf/xskmap.c
+++ b/kernel/bpf/xskmap.c
@@ -21,7 +21,7 @@ static struct bpf_map *xsk_map_alloc(union bpf_attr *attr)
 	int cpu, err;
 	u64 cost;
 
-	if (!capable(CAP_NET_ADMIN))
+	if (!bpf_capable(CAP_NET_ADMIN))
 		return ERR_PTR(-EPERM);
 
 	if (attr->max_entries == 0 || attr->key_size != 4 ||
diff --git a/kernel/fork.c b/kernel/fork.c
index 75675b9bf6df..b312562c1523 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1998,6 +1998,11 @@ static __latent_entropy struct task_struct *copy_process(
 	p->sequential_io_avg	= 0;
 #endif
 
+#ifdef CONFIG_BPF_SYSCALL
+	if ((clone_flags & CLONE_THREAD) == 0)
+		p->bpf_permitted = 0;
+#endif
+
 	/* Perform scheduler related setup. Assign this task to a CPU. */
 	retval = sched_fork(clone_flags, p);
 	if (retval)
diff --git a/net/core/filter.c b/net/core/filter.c
index 2014d76e0d2a..01ccf031849c 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5875,7 +5875,7 @@ bpf_base_func_proto(enum bpf_func_id func_id)
 		break;
 	}
 
-	if (!capable(CAP_SYS_ADMIN))
+	if (!bpf_capable(CAP_SYS_ADMIN))
 		return NULL;
 
 	switch (func_id) {
@@ -6438,7 +6438,7 @@ static bool cg_skb_is_valid_access(int off, int size,
 		return false;
 	case bpf_ctx_range(struct __sk_buff, data):
 	case bpf_ctx_range(struct __sk_buff, data_end):
-		if (!capable(CAP_SYS_ADMIN))
+		if (!bpf_capable(CAP_SYS_ADMIN))
 			return false;
 		break;
 	}
@@ -6450,7 +6450,7 @@ static bool cg_skb_is_valid_access(int off, int size,
 		case bpf_ctx_range_till(struct __sk_buff, cb[0], cb[4]):
 			break;
 		case bpf_ctx_range(struct __sk_buff, tstamp):
-			if (!capable(CAP_SYS_ADMIN))
+			if (!bpf_capable(CAP_SYS_ADMIN))
 				return false;
 			break;
 		default:
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2 bpf-next 0/4] sys_bpf() access control via /dev/bpf
From: Song Liu @ 2019-06-27 20:19 UTC (permalink / raw)
  To: netdev, bpf; +Cc: ast, daniel, kernel-team, lmb, jannh, gregkh, Song Liu

Changes v1 => v2:
1. Make default mode of /dev/bpf 0220 (Greg);
2. Rename ioctl commands as BPF_DEV_IOCTL_ENABLE_SYS_BPF and
   BPF_DEV_IOCTL_DISABLE_SYS_BPF (Daniel);
3. Save space for task_struct by reusing free bit (Daniel);
4. Make the permission per process (Lorenz).

Currently, most access to sys_bpf() is limited to root. However, there are
use cases that would benefit from non-privileged use of sys_bpf(), e.g.
systemd.

This set introduces a new model to control the access to sys_bpf(). A
special device, /dev/bpf, is introduced to manage access to sys_bpf().
Users with access to open /dev/bpf will be able to access most of
sys_bpf() features. The use can get access to sys_bpf() by opening /dev/bpf
and use ioctl to enable/disable the access.

The permission to access sys_bpf() is marked by bit bpf_permitted in
task_struct. During clone(), child will inherit this bit if CLONE_THREAD
is set. Therefore, the permission is shared within same user process,
but not via fork().

libbpf APIs libbpf_[enable|disable]_sys_bpf() are added to help get and
put the permission. bpftool is updated to use these APIs.

Song Liu (4):
  bpf: unprivileged BPF access via /dev/bpf
  bpf: sync tools/include/uapi/linux/bpf.h
  libbpf: add libbpf_[enable|disable]_sys_bpf()
  bpftool: use libbpf_[enable|disable]_sys_bpf()

 Documentation/ioctl/ioctl-number.txt |  1 +
 include/linux/bpf.h                  | 11 +++++
 include/linux/sched.h                |  3 ++
 include/uapi/linux/bpf.h             |  6 +++
 kernel/bpf/arraymap.c                |  2 +-
 kernel/bpf/cgroup.c                  |  2 +-
 kernel/bpf/core.c                    |  4 +-
 kernel/bpf/cpumap.c                  |  2 +-
 kernel/bpf/devmap.c                  |  2 +-
 kernel/bpf/hashtab.c                 |  4 +-
 kernel/bpf/lpm_trie.c                |  2 +-
 kernel/bpf/offload.c                 |  2 +-
 kernel/bpf/queue_stack_maps.c        |  2 +-
 kernel/bpf/reuseport_array.c         |  2 +-
 kernel/bpf/stackmap.c                |  2 +-
 kernel/bpf/syscall.c                 | 71 +++++++++++++++++++++-------
 kernel/bpf/verifier.c                |  2 +-
 kernel/bpf/xskmap.c                  |  2 +-
 kernel/fork.c                        |  5 ++
 net/core/filter.c                    |  6 +--
 tools/bpf/bpftool/feature.c          |  2 +-
 tools/bpf/bpftool/main.c             |  5 ++
 tools/include/uapi/linux/bpf.h       |  6 +++
 tools/lib/bpf/libbpf.c               | 54 +++++++++++++++++++++
 tools/lib/bpf/libbpf.h               |  7 +++
 tools/lib/bpf/libbpf.map             |  2 +
 26 files changed, 174 insertions(+), 35 deletions(-)

--
2.17.1

^ permalink raw reply

* Re: [PATCH V33 24/30] bpf: Restrict bpf when kernel lockdown is in confidentiality mode
From: Stephen Smalley @ 2019-06-27 20:16 UTC (permalink / raw)
  To: James Morris
  Cc: Andy Lutomirski, Andy Lutomirski, Matthew Garrett, linux-security,
	LKML, Linux API, David Howells, Alexei Starovoitov,
	Matthew Garrett, Network Development, Chun-Yi Lee,
	Daniel Borkmann, linux-security-module
In-Reply-To: <alpine.LRH.2.21.1906280332500.17363@namei.org>

On 6/27/19 2:06 PM, James Morris wrote:
> On Thu, 27 Jun 2019, Stephen Smalley wrote:
> 
>> There are two scenarios where finer-grained distinctions make sense:
>>
>> - Users may need to enable specific functionality that falls under the
>> umbrella of "confidentiality" or "integrity" lockdown.  Finer-grained lockdown
>> reasons free them from having to make an all-or-nothing choice between lost
>> functionality or no lockdown at all.
> 
> Agreed. This will be used for more than just UEFI secure boot on desktops,
> e.g. embedded systems using verified boot, where finer grained policy will
> be needed for what are sometimes very specific use-cases (which may be
> also covered by other mitigations).
> 
>> This can be supported directly by the
>> lockdown module without any help from SELinux or other security modules; we
>> just need the ability to specify these finer-grained lockdown levels via the
>> boot parameters and securityfs nodes.
> 
> If the lockdown LSM implements fine grained policy (rather than the simple
> coarse grained policy), I'd suggest adding a new lockdown level of
> 'custom' which by default enables all hooks but allows selective
> disablement via params/sysfs.
> 
> This would be simpler than telling users to use a different lockdown LSM
> for this.
> 
>> - Different processes/programs may need to use different sets of functionality
>> restricted via lockdown confidentiality or integrity categories.  If we have
>> to allow all-or-none for the set of interfaces/functionality covered by the
>> generic confidentiality or integrity categories, then we'll end up having to
>> choose between lost functionality or overprivileged processes, neither of
>> which is optimal.
>>
>> Is it truly the case that everything under the "confidentiality" category
>> poses the same level of risk to kernel confidentiality, and similarly for
>> everything under the "integrity" category?  If not, then being able to
>> distinguish them definitely has benefit.
> 
> Good question. We can't know the answer to this unless we know how an
> attacker might leverage access.
> 
> The value here IMHO is more in allowing tradeoffs to be made by system
> designers vs. disabling lockdown entirely.
> 
>> I'm still not clear though on how/if this will compose with or be overridden
>> by other security modules.  We would need some means for another security
>> module to take over lockdown decisions once it has initialized (including
>> policy load), and to be able to access state that is currently private to the
>> lockdown module, like the level.
> 
> Why not utilize stacking (restrictively), similarly to capabilities?

That would only allow the LSM to further lock down the system above the 
lockdown level set at boot, not grant exemptions for specific 
functionality/interfaces required by the user or by a specific 
process/program. You'd have to boot with lockdown=none (or your 
lockdown=custom suggestion) in order for the LSM to allow anything 
covered by the integrity or confidentiality levels.  And then the kernel 
would be unprotected prior to full initialization of the LSM, including 
policy load.

It seems like one would want to be able to boot with lockdown=integrity 
to protect the kernel initially, then switch over to allowing the LSM to 
selectively override it.

^ permalink raw reply

* Re: [PATCH 80/87] net: hippi: remove memset after pci_alloc_consistent
From: Jes Sorensen @ 2019-06-27 20:06 UTC (permalink / raw)
  To: Fuqian Huang; +Cc: David S. Miller, netdev, linux-kernel
In-Reply-To: <20190627174452.5677-1-huangfq.daxian@gmail.com>

On 6/27/19 1:44 PM, Fuqian Huang wrote:
> pci_alloc_consistent calls dma_alloc_coherent directly.
> In commit af7ddd8a627c
> ("Merge tag 'dma-mapping-4.21' of git://git.infradead.org/users/hch/dma-mapping"),
> dma_alloc_coherent has already zeroed the memory.
> So memset is not needed.
> 
> Signed-off-by: Fuqian Huang <huangfq.daxian@gmail.com>
> ---
>  drivers/net/hippi/rrunner.c | 2 --
>  1 file changed, 2 deletions(-)

Acked-by: Jes Sorensen <Jes.Sorensen@gmail.com>

> diff --git a/drivers/net/hippi/rrunner.c b/drivers/net/hippi/rrunner.c
> index 7b9350dbebdd..2a6ec5394966 100644
> --- a/drivers/net/hippi/rrunner.c
> +++ b/drivers/net/hippi/rrunner.c
> @@ -1196,7 +1196,6 @@ static int rr_open(struct net_device *dev)
>  		goto error;
>  	}
>  	rrpriv->rx_ctrl_dma = dma_addr;
> -	memset(rrpriv->rx_ctrl, 0, 256*sizeof(struct ring_ctrl));
>  
>  	rrpriv->info = pci_alloc_consistent(pdev, sizeof(struct rr_info),
>  					    &dma_addr);
> @@ -1205,7 +1204,6 @@ static int rr_open(struct net_device *dev)
>  		goto error;
>  	}
>  	rrpriv->info_dma = dma_addr;
> -	memset(rrpriv->info, 0, sizeof(struct rr_info));
>  	wmb();
>  
>  	spin_lock_irqsave(&rrpriv->lock, flags);
> 


^ permalink raw reply

* Re: [PATCH net-next v2 1/4] net/sched: Introduce action ct
From: David Miller @ 2019-06-27 19:53 UTC (permalink / raw)
  To: paulb
  Cc: jiri, roid, yossiku, ozsh, marcelo.leitner, netdev, aconole,
	wangzhike, ronye, nst-kernel, john.hurley, simon.horman, jpettit
In-Reply-To: <1561038141-31370-2-git-send-email-paulb@mellanox.com>

From: Paul Blakey <paulb@mellanox.com>
Date: Thu, 20 Jun 2019 16:42:18 +0300

> +struct tcf_ct_params {
 ...
> +	struct rcu_head rcu;
> +
> +};

Please get ride of that empty line after the 'rcu' member.

> +	switch (skb->protocol) {
> +	case htons(ETH_P_IP):
> +		family = NFPROTO_IPV4;
> +		break;
> +	case htons(ETH_P_IPV6):
> +		family = NFPROTO_IPV6;
> +		break;
> +	default:
> +	break;

Break statement is not indented properly.

> +static __net_init int ct_init_net(struct net *net)
> +{
> +	struct tc_ct_action_net *tn = net_generic(net, ct_net_id);
> +	unsigned int n_bits = FIELD_SIZEOF(struct tcf_ct_params, labels) * 8;

Reverse christmas tree here please.

^ permalink raw reply

* Re: [PATCH v4 net-next 1/4] net: core: page_pool: add user cnt preventing pool deletion
From: Jesper Dangaard Brouer @ 2019-06-27 19:44 UTC (permalink / raw)
  To: Ivan Khoronzhuk
  Cc: davem, grygorii.strashko, saeedm, leon, ast, linux-kernel,
	linux-omap, ilias.apalodimas, netdev, daniel, jakub.kicinski,
	john.fastabend, brouer
In-Reply-To: <20190625175948.24771-2-ivan.khoronzhuk@linaro.org>

On Tue, 25 Jun 2019 20:59:45 +0300
Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:

> Add user counter allowing to delete pool only when no users.
> It doesn't prevent pool from flush, only prevents freeing the
> pool instance. Helps when no need to delete the pool and now
> it's user responsibility to free it by calling page_pool_free()
> while destroying procedure. It also makes to use page_pool_free()
> explicitly, not fully hidden in xdp unreg, which looks more
> correct after page pool "create" routine.

I don't think that "create" and "free" routines paring looks "more
correct" together.

Maybe we can scale back your solution(?), via creating a page_pool_get()
and page_pool_put() API that can be used by your driver, to keep the
page_pool object after a xdp_rxq_info_unreg() call.  Then you can use
it for two xdp_rxq_info structs, and call page_pool_put() after you
have unregistered both.

The API would basically be:

diff --git a/net/core/page_pool.c b/net/core/page_pool.c
index b366f59885c1..691ddacfb5a6 100644
--- a/net/core/page_pool.c
+++ b/net/core/page_pool.c
@@ -357,6 +357,10 @@ static void __warn_in_flight(struct page_pool *pool)
 void __page_pool_free(struct page_pool *pool)
 {
        WARN(pool->alloc.count, "API usage violation");
+
+       if (atomic_read(&pool->user_cnt) != 0)
+               return;
+
        WARN(!ptr_ring_empty(&pool->ring), "ptr_ring is not empty");
 
        /* Can happen due to forced shutdown */
@@ -372,6 +376,19 @@ void __page_pool_free(struct page_pool *pool)
 }
 EXPORT_SYMBOL(__page_pool_free);
 
+void page_pool_put(struct page_pool *pool)
+{
+       if (!atomic_dec_and_test(&pool->user_cnt))
+               __page_pool_free(pool);
+}
+EXPORT_SYMBOL(page_pool_put);
+
+void page_pool_get(struct page_pool *pool)
+{
+       atomic_inc(&pool->user_cnt);
+}
+EXPORT_SYMBOL(page_pool_get);
+


-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply related

* Re: [pull request][for-next V2 0/7] Generic DIM lib for netdev and RDMA
From: David Miller @ 2019-06-27 19:43 UTC (permalink / raw)
  To: saeedm; +Cc: dledford, jgg, leonro, ogerlitz, sagi, talgi, netdev, linux-rdma
In-Reply-To: <20190625205701.17849-1-saeedm@mellanox.com>

From: Saeed Mahameed <saeedm@mellanox.com>
Date: Tue, 25 Jun 2019 20:57:27 +0000

> Once we are all happy with the series, please pull to net-next and
> rdma-next trees.

Pulled into net-next, thanks.

I'll push it back out once I am done build testing.

^ permalink raw reply

* [PATCHv2 next 3/3] blackhole_dev: add a selftest
From: Mahesh Bandewar @ 2019-06-27 19:43 UTC (permalink / raw)
  To: Netdev
  Cc: Eric Dumazet, David Miller, Michael Chan, Daniel Axtens,
	Mahesh Bandewar, Mahesh Bandewar

Since this is not really a device with all capabilities, this test
ensures that it has *enough* to make it through the data path
without causing unwanted side-effects (read crash!).

Signed-off-by: Mahesh Bandewar <maheshb@google.com>
---
v1 -> v2
  fixed the conflict resolution in selftests Makefile

 lib/Kconfig.debug                             |   9 ++
 lib/Makefile                                  |   1 +
 lib/test_blackhole_dev.c                      | 100 ++++++++++++++++++
 tools/testing/selftests/net/Makefile          |   2 +-
 tools/testing/selftests/net/config            |   1 +
 .../selftests/net/test_blackhole_dev.sh       |  11 ++
 6 files changed, 123 insertions(+), 1 deletion(-)
 create mode 100644 lib/test_blackhole_dev.c
 create mode 100755 tools/testing/selftests/net/test_blackhole_dev.sh

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index cbdfae379896..79d96e1614f5 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1909,6 +1909,15 @@ config TEST_BPF
 
 	  If unsure, say N.
 
+config TEST_BLACKHOLE_DEV
+	tristate "Test BPF filter functionality"
+	depends on m && NET
+	help
+	  This builds the "test_blackhole_dev" module that validates the
+	  data path through this blackhole netdev.
+
+	  If unsure, say N.
+
 config FIND_BIT_BENCHMARK
 	tristate "Test find_bit functions"
 	help
diff --git a/lib/Makefile b/lib/Makefile
index fb7697031a79..c293624d3664 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -91,6 +91,7 @@ obj-$(CONFIG_TEST_DEBUG_VIRTUAL) += test_debug_virtual.o
 obj-$(CONFIG_TEST_MEMCAT_P) += test_memcat_p.o
 obj-$(CONFIG_TEST_OBJAGG) += test_objagg.o
 obj-$(CONFIG_TEST_STACKINIT) += test_stackinit.o
+obj-$(CONFIG_TEST_BLACKHOLE_DEV) += test_blackhole_dev.o
 
 obj-$(CONFIG_TEST_LIVEPATCH) += livepatch/
 
diff --git a/lib/test_blackhole_dev.c b/lib/test_blackhole_dev.c
new file mode 100644
index 000000000000..4c40580a99a3
--- /dev/null
+++ b/lib/test_blackhole_dev.c
@@ -0,0 +1,100 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * This module tests the blackhole_dev that is created during the
+ * net subsystem initialization. The test this module performs is
+ * by injecting an skb into the stack with skb->dev as the
+ * blackhole_dev and expects kernel to behave in a sane manner
+ * (in other words, *not crash*)!
+ *
+ * Copyright (c) 2018, Mahesh Bandewar <maheshb@google.com>
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/printk.h>
+#include <linux/skbuff.h>
+#include <linux/netdevice.h>
+#include <linux/udp.h>
+#include <linux/ipv6.h>
+
+#include <net/dst.h>
+
+#define SKB_SIZE  256
+#define HEAD_SIZE (14+40+8)	/* Ether + IPv6 + UDP */
+#define TAIL_SIZE 32		/* random tail-room */
+
+#define UDP_PORT 1234
+
+static int __init test_blackholedev_init(void)
+{
+	struct ipv6hdr *ip6h;
+	struct sk_buff *skb;
+	struct ethhdr *ethh;
+	struct udphdr *uh;
+	int data_len;
+	int ret;
+
+	skb = alloc_skb(SKB_SIZE, GFP_KERNEL);
+	if (!skb)
+		return -ENOMEM;
+
+	/* Reserve head-room for the headers */
+	skb_reserve(skb, HEAD_SIZE);
+
+	/* Add data to the skb */
+	data_len = SKB_SIZE - (HEAD_SIZE + TAIL_SIZE);
+	memset(__skb_put(skb, data_len), 0xf, data_len);
+
+	/* Add protocol data */
+	/* (Transport) UDP */
+	uh = (struct udphdr *)skb_push(skb, sizeof(struct udphdr));
+	skb_set_transport_header(skb, 0);
+	uh->source = uh->dest = htons(UDP_PORT);
+	uh->len = htons(data_len);
+	uh->check = 0;
+	/* (Network) IPv6 */
+	ip6h = (struct ipv6hdr *)skb_push(skb, sizeof(struct ipv6hdr));
+	skb_set_network_header(skb, 0);
+	ip6h->hop_limit = 32;
+	ip6h->payload_len = data_len + sizeof(struct udphdr);
+	ip6h->nexthdr = IPPROTO_UDP;
+	ip6h->saddr = in6addr_loopback;
+	ip6h->daddr = in6addr_loopback;
+	/* Ether */
+	ethh = (struct ethhdr *)skb_push(skb, sizeof(struct ethhdr));
+	skb_set_mac_header(skb, 0);
+
+	skb->protocol = htons(ETH_P_IPV6);
+	skb->pkt_type = PACKET_HOST;
+	skb->dev = blackhole_netdev;
+
+	/* Now attempt to send the packet */
+	ret = dev_queue_xmit(skb);
+
+	switch (ret) {
+	case NET_XMIT_SUCCESS:
+		pr_warn("dev_queue_xmit() returned NET_XMIT_SUCCESS\n");
+		break;
+	case NET_XMIT_DROP:
+		pr_warn("dev_queue_xmit() returned NET_XMIT_DROP\n");
+		break;
+	case NET_XMIT_CN:
+		pr_warn("dev_queue_xmit() returned NET_XMIT_CN\n");
+		break;
+	default:
+		pr_err("dev_queue_xmit() returned UNKNOWN(%d)\n", ret);
+	}
+
+	return 0;
+}
+
+static void __exit test_blackholedev_exit(void)
+{
+	pr_warn("test_blackholedev module terminating.\n");
+}
+
+module_init(test_blackholedev_init);
+module_exit(test_blackholedev_exit);
+
+MODULE_AUTHOR("Mahesh Bandewar <maheshb@google.com>");
+MODULE_LICENSE("GPL");
diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 9a275d932fd5..1b24e36b4047 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -5,7 +5,7 @@ CFLAGS =  -Wall -Wl,--no-as-needed -O2 -g
 CFLAGS += -I../../../../usr/include/
 
 TEST_PROGS := run_netsocktests run_afpackettests test_bpf.sh netdevice.sh \
-	      rtnetlink.sh xfrm_policy.sh
+	      rtnetlink.sh xfrm_policy.sh test_blackhole_dev.sh
 TEST_PROGS += fib_tests.sh fib-onlink-tests.sh pmtu.sh udpgso.sh ip_defrag.sh
 TEST_PROGS += udpgso_bench.sh fib_rule_tests.sh msg_zerocopy.sh psock_snd.sh
 TEST_PROGS += udpgro_bench.sh udpgro.sh test_vxlan_under_vrf.sh reuseport_addr_any.sh
diff --git a/tools/testing/selftests/net/config b/tools/testing/selftests/net/config
index 89f84b5118bf..e4b878d95ba0 100644
--- a/tools/testing/selftests/net/config
+++ b/tools/testing/selftests/net/config
@@ -27,3 +27,4 @@ CONFIG_NFT_CHAIN_NAT_IPV6=m
 CONFIG_NFT_CHAIN_NAT_IPV4=m
 CONFIG_NET_SCH_FQ=m
 CONFIG_NET_SCH_ETF=m
+CONFIG_TEST_BLACKHOLE_DEV=m
diff --git a/tools/testing/selftests/net/test_blackhole_dev.sh b/tools/testing/selftests/net/test_blackhole_dev.sh
new file mode 100755
index 000000000000..3119b80e711f
--- /dev/null
+++ b/tools/testing/selftests/net/test_blackhole_dev.sh
@@ -0,0 +1,11 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+# Runs blackhole-dev test using blackhole-dev kernel module
+
+if /sbin/modprobe -q test_blackhole_dev ; then
+	/sbin/modprobe -q -r test_blackhole_dev;
+	echo "test_blackhole_dev: ok";
+else
+	echo "test_blackhole_dev: [FAIL]";
+	exit 1;
+fi
-- 
2.22.0.410.gd8fdbe21b5-goog


^ permalink raw reply related

* [PATCHv2 next 2/3] blackhole_netdev: use blackhole_netdev to invalidate dst entries
From: Mahesh Bandewar @ 2019-06-27 19:43 UTC (permalink / raw)
  To: Netdev
  Cc: Eric Dumazet, David Miller, Michael Chan, Daniel Axtens,
	Mahesh Bandewar, Mahesh Bandewar

Use blackhole_netdev instead of 'lo' device with lower MTU when marking
dst "dead".

Signed-off-by: Mahesh Bandewar <maheshb@google.com>
---
v1 -> v2
  no change

 net/core/dst.c   | 2 +-
 net/ipv4/route.c | 3 +--
 net/ipv6/route.c | 2 +-
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/net/core/dst.c b/net/core/dst.c
index e46366228eaf..1325316d9eab 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -160,7 +160,7 @@ void dst_dev_put(struct dst_entry *dst)
 		dst->ops->ifdown(dst, dev, true);
 	dst->input = dst_discard;
 	dst->output = dst_discard_out;
-	dst->dev = dev_net(dst->dev)->loopback_dev;
+	dst->dev = blackhole_netdev;
 	dev_hold(dst->dev);
 	dev_put(dev);
 }
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 59670fafcd26..d61f43feb7fa 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1532,7 +1532,6 @@ static void ipv4_dst_destroy(struct dst_entry *dst)
 
 void rt_flush_dev(struct net_device *dev)
 {
-	struct net *net = dev_net(dev);
 	struct rtable *rt;
 	int cpu;
 
@@ -1543,7 +1542,7 @@ void rt_flush_dev(struct net_device *dev)
 		list_for_each_entry(rt, &ul->head, rt_uncached) {
 			if (rt->dst.dev != dev)
 				continue;
-			rt->dst.dev = net->loopback_dev;
+			rt->dst.dev = blackhole_netdev;
 			dev_hold(rt->dst.dev);
 			dev_put(dev);
 		}
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index e7c2824435c6..ff44c6287633 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -176,7 +176,7 @@ static void rt6_uncached_list_flush_dev(struct net *net, struct net_device *dev)
 			}
 
 			if (rt_dev == dev) {
-				rt->dst.dev = loopback_dev;
+				rt->dst.dev = blackhole_netdev;
 				dev_hold(rt->dst.dev);
 				dev_put(rt_dev);
 			}
-- 
2.22.0.410.gd8fdbe21b5-goog


^ permalink raw reply related

* [PATCHv2 next 1/3] loopback: create blackhole net device similar to loopack.
From: Mahesh Bandewar @ 2019-06-27 19:43 UTC (permalink / raw)
  To: Netdev
  Cc: Eric Dumazet, David Miller, Michael Chan, Daniel Axtens,
	Mahesh Bandewar, Mahesh Bandewar

Create a blackhole net device that can be used for "dead"
dst entries instead of loopback device. This blackhole device differs
from loopback in few aspects: (a) It's not per-ns. (b)  MTU on this
device is ETH_MIN_MTU (c) The xmit function is essentially kfree_skb().
and (d) since it's not registered it won't have ifindex.

Lower MTU effectively make the device not pass the MTU check during
the route check when a dst associated with the skb is dead.

Signed-off-by: Mahesh Bandewar <maheshb@google.com>
---
v1->v2
  no change

 drivers/net/loopback.c    | 76 ++++++++++++++++++++++++++++++++++-----
 include/linux/netdevice.h |  2 ++
 2 files changed, 69 insertions(+), 9 deletions(-)

diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c
index 87d361666cdd..3b39def5471e 100644
--- a/drivers/net/loopback.c
+++ b/drivers/net/loopback.c
@@ -55,6 +55,13 @@
 #include <net/net_namespace.h>
 #include <linux/u64_stats_sync.h>
 
+/* blackhole_netdev - a device used for dsts that are marked expired!
+ * This is global device (instead of per-net-ns) since it's not needed
+ * to be per-ns and gets initialized at boot time.
+ */
+struct net_device *blackhole_netdev;
+EXPORT_SYMBOL(blackhole_netdev);
+
 /* The higher levels take care of making this non-reentrant (it's
  * called with bh's disabled).
  */
@@ -150,12 +157,14 @@ static const struct net_device_ops loopback_ops = {
 	.ndo_set_mac_address = eth_mac_addr,
 };
 
-/* The loopback device is special. There is only one instance
- * per network namespace.
- */
-static void loopback_setup(struct net_device *dev)
+static void gen_lo_setup(struct net_device *dev,
+			 unsigned int mtu,
+			 const struct ethtool_ops *eth_ops,
+			 const struct header_ops *hdr_ops,
+			 const struct net_device_ops *dev_ops,
+			 void (*dev_destructor)(struct net_device *dev))
 {
-	dev->mtu		= 64 * 1024;
+	dev->mtu		= mtu;
 	dev->hard_header_len	= ETH_HLEN;	/* 14	*/
 	dev->min_header_len	= ETH_HLEN;	/* 14	*/
 	dev->addr_len		= ETH_ALEN;	/* 6	*/
@@ -174,11 +183,20 @@ static void loopback_setup(struct net_device *dev)
 		| NETIF_F_NETNS_LOCAL
 		| NETIF_F_VLAN_CHALLENGED
 		| NETIF_F_LOOPBACK;
-	dev->ethtool_ops	= &loopback_ethtool_ops;
-	dev->header_ops		= &eth_header_ops;
-	dev->netdev_ops		= &loopback_ops;
+	dev->ethtool_ops	= eth_ops;
+	dev->header_ops		= hdr_ops;
+	dev->netdev_ops		= dev_ops;
 	dev->needs_free_netdev	= true;
-	dev->priv_destructor	= loopback_dev_free;
+	dev->priv_destructor	= dev_destructor;
+}
+
+/* The loopback device is special. There is only one instance
+ * per network namespace.
+ */
+static void loopback_setup(struct net_device *dev)
+{
+	gen_lo_setup(dev, (64 * 1024), &loopback_ethtool_ops, &eth_header_ops,
+		     &loopback_ops, loopback_dev_free);
 }
 
 /* Setup and register the loopback device. */
@@ -213,3 +231,43 @@ static __net_init int loopback_net_init(struct net *net)
 struct pernet_operations __net_initdata loopback_net_ops = {
 	.init = loopback_net_init,
 };
+
+/* blackhole netdevice */
+static netdev_tx_t blackhole_netdev_xmit(struct sk_buff *skb,
+					 struct net_device *dev)
+{
+	kfree_skb(skb);
+	net_warn_ratelimited("%s(): Dropping skb.\n", __func__);
+	return NETDEV_TX_OK;
+}
+
+static const struct net_device_ops blackhole_netdev_ops = {
+	.ndo_start_xmit = blackhole_netdev_xmit,
+};
+
+/* This is a dst-dummy device used specifically for invalidated
+ * DSTs and unlike loopback, this is not per-ns.
+ */
+static void blackhole_netdev_setup(struct net_device *dev)
+{
+	gen_lo_setup(dev, ETH_MIN_MTU, NULL, NULL, &blackhole_netdev_ops, NULL);
+}
+
+/* Setup and register the blackhole_netdev. */
+static int __init blackhole_netdev_init(void)
+{
+	blackhole_netdev = alloc_netdev(0, "blackhole_dev", NET_NAME_UNKNOWN,
+					blackhole_netdev_setup);
+	if (!blackhole_netdev)
+		return -ENOMEM;
+
+	dev_init_scheduler(blackhole_netdev);
+	dev_activate(blackhole_netdev);
+
+	blackhole_netdev->flags |= IFF_UP | IFF_RUNNING;
+	dev_net_set(blackhole_netdev, &init_net);
+
+	return 0;
+}
+
+device_initcall(blackhole_netdev_init);
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index eeacebd7debb..88292953aa6f 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4870,4 +4870,6 @@ do {								\
 #define PTYPE_HASH_SIZE	(16)
 #define PTYPE_HASH_MASK	(PTYPE_HASH_SIZE - 1)
 
+extern struct net_device *blackhole_netdev;
+
 #endif	/* _LINUX_NETDEVICE_H */
-- 
2.22.0.410.gd8fdbe21b5-goog


^ permalink raw reply related

* [PATCHv2 next 0/3] blackhole device to invalidate dst
From: Mahesh Bandewar @ 2019-06-27 19:42 UTC (permalink / raw)
  To: Netdev
  Cc: Eric Dumazet, David Miller, Michael Chan, Daniel Axtens,
	Mahesh Bandewar, Mahesh Bandewar

When we invalidate dst or mark it "dead", we assign 'lo' to
dst->dev. First of all this assignment is racy and more over,
it has MTU implications.

The standard dev MTU is 1500 while the Loopback MTU is 64k. TCP
code when dereferencing the dst don't check if the dst is valid
or not. TCP when dereferencing a dead-dst while negotiating a
new connection, may use dst device which is 'lo' instead of
using the correct device. Consider the following scenario:

A SYN arrives on an interface and tcp-layer while processing
SYNACK finds a dst and associates it with SYNACK skb. Now before
skb gets passed to L3 for processing, if that dst gets "dead"
(because of the virtual device getting disappeared & then reappeared),
the 'lo' gets assigned to that dst (lo MTU = 64k). Let's assume
the SYN has ADV_MSS set as 9k while the output device through
which this SYNACK is going to go out has standard MTU of 1500.
The MTU check during the route check passes since MIN(9K, 64K)
is 9k and TCP successfully negotiates 9k MSS. The subsequent
data packet; bigger in size gets passed to the device and it 
won't be marked as GSO since the assumed MTU of the device is
9k.

This either crashes the NIC and we have seen fixes that went
into drivers to handle this scenario. 8914a595110a ('bnx2x:
disable GSO where gso_size is too big for hardware') and
2b16f048729b ('net: create skb_gso_validate_mac_len()') and
with those fixes TCP eventually recovers but not before
few dropped segments.

Well, I'm not a TCP expert and though we have experienced
these corner cases in our environment, I could not reproduce 
this case reliably in my test setup to try this fix myself.
However, Michael Chan <michael.chan@broadcom.com> had a setup
where these fixes helped him mitigate the issue and not cause
the crash.

The idea here is to not alter the data-path with additional
locks or smb()/rmb() barriers to avoid racy assignments but
to create a new device that has really low MTU that has
.ndo_start_xmit essentially a kfree_skb(). Make use of this
device instead of 'lo' when marking the dst dead.

First patch implements the blackhole device and second
patch uses it in IPv4 and IPv6 stack while the third patch
is the self test that ensures the sanity of this device.

v1->v2
  fixed the self-test patch to handle the conflict

Mahesh Bandewar (3):
  loopback: create blackhole net device similar to loopack.
  blackhole_netdev: use blackhole_netdev to invalidate dst entries
  blackhole_dev: add a selftest

 drivers/net/loopback.c                        |  76 +++++++++++--
 include/linux/netdevice.h                     |   2 +
 lib/Kconfig.debug                             |   9 ++
 lib/Makefile                                  |   1 +
 lib/test_blackhole_dev.c                      | 100 ++++++++++++++++++
 net/core/dst.c                                |   2 +-
 net/ipv4/route.c                              |   3 +-
 net/ipv6/route.c                              |   2 +-
 tools/testing/selftests/net/Makefile          |   2 +-
 tools/testing/selftests/net/config            |   1 +
 .../selftests/net/test_blackhole_dev.sh       |  11 ++
 11 files changed, 195 insertions(+), 14 deletions(-)
 create mode 100644 lib/test_blackhole_dev.c
 create mode 100755 tools/testing/selftests/net/test_blackhole_dev.sh

-- 
2.22.0.410.gd8fdbe21b5-goog


^ permalink raw reply

* Re: [RFC] longer netdev names proposal
From: Dan Williams @ 2019-06-27 19:35 UTC (permalink / raw)
  To: Stephen Hemminger, Michal Kubecek
  Cc: netdev, Andrew Lunn, David Ahern, Jiri Pirko, davem,
	jakub.kicinski, mlxsw
In-Reply-To: <20190627122041.18c46daf@hermes.lan>

On Thu, 2019-06-27 at 12:20 -0700, Stephen Hemminger wrote:
> On Thu, 27 Jun 2019 20:39:48 +0200
> Michal Kubecek <mkubecek@suse.cz> wrote:
> 
> > > $ ip li set dev enp3s0 alias "Onboard Ethernet"
> > > # ip link show "Onboard Ethernet"
> > > Device "Onboard Ethernet" does not exist.
> > > 
> > > So it does not really appear to be an alias, it is a label. To be
> > > truly useful, it needs to be more than a label, it needs to be a
> > > real
> > > alias which you can use.  
> > 
> > That's exactly what I meant: to be really useful, one should be
> > able to
> > use the alias(es) for setting device options, for adding routes, in
> > netfilter rules etc.
> > 
> > Michal
> 
> The kernel doesn't enforce uniqueness of alias.

Can we even enforce unique aliases/labels? Given that the kernel hasn't
enforced that in the past there's a good possibility of breaking stuff
if it started. (unfortunately)

Dan

> Also current kernel RTM_GETLINK doesn't do filter by alias (easily
> fixed).
> 
> If it did, then handling it in iproute would be something like:
> 
> diff --git a/lib/ll_map.c b/lib/ll_map.c
> index e0ed54bf77c9..c798ba542224 100644
> --- a/lib/ll_map.c
> +++ b/lib/ll_map.c
> @@ -26,15 +26,18 @@
>  struct ll_cache {
>  	struct hlist_node idx_hash;
>  	struct hlist_node name_hash;
> +	struct hlist_node alias_hash;
>  	unsigned	flags;
>  	unsigned 	index;
>  	unsigned short	type;
> -	char		name[];
> +	char		*alias;
> +	char		name[IFNAMSIZ];
>  };
>  
>  #define IDXMAP_SIZE	1024
>  static struct hlist_head idx_head[IDXMAP_SIZE];
>  static struct hlist_head name_head[IDXMAP_SIZE];
> +static struct hlist_head alias_head[IDXMAP_SIZE];
>  
>  static struct ll_cache *ll_get_by_index(unsigned index)
>  {
> @@ -77,10 +80,26 @@ static struct ll_cache *ll_get_by_name(const char
> *name)
>  	return NULL;
>  }
>  
> +static struct ll_cache *ll_get_by_alias(const char *alias)
> +{
> +	struct hlist_node *n;
> +	unsigned h = namehash(alias) & (IDXMAP_SIZE - 1);
> +
> +	hlist_for_each(n, &alias_head[h]) {
> +		struct ll_cache *im
> +			= container_of(n, struct ll_cache, alias_hash);
> +
> +		if (strcmp(im->alias, alias) == 0)
> +			return im;
> +	}
> +
> +	return NULL;
> +}
> +
>  int ll_remember_index(struct nlmsghdr *n, void *arg)
>  {
>  	unsigned int h;
> -	const char *ifname;
> +	const char *ifname, *ifalias;
>  	struct ifinfomsg *ifi = NLMSG_DATA(n);
>  	struct ll_cache *im;
>  	struct rtattr *tb[IFLA_MAX+1];
> @@ -96,6 +115,10 @@ int ll_remember_index(struct nlmsghdr *n, void
> *arg)
>  		if (im) {
>  			hlist_del(&im->name_hash);
>  			hlist_del(&im->idx_hash);
> +			if (im->alias) {
> +				hlist_del(&im->alias_hash);
> +				free(im->alias);
> +			}
>  			free(im);
>  		}
>  		return 0;
> @@ -106,6 +129,8 @@ int ll_remember_index(struct nlmsghdr *n, void
> *arg)
>  	if (ifname == NULL)
>  		return 0;
>  
> +	ifalias = tb[IFLA_IFALIAS] ? rta_getattr_str(tb[IFLA_IFALIAS])
> : NULL;
> +
>  	if (im) {
>  		/* change to existing entry */
>  		if (strcmp(im->name, ifname) != 0) {
> @@ -114,6 +139,14 @@ int ll_remember_index(struct nlmsghdr *n, void
> *arg)
>  			hlist_add_head(&im->name_hash, &name_head[h]);
>  		}
>  
> +		if (im->alias) {
> +			hlist_del(&im->alias_hash);
> +			if (ifalias) {
> +				h = namehash(ifalias) & (IDXMAP_SIZE -
> 1);
> +				hlist_add_head(&im->alias_hash,
> &alias_head[h]);
> +			}
> +		}
> +
>  		im->flags = ifi->ifi_flags;
>  		return 0;
>  	}
> @@ -132,6 +165,12 @@ int ll_remember_index(struct nlmsghdr *n, void
> *arg)
>  	h = namehash(ifname) & (IDXMAP_SIZE - 1);
>  	hlist_add_head(&im->name_hash, &name_head[h]);
>  
> +	if (ifalias) {
> +		im->alias = strdup(ifalias);
> +		h = namehash(ifalias) & (IDXMAP_SIZE - 1);
> +		hlist_add_head(&im->alias_hash, &alias_head[h]);
> +	}		
> +	
>  	return 0;
>  }
>  
> @@ -152,7 +191,7 @@ static unsigned int ll_idx_a2n(const char *name)
>  	return idx;
>  }
>  
> -static int ll_link_get(const char *name, int index)
> +static int ll_link_get(const char *name, const char *alias, int
> index)
>  {
>  	struct {
>  		struct nlmsghdr		n;
> @@ -176,6 +215,9 @@ static int ll_link_get(const char *name, int
> index)
>  	if (name)
>  		addattr_l(&req.n, sizeof(req), IFLA_IFNAME, name,
>  			  strlen(name) + 1);
> +	if (alias)
> +		addattr_l(&req.n, sizeof(req), IFLA_IFALIAS, alias,
> +			  strlen(alias) + 1);
>  
>  	if (rtnl_talk_suppress_rtnl_errmsg(&rth, &req.n, &answer) < 0)
>  		goto out;
> @@ -206,7 +248,7 @@ const char *ll_index_to_name(unsigned int idx)
>  	if (im)
>  		return im->name;
>  
> -	if (ll_link_get(NULL, idx) == idx) {
> +	if (ll_link_get(NULL, NULL, idx) == idx) {
>  		im = ll_get_by_index(idx);
>  		if (im)
>  			return im->name;
> @@ -252,7 +294,13 @@ unsigned ll_name_to_index(const char *name)
>  	if (im)
>  		return im->index;
>  
> -	idx = ll_link_get(name, 0);
> +	im = ll_get_by_alias(name);
> +	if (im)
> +		return im->index;
> +
> +	idx = ll_link_get(name, NULL, 0);
> +	if (idx == 0)
> +		idx = ll_link_get(NULL, name, 0);
>  	if (idx == 0)
>  		idx = if_nametoindex(name);
>  	if (idx == 0)
> @@ -270,7 +318,10 @@ void ll_drop_by_index(unsigned index)
>  
>  	hlist_del(&im->idx_hash);
>  	hlist_del(&im->name_hash);
> -
> +	if (im->alias) {
> +		hlist_del(&im->alias_hash);
> +		free(im->alias);
> +	}
>  	free(im);
>  }
>  
> 


^ permalink raw reply

* Re: [PATCH RFC net-next 1/5] net: dsa: mt7530: Convert to PHYLINK API
From: Andrew Lunn @ 2019-06-27 19:28 UTC (permalink / raw)
  To: Daniel Santos
  Cc: Russell King - ARM Linux admin, René van Dorst, sean.wang,
	f.fainelli, davem, matthias.bgg, vivien.didelot, frank-w, netdev,
	linux-mediatek, linux-mips
In-Reply-To: <e469daa1-3e28-db9c-e29a-7f68cc676fda@pobox.com>

> > Looking at the data sheet page, you want FORCE_MODE_Pn set. You never
> > want the MAC directly talking to the PHY. Bad things will happen.
> 
> So what exactly do you mean by the MAC directly talking to the PHY?  Do
> you mean setting speed, duplex, etc. via the MAC registers instead of
> via MDIO to the MII registers of the PHY?

The data sheet implies the MAC hardware performs reads on the PHY to
get the status, and then uses that to configure the MAC. This is often
called PHY polling. The MAC hardware however has no idea what the PHY
driver is doing. The MAC does not take the PHY mutex. So the PHY
driver might be doing something at the same time the MAC hardware
polls the PHY, and it gets odd results. Some PHYs have multiple pages,
and for example reading the temperature sensor means swapping
pages. If the MAC hardware was to poll while the sensor is being read,
it would not get the link status, it would read some random
temperature register.

So you want the PHY driver to read the results of auto-neg and it then
tell the MAC the results, so the MAC can be configured correctly.

> > Then use FORCE_RX_FC_Pn and FORCE_TX_Pn to reflect phydev->pause and
> > phydev->asym_pause.
> >
> > The same idea applies when using phylink.
> >
> >     Andrew
> 
> You're help is greatly appreciated here.  Admittedly, I'm also trying to
> get this working in the now deprecated swconfig for a 3.18 kernel that's
> in production.

I'm not very familiar with swconfig. Is there software driving the
PHY? If not, it is then safe for the MAC hardware to poll the PHY.

     Andrew


^ permalink raw reply

* Re: [RFC] longer netdev names proposal
From: Stephen Hemminger @ 2019-06-27 19:20 UTC (permalink / raw)
  To: Michal Kubecek
  Cc: netdev, Andrew Lunn, David Ahern, Jiri Pirko, davem,
	jakub.kicinski, mlxsw
In-Reply-To: <20190627183948.GK27240@unicorn.suse.cz>

On Thu, 27 Jun 2019 20:39:48 +0200
Michal Kubecek <mkubecek@suse.cz> wrote:

> > 
> > $ ip li set dev enp3s0 alias "Onboard Ethernet"
> > # ip link show "Onboard Ethernet"
> > Device "Onboard Ethernet" does not exist.
> > 
> > So it does not really appear to be an alias, it is a label. To be
> > truly useful, it needs to be more than a label, it needs to be a real
> > alias which you can use.  
> 
> That's exactly what I meant: to be really useful, one should be able to
> use the alias(es) for setting device options, for adding routes, in
> netfilter rules etc.
> 
> Michal

The kernel doesn't enforce uniqueness of alias.
Also current kernel RTM_GETLINK doesn't do filter by alias (easily fixed).

If it did, then handling it in iproute would be something like:

diff --git a/lib/ll_map.c b/lib/ll_map.c
index e0ed54bf77c9..c798ba542224 100644
--- a/lib/ll_map.c
+++ b/lib/ll_map.c
@@ -26,15 +26,18 @@
 struct ll_cache {
 	struct hlist_node idx_hash;
 	struct hlist_node name_hash;
+	struct hlist_node alias_hash;
 	unsigned	flags;
 	unsigned 	index;
 	unsigned short	type;
-	char		name[];
+	char		*alias;
+	char		name[IFNAMSIZ];
 };
 
 #define IDXMAP_SIZE	1024
 static struct hlist_head idx_head[IDXMAP_SIZE];
 static struct hlist_head name_head[IDXMAP_SIZE];
+static struct hlist_head alias_head[IDXMAP_SIZE];
 
 static struct ll_cache *ll_get_by_index(unsigned index)
 {
@@ -77,10 +80,26 @@ static struct ll_cache *ll_get_by_name(const char *name)
 	return NULL;
 }
 
+static struct ll_cache *ll_get_by_alias(const char *alias)
+{
+	struct hlist_node *n;
+	unsigned h = namehash(alias) & (IDXMAP_SIZE - 1);
+
+	hlist_for_each(n, &alias_head[h]) {
+		struct ll_cache *im
+			= container_of(n, struct ll_cache, alias_hash);
+
+		if (strcmp(im->alias, alias) == 0)
+			return im;
+	}
+
+	return NULL;
+}
+
 int ll_remember_index(struct nlmsghdr *n, void *arg)
 {
 	unsigned int h;
-	const char *ifname;
+	const char *ifname, *ifalias;
 	struct ifinfomsg *ifi = NLMSG_DATA(n);
 	struct ll_cache *im;
 	struct rtattr *tb[IFLA_MAX+1];
@@ -96,6 +115,10 @@ int ll_remember_index(struct nlmsghdr *n, void *arg)
 		if (im) {
 			hlist_del(&im->name_hash);
 			hlist_del(&im->idx_hash);
+			if (im->alias) {
+				hlist_del(&im->alias_hash);
+				free(im->alias);
+			}
 			free(im);
 		}
 		return 0;
@@ -106,6 +129,8 @@ int ll_remember_index(struct nlmsghdr *n, void *arg)
 	if (ifname == NULL)
 		return 0;
 
+	ifalias = tb[IFLA_IFALIAS] ? rta_getattr_str(tb[IFLA_IFALIAS]) : NULL;
+
 	if (im) {
 		/* change to existing entry */
 		if (strcmp(im->name, ifname) != 0) {
@@ -114,6 +139,14 @@ int ll_remember_index(struct nlmsghdr *n, void *arg)
 			hlist_add_head(&im->name_hash, &name_head[h]);
 		}
 
+		if (im->alias) {
+			hlist_del(&im->alias_hash);
+			if (ifalias) {
+				h = namehash(ifalias) & (IDXMAP_SIZE - 1);
+				hlist_add_head(&im->alias_hash, &alias_head[h]);
+			}
+		}
+
 		im->flags = ifi->ifi_flags;
 		return 0;
 	}
@@ -132,6 +165,12 @@ int ll_remember_index(struct nlmsghdr *n, void *arg)
 	h = namehash(ifname) & (IDXMAP_SIZE - 1);
 	hlist_add_head(&im->name_hash, &name_head[h]);
 
+	if (ifalias) {
+		im->alias = strdup(ifalias);
+		h = namehash(ifalias) & (IDXMAP_SIZE - 1);
+		hlist_add_head(&im->alias_hash, &alias_head[h]);
+	}		
+	
 	return 0;
 }
 
@@ -152,7 +191,7 @@ static unsigned int ll_idx_a2n(const char *name)
 	return idx;
 }
 
-static int ll_link_get(const char *name, int index)
+static int ll_link_get(const char *name, const char *alias, int index)
 {
 	struct {
 		struct nlmsghdr		n;
@@ -176,6 +215,9 @@ static int ll_link_get(const char *name, int index)
 	if (name)
 		addattr_l(&req.n, sizeof(req), IFLA_IFNAME, name,
 			  strlen(name) + 1);
+	if (alias)
+		addattr_l(&req.n, sizeof(req), IFLA_IFALIAS, alias,
+			  strlen(alias) + 1);
 
 	if (rtnl_talk_suppress_rtnl_errmsg(&rth, &req.n, &answer) < 0)
 		goto out;
@@ -206,7 +248,7 @@ const char *ll_index_to_name(unsigned int idx)
 	if (im)
 		return im->name;
 
-	if (ll_link_get(NULL, idx) == idx) {
+	if (ll_link_get(NULL, NULL, idx) == idx) {
 		im = ll_get_by_index(idx);
 		if (im)
 			return im->name;
@@ -252,7 +294,13 @@ unsigned ll_name_to_index(const char *name)
 	if (im)
 		return im->index;
 
-	idx = ll_link_get(name, 0);
+	im = ll_get_by_alias(name);
+	if (im)
+		return im->index;
+
+	idx = ll_link_get(name, NULL, 0);
+	if (idx == 0)
+		idx = ll_link_get(NULL, name, 0);
 	if (idx == 0)
 		idx = if_nametoindex(name);
 	if (idx == 0)
@@ -270,7 +318,10 @@ void ll_drop_by_index(unsigned index)
 
 	hlist_del(&im->idx_hash);
 	hlist_del(&im->name_hash);
-
+	if (im->alias) {
+		hlist_del(&im->alias_hash);
+		free(im->alias);
+	}
 	free(im);
 }
 


^ permalink raw reply related

* Re: [PATCH v4 00/13] net: Add generic and Allwinner YAML bindings
From: Rob Herring @ 2019-06-27 19:17 UTC (permalink / raw)
  To: David Miller
  Cc: Maxime Ripard, Mark Rutland, Frank Rowand, Chen-Yu Tsai,
	Maxime Coquelin, Alexandre Torgue, netdev,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	devicetree, linux-stm32, Maxime Chevallier, Antoine Tenart,
	Andrew Lunn, Florian Fainelli, Heiner Kallweit
In-Reply-To: <20190627.102256.1839462093915893704.davem@davemloft.net>

On Thu, Jun 27, 2019 at 11:22 AM David Miller <davem@davemloft.net> wrote:
>
> From: Maxime Ripard <maxime.ripard@bootlin.com>
> Date: Thu, 27 Jun 2019 17:31:42 +0200
>
> > This is an attempt at getting the main generic DT bindings for the ethernet
> > (and related) devices, and convert some DT bindings for the Allwinner DTs
> > to YAML as well.
> >
> > This should provide some DT validation coverage.
>
> I don't think this should go via my tree as it's all DT stuff.

That's fine. I can take it. There's one conflict with commit
79b647a0c0d5 ("dt-bindings: net: document new usxgmii phy mode"), but
that's easy enough to handle. Any other changes to the binding docs
will need to go thru me this cycle.

Rob

^ permalink raw reply

* Re: [PATCH 2/2 nf-next] netfilter:nft_meta: add NFT_META_VLAN support
From: Pablo Neira Ayuso @ 2019-06-27 19:16 UTC (permalink / raw)
  To: wenxu; +Cc: fw, netfilter-devel, netdev
In-Reply-To: <c9ce6a77-a5db-b3a1-ab48-eb6bc97337e1@ucloud.cn>

On Thu, Jun 27, 2019 at 09:37:53PM +0800, wenxu wrote:
> 
> 在 2019/6/27 20:35, Pablo Neira Ayuso 写道:
> > On Thu, Jun 27, 2019 at 10:09:17AM +0800, wenxu@ucloud.cn wrote:
> >> From: wenxu <wenxu@ucloud.cn>
> >>
> >> This patch provide a meta vlan to set the vlan tag of the packet.
> >>
> >> for q-in-q vlan id 20:
> >> meta vlan set 0x88a8:20
> > Actually, I think this is not very useful for stacked vlan since this
> > just sets/mangles the existing meta vlan data.
> >
> > We'll need infrastructure that uses skb_vlan_push() and _pop().
> >
> > Patch looks good anyway, such infrastructure to push/pop can be added
> > later on.
> >
> > Thanks.
> 
> yes, It's just ste/mangle the meta vlan data. I just wonder if we set for stacked vlan.
> vlan meta 0x88a8:20. The packet should contain a 0x8100 vlan tag, we just push the
> inner vlan and the the vlan meta with the outer 0x88a8:20. Or the packet don't contain
> only vlan tag, we add a inner 0x8100:20 tag and outer 0x88a8:20 tag?

You got me thinking here.

I wonder if we can just make this fit into nft_payload.

Or just add a new nft_vlan extension for this specifically, to push,
to mangle and to pop vlan headers. This would be a simple solution for
this.

I need to explore this by the weekend, will get back to you beginning
next week.

Feedback is welcome in any case :-), thanks.

^ permalink raw reply

* Re: [PATCH 1/2 nf-next v2] netfilter: nft_meta: add NFT_META_BRI_O/IIFVPROTO support
From: Pablo Neira Ayuso @ 2019-06-27 19:12 UTC (permalink / raw)
  To: wenxu; +Cc: fw, netfilter-devel, netdev
In-Reply-To: <1561640835-4507-1-git-send-email-wenxu@ucloud.cn>

On Thu, Jun 27, 2019 at 09:07:14PM +0800, wenxu@ucloud.cn wrote:
> From: wenxu <wenxu@ucloud.cn>
> 
> This patch provide a meta to get the bridge vlan proto
> 
> nft add rule bridge firewall zones counter meta br_iifvproto 0x8100
> 
> Signed-off-by: wenxu <wenxu@ucloud.cn>
> ---
>  include/uapi/linux/netfilter/nf_tables.h |  4 ++++
>  net/netfilter/nft_meta.c                 | 18 ++++++++++++++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
> index 8859535..0f75a6d 100644
> --- a/include/uapi/linux/netfilter/nf_tables.h
> +++ b/include/uapi/linux/netfilter/nf_tables.h
> @@ -796,6 +796,8 @@ enum nft_exthdr_attributes {
>   * @NFT_META_IIFKIND: packet input interface kind name (dev->rtnl_link_ops->kind)
>   * @NFT_META_OIFKIND: packet output interface kind name (dev->rtnl_link_ops->kind)
>   * @NFT_META_BRI_PVID: packet input bridge port pvid

An initial patch to re-name NFT_META_BRI_PVID to NFT_META_BRI_IIFVID
would be good, and to add NFT_META_BRI_OIFVID... if you have a usecase
for this, of course.

Thanks.

^ permalink raw reply

* Re: [PATCH RFC net-next 1/5] net: dsa: mt7530: Convert to PHYLINK API
From: Daniel Santos @ 2019-06-27 19:09 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Russell King - ARM Linux admin, René van Dorst, sean.wang,
	f.fainelli, davem, matthias.bgg, vivien.didelot, frank-w, netdev,
	linux-mediatek, linux-mips
In-Reply-To: <20190625204148.GB27733@lunn.ch>



On 6/25/19 3:41 PM, Andrew Lunn wrote:
> On Tue, Jun 25, 2019 at 02:27:55PM -0500, Daniel Santos wrote:
>> On 6/25/19 2:02 PM, Andrew Lunn wrote:
>>>> But will there still be a mechanism to ignore link partner's advertising
>>>> and force these parameters?
>>> >From man 1 ethtool:
>>>
>>>        -a --show-pause
>>>               Queries the specified Ethernet device for pause parameter information.
>>>
>>>        -A --pause
>>>               Changes the pause parameters of the specified Ethernet device.
>>>
>>>            autoneg on|off
>>>                   Specifies whether pause autonegotiation should be enabled.
>>>
>>>            rx on|off
>>>                   Specifies whether RX pause should be enabled.
>>>
>>>            tx on|off
>>>                   Specifies whether TX pause should be enabled.
>>>
>>> You need to check the driver to see if it actually implements this
>>> ethtool call, but that is how it should be configured.
>>>
>>> 	Andrew
>>>
>> Thank you Andrew,
>>
>> So in this context, my question is the difference between "enabling" and
>> "forcing".  Here's that register for the mt7620 (which has an mt7530 on
>> its die): https://imgur.com/a/pTk0668  I believe this is also what René
>> is seeking clarity on?
> Lets start with normal operation. If the MAC supports pause or asym
> pause, it calls phy_support_sym_pause() or phy_support_asym_pause().
> phylib will then configure the PHY to advertise pause as appropriate.
> Once auto-neg has completed, the results of the negotiation are set in
> phydev. So phdev->pause and phydev->asym_pause. The MAC callback is
> then used to tell the MAC about the autoneg results. The MAC should be
> programmed using the values in phdev->pause and phydev->asym_pause.
>
> For ethtool, the MAC driver needs to implement .get_pauseparam and
> .set_pauseparam. The set_pauseparam needs to validate the settings,
> using phy_validate_pause(). If valid, phy_set_asym_pause() is used to
> tell the PHY about the new configuration. This will trigger a new
> auto-neg if auto-neg is enabled, and the results will be passed back
> in the usual way. If auto-neg is disabled, or pause auto-neg is
> disabled, the MAC should configure pause directly based on the
> settings passed.
>
> Looking at the data sheet page, you want FORCE_MODE_Pn set. You never
> want the MAC directly talking to the PHY. Bad things will happen.

So what exactly do you mean by the MAC directly talking to the PHY?  Do
you mean setting speed, duplex, etc. via the MAC registers instead of
via MDIO to the MII registers of the PHY?

> Then use FORCE_RX_FC_Pn and FORCE_TX_Pn to reflect phydev->pause and
> phydev->asym_pause.
>
> The same idea applies when using phylink.
>
>     Andrew

You're help is greatly appreciated here.  Admittedly, I'm also trying to
get this working in the now deprecated swconfig for a 3.18 kernel that's
in production.  In my code, I had just set the appropriate bits in both
the MAC and mii registers -- did I just shoot myself in the foot or only
toe or two? :)  I should probably start a separate thread for this. 
(And probably attempt to wrestle an mt7530 programmer's guide out of
MediaTek!)

Thanks,
Daniel

PS: I found a rather humorous quote from the mt7621 datasheet regarding
the MAC registers (at 0x3000 for port 0, 0x3100 for port 1, etc.):

    2.4 Link Status

    You can find MAC control register put at 0x3500 for MAC 5, and
    0x3600 for MAC 6. You can change
    MAC ability at this register. We would suggest don’t use the
    register 0x3000 to 0x3400. It may not
    work.

I'm not sure if this only applies to something in between the mt7621 and
it's internal mt7530 or not.

^ permalink raw reply

* Re: [net/tls] Re: KMSAN: uninit-value in aesti_encrypt
From: Eric Biggers @ 2019-06-27 19:01 UTC (permalink / raw)
  To: John Fastabend
  Cc: Boris Pismenny, Aviad Yehezkel, Dave Watson, Daniel Borkmann,
	netdev, davem, glider, herbert, linux-crypto, linux-kernel,
	syzkaller-bugs, syzbot
In-Reply-To: <5d1508c79587a_e392b1ee39f65b45b@john-XPS-13-9370.notmuch>

On Thu, Jun 27, 2019 at 11:19:51AM -0700, John Fastabend wrote:
> Eric Biggers wrote:
> > [+TLS maintainers]
> > 
> > Very likely a net/tls bug, not a crypto bug.
> > 
> > Possibly a duplicate of other reports such as "KMSAN: uninit-value in gf128mul_4k_lle (3)"
> > 
> > See https://lore.kernel.org/netdev/20190625055019.GD17703@sol.localdomain/ for
> > the list of 17 other open syzbot bugs I've assigned to the TLS subsystem.  TLS
> > maintainers, when are you planning to look into these?
> > 
> > On Thu, Jun 27, 2019 at 09:37:05AM -0700, syzbot wrote:
> 
> I'm looking at this issue now. There is a series on bpf list now to address
> many of those 17 open issues but this is a separate issue. I can reproduce
> it locally so should have a fix soon.
> 

Okay, great!  However, just to clarify, the 17 syzbot bugs I assigned to TLS are
in addition to the 30 I assigned to BPF
(https://lore.kernel.org/lkml/20190624050114.GA30702@sol.localdomain/).
(Well, since I sent that it's actually up to 35 now.)

I do expect most of these are duplicates, so when you are fixing the bugs, it
would be really helpful (for everyone, including you in the future :-) ) if you
would include the corresponding Reported-by syzbot line for *every* syzbot
report you think is addressed, so they get closed.

- Eric

^ permalink raw reply

* Re: [PATCH] netfilter: Fix remainder of pseudo-header protocol 0
From: Pablo Neira Ayuso @ 2019-06-27 18:49 UTC (permalink / raw)
  To: zhe.he; +Cc: kadlec, fw, davem, netfilter-devel, coreteam, netdev,
	linux-kernel
In-Reply-To: <1561346258-272481-1-git-send-email-zhe.he@windriver.com>

On Mon, Jun 24, 2019 at 11:17:38AM +0800, zhe.he@windriver.com wrote:
> From: He Zhe <zhe.he@windriver.com>
> 
> Since v5.1-rc1, some types of packets do not get unreachable reply with the
> following iptables setting. Fox example,
> 
> $ iptables -A INPUT -p icmp --icmp-type 8 -j REJECT
> $ ping 127.0.0.1 -c 1
> PING 127.0.0.1 (127.0.0.1) 56(84) bytes of data.
> — 127.0.0.1 ping statistics —
> 1 packets transmitted, 0 received, 100% packet loss, time 0ms
> 
> We should have got the following reply from command line, but we did not.
> From 127.0.0.1 icmp_seq=1 Destination Port Unreachable
> 
> Yi Zhao reported it and narrowed it down to:
> 7fc38225363d ("netfilter: reject: skip csum verification for protocols that don't support it"),
> 
> This is because nf_ip_checksum still expects pseudo-header protocol type 0 for
> packets that are of neither TCP or UDP, and thus ICMP packets are mistakenly
> treated as TCP/UDP.
> 
> This patch corrects the conditions in nf_ip_checksum and all other places that
> still call it with protocol 0.

Looking at 7fc38225363dd8f19e667ad7c77b63bc4a5c065d, I wonder this can
be fixed while simplifying it...

I think nf_reject_verify_csum() is useless?

In your patch, now you explicitly check for IPPROTO_TCP and
IPPROTO_UDP to validate the checksum.

^ permalink raw reply

* Re: [RFC] longer netdev names proposal
From: Michal Kubecek @ 2019-06-27 18:39 UTC (permalink / raw)
  To: netdev
  Cc: Andrew Lunn, Stephen Hemminger, David Ahern, Jiri Pirko, davem,
	jakub.kicinski, sthemmin, mlxsw
In-Reply-To: <20190627183538.GI31189@lunn.ch>

On Thu, Jun 27, 2019 at 08:35:38PM +0200, Andrew Lunn wrote:
> On Thu, Jun 27, 2019 at 11:23:05AM -0700, Stephen Hemminger wrote:
> > On Thu, 27 Jun 2019 20:08:03 +0200 Michal Kubecek <mkubecek@suse.cz> wrote:
> > 
> > > It often feels as a deficiency that unlike block devices where we can
> > > keep one name and create multiple symlinks based on different naming
> > > schemes, network devices can have only one name. There are aliases but
> > > AFAIK they are only used (and can be only used) for SNMP. IMHO this
> > > limitation is part of the mess that left us with so-called "predictable
> > > names" which are in practice neither persistent nor predictable.
> > > 
> > > So perhaps we could introduce actual aliases (or altnames or whatever we
> > > would call them) for network devices that could be used to identify
> > > a network device whenever both kernel and userspace tool supports them.
> > > Old (and ancient) tools would have to use the one canonical name limited
> > > to current IFNAMSIZ, new tools would allow using any alias which could
> > > be longer.
> >  
> > That is already there in current network model.
> > # ip li set dev eno1 alias 'Onboard Ethernet'
> > # ip li show dev eno1
> > 2: eno1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state UP mode DEFAULT group default qlen 1000
> >     link/ether ac:1f:6b:74:38:c0 brd ff:ff:ff:ff:ff:ff
> >     alias Onboard Ethernet
> 
> $ ip li set dev enp3s0 alias "Onboard Ethernet"
> # ip link show "Onboard Ethernet"
> Device "Onboard Ethernet" does not exist.
> 
> So it does not really appear to be an alias, it is a label. To be
> truly useful, it needs to be more than a label, it needs to be a real
> alias which you can use.

That's exactly what I meant: to be really useful, one should be able to
use the alias(es) for setting device options, for adding routes, in
netfilter rules etc.

Michal

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox