* [PATCH net-next v2] net: bpf: make eBPF interpreter images read-only
From: Hannes Frederic Sowa @ 2014-09-02 20:53 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, Brad Spengler, Daniel Borkmann, Alexei Starovoitov,
Kees Cook
In-Reply-To: <aa5126f747594d768e0c518189a13e8674ad8ee5.1409690486.git.hannes@stressinduktion.org>
From: Daniel Borkmann <dborkman@redhat.com>
With eBPF getting more extended and exposure to user space is on it's way,
hardening the memory range the interpreter uses to steer its command flow
seems appropriate. This patch moves the to be interpreted bytecode to
read-only pages.
In case we execute a corrupted BPF interpreter image for some reason e.g.
caused by an attacker which got past a verifier stage, it would not only
provide arbitrary read/write memory access but arbitrary function calls
as well. After setting up the BPF interpreter image, its contents do not
change until destruction time, thus we can setup the image on immutable
made pages in order to mitigate modifications to that code. The idea
is derived from commit 314beb9bcabf ("x86: bpf_jit_comp: secure bpf jit
against spraying attacks").
This is possible because bpf_prog is not part of sk_filter anymore.
After setup bpf_prog cannot be altered during its life-time. This prevents
any modifications to the entire bpf_prog structure (incl. function/JIT
image pointer).
Every eBPF program (including classic BPF that are migrated) have to call
bpf_prog_select_runtime() to select either interpreter or a JIT image
as a last setup step, and they all are being freed via bpf_prog_free(),
including non-JIT. Therefore, we can easily integrate this into the
eBPF life-time, plus since we directly allocate a bpf_prog, we have no
performance penalty.
Tested with seccomp and test_bpf testsuite in JIT/non-JIT mode and manual
inspection of kernel_page_tables. Brad Spengler proposed the same idea
via Twitter during development of this patch.
Joint work with Hannes Frederic Sowa.
Suggested-by: Brad Spengler <spender@grsecurity.net>
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Cc: Alexei Starovoitov <ast@plumgrid.com>
Cc: Kees Cook <keescook@chromium.org>
---
v2) During proofreading I accidentally did not remove the old duplicate
paragraph from the changelog.
arch/arm/net/bpf_jit_32.c | 3 +-
arch/mips/net/bpf_jit.c | 3 +-
arch/powerpc/net/bpf_jit_comp.c | 3 +-
arch/s390/net/bpf_jit_comp.c | 2 +-
arch/sparc/net/bpf_jit_comp.c | 3 +-
arch/x86/net/bpf_jit_comp.c | 18 ++++------
include/linux/filter.h | 49 ++++++++++++++++++++++---
kernel/bpf/core.c | 80 +++++++++++++++++++++++++++++++++++++++--
kernel/seccomp.c | 7 ++--
lib/test_bpf.c | 2 +-
net/core/filter.c | 6 ++--
11 files changed, 144 insertions(+), 32 deletions(-)
diff --git a/arch/arm/net/bpf_jit_32.c b/arch/arm/net/bpf_jit_32.c
index a37b989..a76623b 100644
--- a/arch/arm/net/bpf_jit_32.c
+++ b/arch/arm/net/bpf_jit_32.c
@@ -930,5 +930,6 @@ void bpf_jit_free(struct bpf_prog *fp)
{
if (fp->jited)
module_free(NULL, fp->bpf_func);
- kfree(fp);
+
+ bpf_prog_unlock_free(fp);
}
diff --git a/arch/mips/net/bpf_jit.c b/arch/mips/net/bpf_jit.c
index 05a5661..cfa83cf 100644
--- a/arch/mips/net/bpf_jit.c
+++ b/arch/mips/net/bpf_jit.c
@@ -1427,5 +1427,6 @@ void bpf_jit_free(struct bpf_prog *fp)
{
if (fp->jited)
module_free(NULL, fp->bpf_func);
- kfree(fp);
+
+ bpf_prog_unlock_free(fp);
}
diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
index 3afa6f4..40c53ff 100644
--- a/arch/powerpc/net/bpf_jit_comp.c
+++ b/arch/powerpc/net/bpf_jit_comp.c
@@ -697,5 +697,6 @@ void bpf_jit_free(struct bpf_prog *fp)
{
if (fp->jited)
module_free(NULL, fp->bpf_func);
- kfree(fp);
+
+ bpf_prog_unlock_free(fp);
}
diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
index 61e45b7..f2833c5 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -887,5 +887,5 @@ void bpf_jit_free(struct bpf_prog *fp)
module_free(NULL, header);
free_filter:
- kfree(fp);
+ bpf_prog_unlock_free(fp);
}
diff --git a/arch/sparc/net/bpf_jit_comp.c b/arch/sparc/net/bpf_jit_comp.c
index 1f76c22..f7a736b 100644
--- a/arch/sparc/net/bpf_jit_comp.c
+++ b/arch/sparc/net/bpf_jit_comp.c
@@ -812,5 +812,6 @@ void bpf_jit_free(struct bpf_prog *fp)
{
if (fp->jited)
module_free(NULL, fp->bpf_func);
- kfree(fp);
+
+ bpf_prog_unlock_free(fp);
}
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index b08a98c..39ccfbb 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -972,23 +972,17 @@ out:
kfree(addrs);
}
-static void bpf_jit_free_deferred(struct work_struct *work)
+void bpf_jit_free(struct bpf_prog *fp)
{
- struct bpf_prog *fp = container_of(work, struct bpf_prog, work);
unsigned long addr = (unsigned long)fp->bpf_func & PAGE_MASK;
struct bpf_binary_header *header = (void *)addr;
+ if (!fp->jited)
+ goto free_filter;
+
set_memory_rw(addr, header->pages);
module_free(NULL, header);
- kfree(fp);
-}
-void bpf_jit_free(struct bpf_prog *fp)
-{
- if (fp->jited) {
- INIT_WORK(&fp->work, bpf_jit_free_deferred);
- schedule_work(&fp->work);
- } else {
- kfree(fp);
- }
+free_filter:
+ bpf_prog_unlock_free(fp);
}
diff --git a/include/linux/filter.h b/include/linux/filter.h
index a5227ab..c789945 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -9,6 +9,11 @@
#include <linux/skbuff.h>
#include <linux/workqueue.h>
#include <uapi/linux/filter.h>
+#include <asm/cacheflush.h>
+
+struct sk_buff;
+struct sock;
+struct seccomp_data;
/* Internally used and optimized filter representation with extended
* instruction set based on top of classic BPF.
@@ -320,20 +325,23 @@ struct sock_fprog_kern {
struct sock_filter *filter;
};
-struct sk_buff;
-struct sock;
-struct seccomp_data;
+struct bpf_work_struct {
+ struct bpf_prog *prog;
+ struct work_struct work;
+};
struct bpf_prog {
+ u32 pages; /* Number of allocated pages */
u32 jited:1, /* Is our filter JIT'ed? */
len:31; /* Number of filter blocks */
struct sock_fprog_kern *orig_prog; /* Original BPF program */
+ struct bpf_work_struct *work; /* Deferred free work struct */
unsigned int (*bpf_func)(const struct sk_buff *skb,
const struct bpf_insn *filter);
+ /* Instructions for interpreter */
union {
struct sock_filter insns[0];
struct bpf_insn insnsi[0];
- struct work_struct work;
};
};
@@ -353,6 +361,26 @@ static inline unsigned int bpf_prog_size(unsigned int proglen)
#define bpf_classic_proglen(fprog) (fprog->len * sizeof(fprog->filter[0]))
+#ifdef CONFIG_DEBUG_SET_MODULE_RONX
+static inline void bpf_prog_lock_ro(struct bpf_prog *fp)
+{
+ set_memory_ro((unsigned long)fp, fp->pages);
+}
+
+static inline void bpf_prog_unlock_ro(struct bpf_prog *fp)
+{
+ set_memory_rw((unsigned long)fp, fp->pages);
+}
+#else
+static inline void bpf_prog_lock_ro(struct bpf_prog *fp)
+{
+}
+
+static inline void bpf_prog_unlock_ro(struct bpf_prog *fp)
+{
+}
+#endif /* CONFIG_DEBUG_SET_MODULE_RONX */
+
int sk_filter(struct sock *sk, struct sk_buff *skb);
void bpf_prog_select_runtime(struct bpf_prog *fp);
@@ -361,6 +389,17 @@ void bpf_prog_free(struct bpf_prog *fp);
int bpf_convert_filter(struct sock_filter *prog, int len,
struct bpf_insn *new_prog, int *new_len);
+struct bpf_prog *bpf_prog_alloc(unsigned int size, gfp_t gfp_extra_flags);
+struct bpf_prog *bpf_prog_realloc(struct bpf_prog *fp_old, unsigned int size,
+ gfp_t gfp_extra_flags);
+void __bpf_prog_free(struct bpf_prog *fp);
+
+static inline void bpf_prog_unlock_free(struct bpf_prog *fp)
+{
+ bpf_prog_unlock_ro(fp);
+ __bpf_prog_free(fp);
+}
+
int bpf_prog_create(struct bpf_prog **pfp, struct sock_fprog_kern *fprog);
void bpf_prog_destroy(struct bpf_prog *fp);
@@ -450,7 +489,7 @@ static inline void bpf_jit_compile(struct bpf_prog *fp)
static inline void bpf_jit_free(struct bpf_prog *fp)
{
- kfree(fp);
+ bpf_prog_unlock_free(fp);
}
#endif /* CONFIG_BPF_JIT */
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 7f0dbcb..b54bb2c 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -22,6 +22,7 @@
*/
#include <linux/filter.h>
#include <linux/skbuff.h>
+#include <linux/vmalloc.h>
#include <asm/unaligned.h>
/* Registers */
@@ -63,6 +64,67 @@ void *bpf_internal_load_pointer_neg_helper(const struct sk_buff *skb, int k, uns
return NULL;
}
+struct bpf_prog *bpf_prog_alloc(unsigned int size, gfp_t gfp_extra_flags)
+{
+ gfp_t gfp_flags = GFP_KERNEL | __GFP_HIGHMEM | __GFP_ZERO |
+ gfp_extra_flags;
+ struct bpf_work_struct *ws;
+ struct bpf_prog *fp;
+
+ size = round_up(size, PAGE_SIZE);
+ fp = __vmalloc(size, gfp_flags, PAGE_KERNEL);
+ if (fp == NULL)
+ return NULL;
+
+ ws = kmalloc(sizeof(*ws), GFP_KERNEL | gfp_extra_flags);
+ if (ws == NULL) {
+ vfree(fp);
+ return NULL;
+ }
+
+ fp->pages = size / PAGE_SIZE;
+ fp->work = ws;
+
+ return fp;
+}
+EXPORT_SYMBOL_GPL(bpf_prog_alloc);
+
+struct bpf_prog *bpf_prog_realloc(struct bpf_prog *fp_old, unsigned int size,
+ gfp_t gfp_extra_flags)
+{
+ gfp_t gfp_flags = GFP_KERNEL | __GFP_HIGHMEM | __GFP_ZERO |
+ gfp_extra_flags;
+ struct bpf_prog *fp;
+
+ BUG_ON(fp_old == NULL);
+
+ size = round_up(size, PAGE_SIZE);
+ if (size <= fp_old->pages * PAGE_SIZE)
+ return fp_old;
+
+ fp = __vmalloc(size, gfp_flags, PAGE_KERNEL);
+ if (fp != NULL) {
+ memcpy(fp, fp_old, fp_old->pages * PAGE_SIZE);
+ fp->pages = size / PAGE_SIZE;
+
+ /* We keep fp->work from fp_old around in the new
+ * reallocated structure.
+ */
+ fp_old->work = NULL;
+ __bpf_prog_free(fp_old);
+ }
+
+ return fp;
+}
+EXPORT_SYMBOL_GPL(bpf_prog_realloc);
+
+void __bpf_prog_free(struct bpf_prog *fp)
+{
+ kfree(fp->work);
+ vfree(fp);
+}
+EXPORT_SYMBOL_GPL(__bpf_prog_free);
+
/* Base function for offset calculation. Needs to go into .text section,
* therefore keeping it non-static as well; will also be used by JITs
* anyway later on, so do not let the compiler omit it.
@@ -523,12 +585,26 @@ void bpf_prog_select_runtime(struct bpf_prog *fp)
/* Probe if internal BPF can be JITed */
bpf_int_jit_compile(fp);
+ /* Lock whole bpf_prog as read-only */
+ bpf_prog_lock_ro(fp);
}
EXPORT_SYMBOL_GPL(bpf_prog_select_runtime);
-/* free internal BPF program */
+static void bpf_prog_free_deferred(struct work_struct *work)
+{
+ struct bpf_work_struct *ws;
+
+ ws = container_of(work, struct bpf_work_struct, work);
+ bpf_jit_free(ws->prog);
+}
+
+/* Free internal BPF program */
void bpf_prog_free(struct bpf_prog *fp)
{
- bpf_jit_free(fp);
+ struct bpf_work_struct *ws = fp->work;
+
+ INIT_WORK(&ws->work, bpf_prog_free_deferred);
+ ws->prog = fp;
+ schedule_work(&ws->work);
}
EXPORT_SYMBOL_GPL(bpf_prog_free);
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index 44eb005..84922be 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -395,16 +395,15 @@ static struct seccomp_filter *seccomp_prepare_filter(struct sock_fprog *fprog)
if (!filter)
goto free_prog;
- filter->prog = kzalloc(bpf_prog_size(new_len),
- GFP_KERNEL|__GFP_NOWARN);
+ filter->prog = bpf_prog_alloc(bpf_prog_size(new_len), __GFP_NOWARN);
if (!filter->prog)
goto free_filter;
ret = bpf_convert_filter(fp, fprog->len, filter->prog->insnsi, &new_len);
if (ret)
goto free_filter_prog;
- kfree(fp);
+ kfree(fp);
atomic_set(&filter->usage, 1);
filter->prog->len = new_len;
@@ -413,7 +412,7 @@ static struct seccomp_filter *seccomp_prepare_filter(struct sock_fprog *fprog)
return filter;
free_filter_prog:
- kfree(filter->prog);
+ __bpf_prog_free(filter->prog);
free_filter:
kfree(filter);
free_prog:
diff --git a/lib/test_bpf.c b/lib/test_bpf.c
index 8c66c6a..9a67456 100644
--- a/lib/test_bpf.c
+++ b/lib/test_bpf.c
@@ -1836,7 +1836,7 @@ static struct bpf_prog *generate_filter(int which, int *err)
break;
case INTERNAL:
- fp = kzalloc(bpf_prog_size(flen), GFP_KERNEL);
+ fp = bpf_prog_alloc(bpf_prog_size(flen), 0);
if (fp == NULL) {
pr_cont("UNEXPECTED_FAIL no memory left\n");
*err = -ENOMEM;
diff --git a/net/core/filter.c b/net/core/filter.c
index d814b8a..37f8eb0 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -933,7 +933,7 @@ static struct bpf_prog *bpf_migrate_filter(struct bpf_prog *fp)
/* Expand fp for appending the new filter representation. */
old_fp = fp;
- fp = krealloc(old_fp, bpf_prog_size(new_len), GFP_KERNEL);
+ fp = bpf_prog_realloc(old_fp, bpf_prog_size(new_len), 0);
if (!fp) {
/* The old_fp is still around in case we couldn't
* allocate new memory, so uncharge on that one.
@@ -1013,7 +1013,7 @@ int bpf_prog_create(struct bpf_prog **pfp, struct sock_fprog_kern *fprog)
if (fprog->filter == NULL)
return -EINVAL;
- fp = kmalloc(bpf_prog_size(fprog->len), GFP_KERNEL);
+ fp = bpf_prog_alloc(bpf_prog_size(fprog->len), 0);
if (!fp)
return -ENOMEM;
@@ -1069,7 +1069,7 @@ int sk_attach_filter(struct sock_fprog *fprog, struct sock *sk)
if (fprog->filter == NULL)
return -EINVAL;
- prog = kmalloc(bpf_fsize, GFP_KERNEL);
+ prog = bpf_prog_alloc(bpf_fsize, 0);
if (!prog)
return -ENOMEM;
--
1.9.3
^ permalink raw reply related
* Re: [PATCH net] qeth: don't query for info if hardware not ready.
From: David Miller @ 2014-09-02 20:54 UTC (permalink / raw)
To: blaschka; +Cc: netdev, linux-s390
In-Reply-To: <20140902062017.GA23360@tuxmaker.boeblingen.de.ibm.com>
From: Frank Blaschka <blaschka@linux.vnet.ibm.com>
Date: Tue, 2 Sep 2014 08:20:17 +0200
> From: Eugene Crosser <Eugene.Crosser@ru.ibm.com>
>
> When qeth device is queried for ethtool data, hardware operation
> is performed to extract the necessary information from the card.
> If the card is not online at the moment (e.g. it is undergoing
> recovery), this operation produces undesired effects like
> temporarily freezing the system. This patch prevents execution
> of the hardware query operation when the card is not online.
> In such case, ioctl() operation returns error with errno ENODEV.
>
> Reviewed-by: Ursula Braun <ursula.braun@de.ibm.com>
> Signed-off-by: Eugene Crosser <Eugene.Crosser@ru.ibm.com>
> Signed-off-by: Frank Blaschka <blaschka@linux.vnet.ibm.com>
Applied.
^ permalink raw reply
* Re: [PATCH 5/9] net: Have xmit_list() signal more==true when appropriate.
From: David Miller @ 2014-09-02 20:55 UTC (permalink / raw)
To: brouer; +Cc: netdev
In-Reply-To: <20140902092538.7987db64@redhat.com>
From: Jesper Dangaard Brouer <brouer@redhat.com>
Date: Tue, 2 Sep 2014 09:25:38 +0200
> On Mon, 01 Sep 2014 15:24:59 -0700 (PDT) David Miller <davem@davemloft.net> wrote:
>
>> if (unlikely(!dev_xmit_complete(rc))) {
>> skb->next = next;
>> goto out;
>
> In the exit case (!dev_xmit_complete(rc)) is it,
> 1. the responsibility of the driver to "flush" the tail, or
> 2. do we depend on qdisc or softirq to be reactivated soonish?
I think the driver will have to do it, much like it must do so when
the queue is stopped.
^ permalink raw reply
* Re: [PATCH v2 net] bnx2x: Configure device endianity on driver load and reset endianity on removal.
From: David Miller @ 2014-09-02 20:55 UTC (permalink / raw)
To: manish.chopra; +Cc: netdev, Ariel.Elior, Yuval.Mintz
In-Reply-To: <1409646685-28320-1-git-send-email-manish.chopra@qlogic.com>
From: Manish Chopra <manish.chopra@qlogic.com>
Date: Tue, 2 Sep 2014 04:31:25 -0400
> Some hosts can be both little and big endian.
> In certain scenarios a big endian kernel can kexec a little endian kernel.
>
> This patch fixes this case from both ends:
> 1) Return endianity to original values on shutdown (in case little endian kernel boots after we shutdown).
> 2) Do not rely on HW reset values when loading driver in little endian kernel
> but configure them explicitly (in case previous kernel was big endian and did not reset the HW).
>
> Signed-off-by: Manish Chopra <manish.chopra@qlogic.com>
> Signed-off-by: Ariel Elior <Ariel.Elior@qlogic.com>
Applied.
^ permalink raw reply
* Re: [PATCH 0/7] pull request: Netfilter/IPVS fixes for net
From: David Miller @ 2014-09-02 20:57 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel, netdev
In-Reply-To: <1409652844-10289-1-git-send-email-pablo@netfilter.org>
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Tue, 2 Sep 2014 12:13:57 +0200
> The following patchset contains seven Netfilter fixes for your net
> tree, they are:
...
> You can pull these changes from:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git
Pulled, thanks a lot Pablo.
^ permalink raw reply
* Re: [PATCH net-next 2/2] sunvnet: Re-check for a VIO_DESC_READY data descriptor after short udelay()
From: David Miller @ 2014-09-02 20:59 UTC (permalink / raw)
To: sowmini.varadhan; +Cc: raghuram.kothakota, netdev
In-Reply-To: <54059BAA.1080307@oracle.com>
From: Sowmini Varadhan <sowmini.varadhan@oracle.com>
Date: Tue, 02 Sep 2014 06:27:54 -0400
> when there are no more packets coming, the extra 12 microsecond
> delay is not that big of a deal anyway.
How much other work could the cpu do in those 12 microseconds?
That's almost 3000 cpu cycles on a T4.
I understand your argument, and the fact that there are some existing
pieces of code doing this already, so I'll think about it some more.
Thanks.
^ permalink raw reply
* Re: [PATCH] netfilter: fix missing dependencies in NETFILTER_XT_TARGET_LOG
From: David Miller @ 2014-09-02 21:00 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel, netdev
In-Reply-To: <1409660777-14999-1-git-send-email-pablo@netfilter.org>
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Tue, 2 Sep 2014 14:26:17 +0200
> make defconfig reports:
>
> warning: (NETFILTER_XT_TARGET_LOG) selects NF_LOG_IPV6 which has unmet direct dependencies (NET && INET && IPV6 && NETFILTER && NETFILTER_ADVANCED)
>
> Fixes: d79a61d netfilter: NETFILTER_XT_TARGET_LOG selects NF_LOG_*
> Reported-by: kbuild test robot <fengguang.wu@intel.com>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] openvswitch: fix a memory leak
From: David Miller @ 2014-09-02 21:01 UTC (permalink / raw)
To: roy.qing.li; +Cc: netdev, dev, pshelar
In-Reply-To: <1409662348-2475-1-git-send-email-roy.qing.li@gmail.com>
From: roy.qing.li@gmail.com
Date: Tue, 2 Sep 2014 20:52:28 +0800
> From: Li RongQing <roy.qing.li@gmail.com>
>
> The user_skb maybe be leaked if the operation on it failed and codes
> skipped into the label "out:" without calling genlmsg_unicast.
>
> Cc: Pravin Shelar <pshelar@nicira.com>
> Signed-off-by: Li RongQing <roy.qing.li@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net] core: Don't attempt to load the "" driver.
From: David Miller @ 2014-09-02 21:03 UTC (permalink / raw)
To: David.Laight; +Cc: netdev
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D174856B9@AcuExch.aculab.com>
From: David Laight <David.Laight@ACULAB.COM>
Date: Tue, 2 Sep 2014 13:48:39 +0000
> Requests like 'ifconfig ""' cause dev_load(net, "") be called, since
> request_module("") return 0 (success) this generates the error message:
> "Loading kernel module for a network device with CAP_SYS_MODULE (deprecated). Use CAP_NET_ADMIN and alias netdev- instead."
>
> Since dev_load() doesn't have to work, just ignore such names.
Patch submissions require a signoff.
Please freshly resubmit this with an appropriate signoff, thanks.
^ permalink raw reply
* Re: [PATCH net] amd-xgbe: Fix initialization of the wrong spin lock
From: David Miller @ 2014-09-02 21:03 UTC (permalink / raw)
To: thomas.lendacky; +Cc: netdev
In-Reply-To: <20140902144022.10877.58098.stgit@tlendack-t1.amdoffice.net>
From: Tom Lendacky <thomas.lendacky@amd.com>
Date: Tue, 2 Sep 2014 09:40:22 -0500
> During allocation and initialization of the network driver structures,
> the wrong pointer is used to initialize a spin lock. Fix the spin lock
> initialization by using the proper pointer.
>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Applied, thanks.
^ permalink raw reply
* Re: [net-next PATCH 0/3] qdisc bulk dequeuing and utilizing delayed tailptr updates
From: David Miller @ 2014-09-02 21:05 UTC (permalink / raw)
To: brouer; +Cc: netdev, fw, hannes, dborkman
In-Reply-To: <20140902143254.1918.8419.stgit@dragon>
From: Jesper Dangaard Brouer <brouer@redhat.com>
Date: Tue, 02 Sep 2014 16:35:19 +0200
> Open questions:
>
> - For now set bulk limit to 8 packets, don't want to stress the driver
> avail ring_buffer space.
Maybe we can start out with something even lower, like 5.
> - Is the (!skb->next) check in dequeue necessary?
>
> - Do we need some checks in dev_requeue_skb() as we could be requeuing a SKB list?
The skb->next stuff is to handle GSO software segmented frames.
dev_requeue_skb() naturally handles any list of SKBs already,
GSO software segmented or multi-dequeue, it shouldn't care.
^ permalink raw reply
* Re: [net-next PATCH 1/3] qdisc: adjustments for API allowing skb list xmits
From: David Miller @ 2014-09-02 21:06 UTC (permalink / raw)
To: brouer; +Cc: netdev, fw, hannes, dborkman
In-Reply-To: <20140902143524.1918.70967.stgit@dragon>
From: Jesper Dangaard Brouer <brouer@redhat.com>
Date: Tue, 02 Sep 2014 16:35:33 +0200
> Minor adjustments for merge commit 53fda7f7f9e (Merge branch 'xmit_list')
> that allows us to work with a list of SKBs.
>
> Update code doc to function sch_direct_xmit().
>
> In handle_dev_cpu_collision() use kfree_skb_list() in error handling.
>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
Applied, thanks for catching the kfree_skb_list() issue.
^ permalink raw reply
* Re: [PATCH] 3c59x: avoid panic in boomerang_start_xmit when finding page address:
From: David Miller @ 2014-09-02 21:08 UTC (permalink / raw)
To: nhorman; +Cc: netdev, klassert
In-Reply-To: <1409676715-2291-1-git-send-email-nhorman@tuxdriver.com>
From: Neil Horman <nhorman@tuxdriver.com>
Date: Tue, 2 Sep 2014 12:51:55 -0400
> @@ -2177,10 +2177,10 @@ boomerang_start_xmit(struct sk_buff *skb, struct net_device *dev)
> skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
>
> vp->tx_ring[entry].frag[i+1].addr =
> - cpu_to_le32(pci_map_single(
> - VORTEX_PCI(vp),
> - (void *)skb_frag_address(frag),
> - skb_frag_size(frag), PCI_DMA_TODEVICE));
> + cpu_to_le32(skb_frag_dma_map(
> + &VORTEX_PCI(vp)->dev,
> + frag,
> + frag->page_offset, frag->size, PCI_DMA_TODEVICE));
You need to adjust the final argument to be "DMA_TO_DEVICE".
^ permalink raw reply
* Re: [RFC] net: ipv4: drop unicast encapsulated in L2 multicast
From: Stephen Hemminger @ 2014-09-02 21:16 UTC (permalink / raw)
To: Johannes Berg
Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, Johannes Berg
In-Reply-To: <1408641747-22199-1-git-send-email-johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>
On Thu, 21 Aug 2014 19:22:27 +0200
Johannes Berg <johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org> wrote:
> + /* RFC 1122 3.3.6:
> + *
> + * When a host sends a datagram to a link-layer broadcast address,
> + * the IP destination address MUST be a legal IP broadcast or IP
> + * multicast address.
> + *
> + * A host SHOULD silently discard a datagram that is received via
> + * a link-layer broadcast (see Section 2.4) but does not specify
> + * an IP multicast or broadcast destination address.
> + *
> + * We also do this for link-layer multicast.
> + */
> + if ((skb->pkt_type == PACKET_BROADCAST ||
> + skb->pkt_type == PACKET_MULTICAST) &&
> + res.type != RTN_BROADCAST)
> + goto e_inval;
> +
I think you need to all multicast packet but not broadcast.
The RFC does not specify that you should drop link-layer multicast to a unicast
address. There are several clustering products use that.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net-next] sock: consistent errqueue errors and signals
From: Hannes Frederic Sowa @ 2014-09-02 21:18 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: Network Development, David Miller
In-Reply-To: <CA+FuTScAYY1ZhQPwY=pqW9hXf3UYB70Dxq5+aPH4D4pB7QRPbg@mail.gmail.com>
Hello,
On Di, 2014-09-02 at 11:20 -0400, Willem de Bruijn wrote:
> > From my experience in IPv6 code, we only do sk->sk_err updates directly
> > in protocol error handling code. In case of UDP IPv6 errors for example
> > we now notify sk_error_report two times with this patch (before the
> > patch we did sk_data_ready (this is what you changed) and
> > sk_error_report).
>
> If the event is that an error is ready, is this not correct? The
> wake up key should be POLLERR in both cases. In implementation
> of sock_def_error_report and sock_def_readable, the difference
> otherwise seems slim. I haven't checked all sk_data_ready and
> sk_error_report implementations, though, so may have missed
> differences for specific protocols. If this is not as obviously a strict
> improvement as I thought, I'll just drop it.
I am not sure if waking up the socket two times has unforeseen
consequences, we do so e.g. in __udp6_lib_err if recverr is set.
> > I really wonder if setting sk->sk_err in this function is the right
> > thing to do.
>
> I agree, in that it is hard to verify that this does not overwrite
> an existing error. This patch only makes the behavior
> consistent between enqueue and dequeue, but perhaps a
> better way to achieve that is to change the dequeue side:
> remove the assignment to sk->sk_err there. If so, then all
> locations that currently check the state of sk->sk_err should
> be changed to also check the qlen of the error queue and
> if non-zero return the embedded error of the first skb. I'll
> take a look whether that is feasible without adding locks
> or atomics in the common path.
That would be great.
>
> > It also depends on socket state bits (e.g. np->recverr) if
> > the update happens. So we still cannot get rid of the protocol dependent
> > sk->sk_err updates.
> >
> > It looks like we have to check all error handling functions in the
> > protocols. Maybe timestamp code needs to adapt?
>
> Does the above sound okay, or did you mean something else?
Best thing would be to not keep the error status two times per socket.
Maybe it would make sense to always synchronize on the error queue and
don't check for sk->sk_err at all? It seems to get very hairy without
taking any locks though.
I even don't know what the semantics for sk_err should be. Should we
leave the oldest error in place until it got fetched? Then we could use
cmpxchg in slow path with 0 as the old value. They could easily become
unsynchronized if the user switches off recverr setsockopt. But I don't
think we need to handle that.
I think best effort should would be ok, too. Not having locked
instructions in fast path is much more important.
Thanks,
Hannes
^ permalink raw reply
* Re: [net-next PATCH 3/3] qdisc: sysctl to adjust bulk dequeue limit
From: Cong Wang @ 2014-09-02 21:20 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: David S. Miller, netdev, Florian Westphal, Hannes Frederic Sowa,
Daniel Borkmann
In-Reply-To: <20140902143553.1918.73632.stgit@dragon>
On Tue, Sep 2, 2014 at 7:36 AM, Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
> Allow userspace to adjust how many packet the qdisc is allowed to
> bulk dequeue.
>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
>
> ---
> Question should we allow this to be adjusted?
>
A sysctl is ugly and seems not fit well with Qdisc which always uses netlink,
so I think a netlink flag might be better if we can find a generic one.
Also, you forgot to document it.
^ permalink raw reply
* Re: [Patch net] ipv6: fix rtnl lock assertion failure in ipv6_sock_ac_join()
From: Hannes Frederic Sowa @ 2014-09-02 21:29 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev, David S. Miller, Sabrina Dubroca
In-Reply-To: <1409681239-4852-1-git-send-email-xiyou.wangcong@gmail.com>
Hi Cong,
On Di, 2014-09-02 at 11:07 -0700, Cong Wang wrote:
> @@ -198,7 +198,7 @@ void ipv6_sock_ac_close(struct sock *sk)
> spin_unlock_bh(&ipv6_sk_ac_lock);
>
> prev_index = 0;
> - rcu_read_lock();
> + rtnl_lock();
> while (pac) {
> struct ipv6_ac_socklist *next = pac->acl_next;
>
> @@ -211,7 +211,7 @@ void ipv6_sock_ac_close(struct sock *sk)
> sock_kfree_s(sk, pac, sizeof(*pac));
> pac = next;
> }
> - rcu_read_unlock();
> + rtnl_unlock();
> }
>
Nit:
You forgot to convert a dev_get_by_index_rcu to __dev_get_by_index in
ipv6_sock_ac_close.
Bye,
Hannes
^ permalink raw reply
* [PATCH net-next] dev_ioctl: remove dev_load() CAP_SYS_MODULE message
From: Daniel Borkmann @ 2014-09-02 21:30 UTC (permalink / raw)
To: davem; +Cc: marcel, stephen, netdev, Vasiliy Kulikov
Marcel reported to see the following message when autoloading
is being triggered when adding nlmon device:
Loading kernel module for a network device with
CAP_SYS_MODULE (deprecated). Use CAP_NET_ADMIN and alias
netdev-nlmon instead.
This false-positive happens despite with having correct
capabilities set, e.g. through issuing `ip link del dev nlmon`
more than once on a valid device with name nlmon, but Marcel
has also seen it on creation time when no nlmon module is
previously compiled-in or loaded as module and the device
name equals a link type name (e.g. nlmon, vxlan, team).
Stephen says:
The netdev module alias is a hold over from the past. For
normal devices, people used to create a alias eth0 to and
point it to the type of network device used, that was back
in the bad old ISA days before real discovery.
Also, the tunnels create module alias for the control device
and ip used to use this to autoload the tunnel device.
The message is bogus and should just be removed, I also see
it in a couple of other cases where tap devices are renamed
for other usese.
As mentioned in 8909c9ad8ff0 ("net: don't allow CAP_NET_ADMIN
to load non-netdev kernel modules"), we nevertheless still
might want to leave the old autoloading behaviour in place
as it could break old scripts, so for now, lets just remove
the log message as Stephen suggests.
Reference: http://thread.gmane.org/gmane.linux.kernel/1105168
Reported-by: Marcel Holtmann <marcel@holtmann.org>
Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: Vasiliy Kulikov <segoon@openwall.com>
---
(Sending to net-next as I don't think it's very urgent.)
net/core/dev_ioctl.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
index cf999e0..72e899a 100644
--- a/net/core/dev_ioctl.c
+++ b/net/core/dev_ioctl.c
@@ -365,11 +365,8 @@ void dev_load(struct net *net, const char *name)
no_module = !dev;
if (no_module && capable(CAP_NET_ADMIN))
no_module = request_module("netdev-%s", name);
- if (no_module && capable(CAP_SYS_MODULE)) {
- if (!request_module("%s", name))
- pr_warn("Loading kernel module for a network device with CAP_SYS_MODULE (deprecated). Use CAP_NET_ADMIN and alias netdev-%s instead.\n",
- name);
- }
+ if (no_module && capable(CAP_SYS_MODULE))
+ request_module("%s", name);
}
EXPORT_SYMBOL(dev_load);
--
1.7.11.7
^ permalink raw reply related
* Re: [PATCH net-next v2] net: bpf: make eBPF interpreter images read-only
From: Alexei Starovoitov @ 2014-09-02 21:31 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: Network Development, LKML, Brad Spengler, Daniel Borkmann,
Kees Cook
In-Reply-To: <2bf2e54282097642db88e2b596b06a9ac3742883.1409690849.git.hannes@stressinduktion.org>
On Tue, Sep 2, 2014 at 1:53 PM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
> From: Daniel Borkmann <dborkman@redhat.com>
>
> With eBPF getting more extended and exposure to user space is on it's way,
> hardening the memory range the interpreter uses to steer its command flow
> seems appropriate. This patch moves the to be interpreted bytecode to
> read-only pages.
...
> 11 files changed, 144 insertions(+), 32 deletions(-)
nice. quite short.
> +#ifdef CONFIG_DEBUG_SET_MODULE_RONX
> +static inline void bpf_prog_lock_ro(struct bpf_prog *fp)
> +{
> + set_memory_ro((unsigned long)fp, fp->pages);
since ronx are ifdef checked together,
would probably make sense to set nx too?
> +static inline void bpf_prog_unlock_ro(struct bpf_prog *fp)
> +{
> + set_memory_rw((unsigned long)fp, fp->pages);
why rw is needed?
since fp is allocated with vmalloc, vfree doesn't need
to touch the pages to free them, no?
^ permalink raw reply
* Re: [PATCH net-next v2] net: bpf: make eBPF interpreter images read-only
From: Hannes Frederic Sowa @ 2014-09-02 21:35 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Network Development, LKML, Brad Spengler, Daniel Borkmann,
Kees Cook
In-Reply-To: <CAMEtUuyoWELJ0Yf+=5sDKxFEfgsiQ3kMCK+Wn_bdJgS8qaH2og@mail.gmail.com>
On Tue, Sep 2, 2014, at 23:31, Alexei Starovoitov wrote:
> On Tue, Sep 2, 2014 at 1:53 PM, Hannes Frederic Sowa
> <hannes@stressinduktion.org> wrote:
> > From: Daniel Borkmann <dborkman@redhat.com>
> >
> > With eBPF getting more extended and exposure to user space is on it's way,
> > hardening the memory range the interpreter uses to steer its command flow
> > seems appropriate. This patch moves the to be interpreted bytecode to
> > read-only pages.
> ...
> > 11 files changed, 144 insertions(+), 32 deletions(-)
>
> nice. quite short.
>
> > +#ifdef CONFIG_DEBUG_SET_MODULE_RONX
> > +static inline void bpf_prog_lock_ro(struct bpf_prog *fp)
> > +{
> > + set_memory_ro((unsigned long)fp, fp->pages);
>
> since ronx are ifdef checked together,
> would probably make sense to set nx too?
NX bit is already set, because we didn't request page with
PAGE_KERNEL_EXEC.
E.g. in kernel_page_tables:
0xffffc90000a94000-0xffffc90000a96000 8K ro
GLB NX pte
> > +static inline void bpf_prog_unlock_ro(struct bpf_prog *fp)
> > +{
> > + set_memory_rw((unsigned long)fp, fp->pages);
>
> why rw is needed?
> since fp is allocated with vmalloc, vfree doesn't need
> to touch the pages to free them, no?
We will check that. It basically was copied from jit hardening code.
Maybe we can omit the call.
Thanks,
Hannes
^ permalink raw reply
* [PATCH 16/26] isdn: i4l: Remove ASYNC_CTS_FLOW
From: Peter Hurley @ 2014-09-02 21:39 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Jiri Slaby, One Thousand Gnomes, linux-serial, linux-kernel,
Peter Hurley, Karsten Keil, netdev
In-Reply-To: <1409693975-1028-1-git-send-email-peter@hurleysoftware.com>
ISDN4Linux always enables CTS flow control and does not use the
tty_port_cts_enabled() helper function; remove ASYNC_CTS_FLOW
state enable/disable.
cc: Karsten Keil <isdn@linux-pingi.de>
cc: <netdev@vger.kernel.org>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
drivers/isdn/i4l/isdn_tty.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/isdn/i4l/isdn_tty.c b/drivers/isdn/i4l/isdn_tty.c
index 3c5f249..bc91261 100644
--- a/drivers/isdn/i4l/isdn_tty.c
+++ b/drivers/isdn/i4l/isdn_tty.c
@@ -1043,11 +1043,6 @@ isdn_tty_change_speed(modem_info *info)
if (!(cflag & PARODD))
cval |= UART_LCR_EPAR;
- /* CTS flow control flag and modem status interrupts */
- if (cflag & CRTSCTS) {
- port->flags |= ASYNC_CTS_FLOW;
- } else
- port->flags &= ~ASYNC_CTS_FLOW;
if (cflag & CLOCAL)
port->flags &= ~ASYNC_CHECK_CD;
else {
--
2.1.0
^ permalink raw reply related
* Re: [PATCH net-next v2] net: bpf: make eBPF interpreter images read-only
From: Eric Dumazet @ 2014-09-02 21:40 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Hannes Frederic Sowa, Network Development, LKML, Brad Spengler,
Daniel Borkmann, Kees Cook
In-Reply-To: <CAMEtUuyoWELJ0Yf+=5sDKxFEfgsiQ3kMCK+Wn_bdJgS8qaH2og@mail.gmail.com>
On Tue, 2014-09-02 at 14:31 -0700, Alexei Starovoitov wrote:
> > +static inline void bpf_prog_unlock_ro(struct bpf_prog *fp)
> > +{
> > + set_memory_rw((unsigned long)fp, fp->pages);
>
> why rw is needed?
> since fp is allocated with vmalloc, vfree doesn't need
> to touch the pages to free them, no?
That assumes that vmalloc() do not have any debugging features, like
poisoning content before freeing, to catch some use after free.
Lets be clean and safe, and give back same memory permission we had
after vmalloc()
^ permalink raw reply
* Re: [PATCH net-next v2] net: bpf: make eBPF interpreter images read-only
From: Hannes Frederic Sowa @ 2014-09-02 21:43 UTC (permalink / raw)
To: Eric Dumazet, Alexei Starovoitov
Cc: Network Development, LKML, Brad Spengler, Daniel Borkmann,
Kees Cook
In-Reply-To: <1409694059.26422.18.camel@edumazet-glaptop2.roam.corp.google.com>
On Tue, Sep 2, 2014, at 23:40, Eric Dumazet wrote:
> On Tue, 2014-09-02 at 14:31 -0700, Alexei Starovoitov wrote:
>
> > > +static inline void bpf_prog_unlock_ro(struct bpf_prog *fp)
> > > +{
> > > + set_memory_rw((unsigned long)fp, fp->pages);
> >
> > why rw is needed?
> > since fp is allocated with vmalloc, vfree doesn't need
> > to touch the pages to free them, no?
>
> That assumes that vmalloc() do not have any debugging features, like
> poisoning content before freeing, to catch some use after free.
>
> Lets be clean and safe, and give back same memory permission we had
> after vmalloc()
Yes, I agree. I just went down the kmemleak codepaths and we certainly
don't want to cause issues in there if the implementation changes one
day.
Bye,
Hannes
^ permalink raw reply
* Re: [PATCH net-next v2] net: bpf: make eBPF interpreter images read-only
From: Daniel Borkmann @ 2014-09-02 21:47 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Hannes Frederic Sowa, Network Development, LKML, Brad Spengler,
Kees Cook
In-Reply-To: <CAMEtUuyoWELJ0Yf+=5sDKxFEfgsiQ3kMCK+Wn_bdJgS8qaH2og@mail.gmail.com>
On 09/02/2014 11:31 PM, Alexei Starovoitov wrote:
...
>> +#ifdef CONFIG_DEBUG_SET_MODULE_RONX
>> +static inline void bpf_prog_lock_ro(struct bpf_prog *fp)
>> +{
>> + set_memory_ro((unsigned long)fp, fp->pages);
>
> since ronx are ifdef checked together,
> would probably make sense to set nx too?
In case of JITs, for example, we request pages that are
PAGE_KERNEL_EXEC via module_alloc(), but here we only need
PAGE_KERNEL. At least on x86_64, PAGE_NX is then set already.
^ permalink raw reply
* [PATCH net-next v4 0/3] enic: Add support for rx_copybreak
From: Govindarajulu Varadarajan @ 2014-09-02 21:47 UTC (permalink / raw)
To: davem, netdev; +Cc: ssujith, ben, Govindarajulu Varadarajan
The following series implements rx_copybreak.
dma_map_single()/dma_unmap_single() is more expensive than alloc_skb & memcpy
for smaller packets. By doing this we can reuse the dma buff which is already
mapped. This is very useful when iommu is on. The default skb copybreak value
is 256.
When iommu is on, we can go much higher than 256. All the drivers that supports
rx_copybreak provides module parameter to change this value. Since module
parameter is the least preferred way for changing driver values, this series
adds ethtool support for setting rx_copybreak.
v4:
Validate tunable length in ethtool_get_tunable, not in driver implemented
function.
Loose tunable_ops array for each tunable type. Define one function and let the
driver use switch case for each type.
Use double underscore for data type in UAPI headers.
Use const qualifier where possible.
v3:
Add tunable namespace to ethtool. Use new ethtool cmd ETHTOOL_S/GTUNABLE to
set/get rx_copybreak from userspace.
v2:
Add new ethtool_cmd for DMA buffer parameters, instead of adding new members to
existing ethtool_ringparam.
Govindarajulu Varadarajan (3):
enic: implement rx_copybreak
ethtool: Add generic options for tunables
enic: Add tunable_ops support for rx_copybreak
drivers/net/ethernet/cisco/enic/enic.h | 1 +
drivers/net/ethernet/cisco/enic/enic_ethtool.c | 39 +++++++++++++
drivers/net/ethernet/cisco/enic/enic_main.c | 50 +++++++++++++++-
include/linux/ethtool.h | 4 ++
include/uapi/linux/ethtool.h | 28 +++++++++
net/core/ethtool.c | 81 ++++++++++++++++++++++++++
6 files changed, 200 insertions(+), 3 deletions(-)
--
2.1.0
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox