* Re: [PATCH] [net]ip_tunnel: the lack of vti_link_ops' dellink() cause kernel panic
From: Nicolas Dichtel @ 2014-11-18 17:23 UTC (permalink / raw)
To: Xin Long, network dev; +Cc: Steffen Klassert
In-Reply-To: <1416307059-32732-1-git-send-email-lucien.xin@gmail.com>
Le 18/11/2014 11:37, Xin Long a écrit :
> Now the vti_link_ops do not point the .dellink, rtnl_newlink will invoke
> the default function, unregister_netdevice_queue, which will cause the
> dev to unregister later. so when we delete a vti device, the net_device
> will be removed, but the tunnel still in the tunnel list. then, we add
> a new vti, in ip_tunnel_find():
>
> hlist_for_each_entry_rcu(t, head, hash_node) {
> if (local == t->parms.iph.saddr &&
> remote == t->parms.iph.daddr &&
> link == t->parms.link &&
> type == t->dev->type &&
> ip_tunnel_key_match(&t->parms, flags, key))
> break;
> }
>
> the dev of ip_tunnel *t may be null because of unregister_netdevice_queue
> motioned above. so the panic will happen:
>
> [ 3835.072977] IP: [<ffffffffa04103fd>] ip_tunnel_find+0x9d/0xc0 [ip_tunnel]
> [ 3835.073008] PGD b2c21067 PUD b7277067 PMD 0
> [ 3835.073008] Oops: 0000 [#1] SMP
> .....
> [ 3835.073008] Stack:
> [ 3835.073008] ffff8800b72d77f0 ffffffffa0411924 ffff8800bb956000 ffff8800b72d78e0
> [ 3835.073008] ffff8800b72d78a0 0000000000000000 ffffffffa040d100 ffff8800b72d7858
> [ 3835.073008] ffffffffa040b2e3 0000000000000000 0000000000000000 0000000000000000
> [ 3835.073008] Call Trace:
> [ 3835.073008] [<ffffffffa0411924>] ip_tunnel_newlink+0x64/0x160 [ip_tunnel]
> [ 3835.073008] [<ffffffffa040b2e3>] vti_newlink+0x43/0x70 [ip_vti]
> [ 3835.073008] [<ffffffff8150d4da>] rtnl_newlink+0x4fa/0x5f0
> [ 3835.073008] [<ffffffff812f68bb>] ? nla_strlcpy+0x5b/0x70
> [ 3835.073008] [<ffffffff81508fb0>] ? rtnl_link_ops_get+0x40/0x60
> [ 3835.073008] [<ffffffff8150d11f>] ? rtnl_newlink+0x13f/0x5f0
> [ 3835.073008] [<ffffffff81509cf4>] rtnetlink_rcv_msg+0xa4/0x270
> [ 3835.073008] [<ffffffff8126adf5>] ? sock_has_perm+0x75/0x90
> [ 3835.073008] [<ffffffff81509c50>] ? rtnetlink_rcv+0x30/0x30
> [ 3835.073008] [<ffffffff81529e39>] netlink_rcv_skb+0xa9/0xc0
> [ 3835.073008] [<ffffffff81509c48>] rtnetlink_rcv+0x28/0x30
> ....
>
> the reproduction can be like this:
>
> modprobe ip_vti
> ip link del ip_vti0 type vti
> ip link add ip_vti0 type vti
> rmmod ip_vti
>
> do that one or more time, kernel will panic.
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
A quick look at the ipv6 side seems to show that there is the same problem. Can
you provide the IPv6 patch too?
Note also that the maintainer of this module is Steffen Klassert, please don't
forget to CC him.
> ---
> net/ipv4/ip_vti.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/ipv4/ip_vti.c b/net/ipv4/ip_vti.c
> index 3e86101..1a7e979 100644
> --- a/net/ipv4/ip_vti.c
> +++ b/net/ipv4/ip_vti.c
> @@ -528,6 +528,7 @@ static struct rtnl_link_ops vti_link_ops __read_mostly = {
> .validate = vti_tunnel_validate,
> .newlink = vti_newlink,
> .changelink = vti_changelink,
> + .dellink = ip_tunnel_dellink,
Nitpicking: other lines into this struct uses tabs to align the '=', but
the one you add uses spaces.
Thank you,
Nicolas
^ permalink raw reply
* [PATCH v4 0/4] Add lightweight memory barriers for coherent memory access
From: Alexander Duyck @ 2014-11-18 17:28 UTC (permalink / raw)
To: linux-arch, netdev, linux-kernel
Cc: mathieu.desnoyers, peterz, benh, heiko.carstens, mingo, mikey,
linux, donald.c.skidmore, matthew.vick, geert, jeffrey.t.kirsher,
romieu, paulmck, nic_swsd, will.deacon, michael, tony.luck,
torvalds, oleg, schwidefsky, fweisbec, davem
These patches introduce two new primitives for synchronizing cache coherent
memory writes and reads. These two new primitives are:
coherent_rmb()
coherent_wmb()
The first patch cleans up some unnecessary overhead related to the
definition of read_barrier_depends, smp_read_barrier_depends, and comments
related to the barrier.
The second patch adds the primitives for the applicable architectures and
asm-generic.
The third patch adds the barriers to r8169 which turns out to be a good
example of where the new barriers might be useful as they have full
rmb()/wmb() barriers ordering accesses to the descriptors and the DescOwn
bit.
The fourth patch adds support for coherent_rmb() to the Intel fm10k, igb,
and ixgbe drivers. Testing with the ixgbe driver has shown a processing
time reduction of at least 7ns per 64B frame on a Core i7-4930K.
This patch series is essentially the v4 for:
v3: Add lightweight memory barriers fast_rmb() and fast_wmb()
v2: Introduce load_acquire() and store_release()
v1: Introduce read_acquire()
The key changes in this patch series versus the earlier patches are:
v4:
- Updated ARM barrier domains to outer shareable
- Renamed barriers coherent_rmb and coherent_wmb
- Added smp_lwsync for use in smp_load_acquire/smp_store_release
v3:
- Moved away from acquire()/store() and instead focused on barriers
- Added cleanup of read_barrier_depends
- Added change in r8169 to fix cur_tx/DescOwn ordering
- Simplified changes to just replacing/moving barriers in r8169
- Added update to documentation with code example
v2:
- Renamed read_acquire() to be consistent with smp_load_acquire()
- Changed barrier used to be consistent with smp_load_acquire()
- Updated PowerPC code to use __lwsync based on IBM article
- Added store_release() as this is a viable use case for drivers
- Added r8169 patch which is able to fully use primitives
- Added fm10k/igb/ixgbe patch which is able to test performance
---
Alexander Duyck (4):
arch: Cleanup read_barrier_depends() and comments
arch: Add lightweight memory barriers coherent_rmb() and coherent_wmb()
r8169: Use coherent_rmb() and coherent_wmb() for DescOwn checks
fm10k/igb/ixgbe: Use coherent_rmb on Rx descriptor reads
Documentation/memory-barriers.txt | 41 +++++++++++++++
arch/alpha/include/asm/barrier.h | 51 ++++++++++++++++++
arch/arm/include/asm/barrier.h | 4 +
arch/arm64/include/asm/barrier.h | 3 +
arch/blackfin/include/asm/barrier.h | 51 ++++++++++++++++++
arch/ia64/include/asm/barrier.h | 25 ++++-----
arch/metag/include/asm/barrier.h | 19 ++++---
arch/mips/include/asm/barrier.h | 61 ++--------------------
arch/powerpc/include/asm/barrier.h | 23 +++++---
arch/s390/include/asm/barrier.h | 7 ++-
arch/sparc/include/asm/barrier_64.h | 7 ++-
arch/x86/include/asm/barrier.h | 70 ++++---------------------
arch/x86/um/asm/barrier.h | 20 ++++---
drivers/net/ethernet/intel/fm10k/fm10k_main.c | 6 +-
drivers/net/ethernet/intel/igb/igb_main.c | 6 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 9 +--
drivers/net/ethernet/realtek/r8169.c | 29 ++++++++--
include/asm-generic/barrier.h | 8 +++
18 files changed, 259 insertions(+), 181 deletions(-)
--
^ permalink raw reply
* [PATCH v4 1/4] arch: Cleanup read_barrier_depends() and comments
From: Alexander Duyck @ 2014-11-18 17:29 UTC (permalink / raw)
To: linux-arch, netdev, linux-kernel
Cc: mathieu.desnoyers, peterz, benh, heiko.carstens, mingo, mikey,
linux, donald.c.skidmore, matthew.vick, geert, jeffrey.t.kirsher,
romieu, paulmck, nic_swsd, will.deacon, michael, tony.luck,
torvalds, oleg, schwidefsky, fweisbec, davem
In-Reply-To: <20141118172644.26303.37688.stgit@ahduyck-server>
This patch is meant to cleanup the handling of read_barrier_depends and
smp_read_barrier_depends. In multiple spots in the kernel headers
read_barrier_depends is defined as "do {} while (0)", however we then go
into the SMP vs non-SMP sections and have the SMP version reference
read_barrier_depends, and the non-SMP define it as yet another empty
do/while.
With this commit I went through and cleaned out the duplicate definitions
and reduced the number of definitions down to 2 per header. In addition I
moved the 50 line comments for the macro from the x86 and mips headers that
defined it as an empty do/while to those that were actually defining the
macro, alpha and blackfin.
Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
arch/alpha/include/asm/barrier.h | 51 ++++++++++++++++++++++++++++++
arch/blackfin/include/asm/barrier.h | 51 ++++++++++++++++++++++++++++++
arch/ia64/include/asm/barrier.h | 22 +++++--------
arch/metag/include/asm/barrier.h | 7 ++--
arch/mips/include/asm/barrier.h | 52 -------------------------------
arch/powerpc/include/asm/barrier.h | 6 ++--
arch/s390/include/asm/barrier.h | 5 ++-
arch/sparc/include/asm/barrier_64.h | 4 +-
arch/x86/include/asm/barrier.h | 59 ++---------------------------------
arch/x86/um/asm/barrier.h | 7 ++--
10 files changed, 129 insertions(+), 135 deletions(-)
diff --git a/arch/alpha/include/asm/barrier.h b/arch/alpha/include/asm/barrier.h
index 3832bdb..77516c8 100644
--- a/arch/alpha/include/asm/barrier.h
+++ b/arch/alpha/include/asm/barrier.h
@@ -7,6 +7,57 @@
#define rmb() __asm__ __volatile__("mb": : :"memory")
#define wmb() __asm__ __volatile__("wmb": : :"memory")
+/**
+ * read_barrier_depends - Flush all pending reads that subsequents reads
+ * depend on.
+ *
+ * No data-dependent reads from memory-like regions are ever reordered
+ * over this barrier. All reads preceding this primitive are guaranteed
+ * to access memory (but not necessarily other CPUs' caches) before any
+ * reads following this primitive that depend on the data return by
+ * any of the preceding reads. This primitive is much lighter weight than
+ * rmb() on most CPUs, and is never heavier weight than is
+ * rmb().
+ *
+ * These ordering constraints are respected by both the local CPU
+ * and the compiler.
+ *
+ * Ordering is not guaranteed by anything other than these primitives,
+ * not even by data dependencies. See the documentation for
+ * memory_barrier() for examples and URLs to more information.
+ *
+ * For example, the following code would force ordering (the initial
+ * value of "a" is zero, "b" is one, and "p" is "&a"):
+ *
+ * <programlisting>
+ * CPU 0 CPU 1
+ *
+ * b = 2;
+ * memory_barrier();
+ * p = &b; q = p;
+ * read_barrier_depends();
+ * d = *q;
+ * </programlisting>
+ *
+ * because the read of "*q" depends on the read of "p" and these
+ * two reads are separated by a read_barrier_depends(). However,
+ * the following code, with the same initial values for "a" and "b":
+ *
+ * <programlisting>
+ * CPU 0 CPU 1
+ *
+ * a = 2;
+ * memory_barrier();
+ * b = 3; y = b;
+ * read_barrier_depends();
+ * x = a;
+ * </programlisting>
+ *
+ * does not enforce ordering, since there is no data dependency between
+ * the read of "a" and the read of "b". Therefore, on some CPUs, such
+ * as Alpha, "y" could be set to 3 and "x" to 0. Use rmb()
+ * in cases like this where there are no data dependencies.
+ */
#define read_barrier_depends() __asm__ __volatile__("mb": : :"memory")
#ifdef CONFIG_SMP
diff --git a/arch/blackfin/include/asm/barrier.h b/arch/blackfin/include/asm/barrier.h
index 4200068..dfb66fe 100644
--- a/arch/blackfin/include/asm/barrier.h
+++ b/arch/blackfin/include/asm/barrier.h
@@ -22,6 +22,57 @@
# define mb() do { barrier(); smp_check_barrier(); smp_mark_barrier(); } while (0)
# define rmb() do { barrier(); smp_check_barrier(); } while (0)
# define wmb() do { barrier(); smp_mark_barrier(); } while (0)
+/*
+ * read_barrier_depends - Flush all pending reads that subsequents reads
+ * depend on.
+ *
+ * No data-dependent reads from memory-like regions are ever reordered
+ * over this barrier. All reads preceding this primitive are guaranteed
+ * to access memory (but not necessarily other CPUs' caches) before any
+ * reads following this primitive that depend on the data return by
+ * any of the preceding reads. This primitive is much lighter weight than
+ * rmb() on most CPUs, and is never heavier weight than is
+ * rmb().
+ *
+ * These ordering constraints are respected by both the local CPU
+ * and the compiler.
+ *
+ * Ordering is not guaranteed by anything other than these primitives,
+ * not even by data dependencies. See the documentation for
+ * memory_barrier() for examples and URLs to more information.
+ *
+ * For example, the following code would force ordering (the initial
+ * value of "a" is zero, "b" is one, and "p" is "&a"):
+ *
+ * <programlisting>
+ * CPU 0 CPU 1
+ *
+ * b = 2;
+ * memory_barrier();
+ * p = &b; q = p;
+ * read_barrier_depends();
+ * d = *q;
+ * </programlisting>
+ *
+ * because the read of "*q" depends on the read of "p" and these
+ * two reads are separated by a read_barrier_depends(). However,
+ * the following code, with the same initial values for "a" and "b":
+ *
+ * <programlisting>
+ * CPU 0 CPU 1
+ *
+ * a = 2;
+ * memory_barrier();
+ * b = 3; y = b;
+ * read_barrier_depends();
+ * x = a;
+ * </programlisting>
+ *
+ * does not enforce ordering, since there is no data dependency between
+ * the read of "a" and the read of "b". Therefore, on some CPUs, such
+ * as Alpha, "y" could be set to 3 and "x" to 0. Use rmb()
+ * in cases like this where there are no data dependencies.
+ */
# define read_barrier_depends() do { barrier(); smp_check_barrier(); } while (0)
#endif
diff --git a/arch/ia64/include/asm/barrier.h b/arch/ia64/include/asm/barrier.h
index a48957c..e8fffb0 100644
--- a/arch/ia64/include/asm/barrier.h
+++ b/arch/ia64/include/asm/barrier.h
@@ -35,26 +35,22 @@
* it's (presumably) much slower than mf and (b) mf.a is supported for
* sequential memory pages only.
*/
-#define mb() ia64_mf()
-#define rmb() mb()
-#define wmb() mb()
-#define read_barrier_depends() do { } while(0)
+#define mb() ia64_mf()
+#define rmb() mb()
+#define wmb() mb()
#ifdef CONFIG_SMP
# define smp_mb() mb()
-# define smp_rmb() rmb()
-# define smp_wmb() wmb()
-# define smp_read_barrier_depends() read_barrier_depends()
-
#else
-
# define smp_mb() barrier()
-# define smp_rmb() barrier()
-# define smp_wmb() barrier()
-# define smp_read_barrier_depends() do { } while(0)
-
#endif
+#define smp_rmb() smp_mb()
+#define smp_wmb() smp_mb()
+
+#define read_barrier_depends() do { } while (0)
+#define smp_read_barrier_depends() do { } while (0)
+
#define smp_mb__before_atomic() barrier()
#define smp_mb__after_atomic() barrier()
diff --git a/arch/metag/include/asm/barrier.h b/arch/metag/include/asm/barrier.h
index c7591e8..6d8b8c9 100644
--- a/arch/metag/include/asm/barrier.h
+++ b/arch/metag/include/asm/barrier.h
@@ -47,8 +47,6 @@ static inline void wmb(void)
wr_fence();
}
-#define read_barrier_depends() do { } while (0)
-
#ifndef CONFIG_SMP
#define fence() do { } while (0)
#define smp_mb() barrier()
@@ -82,7 +80,10 @@ static inline void fence(void)
#define smp_wmb() barrier()
#endif
#endif
-#define smp_read_barrier_depends() do { } while (0)
+
+#define read_barrier_depends() do { } while (0)
+#define smp_read_barrier_depends() do { } while (0)
+
#define set_mb(var, value) do { var = value; smp_mb(); } while (0)
#define smp_store_release(p, v) \
diff --git a/arch/mips/include/asm/barrier.h b/arch/mips/include/asm/barrier.h
index d0101dd..3d69aa8 100644
--- a/arch/mips/include/asm/barrier.h
+++ b/arch/mips/include/asm/barrier.h
@@ -10,58 +10,6 @@
#include <asm/addrspace.h>
-/*
- * read_barrier_depends - Flush all pending reads that subsequents reads
- * depend on.
- *
- * No data-dependent reads from memory-like regions are ever reordered
- * over this barrier. All reads preceding this primitive are guaranteed
- * to access memory (but not necessarily other CPUs' caches) before any
- * reads following this primitive that depend on the data return by
- * any of the preceding reads. This primitive is much lighter weight than
- * rmb() on most CPUs, and is never heavier weight than is
- * rmb().
- *
- * These ordering constraints are respected by both the local CPU
- * and the compiler.
- *
- * Ordering is not guaranteed by anything other than these primitives,
- * not even by data dependencies. See the documentation for
- * memory_barrier() for examples and URLs to more information.
- *
- * For example, the following code would force ordering (the initial
- * value of "a" is zero, "b" is one, and "p" is "&a"):
- *
- * <programlisting>
- * CPU 0 CPU 1
- *
- * b = 2;
- * memory_barrier();
- * p = &b; q = p;
- * read_barrier_depends();
- * d = *q;
- * </programlisting>
- *
- * because the read of "*q" depends on the read of "p" and these
- * two reads are separated by a read_barrier_depends(). However,
- * the following code, with the same initial values for "a" and "b":
- *
- * <programlisting>
- * CPU 0 CPU 1
- *
- * a = 2;
- * memory_barrier();
- * b = 3; y = b;
- * read_barrier_depends();
- * x = a;
- * </programlisting>
- *
- * does not enforce ordering, since there is no data dependency between
- * the read of "a" and the read of "b". Therefore, on some CPUs, such
- * as Alpha, "y" could be set to 3 and "x" to 0. Use rmb()
- * in cases like this where there are no data dependencies.
- */
-
#define read_barrier_depends() do { } while(0)
#define smp_read_barrier_depends() do { } while(0)
diff --git a/arch/powerpc/include/asm/barrier.h b/arch/powerpc/include/asm/barrier.h
index bab79a1..cb6d66c 100644
--- a/arch/powerpc/include/asm/barrier.h
+++ b/arch/powerpc/include/asm/barrier.h
@@ -33,7 +33,6 @@
#define mb() __asm__ __volatile__ ("sync" : : : "memory")
#define rmb() __asm__ __volatile__ ("sync" : : : "memory")
#define wmb() __asm__ __volatile__ ("sync" : : : "memory")
-#define read_barrier_depends() do { } while(0)
#define set_mb(var, value) do { var = value; mb(); } while (0)
@@ -50,16 +49,17 @@
#define smp_mb() mb()
#define smp_rmb() __lwsync()
#define smp_wmb() __asm__ __volatile__ (stringify_in_c(SMPWMB) : : :"memory")
-#define smp_read_barrier_depends() read_barrier_depends()
#else
#define __lwsync() barrier()
#define smp_mb() barrier()
#define smp_rmb() barrier()
#define smp_wmb() barrier()
-#define smp_read_barrier_depends() do { } while(0)
#endif /* CONFIG_SMP */
+#define read_barrier_depends() do { } while (0)
+#define smp_read_barrier_depends() do { } while (0)
+
/*
* This is a barrier which prevents following instructions from being
* started until the value of the argument x is known. For example, if
diff --git a/arch/s390/include/asm/barrier.h b/arch/s390/include/asm/barrier.h
index b5dce65..33d191d 100644
--- a/arch/s390/include/asm/barrier.h
+++ b/arch/s390/include/asm/barrier.h
@@ -24,11 +24,12 @@
#define rmb() mb()
#define wmb() mb()
-#define read_barrier_depends() do { } while(0)
#define smp_mb() mb()
#define smp_rmb() rmb()
#define smp_wmb() wmb()
-#define smp_read_barrier_depends() read_barrier_depends()
+
+#define read_barrier_depends() do { } while (0)
+#define smp_read_barrier_depends() do { } while (0)
#define smp_mb__before_atomic() smp_mb()
#define smp_mb__after_atomic() smp_mb()
diff --git a/arch/sparc/include/asm/barrier_64.h b/arch/sparc/include/asm/barrier_64.h
index 305dcc3..6c974c0 100644
--- a/arch/sparc/include/asm/barrier_64.h
+++ b/arch/sparc/include/asm/barrier_64.h
@@ -37,7 +37,6 @@ do { __asm__ __volatile__("ba,pt %%xcc, 1f\n\t" \
#define rmb() __asm__ __volatile__("":::"memory")
#define wmb() __asm__ __volatile__("":::"memory")
-#define read_barrier_depends() do { } while(0)
#define set_mb(__var, __value) \
do { __var = __value; membar_safe("#StoreLoad"); } while(0)
@@ -51,7 +50,8 @@ do { __asm__ __volatile__("ba,pt %%xcc, 1f\n\t" \
#define smp_wmb() __asm__ __volatile__("":::"memory")
#endif
-#define smp_read_barrier_depends() do { } while(0)
+#define read_barrier_depends() do { } while (0)
+#define smp_read_barrier_depends() do { } while (0)
#define smp_store_release(p, v) \
do { \
diff --git a/arch/x86/include/asm/barrier.h b/arch/x86/include/asm/barrier.h
index 0f4460b..5238000 100644
--- a/arch/x86/include/asm/barrier.h
+++ b/arch/x86/include/asm/barrier.h
@@ -24,60 +24,6 @@
#define wmb() asm volatile("sfence" ::: "memory")
#endif
-/**
- * read_barrier_depends - Flush all pending reads that subsequents reads
- * depend on.
- *
- * No data-dependent reads from memory-like regions are ever reordered
- * over this barrier. All reads preceding this primitive are guaranteed
- * to access memory (but not necessarily other CPUs' caches) before any
- * reads following this primitive that depend on the data return by
- * any of the preceding reads. This primitive is much lighter weight than
- * rmb() on most CPUs, and is never heavier weight than is
- * rmb().
- *
- * These ordering constraints are respected by both the local CPU
- * and the compiler.
- *
- * Ordering is not guaranteed by anything other than these primitives,
- * not even by data dependencies. See the documentation for
- * memory_barrier() for examples and URLs to more information.
- *
- * For example, the following code would force ordering (the initial
- * value of "a" is zero, "b" is one, and "p" is "&a"):
- *
- * <programlisting>
- * CPU 0 CPU 1
- *
- * b = 2;
- * memory_barrier();
- * p = &b; q = p;
- * read_barrier_depends();
- * d = *q;
- * </programlisting>
- *
- * because the read of "*q" depends on the read of "p" and these
- * two reads are separated by a read_barrier_depends(). However,
- * the following code, with the same initial values for "a" and "b":
- *
- * <programlisting>
- * CPU 0 CPU 1
- *
- * a = 2;
- * memory_barrier();
- * b = 3; y = b;
- * read_barrier_depends();
- * x = a;
- * </programlisting>
- *
- * does not enforce ordering, since there is no data dependency between
- * the read of "a" and the read of "b". Therefore, on some CPUs, such
- * as Alpha, "y" could be set to 3 and "x" to 0. Use rmb()
- * in cases like this where there are no data dependencies.
- **/
-
-#define read_barrier_depends() do { } while (0)
-
#ifdef CONFIG_SMP
#define smp_mb() mb()
#ifdef CONFIG_X86_PPRO_FENCE
@@ -86,16 +32,17 @@
# define smp_rmb() barrier()
#endif
#define smp_wmb() barrier()
-#define smp_read_barrier_depends() read_barrier_depends()
#define set_mb(var, value) do { (void)xchg(&var, value); } while (0)
#else /* !SMP */
#define smp_mb() barrier()
#define smp_rmb() barrier()
#define smp_wmb() barrier()
-#define smp_read_barrier_depends() do { } while (0)
#define set_mb(var, value) do { var = value; barrier(); } while (0)
#endif /* SMP */
+#define read_barrier_depends() do { } while (0)
+#define smp_read_barrier_depends() do { } while (0)
+
#if defined(CONFIG_X86_PPRO_FENCE)
/*
diff --git a/arch/x86/um/asm/barrier.h b/arch/x86/um/asm/barrier.h
index cc04e67..d6511d9 100644
--- a/arch/x86/um/asm/barrier.h
+++ b/arch/x86/um/asm/barrier.h
@@ -29,8 +29,6 @@
#endif /* CONFIG_X86_32 */
-#define read_barrier_depends() do { } while (0)
-
#ifdef CONFIG_SMP
#define smp_mb() mb()
@@ -42,7 +40,6 @@
#define smp_wmb() barrier()
-#define smp_read_barrier_depends() read_barrier_depends()
#define set_mb(var, value) do { (void)xchg(&var, value); } while (0)
#else /* CONFIG_SMP */
@@ -50,11 +47,13 @@
#define smp_mb() barrier()
#define smp_rmb() barrier()
#define smp_wmb() barrier()
-#define smp_read_barrier_depends() do { } while (0)
#define set_mb(var, value) do { var = value; barrier(); } while (0)
#endif /* CONFIG_SMP */
+#define read_barrier_depends() do { } while (0)
+#define smp_read_barrier_depends() do { } while (0)
+
/*
* Stop RDTSC speculation. This is needed when you need to use RDTSC
* (or get_cycles or vread that possibly accesses the TSC) in a defined
^ permalink raw reply related
* [PATCH v4 2/4] arch: Add lightweight memory barriers coherent_rmb() and coherent_wmb()
From: Alexander Duyck @ 2014-11-18 17:29 UTC (permalink / raw)
To: linux-arch, netdev, linux-kernel
Cc: mathieu.desnoyers, peterz, benh, heiko.carstens, mingo, mikey,
linux, donald.c.skidmore, matthew.vick, geert, jeffrey.t.kirsher,
romieu, paulmck, nic_swsd, will.deacon, michael, tony.luck,
torvalds, oleg, schwidefsky, fweisbec, davem
In-Reply-To: <20141118172644.26303.37688.stgit@ahduyck-server>
There are a number of situations where the mandatory barriers rmb() and
wmb() are used to order memory/memory operations in the device drivers
and those barriers are much heavier than they actually need to be. For
example in the case of PowerPC wmb() calls the heavy-weight sync
instruction when for coherent memory operations all that is really needed
is an lsync or eieio instruction.
This commit adds a coherent only version of the mandatory memory barriers
rmb() and wmb(). In most cases this should result in the barrier being the
same as the SMP barriers for the SMP case, however in some cases we use a
barrier that is somewhere in between rmb() and smp_rmb(). For example on
ARM the rmb barriers break down as follows:
Barrier Call Explanation
-------------- -------- ----------------------------------
rmb() dsb() Data synchronization barrier - system
coherent_rmb() dmb(osh) data memory barrier - outer sharable
smp_rmb() dmb(ish) data memory barrier - inner sharable
These new barriers are not as safe as the standard rmb() and wmb().
Specifically they do not guarantee ordering between coherent and incoherent
memories. The primary use case for these would be to enforce ordering of
reads and writes when accessing coherent memory that is shared between the
CPU and a device.
It may also be noted that there is no coherent_mb(). Most architectures
don't provide a good mechanism for performing a coherent only full barrier
without resorting to the same mechanism used in mb(). As such there isn't
much to be gained in trying to define such a function.
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
Cc: Michael Ellerman <michael@ellerman.id.au>
Cc: Michael Neuling <mikey@neuling.org>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: David Miller <davem@davemloft.net>
Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
Documentation/memory-barriers.txt | 41 +++++++++++++++++++++++++++++++++++
arch/arm/include/asm/barrier.h | 4 +++
arch/arm64/include/asm/barrier.h | 3 +++
arch/ia64/include/asm/barrier.h | 3 +++
arch/metag/include/asm/barrier.h | 14 ++++++------
arch/mips/include/asm/barrier.h | 9 ++++----
arch/powerpc/include/asm/barrier.h | 17 +++++++++------
arch/s390/include/asm/barrier.h | 2 ++
arch/sparc/include/asm/barrier_64.h | 3 +++
arch/x86/include/asm/barrier.h | 11 ++++++---
arch/x86/um/asm/barrier.h | 13 ++++++-----
include/asm-generic/barrier.h | 8 +++++++
12 files changed, 100 insertions(+), 28 deletions(-)
diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt
index 22a969c..d86cdc2 100644
--- a/Documentation/memory-barriers.txt
+++ b/Documentation/memory-barriers.txt
@@ -1615,6 +1615,47 @@ There are some more advanced barrier functions:
operations" subsection for information on where to use these.
+ (*) coherent_wmb();
+ (*) coherent_rmb();
+
+ These are for use with memory based device I/O to guarantee the ordering
+ of cache coherent writes or reads with respect to other writes or reads
+ to cache coherent memory.
+
+ For example, consider a device driver that shares memory with a device
+ and uses a descriptor status value to indicate if the descriptor belongs
+ to the device or the CPU, and a doorbell to notify it when new
+ descriptors are available:
+
+ if (desc->status != DEVICE_OWN) {
+ /* do not read data until we own descriptor */
+ coherent_rmb();
+
+ /* read/modify data */
+ read_data = desc->data;
+ desc->data = write_data;
+
+ /* flush modifications before status update */
+ coherent_wmb();
+
+ /* assign ownership */
+ desc->status = DEVICE_OWN;
+
+ /* force memory to sync before notifying device via MMIO */
+ wmb();
+
+ /* notify device of new descriptors */
+ writel(DESC_NOTIFY, doorbell);
+ }
+
+ The coherent_rmb() allows us guarantee the device has released ownership
+ before we read the data from the descriptor, and he coherent_wmb() allows
+ us to guarantee the data is written to the descriptor before the device
+ can see it now has ownership. The wmb() is needed to guarantee that the
+ cache coherent memory writes have completed before attempting a write to
+ the cache incoherent MMIO region.
+
+
MMIO WRITE BARRIER
------------------
diff --git a/arch/arm/include/asm/barrier.h b/arch/arm/include/asm/barrier.h
index c6a3e73..8d33c9e 100644
--- a/arch/arm/include/asm/barrier.h
+++ b/arch/arm/include/asm/barrier.h
@@ -43,10 +43,14 @@
#define mb() do { dsb(); outer_sync(); } while (0)
#define rmb() dsb()
#define wmb() do { dsb(st); outer_sync(); } while (0)
+#define coherent_rmb() dmb(osh)
+#define coherent_wmb() dmb(oshst)
#else
#define mb() barrier()
#define rmb() barrier()
#define wmb() barrier()
+#define coherent_rmb() barrier()
+#define coherent_wmb() barrier()
#endif
#ifndef CONFIG_SMP
diff --git a/arch/arm64/include/asm/barrier.h b/arch/arm64/include/asm/barrier.h
index 6389d60..d250ac4 100644
--- a/arch/arm64/include/asm/barrier.h
+++ b/arch/arm64/include/asm/barrier.h
@@ -32,6 +32,9 @@
#define rmb() dsb(ld)
#define wmb() dsb(st)
+#define coherent_rmb() dmb(oshld)
+#define coherent_wmb() dmb(oshst)
+
#ifndef CONFIG_SMP
#define smp_mb() barrier()
#define smp_rmb() barrier()
diff --git a/arch/ia64/include/asm/barrier.h b/arch/ia64/include/asm/barrier.h
index e8fffb0..ec4e652 100644
--- a/arch/ia64/include/asm/barrier.h
+++ b/arch/ia64/include/asm/barrier.h
@@ -39,6 +39,9 @@
#define rmb() mb()
#define wmb() mb()
+#define coherent_rmb() mb()
+#define coherent_wmb() mb()
+
#ifdef CONFIG_SMP
# define smp_mb() mb()
#else
diff --git a/arch/metag/include/asm/barrier.h b/arch/metag/include/asm/barrier.h
index 6d8b8c9..56999f3 100644
--- a/arch/metag/include/asm/barrier.h
+++ b/arch/metag/include/asm/barrier.h
@@ -4,8 +4,6 @@
#include <asm/metag_mem.h>
#define nop() asm volatile ("NOP")
-#define mb() wmb()
-#define rmb() barrier()
#ifdef CONFIG_METAG_META21
@@ -41,11 +39,13 @@ static inline void wr_fence(void)
#endif /* !CONFIG_METAG_META21 */
-static inline void wmb(void)
-{
- /* flush writes through the write combiner */
- wr_fence();
-}
+/* flush writes through the write combiner */
+#define mb() wr_fence()
+#define rmb() barrier()
+#define wmb() mb()
+
+#define coherent_rmb() rmb()
+#define coherent_wmb() wmb()
#ifndef CONFIG_SMP
#define fence() do { } while (0)
diff --git a/arch/mips/include/asm/barrier.h b/arch/mips/include/asm/barrier.h
index 3d69aa8..2d54df7 100644
--- a/arch/mips/include/asm/barrier.h
+++ b/arch/mips/include/asm/barrier.h
@@ -75,20 +75,21 @@
#include <asm/wbflush.h>
-#define wmb() fast_wmb()
-#define rmb() fast_rmb()
#define mb() wbflush()
#define iob() wbflush()
#else /* !CONFIG_CPU_HAS_WB */
-#define wmb() fast_wmb()
-#define rmb() fast_rmb()
#define mb() fast_mb()
#define iob() fast_iob()
#endif /* !CONFIG_CPU_HAS_WB */
+#define wmb() fast_wmb()
+#define rmb() fast_rmb()
+#define coherent_wmb() fast_wmb()
+#define coherent_rmb() fast_rmb()
+
#if defined(CONFIG_WEAK_ORDERING) && defined(CONFIG_SMP)
# ifdef CONFIG_CPU_CAVIUM_OCTEON
# define smp_mb() __sync()
diff --git a/arch/powerpc/include/asm/barrier.h b/arch/powerpc/include/asm/barrier.h
index cb6d66c..40c668b 100644
--- a/arch/powerpc/include/asm/barrier.h
+++ b/arch/powerpc/include/asm/barrier.h
@@ -36,8 +36,6 @@
#define set_mb(var, value) do { var = value; mb(); } while (0)
-#ifdef CONFIG_SMP
-
#ifdef __SUBARCH_HAS_LWSYNC
# define SMPWMB LWSYNC
#else
@@ -45,12 +43,17 @@
#endif
#define __lwsync() __asm__ __volatile__ (stringify_in_c(LWSYNC) : : :"memory")
+#define coherent_rmb() __lwsync()
+#define coherent_wmb() __asm__ __volatile__ (stringify_in_c(SMPWMB) : : :"memory")
+
+#ifdef CONFIG_SMP
+#define smp_lwsync() __lwsync()
#define smp_mb() mb()
-#define smp_rmb() __lwsync()
-#define smp_wmb() __asm__ __volatile__ (stringify_in_c(SMPWMB) : : :"memory")
+#define smp_rmb() coherent_rmb()
+#define smp_wmb() coherent_wmb()
#else
-#define __lwsync() barrier()
+#define smp_lwsync() barrier()
#define smp_mb() barrier()
#define smp_rmb() barrier()
@@ -72,7 +75,7 @@
#define smp_store_release(p, v) \
do { \
compiletime_assert_atomic_type(*p); \
- __lwsync(); \
+ smp_lwsync(); \
ACCESS_ONCE(*p) = (v); \
} while (0)
@@ -80,7 +83,7 @@ do { \
({ \
typeof(*p) ___p1 = ACCESS_ONCE(*p); \
compiletime_assert_atomic_type(*p); \
- __lwsync(); \
+ smp_lwsync(); \
___p1; \
})
diff --git a/arch/s390/include/asm/barrier.h b/arch/s390/include/asm/barrier.h
index 33d191d..a50117d 100644
--- a/arch/s390/include/asm/barrier.h
+++ b/arch/s390/include/asm/barrier.h
@@ -24,6 +24,8 @@
#define rmb() mb()
#define wmb() mb()
+#define coherent_rmb() rmb()
+#define coherent_wmb() wmb()
#define smp_mb() mb()
#define smp_rmb() rmb()
#define smp_wmb() wmb()
diff --git a/arch/sparc/include/asm/barrier_64.h b/arch/sparc/include/asm/barrier_64.h
index 6c974c0..97bdcd4 100644
--- a/arch/sparc/include/asm/barrier_64.h
+++ b/arch/sparc/include/asm/barrier_64.h
@@ -37,6 +37,9 @@ do { __asm__ __volatile__("ba,pt %%xcc, 1f\n\t" \
#define rmb() __asm__ __volatile__("":::"memory")
#define wmb() __asm__ __volatile__("":::"memory")
+#define coherent_rmb() rmb()
+#define coherent_wmb() wmb()
+
#define set_mb(__var, __value) \
do { __var = __value; membar_safe("#StoreLoad"); } while(0)
diff --git a/arch/x86/include/asm/barrier.h b/arch/x86/include/asm/barrier.h
index 5238000..340a7fe 100644
--- a/arch/x86/include/asm/barrier.h
+++ b/arch/x86/include/asm/barrier.h
@@ -24,13 +24,16 @@
#define wmb() asm volatile("sfence" ::: "memory")
#endif
-#ifdef CONFIG_SMP
-#define smp_mb() mb()
#ifdef CONFIG_X86_PPRO_FENCE
-# define smp_rmb() rmb()
+#define coherent_rmb() rmb()
#else
-# define smp_rmb() barrier()
+#define coherent_rmb() barrier()
#endif
+#define coherent_wmb() barrier()
+
+#ifdef CONFIG_SMP
+#define smp_mb() mb()
+#define smp_rmb() coherent_rmb()
#define smp_wmb() barrier()
#define set_mb(var, value) do { (void)xchg(&var, value); } while (0)
#else /* !SMP */
diff --git a/arch/x86/um/asm/barrier.h b/arch/x86/um/asm/barrier.h
index d6511d9..d4952e0 100644
--- a/arch/x86/um/asm/barrier.h
+++ b/arch/x86/um/asm/barrier.h
@@ -29,17 +29,18 @@
#endif /* CONFIG_X86_32 */
-#ifdef CONFIG_SMP
-
-#define smp_mb() mb()
#ifdef CONFIG_X86_PPRO_FENCE
-#define smp_rmb() rmb()
+#define coherent_rmb() rmb()
#else /* CONFIG_X86_PPRO_FENCE */
-#define smp_rmb() barrier()
+#define coherent_rmb() barrier()
#endif /* CONFIG_X86_PPRO_FENCE */
+#define coherent_wmb() barrier()
-#define smp_wmb() barrier()
+#ifdef CONFIG_SMP
+#define smp_mb() mb()
+#define smp_rmb() coherent_rmb()
+#define smp_wmb() coherent_wmb()
#define set_mb(var, value) do { (void)xchg(&var, value); } while (0)
#else /* CONFIG_SMP */
diff --git a/include/asm-generic/barrier.h b/include/asm-generic/barrier.h
index 1402fa8..52a9a71 100644
--- a/include/asm-generic/barrier.h
+++ b/include/asm-generic/barrier.h
@@ -42,6 +42,14 @@
#define wmb() mb()
#endif
+#ifndef coherent_rmb
+#define coherent_rmb() rmb()
+#endif
+
+#ifndef coherent_wmb
+#define coherent_wmb() wmb()
+#endif
+
#ifndef read_barrier_depends
#define read_barrier_depends() do { } while (0)
#endif
^ permalink raw reply related
* [PATCH v4 4/4] fm10k/igb/ixgbe: Use coherent_rmb on Rx descriptor reads
From: Alexander Duyck @ 2014-11-18 17:29 UTC (permalink / raw)
To: linux-arch, netdev, linux-kernel
Cc: mathieu.desnoyers, peterz, benh, heiko.carstens, mingo, mikey,
linux, donald.c.skidmore, matthew.vick, geert, jeffrey.t.kirsher,
romieu, paulmck, nic_swsd, will.deacon, michael, tony.luck,
torvalds, oleg, schwidefsky, fweisbec, davem
In-Reply-To: <20141118172644.26303.37688.stgit@ahduyck-server>
This change makes it so that coherent_rmb is used when reading the Rx
descriptor. The advantage of coherent_rmb is that it allows for a much
lower cost barrier on x86, powerpc, arm, and arm64 architectures than a
traditional memory barrier when dealing with reads that only have to
synchronize to coherent memory.
In addition I have updated the code so that it just checks to see if any
bits have been set instead of just the DD bit since the DD bit will always
be set as a part of a descriptor write-back so we just need to check for a
non-zero value being present at that memory location rather than just
checking for any specific bit. This allows the code itself to appear much
cleaner and allows the compiler more room to optimize.
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: Matthew Vick <matthew.vick@intel.com>
Cc: Don Skidmore <donald.c.skidmore@intel.com>
Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
drivers/net/ethernet/intel/fm10k/fm10k_main.c | 6 +++---
drivers/net/ethernet/intel/igb/igb_main.c | 6 +++---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 9 ++++-----
3 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_main.c b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
index e645af4..1819311 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_main.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
@@ -620,14 +620,14 @@ static bool fm10k_clean_rx_irq(struct fm10k_q_vector *q_vector,
rx_desc = FM10K_RX_DESC(rx_ring, rx_ring->next_to_clean);
- if (!fm10k_test_staterr(rx_desc, FM10K_RXD_STATUS_DD))
+ if (!rx_desc->d.staterr)
break;
/* This memory barrier is needed to keep us from reading
* any other fields out of the rx_desc until we know the
- * RXD_STATUS_DD bit is set
+ * descriptor has been written back
*/
- rmb();
+ coherent_rmb();
/* retrieve a buffer from the ring */
skb = fm10k_fetch_rx_buffer(rx_ring, rx_desc, skb);
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index a2d72a8..51cda23 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -6918,14 +6918,14 @@ static bool igb_clean_rx_irq(struct igb_q_vector *q_vector, const int budget)
rx_desc = IGB_RX_DESC(rx_ring, rx_ring->next_to_clean);
- if (!igb_test_staterr(rx_desc, E1000_RXD_STAT_DD))
+ if (!rx_desc->wb.upper.status_error)
break;
/* This memory barrier is needed to keep us from reading
* any other fields out of the rx_desc until we know the
- * RXD_STAT_DD bit is set
+ * descriptor has been written back
*/
- rmb();
+ coherent_rmb();
/* retrieve a buffer from the ring */
skb = igb_fetch_rx_buffer(rx_ring, rx_desc, skb);
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index d2df4e3..fdb49c0 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -2003,15 +2003,14 @@ static int ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
rx_desc = IXGBE_RX_DESC(rx_ring, rx_ring->next_to_clean);
- if (!ixgbe_test_staterr(rx_desc, IXGBE_RXD_STAT_DD))
+ if (!rx_desc->wb.upper.status_error)
break;
- /*
- * This memory barrier is needed to keep us from reading
+ /* This memory barrier is needed to keep us from reading
* any other fields out of the rx_desc until we know the
- * RXD_STAT_DD bit is set
+ * descriptor has been written back
*/
- rmb();
+ coherent_rmb();
/* retrieve a buffer from the ring */
skb = ixgbe_fetch_rx_buffer(rx_ring, rx_desc);
^ permalink raw reply related
* [PATCH v4 3/4] r8169: Use coherent_rmb() and coherent_wmb() for DescOwn checks
From: Alexander Duyck @ 2014-11-18 17:29 UTC (permalink / raw)
To: linux-arch, netdev, linux-kernel
Cc: mathieu.desnoyers, peterz, benh, heiko.carstens, mingo, mikey,
linux, donald.c.skidmore, matthew.vick, geert, jeffrey.t.kirsher,
romieu, paulmck, nic_swsd, will.deacon, michael, tony.luck,
torvalds, oleg, schwidefsky, fweisbec, davem
In-Reply-To: <20141118172644.26303.37688.stgit@ahduyck-server>
The r8169 use a pair of wmb() calls when setting up the descriptor rings.
The first is to synchronize the descriptor data with the descriptor status,
and the second is to synchronize the descriptor status with the use of the
MMIO doorbell to notify the device that descriptors are ready. This can
come at a heavy price on some systems, and is not really necessary on
systems such as x86 as a simple barrier() would suffice to order store/store
accesses. As such we can replace the first memory barrier with
coherent_wmb() to reduce the cost for these accesses.
In addition the r8169 uses a rmb() to prevent compiler optimization in the
cleanup paths, however by moving the barrier down a few lines and replacing
it with a coherent_rmb() we should be able to use it to guarantee
descriptor accesses do not occur until the device has updated the DescOwn
bit from its end.
One last change I made is to move the update of cur_tx in the xmit path to
after the wmb. This way we can guarantee the device and all CPUs should
see the DescOwn update before they see the cur_tx value update.
Cc: Realtek linux nic maintainers <nic_swsd@realtek.com>
Cc: Francois Romieu <romieu@fr.zoreil.com>
Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
drivers/net/ethernet/realtek/r8169.c | 29 +++++++++++++++++++++--------
1 file changed, 21 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index cf154f7..2886c4a 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -6601,6 +6601,9 @@ static inline void rtl8169_mark_to_asic(struct RxDesc *desc, u32 rx_buf_sz)
{
u32 eor = le32_to_cpu(desc->opts1) & RingEnd;
+ /* Force memory writes to complete before releasing descriptor */
+ coherent_wmb();
+
desc->opts1 = cpu_to_le32(DescOwn | eor | rx_buf_sz);
}
@@ -6608,7 +6611,6 @@ static inline void rtl8169_map_to_asic(struct RxDesc *desc, dma_addr_t mapping,
u32 rx_buf_sz)
{
desc->addr = cpu_to_le64(mapping);
- wmb();
rtl8169_mark_to_asic(desc, rx_buf_sz);
}
@@ -7077,16 +7079,18 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
skb_tx_timestamp(skb);
- wmb();
+ /* Force memory writes to complete before releasing descriptor */
+ coherent_wmb();
/* Anti gcc 2.95.3 bugware (sic) */
status = opts[0] | len | (RingEnd * !((entry + 1) % NUM_TX_DESC));
txd->opts1 = cpu_to_le32(status);
- tp->cur_tx += frags + 1;
-
+ /* Force all memory writes to complete before notifying device */
wmb();
+ tp->cur_tx += frags + 1;
+
RTL_W8(TxPoll, NPQ);
mmiowb();
@@ -7185,11 +7189,16 @@ static void rtl_tx(struct net_device *dev, struct rtl8169_private *tp)
struct ring_info *tx_skb = tp->tx_skb + entry;
u32 status;
- rmb();
status = le32_to_cpu(tp->TxDescArray[entry].opts1);
if (status & DescOwn)
break;
+ /* This barrier is needed to keep us from reading
+ * any other fields out of the Tx descriptor until
+ * we know the status of DescOwn
+ */
+ coherent_rmb();
+
rtl8169_unmap_tx_skb(&tp->pci_dev->dev, tx_skb,
tp->TxDescArray + entry);
if (status & LastFrag) {
@@ -7284,11 +7293,16 @@ static int rtl_rx(struct net_device *dev, struct rtl8169_private *tp, u32 budget
struct RxDesc *desc = tp->RxDescArray + entry;
u32 status;
- rmb();
status = le32_to_cpu(desc->opts1) & tp->opts1_mask;
-
if (status & DescOwn)
break;
+
+ /* This barrier is needed to keep us from reading
+ * any other fields out of the Rx descriptor until
+ * we know the status of DescOwn
+ */
+ coherent_rmb();
+
if (unlikely(status & RxRES)) {
netif_info(tp, rx_err, dev, "Rx ERROR. status = %08x\n",
status);
@@ -7350,7 +7364,6 @@ process_pkt:
}
release_descriptor:
desc->opts2 = 0;
- wmb();
rtl8169_mark_to_asic(desc, rx_buf_sz);
}
^ permalink raw reply related
* [PATCH net-next 1/4] net: allow large number of rx queues
From: Pankaj Gupta @ 2014-11-18 16:22 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: davem, jasowang, mst, dgibson, vfalico, edumazet, vyasevic, hkchu,
wuzhy, xemul, therbert, bhutchings, xii, stephen, jiri,
sergei.shtylyov, Pankaj Gupta
In-Reply-To: <1416327778-17716-1-git-send-email-pagupta@redhat.com>
netif_alloc_rx_queues() uses kcalloc() to allocate memory
for "struct netdev_queue *_rx" array.
If we are doing large rx queue allocation kcalloc() might
fail, so this patch does a fallback to vzalloc().
Similar implementation is done for tx queue allocation in
netif_alloc_netdev_queues().
We avoid failure of high order memory allocation
with the help of vzalloc(), this allows us to do large
rx and tx queue allocation which in turn helps us to
increase the number of queues in tun.
As vmalloc() adds overhead on a critical network path,
__GFP_REPEAT flag is used with kzalloc() to do this fallback
only when really needed.
Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: David Gibson <dgibson@redhat.com>
---
net/core/dev.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index e916ba8..abe9560 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6059,17 +6059,25 @@ void netif_stacked_transfer_operstate(const struct net_device *rootdev,
EXPORT_SYMBOL(netif_stacked_transfer_operstate);
#ifdef CONFIG_SYSFS
+static void netif_free_rx_queues(struct net_device *dev)
+{
+ kvfree(dev->_rx);
+}
+
static int netif_alloc_rx_queues(struct net_device *dev)
{
unsigned int i, count = dev->num_rx_queues;
struct netdev_rx_queue *rx;
+ size_t sz = count * sizeof(*rx);
BUG_ON(count < 1);
- rx = kcalloc(count, sizeof(struct netdev_rx_queue), GFP_KERNEL);
- if (!rx)
- return -ENOMEM;
-
+ rx = kzalloc(sz, GFP_KERNEL | __GFP_NOWARN | __GFP_REPEAT);
+ if (!rx) {
+ rx = vzalloc(sz);
+ if (!rx)
+ return -ENOMEM;
+ }
dev->_rx = rx;
for (i = 0; i < count; i++)
@@ -6698,9 +6706,8 @@ void free_netdev(struct net_device *dev)
netif_free_tx_queues(dev);
#ifdef CONFIG_SYSFS
- kfree(dev->_rx);
+ netif_free_rx_queues(dev);
#endif
-
kfree(rcu_dereference_protected(dev->ingress_queue, 1));
/* Flush device addresses */
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH net-next] PPC: bpf_jit_comp: Unify BPF_MOD | BPF_X and BPF_DIV | BPF_X
From: David Miller @ 2014-11-18 18:20 UTC (permalink / raw)
To: alexei.starovoitov; +Cc: kda, mpe, netdev, felix, linuxppc-dev, dborkman
In-Reply-To: <CAADnVQ+w9=aezS9uCursLS4Q40S4jcyn=5vNhoc-n5qcyrF0wQ@mail.gmail.com>
From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Date: Tue, 18 Nov 2014 07:37:21 -0800
> On Mon, Nov 17, 2014 at 10:58 PM, Denis Kirjanov <kda@linux-powerpc.org> wrote:
>> Hi Michael,
>>
>> This patch added no new functionality so I haven't put the test
>> results (of course I ran the test suite to check the patch).
>>
>> The output :
>> [ 650.198958] test_bpf: Summary: 60 PASSED, 0 FAILED
>
> Acked-by: Alexei Starovoitov <ast@plumgrid.com>
Applied, thanks everyone.
> btw, please don't top post.
Seriously, it's my biggest pet peeve. It's like going to a dinner party
and eating with your feet.
^ permalink raw reply
* [ANN] Kernel integration now merged on backports
From: Luis R. Rodriguez @ 2014-11-18 18:21 UTC (permalink / raw)
To: backports@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, linux-wireless, linux-media,
linux-bluetooth, netdev@vger.kernel.org, linux-wpan
Full kernel integration is now merged as part of Linux
backports-20141114. I've written a bit about it [0] [1], what we need
now are users and developer to give this a good spin as we wind down
for the v3.19 release, which will be the first release that will
support kernel integration down to any kernel >= 3.0 -- for now you
can use the backports-20141114 tag which uses as base supported
drivers from next-20141114. What this will mean is that you can opt in
to integrate any device driver we support from any future backports
release into any of >= 3.0 kernel with full kconfig support, enabling
you to build everything as built-in. For all this you won't be using
the packaged releases [2], instead you'll use the git tree directly as
documented.
[0] http://www.do-not-panic.com/2014/11/automating-backport-kernel-integration.html
[1] https://backports.wiki.kernel.org/index.php/Documentation/integration
[2] https://backports.wiki.kernel.org/index.php/Documentation/packaging
Luis
^ permalink raw reply
* Re: Device Tree Binding for Marvell DSA Switch on imx28 board over Mdio Interface
From: Florian Fainelli @ 2014-11-18 18:23 UTC (permalink / raw)
To: Oliver Graute, Andrew Lunn; +Cc: netdev@vger.kernel.org, buytenh
In-Reply-To: <CA+KjHfauqO=BxOogexO3azW27cb1gHqEL5Mh-c+qxfUMYvf_mw@mail.gmail.com>
On 11/18/2014 12:30 AM, Oliver Graute wrote:
>> Hi Oliver
>>
>> How do you have the strapping pins on the switch set? They determine
>> what address on the mdio bus the chip responds to.
>
> On the circuit diagram the PIN 54 (P5_IND1/P5ID1) is set to
> "Configuration Address: 0101"
> P5_MODE[3:0]=0111 = Single RMII MAC Mode (100Mbps FD with 50 MHz clock input)
> PIN 59 R1_LED/NO_CPU Configuration: CPU is attached SMI address is 0x10 to 0x1F
>
> But what is the mdio address of the whole switch? or can I only
> address individual phy ports?
You should specify in the Device Tree the switch pseudo-PHY address,
typically 16 for Marvell switches. You can still access the individual
ports' PHY addresses using address 0 through N.
>
> The Port 5 Pins of the Marvell 88e6071 switch are connected via RMII
> to a Micrel Phy KSZ8051. And this Micrel Phy is connected to the MAC
> of IMX28 CPU (also RMII).
>
>> Does your u-boot have commands to read arbitrary phy registers?
>> Generally, reading a register that does not exist gives 0xffff. So
>> try some reads at different addresses and see what you can find.
>
> I can read phy registers from the Marvell switch by a Tool called
> mii_demo it is a bit like mii_diag. It can read PHY registers from the
> switch Ports via IOCTL.
> This is working via some undefined ioctl SIOCSMIIREG2, SIOCGMIIPHY2.
>
>
> Example:
> #read SMI PHY Command Register 18 from Switch with IEEE 802.3 Clause 22
> mii_demo -rd eth0 23 0x18
>
> 0x1680
>
> Best regards,
>
> Oliver
>
^ permalink raw reply
* Re: [net-next v2 01/11] i40e: only warn once of PTP nonsupport in 100Mbit speed
From: Florian Fainelli @ 2014-11-18 18:24 UTC (permalink / raw)
To: Jeff Kirsher, davem, Joe Perches
Cc: Shannon Nelson, netdev, nhorman, sassmann, jogreene
In-Reply-To: <1416303220.2295.19.camel@jtkirshe-mobl>
On 11/18/2014 01:33 AM, Jeff Kirsher wrote:
> On Tue, 2014-11-18 at 01:25 -0800, Jeff Kirsher wrote:
>> From: Shannon Nelson <shannon.nelson@intel.com>
>>
>> Only warn once that PTP is not supported when linked at 100Mbit.
>>
>> Yes, using a static this way means that this once-only message is not
>> port specific, but once only for the life of the driver, regardless of
>> the number of ports. That should be plenty.
>>
>> Change-ID: Ie6476530056df408452e195ef06afd4f57caa4b2
>> Signed-off-by: Shannon Nelson <shannon.nelson@intel.com>
>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>> ---
>> drivers/net/ethernet/intel/i40e/i40e_ptp.c | 12 +++++++++---
>> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> Florian/Joe-
>
> Since your patch to update the dev_level_once() function is going
> through Greg KH's tree, we will stick with this patch until it gets
> accepted and merged into Dave's tree. Then we will provide a patch to
> update the driver.
Sounds absolutely reasonable to me.
>
>>
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
>> index 537b621..f9151037 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
>> @@ -382,11 +382,17 @@ void i40e_ptp_set_increment(struct i40e_pf *pf)
>> incval = I40E_PTP_1GB_INCVAL;
>> break;
>> case I40E_LINK_SPEED_100MB:
>> - dev_warn(&pf->pdev->dev,
>> - "%s: 1588 functionality is not supported at 100 Mbps. Stopping the PHC.\n",
>> - __func__);
>> + {
>> + static int warn_once;
>> +
>> + if (!warn_once) {
>> + dev_warn(&pf->pdev->dev,
>> + "1588 functionality is not supported at 100 Mbps. Stopping the PHC.\n");
>> + warn_once++;
>> + }
>> incval = 0;
>> break;
>> + }
>> case I40E_LINK_SPEED_40GB:
>> default:
>> incval = I40E_PTP_40GB_INCVAL;
>
^ permalink raw reply
* Re: [net-next v2 00/11][pull request] Intel Wired LAN Driver Updates 2014-11-18
From: David Miller @ 2014-11-18 18:38 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, nhorman, sassmann, jogreene
In-Reply-To: <1416302731-11327-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Tue, 18 Nov 2014 01:25:20 -0800
> This series contains updates to i40e only.
>
> Shannon provides a patch to clean up the driver to only warn once that
> PTP is not supported when linked at 100Mbps.
>
> Mitch provides a fix for i40e where the VF interrupt processing takes
> a long time and it is possible that we could lose a VFLR event if it
> happens while processing a VFLR on another VF. To correct this situation,
> we enable the VFLR interrupt cause before we begin processing any pending
> resets.
>
> Neerav provides several patches to update DCB support in i40e. When
> there are DCB configuration changes based on DCBx, the firmware suspends
> the port's Tx and generates an event to the PF. The PF is then
> responsible to reconfigure the PF VSIs and switching topology as per the
> updated DCB configuration and then resume the port's Tx by calling the
> "Resume Port Tx" AQ command, so add this call to the flow that handles
> DCB re-configuration in the PF. Allow the driver to query and use DCB
> configuration from firmware when firmware DCBx agent is in CEE mode.
> Add a check whether LLDP Agent's default AdminStatus is enabled or
> disabled on a given port, and sets DCBx status to disabled if the
> status is disabled. Fix an issue when the port TC configuration
> changes as a result of DCBx and the driver modifies the enabled TCs for
> the VEBs it manages but does not update the enabled_tc value that
> was cached on a per VEB basis. Add a new PF state so that if a port's
> Tx is in suspended state the Tx queue disable flow would just put the
> request for the queue to be disabled and return without waiting for the
> queue to be actually disabled. Allows the driver to enable/disable
> the XPS based on the number of TCs being enabled for the given VSI.
Pulled, thanks Jeff.
> v2: Dropped patch "i40e: Handle a single mss packet with more than 8 frags"
> while we rework the patch after we test a bit more based on feedback from
> Eric Dumazet.
Since, as Eric showed, it's easy to hang the card with a simple
netperf command line due to this issue, you might want to consider
sending that fix to 'net' instead.
^ permalink raw reply
* [PATCH net] openvswitch: Fix mask generation for IPv6 labels.
From: Joe Stringer @ 2014-11-18 18:54 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: dev-yBygre7rU0TnMu66kgdUjQ, linux-kernel-u79uwXL29TY76Z2rM5mHXA
When userspace doesn't provide a mask, OVS datapath generates a fully
unwildcarded mask for the flow. This is done by taking a copy of the
flow key, then iterating across its attributes, setting all values to
0xff. This works for most attributes, as the length of the netlink
attribute typically matches the length of the value. However, IPv6
labels only use the lower 20 bits of the field. This patch makes a
special case to handle this.
This fixes the following error seen when installing IPv6 flows without a mask:
openvswitch: netlink: Invalid IPv6 flow label value (value=ffffffff, max=fffff)
Signed-off-by: Joe Stringer <joestringer@nicira.com>
---
net/openvswitch/flow_netlink.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index fa4ec2e..7a5b28f 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -825,7 +825,7 @@ static int ovs_key_from_nlattrs(struct sw_flow_match *match, u64 attrs,
return 0;
}
-static void nlattr_set(struct nlattr *attr, u8 val, bool is_attr_mask_key)
+static void mask_set_nlattr(struct nlattr *attr)
{
struct nlattr *nla;
int rem;
@@ -835,16 +835,18 @@ static void nlattr_set(struct nlattr *attr, u8 val, bool is_attr_mask_key)
/* We assume that ovs_key_lens[type] == -1 means that type is a
* nested attribute
*/
- if (is_attr_mask_key && ovs_key_lens[nla_type(nla)] == -1)
- nlattr_set(nla, val, false);
+ if (ovs_key_lens[nla_type(nla)] == -1)
+ nla_for_each_nested(nla, attr, rem)
+ memset(nla_data(nla), 0xff, nla_len(nla));
else
- memset(nla_data(nla), val, nla_len(nla));
- }
-}
+ memset(nla_data(nla), 0xff, nla_len(nla));
-static void mask_set_nlattr(struct nlattr *attr, u8 val)
-{
- nlattr_set(attr, val, true);
+ if (nla_type(nla) == OVS_KEY_ATTR_IPV6) {
+ struct ovs_key_ipv6 *ipv6_key = nla_data(nla);
+
+ ipv6_key->ipv6_label &= htonl(0x000FFFFF);
+ }
+ }
}
/**
@@ -926,7 +928,7 @@ int ovs_nla_get_match(struct sw_flow_match *match,
if (!newmask)
return -ENOMEM;
- mask_set_nlattr(newmask, 0xff);
+ mask_set_nlattr(newmask);
/* The userspace does not send tunnel attributes that are 0,
* but we should not wildcard them nonetheless.
--
1.7.10.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
^ permalink raw reply related
* Re: [PATCH] ipv6: delete protocol when cleanup
From: Cong Wang @ 2014-11-18 18:56 UTC (permalink / raw)
To: Duan Jiong; +Cc: David Miller, netdev
In-Reply-To: <546B20F8.6010806@cn.fujitsu.com>
On Tue, Nov 18, 2014 at 2:35 AM, Duan Jiong <duanj.fnst@cn.fujitsu.com> wrote:
>
> pim6_protocol was added when initiation, but it not deleted.
>
> Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
> ---
> net/ipv6/ip6mr.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
> index 0171f08..6dcdecd 100644
> --- a/net/ipv6/ip6mr.c
> +++ b/net/ipv6/ip6mr.c
> @@ -1439,6 +1439,9 @@ reg_pernet_fail:
>
> void ip6_mr_cleanup(void)
> {
> +#ifdef CONFIG_IPV6_PIMSM_V2
> + inet6_del_protocol(&pim6_protocol, IPPROTO_PIM);
> +#endif
> unregister_netdevice_notifier(&ip6_mr_notifier);
> unregister_pernet_subsys(&ip6mr_net_ops);
> kmem_cache_destroy(mrt_cachep);
Looks like rtnl_unregister() is missing as well.
^ permalink raw reply
* Re: [PATCH] ipv6: delete protocol when cleanup
From: Eric Dumazet @ 2014-11-18 19:14 UTC (permalink / raw)
To: Cong Wang; +Cc: Duan Jiong, David Miller, netdev
In-Reply-To: <CAHA+R7PNmac7KQpSK-vO9zVd3+926f6n_GpaGrbpBnSn98xiyQ@mail.gmail.com>
On Tue, 2014-11-18 at 10:56 -0800, Cong Wang wrote:
> On Tue, Nov 18, 2014 at 2:35 AM, Duan Jiong <duanj.fnst@cn.fujitsu.com> wrote:
> >
> > pim6_protocol was added when initiation, but it not deleted.
> >
> > Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
...
> Looks like rtnl_unregister() is missing as well.
Is it really worth bothering ?
May I remind you we took the decision one year ago to not bother about
removing ipv6 module.
commit 8ce440610357b77587433d0df647cea69a6890a8
Author: Cong Wang <amwang@redhat.com>
Date: Sat Sep 21 11:12:21 2013 +0800
ipv6: do not allow ipv6 module to be removed
There was some bug report on ipv6 module removal path before.
Also, as Stephen pointed out, after vxlan module gets ipv6 support,
the ipv6 stub it used is not safe against this module removal either.
So, let's just remove inet6_exit() so that ipv6 module will not be
able to be unloaded.
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: Stephen Hemminger <stephen@networkplumber.org>
Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: Cong Wang <amwang@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* [PATCH 1/1] net: pktgen: Deletion of an unnecessary check before the function call "proc_remove"
From: SF Markus Elfring @ 2014-11-18 19:16 UTC (permalink / raw)
To: David S. Miller, netdev; +Cc: LKML, kernel-janitors, Coccinelle
In-Reply-To: <5317A59D.4@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 18 Nov 2014 20:10:34 +0100
The proc_remove() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
net/core/pktgen.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 8b849dd..35046a8 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -3698,8 +3698,7 @@ static int pktgen_remove_device(struct pktgen_thread *t,
/* Remove proc before if_list entry, because add_device uses
* list to determine if interface already exist, avoid race
* with proc_create_data() */
- if (pkt_dev->entry)
- proc_remove(pkt_dev->entry);
+ proc_remove(pkt_dev->entry);
/* And update the thread if_list */
_rem_dev_from_if_list(t, pkt_dev);
--
2.1.3
^ permalink raw reply related
* [PATCH] ixgbe: Correctly disable vlan filter in promiscuous mode
From: Vladislav Yasevich @ 2014-11-18 19:28 UTC (permalink / raw)
To: netdev; +Cc: Vladislav Yasevich, Jeff Kirsher, Jacob Keller
IXGBE adapater seems to require that vlan filtering be enabled if
VMDQ
or SRIOV are enabled. When those functions are disabled,
vlan filtering may be disabled in promiscuous mode.
Prior to commit a9b8943ee129e11045862d6d6e25c5b63c95403c
ixgbe: remove vlan_filter_disable and enable functions
the logic was correct. However, after the commit the logic
got reversed and vlan filtered in now turned on when VMDQ/SRIOV
is disabled.
This patch changes the condition to enable hw vlan filtered
when VMDQ or SRIOV is enabled.
Fixes: a9b8943ee129e11045862d6d6e25c5b63c95403c (ixgbe: remove
vlan_filter_disable and enable functions)
CC: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
CC: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index d2df4e3..3f81c7a 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -3936,8 +3936,8 @@ void ixgbe_set_rx_mode(struct net_device *netdev)
* if SR-IOV and VMDQ are disabled - otherwise ensure
* that hardware VLAN filters remain enabled.
*/
- if (!(adapter->flags & (IXGBE_FLAG_VMDQ_ENABLED |
- IXGBE_FLAG_SRIOV_ENABLED)))
+ if (adapter->flags & (IXGBE_FLAG_VMDQ_ENABLED |
+ IXGBE_FLAG_SRIOV_ENABLED))
vlnctrl |= (IXGBE_VLNCTRL_VFE | IXGBE_VLNCTRL_CFIEN);
} else {
if (netdev->flags & IFF_ALLMULTI) {
--
1.9.3
^ permalink raw reply related
* loan test
From: Frank.Laurent @ 2014-11-18 19:33 UTC (permalink / raw)
To: netdev
NEED LOAN?
We offer our Loans in the following categories.
Auto Loans Mortgage loans Business Loans Personal Loans Real Estate Loan.
For more information and Application,Contact our representative with the information below,
Loan Agent: Donald Gotze
Email: gotzedonald@yahoo.com
Thank you for your response.
Yours Faithfully,
Frank Laurent
^ permalink raw reply
* [patches][RFC] situation with csum_and_copy_... API
From: Al Viro @ 2014-11-18 19:40 UTC (permalink / raw)
To: netdev; +Cc: Linus Torvalds, David Miller, linux-kernel
In-Reply-To: <20141118084745.GT7996@ZenIV.linux.org.uk>
On Tue, Nov 18, 2014 at 08:47:45AM +0000, Al Viro wrote:
> I do have a patch doing just that; the question is what to do with csum-and-copy
> primitives. Originally I planned to simply strip those access_ok() from those
> (both the explicit calls and use of copy_from_user() where we ought to use
> __copy_from_user(), etc.), but that's not nice to potential out-of-tree callers
> of those suckers. If any of those exist and manage to cope with the wonderful
> calling conventions, that is. As it is, we have the total of 4 callers of
> csum_and_copy_from_user() and 2 callers of csum_and_copy_to_user(), all in
> networking code. Do we care about potential out-of-tree users existing and
> getting screwed by such change? Davem, Linus?
FWIW, the beginning of series in question follows; removal of those
access_ok() is 3/5. The series is longer than that (see vfs.git#iov_iter-net
for a bit more, and there's more stuff in local queue still too much in flux
to push them out), but all the stuff relevant to validating iovecs on
sendmsg/recvmsg and getting rid of excessive access_ok() is in the first 5
commits.
^ permalink raw reply
* [PATCH 1/5] separate kernel- and userland-side msghdr
From: Al Viro @ 2014-11-18 19:41 UTC (permalink / raw)
To: netdev; +Cc: Linus Torvalds, David Miller, linux-kernel
In-Reply-To: <20141118194053.GA14641@ZenIV.linux.org.uk>
Kernel-side struct msghdr is (currently) using the same layout as
userland one, but it's not a one-to-one copy - even without considering
32bit compat issues, we have msg_iov, msg_name and msg_control copied
to kernel[1]. It's fairly localized, so we get away with a few functions
where that knowledge is needed (and we could shrink that set even
more). Pretty much everything deals with the kernel-side variant and
the few places that want userland one just use a bunch of force-casts
to paper over the differences.
The thing is, kernel-side definition of struct msghdr is *not* exposed
in include/uapi - libc doesn't see it, etc. So we can add struct user_msghdr,
with proper annotations and let the few places that ever deal with those
beasts use it for userland pointers. Saner typechecking aside, that will
allow to change the layout of kernel-side msghdr - e.g. replace
msg_iov/msg_iovlen there with struct iov_iter, getting rid of the need
to modify the iovec as we copy data to/from it, etc.
We could introduce kernel_msghdr instead, but that would create much more
noise - the absolute majority of the instances would need to have the
type switched to kernel_msghdr and definition of struct msghdr in
include/linux/socket.h is not going to be seen by userland anyway.
This commit just introduces user_msghdr and switches the few places that
are dealing with userland-side msghdr to it.
[1] actually, it's even trickier than that - we copy msg_control for
sendmsg, but keep the userland address on recvmsg.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
arch/arm/kernel/sys_oabi-compat.c | 4 ++--
include/linux/socket.h | 16 +++++++++++++---
include/linux/syscalls.h | 6 +++---
net/compat.c | 4 ++--
net/socket.c | 29 ++++++++++++++++-------------
5 files changed, 36 insertions(+), 23 deletions(-)
diff --git a/arch/arm/kernel/sys_oabi-compat.c b/arch/arm/kernel/sys_oabi-compat.c
index e90a314..b83f3b7 100644
--- a/arch/arm/kernel/sys_oabi-compat.c
+++ b/arch/arm/kernel/sys_oabi-compat.c
@@ -400,7 +400,7 @@ asmlinkage long sys_oabi_sendto(int fd, void __user *buff,
return sys_sendto(fd, buff, len, flags, addr, addrlen);
}
-asmlinkage long sys_oabi_sendmsg(int fd, struct msghdr __user *msg, unsigned flags)
+asmlinkage long sys_oabi_sendmsg(int fd, struct user_msghdr __user *msg, unsigned flags)
{
struct sockaddr __user *addr;
int msg_namelen;
@@ -446,7 +446,7 @@ asmlinkage long sys_oabi_socketcall(int call, unsigned long __user *args)
break;
case SYS_SENDMSG:
if (copy_from_user(a, args, 3 * sizeof(long)) == 0)
- r = sys_oabi_sendmsg(a[0], (struct msghdr __user *)a[1], a[2]);
+ r = sys_oabi_sendmsg(a[0], (struct user_msghdr __user *)a[1], a[2]);
break;
default:
r = sys_socketcall(call, args);
diff --git a/include/linux/socket.h b/include/linux/socket.h
index bb9b836..51bd666 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -53,10 +53,20 @@ struct msghdr {
__kernel_size_t msg_controllen; /* ancillary data buffer length */
unsigned int msg_flags; /* flags on received message */
};
+
+struct user_msghdr {
+ void __user *msg_name; /* ptr to socket address structure */
+ int msg_namelen; /* size of socket address structure */
+ struct iovec __user *msg_iov; /* scatter/gather array */
+ __kernel_size_t msg_iovlen; /* # elements in msg_iov */
+ void __user *msg_control; /* ancillary data */
+ __kernel_size_t msg_controllen; /* ancillary data buffer length */
+ unsigned int msg_flags; /* flags on received message */
+};
/* For recvmmsg/sendmmsg */
struct mmsghdr {
- struct msghdr msg_hdr;
+ struct user_msghdr msg_hdr;
unsigned int msg_len;
};
@@ -319,8 +329,8 @@ extern int put_cmsg(struct msghdr*, int level, int type, int len, void *data);
struct timespec;
/* The __sys_...msg variants allow MSG_CMSG_COMPAT */
-extern long __sys_recvmsg(int fd, struct msghdr __user *msg, unsigned flags);
-extern long __sys_sendmsg(int fd, struct msghdr __user *msg, unsigned flags);
+extern long __sys_recvmsg(int fd, struct user_msghdr __user *msg, unsigned flags);
+extern long __sys_sendmsg(int fd, struct user_msghdr __user *msg, unsigned flags);
extern int __sys_recvmmsg(int fd, struct mmsghdr __user *mmsg, unsigned int vlen,
unsigned int flags, struct timespec *timeout);
extern int __sys_sendmmsg(int fd, struct mmsghdr __user *mmsg,
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index bda9b81..c9afdc7 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -25,7 +25,7 @@ struct linux_dirent64;
struct list_head;
struct mmap_arg_struct;
struct msgbuf;
-struct msghdr;
+struct user_msghdr;
struct mmsghdr;
struct msqid_ds;
struct new_utsname;
@@ -601,13 +601,13 @@ asmlinkage long sys_getpeername(int, struct sockaddr __user *, int __user *);
asmlinkage long sys_send(int, void __user *, size_t, unsigned);
asmlinkage long sys_sendto(int, void __user *, size_t, unsigned,
struct sockaddr __user *, int);
-asmlinkage long sys_sendmsg(int fd, struct msghdr __user *msg, unsigned flags);
+asmlinkage long sys_sendmsg(int fd, struct user_msghdr __user *msg, unsigned flags);
asmlinkage long sys_sendmmsg(int fd, struct mmsghdr __user *msg,
unsigned int vlen, unsigned flags);
asmlinkage long sys_recv(int, void __user *, size_t, unsigned);
asmlinkage long sys_recvfrom(int, void __user *, size_t, unsigned,
struct sockaddr __user *, int __user *);
-asmlinkage long sys_recvmsg(int fd, struct msghdr __user *msg, unsigned flags);
+asmlinkage long sys_recvmsg(int fd, struct user_msghdr __user *msg, unsigned flags);
asmlinkage long sys_recvmmsg(int fd, struct mmsghdr __user *msg,
unsigned int vlen, unsigned flags,
struct timespec __user *timeout);
diff --git a/net/compat.c b/net/compat.c
index bc8aeef..562e920 100644
--- a/net/compat.c
+++ b/net/compat.c
@@ -740,7 +740,7 @@ COMPAT_SYSCALL_DEFINE3(sendmsg, int, fd, struct compat_msghdr __user *, msg, uns
{
if (flags & MSG_CMSG_COMPAT)
return -EINVAL;
- return __sys_sendmsg(fd, (struct msghdr __user *)msg, flags | MSG_CMSG_COMPAT);
+ return __sys_sendmsg(fd, (struct user_msghdr __user *)msg, flags | MSG_CMSG_COMPAT);
}
COMPAT_SYSCALL_DEFINE4(sendmmsg, int, fd, struct compat_mmsghdr __user *, mmsg,
@@ -756,7 +756,7 @@ COMPAT_SYSCALL_DEFINE3(recvmsg, int, fd, struct compat_msghdr __user *, msg, uns
{
if (flags & MSG_CMSG_COMPAT)
return -EINVAL;
- return __sys_recvmsg(fd, (struct msghdr __user *)msg, flags | MSG_CMSG_COMPAT);
+ return __sys_recvmsg(fd, (struct user_msghdr __user *)msg, flags | MSG_CMSG_COMPAT);
}
COMPAT_SYSCALL_DEFINE4(recv, int, fd, void __user *, buf, compat_size_t, len, unsigned int, flags)
diff --git a/net/socket.c b/net/socket.c
index fe20c31..0ae8147 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1989,8 +1989,11 @@ struct used_address {
};
static int copy_msghdr_from_user(struct msghdr *kmsg,
- struct msghdr __user *umsg)
+ struct user_msghdr __user *umsg)
{
+ /* We are relying on the (currently) identical layouts. Once
+ * the kernel-side changes, this place will need to be updated
+ */
if (copy_from_user(kmsg, umsg, sizeof(struct msghdr)))
return -EFAULT;
@@ -2005,7 +2008,7 @@ static int copy_msghdr_from_user(struct msghdr *kmsg,
return 0;
}
-static int ___sys_sendmsg(struct socket *sock, struct msghdr __user *msg,
+static int ___sys_sendmsg(struct socket *sock, struct user_msghdr __user *msg,
struct msghdr *msg_sys, unsigned int flags,
struct used_address *used_address)
{
@@ -2123,7 +2126,7 @@ out:
* BSD sendmsg interface
*/
-long __sys_sendmsg(int fd, struct msghdr __user *msg, unsigned flags)
+long __sys_sendmsg(int fd, struct user_msghdr __user *msg, unsigned flags)
{
int fput_needed, err;
struct msghdr msg_sys;
@@ -2140,7 +2143,7 @@ out:
return err;
}
-SYSCALL_DEFINE3(sendmsg, int, fd, struct msghdr __user *, msg, unsigned int, flags)
+SYSCALL_DEFINE3(sendmsg, int, fd, struct user_msghdr __user *, msg, unsigned int, flags)
{
if (flags & MSG_CMSG_COMPAT)
return -EINVAL;
@@ -2177,7 +2180,7 @@ int __sys_sendmmsg(int fd, struct mmsghdr __user *mmsg, unsigned int vlen,
while (datagrams < vlen) {
if (MSG_CMSG_COMPAT & flags) {
- err = ___sys_sendmsg(sock, (struct msghdr __user *)compat_entry,
+ err = ___sys_sendmsg(sock, (struct user_msghdr __user *)compat_entry,
&msg_sys, flags, &used_address);
if (err < 0)
break;
@@ -2185,7 +2188,7 @@ int __sys_sendmmsg(int fd, struct mmsghdr __user *mmsg, unsigned int vlen,
++compat_entry;
} else {
err = ___sys_sendmsg(sock,
- (struct msghdr __user *)entry,
+ (struct user_msghdr __user *)entry,
&msg_sys, flags, &used_address);
if (err < 0)
break;
@@ -2215,7 +2218,7 @@ SYSCALL_DEFINE4(sendmmsg, int, fd, struct mmsghdr __user *, mmsg,
return __sys_sendmmsg(fd, mmsg, vlen, flags);
}
-static int ___sys_recvmsg(struct socket *sock, struct msghdr __user *msg,
+static int ___sys_recvmsg(struct socket *sock, struct user_msghdr __user *msg,
struct msghdr *msg_sys, unsigned int flags, int nosec)
{
struct compat_msghdr __user *msg_compat =
@@ -2311,7 +2314,7 @@ out:
* BSD recvmsg interface
*/
-long __sys_recvmsg(int fd, struct msghdr __user *msg, unsigned flags)
+long __sys_recvmsg(int fd, struct user_msghdr __user *msg, unsigned flags)
{
int fput_needed, err;
struct msghdr msg_sys;
@@ -2328,7 +2331,7 @@ out:
return err;
}
-SYSCALL_DEFINE3(recvmsg, int, fd, struct msghdr __user *, msg,
+SYSCALL_DEFINE3(recvmsg, int, fd, struct user_msghdr __user *, msg,
unsigned int, flags)
{
if (flags & MSG_CMSG_COMPAT)
@@ -2373,7 +2376,7 @@ int __sys_recvmmsg(int fd, struct mmsghdr __user *mmsg, unsigned int vlen,
* No need to ask LSM for more than the first datagram.
*/
if (MSG_CMSG_COMPAT & flags) {
- err = ___sys_recvmsg(sock, (struct msghdr __user *)compat_entry,
+ err = ___sys_recvmsg(sock, (struct user_msghdr __user *)compat_entry,
&msg_sys, flags & ~MSG_WAITFORONE,
datagrams);
if (err < 0)
@@ -2382,7 +2385,7 @@ int __sys_recvmmsg(int fd, struct mmsghdr __user *mmsg, unsigned int vlen,
++compat_entry;
} else {
err = ___sys_recvmsg(sock,
- (struct msghdr __user *)entry,
+ (struct user_msghdr __user *)entry,
&msg_sys, flags & ~MSG_WAITFORONE,
datagrams);
if (err < 0)
@@ -2571,13 +2574,13 @@ SYSCALL_DEFINE2(socketcall, int, call, unsigned long __user *, args)
(int __user *)a[4]);
break;
case SYS_SENDMSG:
- err = sys_sendmsg(a0, (struct msghdr __user *)a1, a[2]);
+ err = sys_sendmsg(a0, (struct user_msghdr __user *)a1, a[2]);
break;
case SYS_SENDMMSG:
err = sys_sendmmsg(a0, (struct mmsghdr __user *)a1, a[2], a[3]);
break;
case SYS_RECVMSG:
- err = sys_recvmsg(a0, (struct msghdr __user *)a1, a[2]);
+ err = sys_recvmsg(a0, (struct user_msghdr __user *)a1, a[2]);
break;
case SYS_RECVMMSG:
err = sys_recvmmsg(a0, (struct mmsghdr __user *)a1, a[2], a[3],
--
1.7.10.4
^ permalink raw reply related
* [PATCH 2/5] {compat_,}verify_iovec(): switch to generic copying of iovecs
From: Al Viro @ 2014-11-18 19:42 UTC (permalink / raw)
To: netdev; +Cc: Linus Torvalds, David Miller, linux-kernel
In-Reply-To: <20141118194053.GA14641@ZenIV.linux.org.uk>
use {compat_,}rw_copy_check_uvector(). As the result, we are
guaranteed that all iovecs seen in ->msg_iov by ->sendmsg()
and ->recvmsg() will pass access_ok(). The next commit removes
now redundant checks in callees...
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
net/compat.c | 51 +++++++++++++++------------------------------------
net/core/iovec.c | 37 ++++++++++++++-----------------------
net/socket.c | 38 ++++++++------------------------------
3 files changed, 37 insertions(+), 89 deletions(-)
diff --git a/net/compat.c b/net/compat.c
index 562e920..7b4b6ad 100644
--- a/net/compat.c
+++ b/net/compat.c
@@ -31,33 +31,6 @@
#include <asm/uaccess.h>
#include <net/compat.h>
-static inline int iov_from_user_compat_to_kern(struct iovec *kiov,
- struct compat_iovec __user *uiov32,
- int niov)
-{
- int tot_len = 0;
-
- while (niov > 0) {
- compat_uptr_t buf;
- compat_size_t len;
-
- if (get_user(len, &uiov32->iov_len) ||
- get_user(buf, &uiov32->iov_base))
- return -EFAULT;
-
- if (len > INT_MAX - tot_len)
- len = INT_MAX - tot_len;
-
- tot_len += len;
- kiov->iov_base = compat_ptr(buf);
- kiov->iov_len = (__kernel_size_t) len;
- uiov32++;
- kiov++;
- niov--;
- }
- return tot_len;
-}
-
int get_compat_msghdr(struct msghdr *kmsg, struct compat_msghdr __user *umsg)
{
compat_uptr_t tmp1, tmp2, tmp3;
@@ -80,13 +53,15 @@ int get_compat_msghdr(struct msghdr *kmsg, struct compat_msghdr __user *umsg)
}
/* I've named the args so it is easy to tell whose space the pointers are in. */
-int verify_compat_iovec(struct msghdr *kern_msg, struct iovec *kern_iov,
+int verify_compat_iovec(struct msghdr *kern_msg, struct iovec *iov,
struct sockaddr_storage *kern_address, int mode)
{
- int tot_len;
+ struct compat_iovec __user *p;
+ struct iovec *res;
+ int err;
if (kern_msg->msg_name && kern_msg->msg_namelen) {
- if (mode == VERIFY_READ) {
+ if (mode == WRITE) {
int err = move_addr_to_kernel(kern_msg->msg_name,
kern_msg->msg_namelen,
kern_address);
@@ -99,13 +74,17 @@ int verify_compat_iovec(struct msghdr *kern_msg, struct iovec *kern_iov,
kern_msg->msg_namelen = 0;
}
- tot_len = iov_from_user_compat_to_kern(kern_iov,
- (struct compat_iovec __user *)kern_msg->msg_iov,
- kern_msg->msg_iovlen);
- if (tot_len >= 0)
- kern_msg->msg_iov = kern_iov;
+ if (kern_msg->msg_iovlen > UIO_MAXIOV)
+ return -EMSGSIZE;
- return tot_len;
+ p = (struct compat_iovec __user *)kern_msg->msg_iov;
+ err = compat_rw_copy_check_uvector(mode, p, kern_msg->msg_iovlen,
+ UIO_FASTIOV, iov, &res);
+ if (err >= 0)
+ kern_msg->msg_iov = res;
+ else if (res != iov)
+ kfree(res);
+ return err;
}
/* Bleech... */
diff --git a/net/core/iovec.c b/net/core/iovec.c
index e1ec45a..86beeea 100644
--- a/net/core/iovec.c
+++ b/net/core/iovec.c
@@ -37,13 +37,13 @@
int verify_iovec(struct msghdr *m, struct iovec *iov, struct sockaddr_storage *address, int mode)
{
- int size, ct, err;
+ struct iovec *res;
+ int err;
if (m->msg_name && m->msg_namelen) {
- if (mode == VERIFY_READ) {
- void __user *namep;
- namep = (void __user __force *) m->msg_name;
- err = move_addr_to_kernel(namep, m->msg_namelen,
+ if (mode == WRITE) {
+ void __user *namep = (void __user __force *)m->msg_name;
+ int err = move_addr_to_kernel(namep, m->msg_namelen,
address);
if (err < 0)
return err;
@@ -53,24 +53,15 @@ int verify_iovec(struct msghdr *m, struct iovec *iov, struct sockaddr_storage *a
m->msg_name = NULL;
m->msg_namelen = 0;
}
-
- size = m->msg_iovlen * sizeof(struct iovec);
- if (copy_from_user(iov, (void __user __force *) m->msg_iov, size))
- return -EFAULT;
-
- m->msg_iov = iov;
- err = 0;
-
- for (ct = 0; ct < m->msg_iovlen; ct++) {
- size_t len = iov[ct].iov_len;
-
- if (len > INT_MAX - err) {
- len = INT_MAX - err;
- iov[ct].iov_len = len;
- }
- err += len;
- }
-
+ if (m->msg_iovlen > UIO_MAXIOV)
+ return -EMSGSIZE;
+
+ err = rw_copy_check_uvector(mode, (void __user __force *)m->msg_iov,
+ m->msg_iovlen, UIO_FASTIOV, iov, &res);
+ if (err >= 0)
+ m->msg_iov = res;
+ else if (res != iov)
+ kfree(res);
return err;
}
diff --git a/net/socket.c b/net/socket.c
index 0ae8147..59020f0 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -2032,24 +2032,14 @@ static int ___sys_sendmsg(struct socket *sock, struct user_msghdr __user *msg,
return err;
}
- if (msg_sys->msg_iovlen > UIO_FASTIOV) {
- err = -EMSGSIZE;
- if (msg_sys->msg_iovlen > UIO_MAXIOV)
- goto out;
- err = -ENOMEM;
- iov = kmalloc(msg_sys->msg_iovlen * sizeof(struct iovec),
- GFP_KERNEL);
- if (!iov)
- goto out;
- }
-
/* This will also move the address data into kernel space */
- if (MSG_CMSG_COMPAT & flags) {
- err = verify_compat_iovec(msg_sys, iov, &address, VERIFY_READ);
- } else
- err = verify_iovec(msg_sys, iov, &address, VERIFY_READ);
+ if (MSG_CMSG_COMPAT & flags)
+ err = verify_compat_iovec(msg_sys, iovstack, &address, WRITE);
+ else
+ err = verify_iovec(msg_sys, iovstack, &address, WRITE);
if (err < 0)
goto out_freeiov;
+ iov = msg_sys->msg_iov;
total_len = err;
err = -ENOBUFS;
@@ -2118,7 +2108,6 @@ out_freectl:
out_freeiov:
if (iov != iovstack)
kfree(iov);
-out:
return err;
}
@@ -2244,28 +2233,18 @@ static int ___sys_recvmsg(struct socket *sock, struct user_msghdr __user *msg,
return err;
}
- if (msg_sys->msg_iovlen > UIO_FASTIOV) {
- err = -EMSGSIZE;
- if (msg_sys->msg_iovlen > UIO_MAXIOV)
- goto out;
- err = -ENOMEM;
- iov = kmalloc(msg_sys->msg_iovlen * sizeof(struct iovec),
- GFP_KERNEL);
- if (!iov)
- goto out;
- }
-
/* Save the user-mode address (verify_iovec will change the
* kernel msghdr to use the kernel address space)
*/
uaddr = (__force void __user *)msg_sys->msg_name;
uaddr_len = COMPAT_NAMELEN(msg);
if (MSG_CMSG_COMPAT & flags)
- err = verify_compat_iovec(msg_sys, iov, &addr, VERIFY_WRITE);
+ err = verify_compat_iovec(msg_sys, iovstack, &addr, READ);
else
- err = verify_iovec(msg_sys, iov, &addr, VERIFY_WRITE);
+ err = verify_iovec(msg_sys, iovstack, &addr, READ);
if (err < 0)
goto out_freeiov;
+ iov = msg_sys->msg_iov;
total_len = err;
cmsg_ptr = (unsigned long)msg_sys->msg_control;
@@ -2306,7 +2285,6 @@ static int ___sys_recvmsg(struct socket *sock, struct user_msghdr __user *msg,
out_freeiov:
if (iov != iovstack)
kfree(iov);
-out:
return err;
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH 3/5] remove a bunch of now-pointless access_ok() in net
From: Al Viro @ 2014-11-18 19:42 UTC (permalink / raw)
To: netdev; +Cc: Linus Torvalds, David Miller, linux-kernel
In-Reply-To: <20141118194053.GA14641@ZenIV.linux.org.uk>
The following set of functions
skb_add_data_nocache
skb_copy_to_page_nocache
skb_do_copy_data_nocache
skb_copy_to_page
skb_add_data
csum_and_copy_from_user
skb_copy_and_csum_datagram
csum_and_copy_to_user
memcpy_fromiovec
memcpy_toiovec
memcpy_toiovecend
memcpy_fromiovecend
are never given a userland range that would not satisfy access_ok().
Proof:
1) skb_add_data_nocache() and skb_copy_to_page_nocache() are called only
by tcp_sendmsg() and given a range covered by ->msg_iov[].
2) skb_do_copy_data_nocache() is called only by skb_add_data_nocache() and
skb_copy_to_page_nocache() and range comes from their arguments.
3) skb_copy_to_page() is never called at all (dead code since 3.0; killed in
the next commit).
4) skb_add_data() is called by rxrpc_send_data() and tcp_send_syn_data(),
both passing it a range covered by ->msg_iov[].
5) all callers of csum_partial_copy_fromiovecend() are giving it iovecs from
->msg_iov[]. Proof: it is called by ip_generic_getfrag() and ping_getfrag(),
both are called only as callbacks by ip_append_data(), ip6_append_data() and
ip_make_skb() and argument passed to those callbacks in all such invocations
is ->msg_iov.
6) csum_and_copy_from_user() is called by skb_add_data(), skb_copy_to_page(),
skb_do_copy_data_nocache() and csum_partial_copy_fromiovecend(). In all cases
the range is covered by ->msg_iov[]
7) skb_copy_and_csum_datagram_iovec() is always getting an iovec from ->msg_iov.
Proof: it is called by tcp_copy_to_iovec(), which gives it tp->ucopy.iov,
and by several recvmsg instances (udp, raw, raw6) which give it ->msg_iov.
But tp->ucopy.iov is initialized only by ->msg_iov.
8) skb_copy_and_csum_datagram() is called only by itself (for fragments)
and by skb_copy_and_csum_datagram_iovec(). The range is covered by the range
passed to caller (in the first case) or by an iovec passed to
skb_copy_and_csum_datagram_iovec().
9) csum_and_copy_to_user() is called only by skb_copy_and_csum_datagram().
Range is covered by the range given to caller...
10) skb_copy_datagram_iovec() is always getting an iovec that would pass
access_ok() on all elements. Proof: cases when ->msg_iov or ->ucopy.iov are
passed are trivial. Other than those, we have
* ppp_read() (single-element iovec, range passed to ->read() has been
validated by caller)
* skb_copy_and_csum_datagram_iovec() (see (7)), itself (covered by
the ranges in array given to its caller)
* rds_tcp_inc_copy_to_user(), which is called only as
->inc_copy_to_user(), which is always given ->msg_iov.
11) aside of the callers of memcpy_toiovec() that immediately pass it ->msg_iov,
there are 3 call sites: one in __qp_memcpy_from_queue() (when called from
qp_memcpy_from_queue_iov()) and two in skb_copy_datagram_iovec(). The latter
is OK due to (10), the former has the call chain coming through
vmci_qpair_dequev() and vmci_transport_stream_dequeue(), which is called as
->stream_dequeue(), which is always given ->msg_iov.
Types in vmw_vmci blow, film at 11...
12) memcpy_toiovecend() is always given a subset of something we'd just given
to ->recvmsg() in ->msg_iov.
13) most of the memcpy_fromiovec() callers are explicitly passing it ->msg_iov.
There are few exceptions:
* l2cap_skbuff_fromiovec(). Called only as bluetooth
->memcpy_fromiovec(), which always gets ->msg_iov as argument.
* __qp_memcpy_to_queue(), from qp_memcpy_to_queue_iov(), from
vmci_qpair_enquev(), from vmci_transport_stream_enqueue(), which is always
called as ->stream_enqueue(), which always gets ->msg_iov. Don't ask me what
I think of vmware...
* ipxrtr_route_packet(), which is always given ->msg_iov by its caller.
* vmci_transport_dgram_enqueue(), which is always called as
->dgram_enqueue(), which always gets ->msg_iov.
* vhost get_indirect(), which is passing it iovec filled by
translate_desc(). Ranges are subsets of those that had been validated by
vq_memory_access_ok() back when we did vhost_set_memory().
14) zerocopy_sg_from_iovec() always gets a validated iovec.
Proof: callers are {tun,macvtap}_get_user(), which are called either from
->aio_write() (and given iovec validated by caller of ->aio_write()), or
from ->sendmsg() (and given ->msg_iov).
15) skb_copy_datagram_from_iovec() always gets an validated iovec.
Proof: for callers in macvtap and tun, same as in (14). Ones in net/unix
and net/packet are given ->msg_iov. Other than those, there's one
in zerocopy_sg_from_iovec() (see (14)) and one in skb_copy_datagram_from_iovec()
itself (fragments handling) - that one gets the iovec its caller was given.
16) callers of memcpy_fromiovecend() are
* {macvtap,tun}_get_user(). Same as (14).
* skb_copy_datagram_from_iovec(). See (15).
* raw_send_hdrinc(), raw6_send_hdrinv(). Both get ->msg_iov as argument
from their callers and pass it to memcpy_fromiovecend().
* sctp_user_addto_chunk(). Ditto.
* tipc_msg_build(). Again, iovec argument is always ->msg_iov.
* ip_generic_getfrag() and udplite_getfrag(). Same as (5).
* vhost_scsi_handle_vq() - that one gets vq->iov as iovec, and
vq->iov is filled ultimately by translate_desc(). Validated by
vq_memory_access_ok() back when we did vhost_set_memory().
And anything that might end up in ->msg_iov[] has to pass access_ok(). It's
trivial
for kernel_{send,recv}msg() users (there we are under set_fs(KERNEL_DS)), it's
verified by rw_copy_check_uvector() in sendmsg()/recvmsg()/sendmmsg()/recvmmsg()
and in the only place where we call ->sendmsg()/->recvmsg() not via net/socket.c
helpers (drivers/vhost/net.c) they are getting vq->iov. As mentioned above,
this one is guaranteed to pass the checks since it's filled by translate_desc(),
and ranges it fills are subsets of ranges that had been validated when we
did vhost_set_memory().
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
arch/alpha/lib/csum_partial_copy.c | 5 ----
arch/frv/lib/checksum.c | 2 +-
arch/m32r/lib/csum_partial_copy.c | 2 +-
arch/mips/include/asm/checksum.h | 30 +++++++--------------
arch/mn10300/lib/checksum.c | 4 +--
arch/parisc/include/asm/checksum.h | 2 +-
arch/parisc/lib/checksum.c | 2 +-
arch/powerpc/lib/checksum_wrappers_64.c | 6 ++---
arch/s390/include/asm/checksum.h | 2 +-
arch/score/include/asm/checksum.h | 2 +-
arch/score/lib/checksum_copy.c | 2 +-
arch/sh/include/asm/checksum_32.h | 8 +-----
arch/sparc/include/asm/checksum_32.h | 43 ++++++++++++++-----------------
arch/x86/include/asm/checksum_32.h | 17 ++++--------
arch/x86/lib/csum-wrappers_64.c | 3 ---
arch/x86/um/asm/checksum.h | 2 +-
arch/x86/um/asm/checksum_32.h | 12 ++++-----
arch/xtensa/include/asm/checksum.h | 8 +-----
include/linux/skbuff.h | 2 +-
include/net/checksum.h | 14 +++-------
include/net/sock.h | 7 +++--
lib/iovec.c | 8 +++---
net/core/iovec.c | 6 ++---
23 files changed, 67 insertions(+), 122 deletions(-)
diff --git a/arch/alpha/lib/csum_partial_copy.c b/arch/alpha/lib/csum_partial_copy.c
index 5675dca..1ee80e8 100644
--- a/arch/alpha/lib/csum_partial_copy.c
+++ b/arch/alpha/lib/csum_partial_copy.c
@@ -338,11 +338,6 @@ csum_partial_copy_from_user(const void __user *src, void *dst, int len,
unsigned long doff = 7 & (unsigned long) dst;
if (len) {
- if (!access_ok(VERIFY_READ, src, len)) {
- if (errp) *errp = -EFAULT;
- memset(dst, 0, len);
- return sum;
- }
if (!doff) {
if (!soff)
checksum = csum_partial_cfu_aligned(
diff --git a/arch/frv/lib/checksum.c b/arch/frv/lib/checksum.c
index 44e16d5..f3fd6cd 100644
--- a/arch/frv/lib/checksum.c
+++ b/arch/frv/lib/checksum.c
@@ -140,7 +140,7 @@ csum_partial_copy_from_user(const void __user *src, void *dst,
if (csum_err)
*csum_err = 0;
- rem = copy_from_user(dst, src, len);
+ rem = __copy_from_user(dst, src, len);
if (rem != 0) {
if (csum_err)
*csum_err = -EFAULT;
diff --git a/arch/m32r/lib/csum_partial_copy.c b/arch/m32r/lib/csum_partial_copy.c
index 5596f3d..f307c53 100644
--- a/arch/m32r/lib/csum_partial_copy.c
+++ b/arch/m32r/lib/csum_partial_copy.c
@@ -47,7 +47,7 @@ csum_partial_copy_from_user (const void __user *src, void *dst,
{
int missing;
- missing = copy_from_user(dst, src, len);
+ missing = __copy_from_user(dst, src, len);
if (missing) {
memset(dst + len - missing, 0, missing);
*err_ptr = -EFAULT;
diff --git a/arch/mips/include/asm/checksum.h b/arch/mips/include/asm/checksum.h
index 3418c51..80f48c4 100644
--- a/arch/mips/include/asm/checksum.h
+++ b/arch/mips/include/asm/checksum.h
@@ -59,13 +59,7 @@ static inline
__wsum csum_and_copy_from_user(const void __user *src, void *dst,
int len, __wsum sum, int *err_ptr)
{
- if (access_ok(VERIFY_READ, src, len))
- return csum_partial_copy_from_user(src, dst, len, sum,
- err_ptr);
- if (len)
- *err_ptr = -EFAULT;
-
- return sum;
+ return csum_partial_copy_from_user(src, dst, len, sum, err_ptr);
}
/*
@@ -77,20 +71,14 @@ __wsum csum_and_copy_to_user(const void *src, void __user *dst, int len,
__wsum sum, int *err_ptr)
{
might_fault();
- if (access_ok(VERIFY_WRITE, dst, len)) {
- if (segment_eq(get_fs(), get_ds()))
- return __csum_partial_copy_kernel(src,
- (__force void *)dst,
- len, sum, err_ptr);
- else
- return __csum_partial_copy_to_user(src,
- (__force void *)dst,
- len, sum, err_ptr);
- }
- if (len)
- *err_ptr = -EFAULT;
-
- return (__force __wsum)-1; /* invalid checksum */
+ if (segment_eq(get_fs(), get_ds()))
+ return __csum_partial_copy_kernel(src,
+ (__force void *)dst,
+ len, sum, err_ptr);
+ else
+ return __csum_partial_copy_to_user(src,
+ (__force void *)dst,
+ len, sum, err_ptr);
}
/*
diff --git a/arch/mn10300/lib/checksum.c b/arch/mn10300/lib/checksum.c
index b6580f5..d080d5e 100644
--- a/arch/mn10300/lib/checksum.c
+++ b/arch/mn10300/lib/checksum.c
@@ -73,7 +73,7 @@ __wsum csum_partial_copy_from_user(const void *src, void *dst,
{
int missing;
- missing = copy_from_user(dst, src, len);
+ missing = __copy_from_user(dst, src, len);
if (missing) {
memset(dst + len - missing, 0, missing);
*err_ptr = -EFAULT;
@@ -89,7 +89,7 @@ __wsum csum_and_copy_to_user(const void *src, void *dst,
{
int missing;
- missing = copy_to_user(dst, src, len);
+ missing = __copy_to_user(dst, src, len);
if (missing) {
memset(dst + len - missing, 0, missing);
*err_ptr = -EFAULT;
diff --git a/arch/parisc/include/asm/checksum.h b/arch/parisc/include/asm/checksum.h
index c84b2fc..6ca9a6f 100644
--- a/arch/parisc/include/asm/checksum.h
+++ b/arch/parisc/include/asm/checksum.h
@@ -198,7 +198,7 @@ static __inline__ __wsum csum_and_copy_to_user(const void *src,
/* code stolen from include/asm-mips64 */
sum = csum_partial(src, len, sum);
- if (copy_to_user(dst, src, len)) {
+ if (__copy_to_user(dst, src, len)) {
*err_ptr = -EFAULT;
return (__force __wsum)-1;
}
diff --git a/arch/parisc/lib/checksum.c b/arch/parisc/lib/checksum.c
index ae66d31..e52fdba 100644
--- a/arch/parisc/lib/checksum.c
+++ b/arch/parisc/lib/checksum.c
@@ -138,7 +138,7 @@ __wsum csum_partial_copy_from_user(const void __user *src,
{
int missing;
- missing = copy_from_user(dst, src, len);
+ missing = __copy_from_user(dst, src, len);
if (missing) {
memset(dst + len - missing, 0, missing);
*err_ptr = -EFAULT;
diff --git a/arch/powerpc/lib/checksum_wrappers_64.c b/arch/powerpc/lib/checksum_wrappers_64.c
index 08e3a33..4c783c8 100644
--- a/arch/powerpc/lib/checksum_wrappers_64.c
+++ b/arch/powerpc/lib/checksum_wrappers_64.c
@@ -37,7 +37,7 @@ __wsum csum_and_copy_from_user(const void __user *src, void *dst,
goto out;
}
- if (unlikely((len < 0) || !access_ok(VERIFY_READ, src, len))) {
+ if (unlikely(len < 0)) {
*err_ptr = -EFAULT;
csum = (__force unsigned int)sum;
goto out;
@@ -78,7 +78,7 @@ __wsum csum_and_copy_to_user(const void *src, void __user *dst, int len,
goto out;
}
- if (unlikely((len < 0) || !access_ok(VERIFY_WRITE, dst, len))) {
+ if (unlikely(len < 0)) {
*err_ptr = -EFAULT;
csum = -1; /* invalid checksum */
goto out;
@@ -90,7 +90,7 @@ __wsum csum_and_copy_to_user(const void *src, void __user *dst, int len,
if (unlikely(*err_ptr)) {
csum = csum_partial(src, len, sum);
- if (copy_to_user(dst, src, len)) {
+ if (__copy_to_user(dst, src, len)) {
*err_ptr = -EFAULT;
csum = -1; /* invalid checksum */
}
diff --git a/arch/s390/include/asm/checksum.h b/arch/s390/include/asm/checksum.h
index 7403648..fdc9532 100644
--- a/arch/s390/include/asm/checksum.h
+++ b/arch/s390/include/asm/checksum.h
@@ -51,7 +51,7 @@ csum_partial_copy_from_user(const void __user *src, void *dst,
int len, __wsum sum,
int *err_ptr)
{
- if (unlikely(copy_from_user(dst, src, len)))
+ if (unlikely(__copy_from_user(dst, src, len)))
*err_ptr = -EFAULT;
return csum_partial(dst, len, sum);
}
diff --git a/arch/score/include/asm/checksum.h b/arch/score/include/asm/checksum.h
index 961bd64..d783634 100644
--- a/arch/score/include/asm/checksum.h
+++ b/arch/score/include/asm/checksum.h
@@ -36,7 +36,7 @@ __wsum csum_and_copy_to_user(const void *src, void __user *dst, int len,
__wsum sum, int *err_ptr)
{
sum = csum_partial(src, len, sum);
- if (copy_to_user(dst, src, len)) {
+ if (__copy_to_user(dst, src, len)) {
*err_ptr = -EFAULT;
return (__force __wsum) -1; /* invalid checksum */
}
diff --git a/arch/score/lib/checksum_copy.c b/arch/score/lib/checksum_copy.c
index 9b770b3..7d6b9b4 100644
--- a/arch/score/lib/checksum_copy.c
+++ b/arch/score/lib/checksum_copy.c
@@ -42,7 +42,7 @@ unsigned int csum_partial_copy_from_user(const char *src, char *dst,
{
int missing;
- missing = copy_from_user(dst, src, len);
+ missing = __copy_from_user(dst, src, len);
if (missing) {
memset(dst + len - missing, 0, missing);
*err_ptr = -EFAULT;
diff --git a/arch/sh/include/asm/checksum_32.h b/arch/sh/include/asm/checksum_32.h
index 14b7ac2..9749277 100644
--- a/arch/sh/include/asm/checksum_32.h
+++ b/arch/sh/include/asm/checksum_32.h
@@ -203,13 +203,7 @@ static inline __wsum csum_and_copy_to_user(const void *src,
int len, __wsum sum,
int *err_ptr)
{
- if (access_ok(VERIFY_WRITE, dst, len))
- return csum_partial_copy_generic((__force const void *)src,
+ return csum_partial_copy_generic((__force const void *)src,
dst, len, sum, NULL, err_ptr);
-
- if (len)
- *err_ptr = -EFAULT;
-
- return (__force __wsum)-1; /* invalid checksum */
}
#endif /* __ASM_SH_CHECKSUM_H */
diff --git a/arch/sparc/include/asm/checksum_32.h b/arch/sparc/include/asm/checksum_32.h
index 426b238..be2e295 100644
--- a/arch/sparc/include/asm/checksum_32.h
+++ b/arch/sparc/include/asm/checksum_32.h
@@ -86,30 +86,25 @@ static inline __wsum
csum_partial_copy_to_user(const void *src, void __user *dst, int len,
__wsum sum, int *err)
{
- if (!access_ok (VERIFY_WRITE, dst, len)) {
- *err = -EFAULT;
- return sum;
- } else {
- register unsigned long ret asm("o0") = (unsigned long)src;
- register char __user *d asm("o1") = dst;
- register int l asm("g1") = len;
- register __wsum s asm("g7") = sum;
-
- __asm__ __volatile__ (
- ".section __ex_table,#alloc\n\t"
- ".align 4\n\t"
- ".word 1f,1\n\t"
- ".previous\n"
- "1:\n\t"
- "call __csum_partial_copy_sparc_generic\n\t"
- " st %8, [%%sp + 64]\n"
- : "=&r" (ret), "=&r" (d), "=&r" (l), "=&r" (s)
- : "0" (ret), "1" (d), "2" (l), "3" (s), "r" (err)
- : "o2", "o3", "o4", "o5", "o7",
- "g2", "g3", "g4", "g5",
- "cc", "memory");
- return (__force __wsum)ret;
- }
+ register unsigned long ret asm("o0") = (unsigned long)src;
+ register char __user *d asm("o1") = dst;
+ register int l asm("g1") = len;
+ register __wsum s asm("g7") = sum;
+
+ __asm__ __volatile__ (
+ ".section __ex_table,#alloc\n\t"
+ ".align 4\n\t"
+ ".word 1f,1\n\t"
+ ".previous\n"
+ "1:\n\t"
+ "call __csum_partial_copy_sparc_generic\n\t"
+ " st %8, [%%sp + 64]\n"
+ : "=&r" (ret), "=&r" (d), "=&r" (l), "=&r" (s)
+ : "0" (ret), "1" (d), "2" (l), "3" (s), "r" (err)
+ : "o2", "o3", "o4", "o5", "o7",
+ "g2", "g3", "g4", "g5",
+ "cc", "memory");
+ return (__force __wsum)ret;
}
#define HAVE_CSUM_COPY_USER
diff --git a/arch/x86/include/asm/checksum_32.h b/arch/x86/include/asm/checksum_32.h
index f50de69..4d96e08 100644
--- a/arch/x86/include/asm/checksum_32.h
+++ b/arch/x86/include/asm/checksum_32.h
@@ -185,18 +185,11 @@ static inline __wsum csum_and_copy_to_user(const void *src,
__wsum ret;
might_sleep();
- if (access_ok(VERIFY_WRITE, dst, len)) {
- stac();
- ret = csum_partial_copy_generic(src, (__force void *)dst,
- len, sum, NULL, err_ptr);
- clac();
- return ret;
- }
-
- if (len)
- *err_ptr = -EFAULT;
-
- return (__force __wsum)-1; /* invalid checksum */
+ stac();
+ ret = csum_partial_copy_generic(src, (__force void *)dst,
+ len, sum, NULL, err_ptr);
+ clac();
+ return ret;
}
#endif /* _ASM_X86_CHECKSUM_32_H */
diff --git a/arch/x86/lib/csum-wrappers_64.c b/arch/x86/lib/csum-wrappers_64.c
index 7609e0e..b6b5626 100644
--- a/arch/x86/lib/csum-wrappers_64.c
+++ b/arch/x86/lib/csum-wrappers_64.c
@@ -26,9 +26,6 @@ csum_partial_copy_from_user(const void __user *src, void *dst,
might_sleep();
*errp = 0;
- if (!likely(access_ok(VERIFY_READ, src, len)))
- goto out_err;
-
/*
* Why 6, not 7? To handle odd addresses aligned we
* would need to do considerable complications to fix the
diff --git a/arch/x86/um/asm/checksum.h b/arch/x86/um/asm/checksum.h
index 4b181b7..3a5166c 100644
--- a/arch/x86/um/asm/checksum.h
+++ b/arch/x86/um/asm/checksum.h
@@ -46,7 +46,7 @@ static __inline__
__wsum csum_partial_copy_from_user(const void __user *src, void *dst,
int len, __wsum sum, int *err_ptr)
{
- if (copy_from_user(dst, src, len)) {
+ if (__copy_from_user(dst, src, len)) {
*err_ptr = -EFAULT;
return (__force __wsum)-1;
}
diff --git a/arch/x86/um/asm/checksum_32.h b/arch/x86/um/asm/checksum_32.h
index ab77b6f..e047748 100644
--- a/arch/x86/um/asm/checksum_32.h
+++ b/arch/x86/um/asm/checksum_32.h
@@ -43,14 +43,12 @@ static __inline__ __wsum csum_and_copy_to_user(const void *src,
void __user *dst,
int len, __wsum sum, int *err_ptr)
{
- if (access_ok(VERIFY_WRITE, dst, len)) {
- if (copy_to_user(dst, src, len)) {
- *err_ptr = -EFAULT;
- return (__force __wsum)-1;
- }
-
- return csum_partial(src, len, sum);
+ if (__copy_to_user(dst, src, len)) {
+ *err_ptr = -EFAULT;
+ return (__force __wsum)-1;
}
+ return csum_partial(src, len, sum);
+}
if (len)
*err_ptr = -EFAULT;
diff --git a/arch/xtensa/include/asm/checksum.h b/arch/xtensa/include/asm/checksum.h
index 0593de6..e2fd018 100644
--- a/arch/xtensa/include/asm/checksum.h
+++ b/arch/xtensa/include/asm/checksum.h
@@ -245,12 +245,6 @@ static __inline__ __wsum csum_and_copy_to_user(const void *src,
void __user *dst, int len,
__wsum sum, int *err_ptr)
{
- if (access_ok(VERIFY_WRITE, dst, len))
- return csum_partial_copy_generic(src,dst,len,sum,NULL,err_ptr);
-
- if (len)
- *err_ptr = -EFAULT;
-
- return (__force __wsum)-1; /* invalid checksum */
+ return csum_partial_copy_generic(src,dst,len,sum,NULL,err_ptr);
}
#endif
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 73c370e..df99075 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2483,7 +2483,7 @@ static inline int skb_add_data(struct sk_buff *skb,
skb->csum = csum_block_add(skb->csum, csum, off);
return 0;
}
- } else if (!copy_from_user(skb_put(skb, copy), from, copy))
+ } else if (!__copy_from_user(skb_put(skb, copy), from, copy))
return 0;
__skb_trim(skb, off);
diff --git a/include/net/checksum.h b/include/net/checksum.h
index 6465bae..3ed85c8 100644
--- a/include/net/checksum.h
+++ b/include/net/checksum.h
@@ -30,13 +30,7 @@ static inline
__wsum csum_and_copy_from_user (const void __user *src, void *dst,
int len, __wsum sum, int *err_ptr)
{
- if (access_ok(VERIFY_READ, src, len))
- return csum_partial_copy_from_user(src, dst, len, sum, err_ptr);
-
- if (len)
- *err_ptr = -EFAULT;
-
- return sum;
+ return csum_partial_copy_from_user(src, dst, len, sum, err_ptr);
}
#endif
@@ -46,10 +40,8 @@ static __inline__ __wsum csum_and_copy_to_user
{
sum = csum_partial(src, len, sum);
- if (access_ok(VERIFY_WRITE, dst, len)) {
- if (copy_to_user(dst, src, len) == 0)
- return sum;
- }
+ if (__copy_to_user(dst, src, len) == 0)
+ return sum;
if (len)
*err_ptr = -EFAULT;
diff --git a/include/net/sock.h b/include/net/sock.h
index 83a669f..94e0ead 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1842,10 +1842,9 @@ static inline int skb_do_copy_data_nocache(struct sock *sk, struct sk_buff *skb,
return err;
skb->csum = csum_block_add(skb->csum, csum, offset);
} else if (sk->sk_route_caps & NETIF_F_NOCACHE_COPY) {
- if (!access_ok(VERIFY_READ, from, copy) ||
- __copy_from_user_nocache(to, from, copy))
+ if (__copy_from_user_nocache(to, from, copy))
return -EFAULT;
- } else if (copy_from_user(to, from, copy))
+ } else if (__copy_from_user(to, from, copy))
return -EFAULT;
return 0;
@@ -1896,7 +1895,7 @@ static inline int skb_copy_to_page(struct sock *sk, char __user *from,
if (err)
return err;
skb->csum = csum_block_add(skb->csum, csum, skb->len);
- } else if (copy_from_user(page_address(page) + off, from, copy))
+ } else if (__copy_from_user(page_address(page) + off, from, copy))
return -EFAULT;
skb->len += copy;
diff --git a/lib/iovec.c b/lib/iovec.c
index df3abd1..021d53f 100644
--- a/lib/iovec.c
+++ b/lib/iovec.c
@@ -13,7 +13,7 @@ int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len)
while (len > 0) {
if (iov->iov_len) {
int copy = min_t(unsigned int, len, iov->iov_len);
- if (copy_from_user(kdata, iov->iov_base, copy))
+ if (__copy_from_user(kdata, iov->iov_base, copy))
return -EFAULT;
len -= copy;
kdata += copy;
@@ -38,7 +38,7 @@ int memcpy_toiovec(struct iovec *iov, unsigned char *kdata, int len)
while (len > 0) {
if (iov->iov_len) {
int copy = min_t(unsigned int, iov->iov_len, len);
- if (copy_to_user(iov->iov_base, kdata, copy))
+ if (__copy_to_user(iov->iov_base, kdata, copy))
return -EFAULT;
kdata += copy;
len -= copy;
@@ -67,7 +67,7 @@ int memcpy_toiovecend(const struct iovec *iov, unsigned char *kdata,
continue;
}
copy = min_t(unsigned int, iov->iov_len - offset, len);
- if (copy_to_user(iov->iov_base + offset, kdata, copy))
+ if (__copy_to_user(iov->iov_base + offset, kdata, copy))
return -EFAULT;
offset = 0;
kdata += copy;
@@ -100,7 +100,7 @@ int memcpy_fromiovecend(unsigned char *kdata, const struct iovec *iov,
int copy = min_t(unsigned int, len, iov->iov_len - offset);
offset = 0;
- if (copy_from_user(kdata, base, copy))
+ if (__copy_from_user(kdata, base, copy))
return -EFAULT;
len -= copy;
kdata += copy;
diff --git a/net/core/iovec.c b/net/core/iovec.c
index 86beeea..4a35b21 100644
--- a/net/core/iovec.c
+++ b/net/core/iovec.c
@@ -97,7 +97,7 @@ int csum_partial_copy_fromiovecend(unsigned char *kdata, struct iovec *iov,
/* iov component is too short ... */
if (par_len > copy) {
- if (copy_from_user(kdata, base, copy))
+ if (__copy_from_user(kdata, base, copy))
goto out_fault;
kdata += copy;
base += copy;
@@ -110,7 +110,7 @@ int csum_partial_copy_fromiovecend(unsigned char *kdata, struct iovec *iov,
partial_cnt, csum);
goto out;
}
- if (copy_from_user(kdata, base, par_len))
+ if (__copy_from_user(kdata, base, par_len))
goto out_fault;
csum = csum_partial(kdata - partial_cnt, 4, csum);
kdata += par_len;
@@ -124,7 +124,7 @@ int csum_partial_copy_fromiovecend(unsigned char *kdata, struct iovec *iov,
partial_cnt = copy % 4;
if (partial_cnt) {
copy -= partial_cnt;
- if (copy_from_user(kdata + copy, base + copy,
+ if (__copy_from_user(kdata + copy, base + copy,
partial_cnt))
goto out_fault;
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH 4/5] bury skb_copy_to_page()
From: Al Viro @ 2014-11-18 19:43 UTC (permalink / raw)
To: netdev; +Cc: Linus Torvalds, David Miller, linux-kernel
In-Reply-To: <20141118194053.GA14641@ZenIV.linux.org.uk>
no callers since 3.0
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
include/net/sock.h | 23 -----------------------
1 file changed, 23 deletions(-)
diff --git a/include/net/sock.h b/include/net/sock.h
index 94e0ead..d7d43e9 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1883,29 +1883,6 @@ static inline int skb_copy_to_page_nocache(struct sock *sk, char __user *from,
return 0;
}
-static inline int skb_copy_to_page(struct sock *sk, char __user *from,
- struct sk_buff *skb, struct page *page,
- int off, int copy)
-{
- if (skb->ip_summed == CHECKSUM_NONE) {
- int err = 0;
- __wsum csum = csum_and_copy_from_user(from,
- page_address(page) + off,
- copy, 0, &err);
- if (err)
- return err;
- skb->csum = csum_block_add(skb->csum, csum, skb->len);
- } else if (__copy_from_user(page_address(page) + off, from, copy))
- return -EFAULT;
-
- skb->len += copy;
- skb->data_len += copy;
- skb->truesize += copy;
- sk->sk_wmem_queued += copy;
- sk_mem_charge(sk, copy);
- return 0;
-}
-
/**
* sk_wmem_alloc_get - returns write allocations
* @sk: socket
--
1.7.10.4
^ permalink raw reply related
* [PATCH 5/5] fold verify_iovec() into copy_msghdr_from_user()
From: Al Viro @ 2014-11-18 19:43 UTC (permalink / raw)
To: netdev; +Cc: Linus Torvalds, David Miller, linux-kernel
In-Reply-To: <20141118194053.GA14641@ZenIV.linux.org.uk>
... and do the same on the compat side of things.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
include/linux/socket.h | 1 -
include/net/compat.h | 5 ++-
net/compat.c | 52 ++++++++++++---------------
net/core/iovec.c | 38 --------------------
net/socket.c | 93 +++++++++++++++++++++++++++---------------------
5 files changed, 77 insertions(+), 112 deletions(-)
diff --git a/include/linux/socket.h b/include/linux/socket.h
index 51bd666..de52228 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -322,7 +322,6 @@ extern int csum_partial_copy_fromiovecend(unsigned char *kdata,
extern unsigned long iov_pages(const struct iovec *iov, int offset,
unsigned long nr_segs);
-extern int verify_iovec(struct msghdr *m, struct iovec *iov, struct sockaddr_storage *address, int mode);
extern int move_addr_to_kernel(void __user *uaddr, int ulen, struct sockaddr_storage *kaddr);
extern int put_cmsg(struct msghdr*, int level, int type, int len, void *data);
diff --git a/include/net/compat.h b/include/net/compat.h
index 3b603b1..42a9c84 100644
--- a/include/net/compat.h
+++ b/include/net/compat.h
@@ -40,9 +40,8 @@ int compat_sock_get_timestampns(struct sock *, struct timespec __user *);
#define compat_mmsghdr mmsghdr
#endif /* defined(CONFIG_COMPAT) */
-int get_compat_msghdr(struct msghdr *, struct compat_msghdr __user *);
-int verify_compat_iovec(struct msghdr *, struct iovec *,
- struct sockaddr_storage *, int);
+ssize_t get_compat_msghdr(struct msghdr *, struct compat_msghdr __user *,
+ struct sockaddr __user **, struct iovec **);
asmlinkage long compat_sys_sendmsg(int, struct compat_msghdr __user *,
unsigned int);
asmlinkage long compat_sys_sendmmsg(int, struct compat_mmsghdr __user *,
diff --git a/net/compat.c b/net/compat.c
index 7b4b6ad..062f157 100644
--- a/net/compat.c
+++ b/net/compat.c
@@ -31,14 +31,18 @@
#include <asm/uaccess.h>
#include <net/compat.h>
-int get_compat_msghdr(struct msghdr *kmsg, struct compat_msghdr __user *umsg)
+ssize_t get_compat_msghdr(struct msghdr *kmsg,
+ struct compat_msghdr __user *umsg,
+ struct sockaddr __user **save_addr,
+ struct iovec **iov)
{
- compat_uptr_t tmp1, tmp2, tmp3;
+ compat_uptr_t uaddr, uiov, tmp3;
+ ssize_t err;
if (!access_ok(VERIFY_READ, umsg, sizeof(*umsg)) ||
- __get_user(tmp1, &umsg->msg_name) ||
+ __get_user(uaddr, &umsg->msg_name) ||
__get_user(kmsg->msg_namelen, &umsg->msg_namelen) ||
- __get_user(tmp2, &umsg->msg_iov) ||
+ __get_user(uiov, &umsg->msg_iov) ||
__get_user(kmsg->msg_iovlen, &umsg->msg_iovlen) ||
__get_user(tmp3, &umsg->msg_control) ||
__get_user(kmsg->msg_controllen, &umsg->msg_controllen) ||
@@ -46,44 +50,32 @@ int get_compat_msghdr(struct msghdr *kmsg, struct compat_msghdr __user *umsg)
return -EFAULT;
if (kmsg->msg_namelen > sizeof(struct sockaddr_storage))
kmsg->msg_namelen = sizeof(struct sockaddr_storage);
- kmsg->msg_name = compat_ptr(tmp1);
- kmsg->msg_iov = compat_ptr(tmp2);
kmsg->msg_control = compat_ptr(tmp3);
- return 0;
-}
-/* I've named the args so it is easy to tell whose space the pointers are in. */
-int verify_compat_iovec(struct msghdr *kern_msg, struct iovec *iov,
- struct sockaddr_storage *kern_address, int mode)
-{
- struct compat_iovec __user *p;
- struct iovec *res;
- int err;
+ if (save_addr)
+ *save_addr = compat_ptr(uaddr);
- if (kern_msg->msg_name && kern_msg->msg_namelen) {
- if (mode == WRITE) {
- int err = move_addr_to_kernel(kern_msg->msg_name,
- kern_msg->msg_namelen,
- kern_address);
+ if (uaddr && kmsg->msg_namelen) {
+ if (!save_addr) {
+ err = move_addr_to_kernel(compat_ptr(uaddr),
+ kmsg->msg_namelen,
+ kmsg->msg_name);
if (err < 0)
return err;
}
- kern_msg->msg_name = kern_address;
} else {
- kern_msg->msg_name = NULL;
- kern_msg->msg_namelen = 0;
+ kmsg->msg_name = NULL;
+ kmsg->msg_namelen = 0;
}
- if (kern_msg->msg_iovlen > UIO_MAXIOV)
+ if (kmsg->msg_iovlen > UIO_MAXIOV)
return -EMSGSIZE;
- p = (struct compat_iovec __user *)kern_msg->msg_iov;
- err = compat_rw_copy_check_uvector(mode, p, kern_msg->msg_iovlen,
- UIO_FASTIOV, iov, &res);
+ err = compat_rw_copy_check_uvector(save_addr ? READ : WRITE,
+ compat_ptr(uiov), kmsg->msg_iovlen,
+ UIO_FASTIOV, *iov, iov);
if (err >= 0)
- kern_msg->msg_iov = res;
- else if (res != iov)
- kfree(res);
+ kmsg->msg_iov = *iov;
return err;
}
diff --git a/net/core/iovec.c b/net/core/iovec.c
index 4a35b21..fa3eed9 100644
--- a/net/core/iovec.c
+++ b/net/core/iovec.c
@@ -28,44 +28,6 @@
#include <net/sock.h>
/*
- * Verify iovec. The caller must ensure that the iovec is big enough
- * to hold the message iovec.
- *
- * Save time not doing access_ok. copy_*_user will make this work
- * in any case.
- */
-
-int verify_iovec(struct msghdr *m, struct iovec *iov, struct sockaddr_storage *address, int mode)
-{
- struct iovec *res;
- int err;
-
- if (m->msg_name && m->msg_namelen) {
- if (mode == WRITE) {
- void __user *namep = (void __user __force *)m->msg_name;
- int err = move_addr_to_kernel(namep, m->msg_namelen,
- address);
- if (err < 0)
- return err;
- }
- m->msg_name = address;
- } else {
- m->msg_name = NULL;
- m->msg_namelen = 0;
- }
- if (m->msg_iovlen > UIO_MAXIOV)
- return -EMSGSIZE;
-
- err = rw_copy_check_uvector(mode, (void __user __force *)m->msg_iov,
- m->msg_iovlen, UIO_FASTIOV, iov, &res);
- if (err >= 0)
- m->msg_iov = res;
- else if (res != iov)
- kfree(res);
- return err;
-}
-
-/*
* And now for the all-in-one: copy and checksum from a user iovec
* directly to a datagram
* Calls to csum_partial but the last must be in 32 bit chunks
diff --git a/net/socket.c b/net/socket.c
index 59020f0..ee3ee39 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1988,16 +1988,26 @@ struct used_address {
unsigned int name_len;
};
-static int copy_msghdr_from_user(struct msghdr *kmsg,
- struct user_msghdr __user *umsg)
+static ssize_t copy_msghdr_from_user(struct msghdr *kmsg,
+ struct user_msghdr __user *umsg,
+ struct sockaddr __user **save_addr,
+ struct iovec **iov)
{
- /* We are relying on the (currently) identical layouts. Once
- * the kernel-side changes, this place will need to be updated
- */
- if (copy_from_user(kmsg, umsg, sizeof(struct msghdr)))
+ struct sockaddr __user *uaddr;
+ struct iovec __user *uiov;
+ ssize_t err;
+
+ if (!access_ok(VERIFY_READ, umsg, sizeof(*umsg)) ||
+ __get_user(uaddr, &umsg->msg_name) ||
+ __get_user(kmsg->msg_namelen, &umsg->msg_namelen) ||
+ __get_user(uiov, &umsg->msg_iov) ||
+ __get_user(kmsg->msg_iovlen, &umsg->msg_iovlen) ||
+ __get_user(kmsg->msg_control, &umsg->msg_control) ||
+ __get_user(kmsg->msg_controllen, &umsg->msg_controllen) ||
+ __get_user(kmsg->msg_flags, &umsg->msg_flags))
return -EFAULT;
- if (kmsg->msg_name == NULL)
+ if (!uaddr)
kmsg->msg_namelen = 0;
if (kmsg->msg_namelen < 0)
@@ -2005,7 +2015,31 @@ static int copy_msghdr_from_user(struct msghdr *kmsg,
if (kmsg->msg_namelen > sizeof(struct sockaddr_storage))
kmsg->msg_namelen = sizeof(struct sockaddr_storage);
- return 0;
+
+ if (save_addr)
+ *save_addr = uaddr;
+
+ if (uaddr && kmsg->msg_namelen) {
+ if (!save_addr) {
+ err = move_addr_to_kernel(uaddr, kmsg->msg_namelen,
+ kmsg->msg_name);
+ if (err < 0)
+ return err;
+ }
+ } else {
+ kmsg->msg_name = NULL;
+ kmsg->msg_namelen = 0;
+ }
+
+ if (kmsg->msg_iovlen > UIO_MAXIOV)
+ return -EMSGSIZE;
+
+ err = rw_copy_check_uvector(save_addr ? READ : WRITE,
+ uiov, kmsg->msg_iovlen,
+ UIO_FASTIOV, *iov, iov);
+ if (err >= 0)
+ kmsg->msg_iov = *iov;
+ return err;
}
static int ___sys_sendmsg(struct socket *sock, struct user_msghdr __user *msg,
@@ -2020,26 +2054,17 @@ static int ___sys_sendmsg(struct socket *sock, struct user_msghdr __user *msg,
__attribute__ ((aligned(sizeof(__kernel_size_t))));
/* 20 is size of ipv6_pktinfo */
unsigned char *ctl_buf = ctl;
- int err, ctl_len, total_len;
+ int ctl_len, total_len;
+ ssize_t err;
- err = -EFAULT;
- if (MSG_CMSG_COMPAT & flags) {
- if (get_compat_msghdr(msg_sys, msg_compat))
- return -EFAULT;
- } else {
- err = copy_msghdr_from_user(msg_sys, msg);
- if (err)
- return err;
- }
+ msg_sys->msg_name = &address;
- /* This will also move the address data into kernel space */
if (MSG_CMSG_COMPAT & flags)
- err = verify_compat_iovec(msg_sys, iovstack, &address, WRITE);
+ err = get_compat_msghdr(msg_sys, msg_compat, NULL, &iov);
else
- err = verify_iovec(msg_sys, iovstack, &address, WRITE);
+ err = copy_msghdr_from_user(msg_sys, msg, NULL, &iov);
if (err < 0)
goto out_freeiov;
- iov = msg_sys->msg_iov;
total_len = err;
err = -ENOBUFS;
@@ -2215,36 +2240,24 @@ static int ___sys_recvmsg(struct socket *sock, struct user_msghdr __user *msg,
struct iovec iovstack[UIO_FASTIOV];
struct iovec *iov = iovstack;
unsigned long cmsg_ptr;
- int err, total_len, len;
+ int total_len, len;
+ ssize_t err;
/* kernel mode address */
struct sockaddr_storage addr;
/* user mode address pointers */
struct sockaddr __user *uaddr;
- int __user *uaddr_len;
+ int __user *uaddr_len = COMPAT_NAMELEN(msg);
- if (MSG_CMSG_COMPAT & flags) {
- if (get_compat_msghdr(msg_sys, msg_compat))
- return -EFAULT;
- } else {
- err = copy_msghdr_from_user(msg_sys, msg);
- if (err)
- return err;
- }
+ msg_sys->msg_name = &addr;
- /* Save the user-mode address (verify_iovec will change the
- * kernel msghdr to use the kernel address space)
- */
- uaddr = (__force void __user *)msg_sys->msg_name;
- uaddr_len = COMPAT_NAMELEN(msg);
if (MSG_CMSG_COMPAT & flags)
- err = verify_compat_iovec(msg_sys, iovstack, &addr, READ);
+ err = get_compat_msghdr(msg_sys, msg_compat, &uaddr, &iov);
else
- err = verify_iovec(msg_sys, iovstack, &addr, READ);
+ err = copy_msghdr_from_user(msg_sys, msg, &uaddr, &iov);
if (err < 0)
goto out_freeiov;
- iov = msg_sys->msg_iov;
total_len = err;
cmsg_ptr = (unsigned long)msg_sys->msg_control;
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH] ipv6: delete protocol when cleanup
From: Cong Wang @ 2014-11-18 19:46 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Duan Jiong, David Miller, netdev
In-Reply-To: <1416338055.14060.24.camel@edumazet-glaptop2.roam.corp.google.com>
On Tue, Nov 18, 2014 at 11:14 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Tue, 2014-11-18 at 10:56 -0800, Cong Wang wrote:
>> On Tue, Nov 18, 2014 at 2:35 AM, Duan Jiong <duanj.fnst@cn.fujitsu.com> wrote:
>> >
>> > pim6_protocol was added when initiation, but it not deleted.
>> >
>> > Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
>
> ...
>
>> Looks like rtnl_unregister() is missing as well.
>
> Is it really worth bothering ?
>
> May I remind you we took the decision one year ago to not bother about
> removing ipv6 module.
>
It is still called in error path of init, so just for completeness.
^ 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