* Re: [PATCH tip/core/rcu 21/21] drivers/vhost: Remove now-redundant read_barrier_depends()
From: Paul E. McKenney @ 2017-12-05 23:39 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Peter Zijlstra, linux-kernel, mingo, jiangshanlai, dipankar, akpm,
mathieu.desnoyers, josh, tglx, rostedt, dhowells, edumazet,
fweisbec, oleg, Jason Wang, kvm, virtualization, netdev
In-Reply-To: <20171206000541-mutt-send-email-mst@kernel.org>
On Wed, Dec 06, 2017 at 12:09:36AM +0200, Michael S. Tsirkin wrote:
> On Tue, Dec 05, 2017 at 10:57:00PM +0100, Peter Zijlstra wrote:
> > On Tue, Dec 05, 2017 at 11:24:49PM +0200, Michael S. Tsirkin wrote:
> > > READ_ONCE is really all over the place (some code literally replaced all
> > > memory accesses with READ/WRITE ONCE).
> >
> > Yeah, so?
>
> Oh my point was I can't just look for READ_ONCE and go
> *that's the pair*. there are too many of these.
> At Paul's suggestion I will document the pairing *this read once has a
> barrier that is paired with that barrier*.
>
> > Complain to the compiler people for forcing us into that.
>
> In some cases when you end up with all accesses
> going through read/write once volatile just might better.
That is in fact what the jiffies counter does. But you lose READ_ONCE()'s
automatic handling of DEC Alpha when you take that approach.
> > > Would an API like WRITE_POINTER()/smp_store_pointer make sense,
> > > and READ_POINTER for symmetry?
> >
> > No, the whole point of the exercise was to get away from the fact that
> > dependent loads are special.
>
> It's a pity that dependent stores are still special.
We can make READ_ONCE() not be special at zero cost on non-Alpha
systems, but both smp_wmb() and smp_store_release() are decidedly
not free of added overhead.
Thanx, Paul
^ permalink raw reply
* Re: [PATCH V11 3/5] printk: hash addresses printed with %p
From: Linus Torvalds @ 2017-12-05 23:33 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Tobin C. Harding, kernel-hardening@lists.openwall.com,
Jason A. Donenfeld, Theodore Ts'o, Kees Cook, Paolo Bonzini,
Tycho Andersen, Roberts, William C, Tejun Heo, Jordan Glover,
Greg KH, Petr Mladek, Joe Perches, Ian Campbell,
Sergey Senozhatsky, Catalin Marinas, Will Deacon, Steven Rostedt,
Chris Fries
In-Reply-To: <CAMuHMdVuc5yUNXhShb_vaR6T=qNexXm38mqBALeWzTNyGdHe+Q@mail.gmail.com>
On Tue, Dec 5, 2017 at 2:57 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Lowest 3 is good enough for all natural types, up to long long.
> We may still receive complaints from people who care about seeing if
> a pointer is cacheline-aligned or not. Fixing that may need up to 7 bits, I'm
> afraid, which is a bit too much to give up.
I really think even the lowest three is a bit too much.
Who really cares? If it's something that is particularly _about_
alignment (ie an alignment trap), maybe just print out those bits
separately.
And for everything else? It's purely about getting used to it.
I will just cut-and-paste what I wrote in another thread about the hashing:
I'm going to require some actual proof of an actual case where it
mattered that a particular value was hashed.
Not hand-waving.
Not "it surprised and confused me" because it looked different. You'll
get used to it.
So an actual "this was critical information that mattered for this
particular bug, and it was missing due to the hashing of this
particular value and debugging was harder in actual reality due to
that".
Because the actual example I have seen so far, not only didn't the
hashing matter AT ALL, most of the _unhashed_ values shouldn't have
been there either, and were due to arm still printing stuff that
shouldn't have been printed at all and just made the oops more complex
and harder to read and report.
This subject is really easy to bike-shed around. Everybody can have an
opinion. I want actual hard data and facts, not opinions.
And if the hashing _really_ is a problem, we'll just change that
particular thing to %px. But it needs actual hard data and real
reasons first, ok?
Linus
Linus
^ permalink raw reply
* RE: [PATCH net-next 1/1] net: dsa: microchip: Add Microchip KSZ8895 DSA driver
From: Tristram.Ha @ 2017-12-05 23:29 UTC (permalink / raw)
To: pavel
Cc: andrew, f.fainelli, muvarov, nathan.leigh.conrad, vivien.didelot,
UNGLinuxDriver, netdev
In-Reply-To: <20171205222342.GC24945@amd>
> > Sorry to be this late for the reply. I finally got hold of a KSZ8895 board that
> > works with my SoC board to confirm the network communication.
> >
> > As expected the KSZ8895 board works correctly as the chip uses the same
> > tail tagging feature in KSZ8795, and I did verify that board is working.
> >
> > One thing to debug this problem is to dump the MIB counters. Use the
> ethtool
> > utility to show MIB counters of both ports:
> >
> > ethtool -S lan3
> > ethtool -S eth0
> >
> > Assuming eth0 is the MAC controller that drives the switch, the receive
> counters of
> > the host port of the switch should match the transmit counters of
> > lan3, and vice versa.
>
> Thanks for reply. I'll get to the tests shortly. Could I get .dts
> snippet that works for you and commands you are using for testing?
>
You said your previous patch works, so I do not think there is anything wrong
with the device tree, unless you are using a completely different one.
The tricky part of network communication is the RMII/MII interface where the
host port connects to the MAC controller. But again your patch works so there is
nothing wrong with the hardware.
I also use a simple setup to test the network:
ifconfig eth0 up
ifconfig lan1 192.168.0.1
ping -c 2 192.168.0.100
If I want a complete test I setup a bridge:
ifconfig eth0 up
ifconfig lan1 up
ifconfig lan2 up
ifconfig lan3 up
ifconfig lan4 up
brctl addbr br0
brctl addif br0 lan1
brctl addif br0 lan2
brctl addif br0 lan3
brctl addif br0 lan4
ifconfig br0 192.168.0.1
ping -c 2 192.168.0.100
^ permalink raw reply
* Re: [RFC][PATCHES] sock_alloc_file() cleanups and fixes
From: Al Viro @ 2017-12-05 23:29 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20171205.144443.2120550314318217897.davem@davemloft.net>
On Tue, Dec 05, 2017 at 02:44:43PM -0500, David Miller wrote:
> From: Al Viro <viro@ZenIV.linux.org.uk>
> Date: Mon, 4 Dec 2017 16:41:01 +0000
>
> > On Mon, Dec 04, 2017 at 10:35:24AM -0500, David Miller wrote:
> >> From: Al Viro <viro@ZenIV.linux.org.uk>
> >> Date: Fri, 1 Dec 2017 00:20:27 +0000
> >>
> >> > 1) massage sys_socketpair() (should be a pure cleanup)
> >> > 2) fix and clean up kcm_clone() (-stable fodder)
> >> > 3) switch sock_alloc_file() to new calling conventions.
> >> >
> >> > It got some local testing, but it certainly needs more review.
> >> > Diffstat for the entire thing is
> >>
> >> Series looks great to me:
> >>
> >> Acked-by: David S. Miller <davem@davemloft.net>
> >
> > How do you prefer it to be handled? KCM one should go into everything
> > since 4.6 (with trivial modifications in 4.11 and 4.12 - both had
> > massaged the place around the call of kcm_clone() a bit, but this fix
> > overwrites the entire area and that can be dropped into earlier
> > kernels without any problems). I've put that into vfs.git#net-fixes
> > and have the other two in vfs.git#for-davem on top of that, with
> > you merging the latter into net-next.git and the former - into net.git.
> > Is that OK with you, or would you prefer some other way of handling
> > that kind of stuff?
>
> Why don't you resubmit this to netdev as a non-RFC, I'll queue it up to
> 'net' and -stable as well.
Sent...
^ permalink raw reply
* [PATCH 3/3] make sock_alloc_file() do sock_release() on failures
From: Al Viro @ 2017-12-05 23:29 UTC (permalink / raw)
To: netdev
This changes calling conventions (and simplifies the hell out
the callers). New rules: once struct socket had been passed
to sock_alloc_file(), it's been consumed either by struct file
or by sock_release() done by sock_alloc_file(). Either way
the caller should not do sock_release() after that point.
Reviewed-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
drivers/staging/lustre/lnet/lnet/lib-socket.c | 8 ++------
net/9p/trans_fd.c | 1 -
net/kcm/kcmsock.c | 7 +------
net/sctp/socket.c | 1 -
net/socket.c | 25 ++++++++-----------------
5 files changed, 11 insertions(+), 31 deletions(-)
diff --git a/drivers/staging/lustre/lnet/lnet/lib-socket.c b/drivers/staging/lustre/lnet/lnet/lib-socket.c
index 539a26444f31..7d49d4865298 100644
--- a/drivers/staging/lustre/lnet/lnet/lib-socket.c
+++ b/drivers/staging/lustre/lnet/lnet/lib-socket.c
@@ -71,16 +71,12 @@ lnet_sock_ioctl(int cmd, unsigned long arg)
}
sock_filp = sock_alloc_file(sock, 0, NULL);
- if (IS_ERR(sock_filp)) {
- sock_release(sock);
- rc = PTR_ERR(sock_filp);
- goto out;
- }
+ if (IS_ERR(sock_filp))
+ return PTR_ERR(sock_filp);
rc = kernel_sock_unlocked_ioctl(sock_filp, cmd, arg);
fput(sock_filp);
-out:
return rc;
}
diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c
index 985046ae4231..80f5c79053a4 100644
--- a/net/9p/trans_fd.c
+++ b/net/9p/trans_fd.c
@@ -839,7 +839,6 @@ static int p9_socket_open(struct p9_client *client, struct socket *csocket)
if (IS_ERR(file)) {
pr_err("%s (%d): failed to map fd\n",
__func__, task_pid_nr(current));
- sock_release(csocket);
kfree(p);
return PTR_ERR(file);
}
diff --git a/net/kcm/kcmsock.c b/net/kcm/kcmsock.c
index c5fa634e63ca..d4e98f20fc2a 100644
--- a/net/kcm/kcmsock.c
+++ b/net/kcm/kcmsock.c
@@ -1629,7 +1629,6 @@ static struct file *kcm_clone(struct socket *osock)
{
struct socket *newsock;
struct sock *newsk;
- struct file *file;
newsock = sock_alloc();
if (!newsock)
@@ -1649,11 +1648,7 @@ static struct file *kcm_clone(struct socket *osock)
sock_init_data(newsock, newsk);
init_kcm_sock(kcm_sk(newsk), kcm_sk(osock->sk)->mux);
- file = sock_alloc_file(newsock, 0, osock->sk->sk_prot_creator->name);
- if (IS_ERR(file))
- sock_release(newsock);
-
- return file;
+ return sock_alloc_file(newsock, 0, osock->sk->sk_prot_creator->name);
}
static int kcm_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 3204a9b29407..8bb5163d6331 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -5080,7 +5080,6 @@ static int sctp_getsockopt_peeloff_common(struct sock *sk, sctp_peeloff_arg_t *p
*newfile = sock_alloc_file(newsock, 0, NULL);
if (IS_ERR(*newfile)) {
put_unused_fd(retval);
- sock_release(newsock);
retval = PTR_ERR(*newfile);
*newfile = NULL;
return retval;
diff --git a/net/socket.c b/net/socket.c
index 2df83c0bfde9..05f361faec45 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -406,8 +406,10 @@ struct file *sock_alloc_file(struct socket *sock, int flags, const char *dname)
name.len = strlen(name.name);
}
path.dentry = d_alloc_pseudo(sock_mnt->mnt_sb, &name);
- if (unlikely(!path.dentry))
+ if (unlikely(!path.dentry)) {
+ sock_release(sock);
return ERR_PTR(-ENOMEM);
+ }
path.mnt = mntget(sock_mnt);
d_instantiate(path.dentry, SOCK_INODE(sock));
@@ -415,9 +417,11 @@ struct file *sock_alloc_file(struct socket *sock, int flags, const char *dname)
file = alloc_file(&path, FMODE_READ | FMODE_WRITE,
&socket_file_ops);
if (IS_ERR(file)) {
- /* drop dentry, keep inode */
+ /* drop dentry, keep inode for a bit */
ihold(d_inode(path.dentry));
path_put(&path);
+ /* ... and now kill it properly */
+ sock_release(sock);
return file;
}
@@ -1330,19 +1334,9 @@ SYSCALL_DEFINE3(socket, int, family, int, type, int, protocol)
retval = sock_create(family, type, protocol, &sock);
if (retval < 0)
- goto out;
-
- retval = sock_map_fd(sock, flags & (O_CLOEXEC | O_NONBLOCK));
- if (retval < 0)
- goto out_release;
-
-out:
- /* It may be already another descriptor 8) Not kernel problem. */
- return retval;
+ return retval;
-out_release:
- sock_release(sock);
- return retval;
+ return sock_map_fd(sock, flags & (O_CLOEXEC | O_NONBLOCK));
}
/*
@@ -1412,7 +1406,6 @@ SYSCALL_DEFINE4(socketpair, int, family, int, type, int, protocol,
newfile1 = sock_alloc_file(sock1, flags, NULL);
if (IS_ERR(newfile1)) {
err = PTR_ERR(newfile1);
- sock_release(sock1);
sock_release(sock2);
goto out;
}
@@ -1420,7 +1413,6 @@ SYSCALL_DEFINE4(socketpair, int, family, int, type, int, protocol,
newfile2 = sock_alloc_file(sock2, flags, NULL);
if (IS_ERR(newfile2)) {
err = PTR_ERR(newfile2);
- sock_release(sock2);
fput(newfile1);
goto out;
}
@@ -1549,7 +1541,6 @@ SYSCALL_DEFINE4(accept4, int, fd, struct sockaddr __user *, upeer_sockaddr,
if (IS_ERR(newfile)) {
err = PTR_ERR(newfile);
put_unused_fd(newfd);
- sock_release(newsock);
goto out_put;
}
--
2.11.0
^ permalink raw reply related
* [PATCH 2/3] socketpair(): allocate descriptors first
From: Al Viro @ 2017-12-05 23:28 UTC (permalink / raw)
To: netdev
simplifies failure exits considerably...
Reviewed-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
net/socket.c | 89 ++++++++++++++++++++++++++----------------------------------
1 file changed, 38 insertions(+), 51 deletions(-)
diff --git a/net/socket.c b/net/socket.c
index 42d8e9c9ccd5..2df83c0bfde9 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1366,87 +1366,74 @@ SYSCALL_DEFINE4(socketpair, int, family, int, type, int, protocol,
flags = (flags & ~SOCK_NONBLOCK) | O_NONBLOCK;
/*
+ * reserve descriptors and make sure we won't fail
+ * to return them to userland.
+ */
+ fd1 = get_unused_fd_flags(flags);
+ if (unlikely(fd1 < 0))
+ return fd1;
+
+ fd2 = get_unused_fd_flags(flags);
+ if (unlikely(fd2 < 0)) {
+ put_unused_fd(fd1);
+ return fd2;
+ }
+
+ err = put_user(fd1, &usockvec[0]);
+ if (err)
+ goto out;
+
+ err = put_user(fd2, &usockvec[1]);
+ if (err)
+ goto out;
+
+ /*
* Obtain the first socket and check if the underlying protocol
* supports the socketpair call.
*/
err = sock_create(family, type, protocol, &sock1);
- if (err < 0)
+ if (unlikely(err < 0))
goto out;
err = sock_create(family, type, protocol, &sock2);
- if (err < 0)
- goto out_release_1;
-
- err = sock1->ops->socketpair(sock1, sock2);
- if (err < 0)
- goto out_release_both;
-
- fd1 = get_unused_fd_flags(flags);
- if (unlikely(fd1 < 0)) {
- err = fd1;
- goto out_release_both;
+ if (unlikely(err < 0)) {
+ sock_release(sock1);
+ goto out;
}
- fd2 = get_unused_fd_flags(flags);
- if (unlikely(fd2 < 0)) {
- err = fd2;
- goto out_put_unused_1;
+ err = sock1->ops->socketpair(sock1, sock2);
+ if (unlikely(err < 0)) {
+ sock_release(sock2);
+ sock_release(sock1);
+ goto out;
}
newfile1 = sock_alloc_file(sock1, flags, NULL);
if (IS_ERR(newfile1)) {
err = PTR_ERR(newfile1);
- goto out_put_unused_both;
+ sock_release(sock1);
+ sock_release(sock2);
+ goto out;
}
newfile2 = sock_alloc_file(sock2, flags, NULL);
if (IS_ERR(newfile2)) {
err = PTR_ERR(newfile2);
- goto out_fput_1;
+ sock_release(sock2);
+ fput(newfile1);
+ goto out;
}
- err = put_user(fd1, &usockvec[0]);
- if (err)
- goto out_fput_both;
-
- err = put_user(fd2, &usockvec[1]);
- if (err)
- goto out_fput_both;
-
audit_fd_pair(fd1, fd2);
fd_install(fd1, newfile1);
fd_install(fd2, newfile2);
- /* fd1 and fd2 may be already another descriptors.
- * Not kernel problem.
- */
-
return 0;
-out_fput_both:
- fput(newfile2);
- fput(newfile1);
- put_unused_fd(fd2);
- put_unused_fd(fd1);
- goto out;
-
-out_fput_1:
- fput(newfile1);
- put_unused_fd(fd2);
- put_unused_fd(fd1);
- sock_release(sock2);
- goto out;
-
-out_put_unused_both:
+out:
put_unused_fd(fd2);
-out_put_unused_1:
put_unused_fd(fd1);
-out_release_both:
- sock_release(sock2);
-out_release_1:
- sock_release(sock1);
-out:
return err;
}
--
2.11.0
^ permalink raw reply related
* [PATCH 1/3] fix kcm_clone()
From: Al Viro @ 2017-12-05 23:27 UTC (permalink / raw)
To: netdev
1) it's fput() or sock_release(), not both
2) don't do fd_install() until the last failure exit.
3) not a bug per se, but... don't attach socket to struct file
until it's set up.
Take reserving descriptor into the caller, move fd_install() to the
caller, sanitize failure exits and calling conventions.
Cc: stable@vger.kernel.org # v4.6+
Acked-by: Tom Herbert <tom@herbertland.com>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
net/kcm/kcmsock.c | 71 +++++++++++++++++++++----------------------------------
1 file changed, 27 insertions(+), 44 deletions(-)
diff --git a/net/kcm/kcmsock.c b/net/kcm/kcmsock.c
index 0b750a22c4b9..c5fa634e63ca 100644
--- a/net/kcm/kcmsock.c
+++ b/net/kcm/kcmsock.c
@@ -1625,60 +1625,35 @@ static struct proto kcm_proto = {
};
/* Clone a kcm socket. */
-static int kcm_clone(struct socket *osock, struct kcm_clone *info,
- struct socket **newsockp)
+static struct file *kcm_clone(struct socket *osock)
{
struct socket *newsock;
struct sock *newsk;
- struct file *newfile;
- int err, newfd;
+ struct file *file;
- err = -ENFILE;
newsock = sock_alloc();
if (!newsock)
- goto out;
+ return ERR_PTR(-ENFILE);
newsock->type = osock->type;
newsock->ops = osock->ops;
__module_get(newsock->ops->owner);
- newfd = get_unused_fd_flags(0);
- if (unlikely(newfd < 0)) {
- err = newfd;
- goto out_fd_fail;
- }
-
- newfile = sock_alloc_file(newsock, 0, osock->sk->sk_prot_creator->name);
- if (IS_ERR(newfile)) {
- err = PTR_ERR(newfile);
- goto out_sock_alloc_fail;
- }
-
newsk = sk_alloc(sock_net(osock->sk), PF_KCM, GFP_KERNEL,
&kcm_proto, true);
if (!newsk) {
- err = -ENOMEM;
- goto out_sk_alloc_fail;
+ sock_release(newsock);
+ return ERR_PTR(-ENOMEM);
}
-
sock_init_data(newsock, newsk);
init_kcm_sock(kcm_sk(newsk), kcm_sk(osock->sk)->mux);
- fd_install(newfd, newfile);
- *newsockp = newsock;
- info->fd = newfd;
-
- return 0;
+ file = sock_alloc_file(newsock, 0, osock->sk->sk_prot_creator->name);
+ if (IS_ERR(file))
+ sock_release(newsock);
-out_sk_alloc_fail:
- fput(newfile);
-out_sock_alloc_fail:
- put_unused_fd(newfd);
-out_fd_fail:
- sock_release(newsock);
-out:
- return err;
+ return file;
}
static int kcm_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
@@ -1708,17 +1683,25 @@ static int kcm_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
}
case SIOCKCMCLONE: {
struct kcm_clone info;
- struct socket *newsock = NULL;
-
- err = kcm_clone(sock, &info, &newsock);
- if (!err) {
- if (copy_to_user((void __user *)arg, &info,
- sizeof(info))) {
- err = -EFAULT;
- sys_close(info.fd);
- }
- }
+ struct file *file;
+
+ info.fd = get_unused_fd_flags(0);
+ if (unlikely(info.fd < 0))
+ return info.fd;
+ file = kcm_clone(sock);
+ if (IS_ERR(file)) {
+ put_unused_fd(info.fd);
+ return PTR_ERR(file);
+ }
+ if (copy_to_user((void __user *)arg, &info,
+ sizeof(info))) {
+ put_unused_fd(info.fd);
+ fput(file);
+ return -EFAULT;
+ }
+ fd_install(info.fd, file);
+ err = 0;
break;
}
default:
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 1/2] ip6_gre: add ip6 erspan collect_md mode
From: William Tu @ 2017-12-05 23:15 UTC (permalink / raw)
To: netdev
In-Reply-To: <1512515745-21003-1-git-send-email-u9012063@gmail.com>
Similar to ip6 gretap and ip4 gretap, the patch allows
erspan tunnel to operate in collect metadata mode.
bpf_skb_[gs]et_tunnel_key() helpers can make use of
it right away.
Signed-off-by: William Tu <u9012063@gmail.com>
---
net/ipv6/ip6_gre.c | 110 +++++++++++++++++++++++++++++++++++++++++------------
1 file changed, 85 insertions(+), 25 deletions(-)
diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index 1510ce9a4e4e..4562579797d1 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -524,8 +524,37 @@ static int ip6erspan_rcv(struct sk_buff *skb, int gre_hdr_len,
false, false) < 0)
return PACKET_REJECT;
- tunnel->parms.index = ntohl(index);
- ip6_tnl_rcv(tunnel, skb, tpi, NULL, log_ecn_error);
+ if (tunnel->parms.collect_md) {
+ struct metadata_dst *tun_dst;
+ struct ip_tunnel_info *info;
+ struct erspan_metadata *md;
+ __be64 tun_id;
+ __be16 flags;
+
+ tpi->flags |= TUNNEL_KEY;
+ flags = tpi->flags;
+ tun_id = key32_to_tunnel_id(tpi->key);
+
+ tun_dst = ipv6_tun_rx_dst(skb, flags, tun_id,
+ sizeof(*md));
+ if (!tun_dst)
+ return PACKET_REJECT;
+
+ info = &tun_dst->u.tun_info;
+ md = ip_tunnel_info_opts(info);
+ if (!md)
+ return PACKET_REJECT;
+
+ md->index = index;
+ info->key.tun_flags |= TUNNEL_ERSPAN_OPT;
+ info->options_len = sizeof(*md);
+
+ ip6_tnl_rcv(tunnel, skb, tpi, tun_dst, log_ecn_error);
+
+ } else {
+ tunnel->parms.index = ntohl(index);
+ ip6_tnl_rcv(tunnel, skb, tpi, NULL, log_ecn_error);
+ }
return PACKET_RCVD;
}
@@ -857,42 +886,73 @@ static netdev_tx_t ip6erspan_tunnel_xmit(struct sk_buff *skb,
if (gre_handle_offloads(skb, false))
goto tx_err;
- switch (skb->protocol) {
- case htons(ETH_P_IP):
- memset(&(IPCB(skb)->opt), 0, sizeof(IPCB(skb)->opt));
- prepare_ip6gre_xmit_ipv4(skb, dev, &fl6,
- &dsfield, &encap_limit);
- break;
- case htons(ETH_P_IPV6):
- if (ipv6_addr_equal(&t->parms.raddr, &ipv6h->saddr))
- goto tx_err;
- if (prepare_ip6gre_xmit_ipv6(skb, dev, &fl6,
- &dsfield, &encap_limit))
- goto tx_err;
- break;
- default:
- memcpy(&fl6, &t->fl.u.ip6, sizeof(fl6));
- break;
- }
-
if (skb->len > dev->mtu + dev->hard_header_len) {
pskb_trim(skb, dev->mtu + dev->hard_header_len);
truncate = true;
}
- erspan_build_header(skb, t->parms.o_key, t->parms.index,
- truncate, false);
t->parms.o_flags &= ~TUNNEL_KEY;
-
IPCB(skb)->flags = 0;
- fl6.daddr = t->parms.raddr;
+
+ /* For collect_md mode, derive fl6 from the tunnel key,
+ * for native mode, call prepare_ip6gre_xmit_{ipv4,ipv6}.
+ */
+ if (t->parms.collect_md) {
+ struct ip_tunnel_info *tun_info;
+ const struct ip_tunnel_key *key;
+ struct erspan_metadata *md;
+
+ tun_info = skb_tunnel_info(skb);
+ if (unlikely(!tun_info ||
+ !(tun_info->mode & IP_TUNNEL_INFO_TX) ||
+ ip_tunnel_info_af(tun_info) != AF_INET6))
+ return -EINVAL;
+
+ key = &tun_info->key;
+ memset(&fl6, 0, sizeof(fl6));
+ fl6.flowi6_proto = IPPROTO_GRE;
+ fl6.daddr = key->u.ipv6.dst;
+ fl6.flowlabel = key->label;
+ fl6.flowi6_uid = sock_net_uid(dev_net(dev), NULL);
+
+ dsfield = key->tos;
+ md = ip_tunnel_info_opts(tun_info);
+ if (!md)
+ goto tx_err;
+
+ erspan_build_header(skb, tunnel_id_to_key32(key->tun_id),
+ ntohl(md->index), truncate, false);
+
+ } else {
+ switch (skb->protocol) {
+ case htons(ETH_P_IP):
+ memset(&(IPCB(skb)->opt), 0, sizeof(IPCB(skb)->opt));
+ prepare_ip6gre_xmit_ipv4(skb, dev, &fl6,
+ &dsfield, &encap_limit);
+ break;
+ case htons(ETH_P_IPV6):
+ if (ipv6_addr_equal(&t->parms.raddr, &ipv6h->saddr))
+ goto tx_err;
+ if (prepare_ip6gre_xmit_ipv6(skb, dev, &fl6,
+ &dsfield, &encap_limit))
+ goto tx_err;
+ break;
+ default:
+ memcpy(&fl6, &t->fl.u.ip6, sizeof(fl6));
+ break;
+ }
+
+ erspan_build_header(skb, t->parms.o_key, t->parms.index,
+ truncate, false);
+ fl6.daddr = t->parms.raddr;
+ }
/* Push GRE header. */
gre_build_header(skb, 8, TUNNEL_SEQ,
htons(ETH_P_ERSPAN), 0, htonl(t->o_seqno++));
/* TooBig packet may have updated dst->dev's mtu */
- if (dst && dst_mtu(dst) > dst->dev->mtu)
+ if (!t->parms.collect_md && dst && dst_mtu(dst) > dst->dev->mtu)
dst->ops->update_pmtu(dst, NULL, skb, dst->dev->mtu);
err = ip6_tnl_xmit(skb, dev, dsfield, &fl6, encap_limit, &mtu,
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 2/2] samples/bpf: add ip6erspan sample code
From: William Tu @ 2017-12-05 23:15 UTC (permalink / raw)
To: netdev
In-Reply-To: <1512515745-21003-1-git-send-email-u9012063@gmail.com>
Extend the existing tests for ip6erspan.
Signed-off-by: William Tu <u9012063@gmail.com>
---
samples/bpf/tcbpf2_kern.c | 58 ++++++++++++++++++++++++++++++++++++++++++
samples/bpf/test_tunnel_bpf.sh | 37 +++++++++++++++++++++++++++
2 files changed, 95 insertions(+)
diff --git a/samples/bpf/tcbpf2_kern.c b/samples/bpf/tcbpf2_kern.c
index 15a469220e19..79ad061079dd 100644
--- a/samples/bpf/tcbpf2_kern.c
+++ b/samples/bpf/tcbpf2_kern.c
@@ -181,6 +181,64 @@ int _erspan_get_tunnel(struct __sk_buff *skb)
return TC_ACT_OK;
}
+SEC("ip4ip6erspan_set_tunnel")
+int _ip4ip6erspan_set_tunnel(struct __sk_buff *skb)
+{
+ struct bpf_tunnel_key key;
+ struct erspan_metadata md;
+ int ret;
+
+ __builtin_memset(&key, 0x0, sizeof(key));
+ key.remote_ipv6[3] = _htonl(0x11);
+ key.tunnel_id = 2;
+ key.tunnel_tos = 0;
+ key.tunnel_ttl = 64;
+
+ ret = bpf_skb_set_tunnel_key(skb, &key, sizeof(key),
+ BPF_F_TUNINFO_IPV6);
+ if (ret < 0) {
+ ERROR(ret);
+ return TC_ACT_SHOT;
+ }
+
+ md.index = htonl(123);
+ ret = bpf_skb_set_tunnel_opt(skb, &md, sizeof(md));
+ if (ret < 0) {
+ ERROR(ret);
+ return TC_ACT_SHOT;
+ }
+
+ return TC_ACT_OK;
+}
+
+SEC("ip4ip6erspan_get_tunnel")
+int _ip4ip6erspan_get_tunnel(struct __sk_buff *skb)
+{
+ char fmt[] = "key %d remote ip6 ::%x erspan index 0x%x\n";
+ struct bpf_tunnel_key key;
+ struct erspan_metadata md;
+ u32 index;
+ int ret;
+
+ ret = bpf_skb_get_tunnel_key(skb, &key, sizeof(key), BPF_F_TUNINFO_IPV6);
+ if (ret < 0) {
+ ERROR(ret);
+ return TC_ACT_SHOT;
+ }
+
+ ret = bpf_skb_get_tunnel_opt(skb, &md, sizeof(md));
+ if (ret < 0) {
+ ERROR(ret);
+ return TC_ACT_SHOT;
+ }
+
+ index = bpf_ntohl(md.index);
+ bpf_trace_printk(fmt, sizeof(fmt),
+ key.tunnel_id, key.remote_ipv6[0], index);
+
+ return TC_ACT_OK;
+}
+
SEC("vxlan_set_tunnel")
int _vxlan_set_tunnel(struct __sk_buff *skb)
{
diff --git a/samples/bpf/test_tunnel_bpf.sh b/samples/bpf/test_tunnel_bpf.sh
index 226f45381b76..f53efb62f699 100755
--- a/samples/bpf/test_tunnel_bpf.sh
+++ b/samples/bpf/test_tunnel_bpf.sh
@@ -70,6 +70,28 @@ function add_erspan_tunnel {
ip addr add dev $DEV 10.1.1.200/24
}
+function add_ip6erspan_tunnel {
+
+ # assign ipv6 address
+ ip netns exec at_ns0 ip addr add ::11/96 dev veth0
+ ip netns exec at_ns0 ip link set dev veth0 up
+ ip addr add dev veth1 ::22/96
+ ip link set dev veth1 up
+
+ # in namespace
+ ip netns exec at_ns0 \
+ ip link add dev $DEV_NS type $TYPE seq key 2 erspan 123 \
+ local ::11 remote ::22
+
+ ip netns exec at_ns0 ip addr add dev $DEV_NS 10.1.1.100/24
+ ip netns exec at_ns0 ip link set dev $DEV_NS up
+
+ # out of namespace
+ ip link add dev $DEV type $TYPE external
+ ip addr add dev $DEV 10.1.1.200/24
+ ip link set dev $DEV up
+}
+
function add_vxlan_tunnel {
# Set static ARP entry here because iptables set-mark works
# on L3 packet, as a result not applying to ARP packets,
@@ -184,6 +206,18 @@ function test_erspan {
cleanup
}
+function test_ip6erspan {
+ TYPE=ip6erspan
+ DEV_NS=ip6erspan00
+ DEV=ip6erspan11
+ config_device
+ add_ip6erspan_tunnel
+ attach_bpf $DEV ip4ip6erspan_set_tunnel ip4ip6erspan_get_tunnel
+ ping6 -c 3 ::11
+ ip netns exec at_ns0 ping -c 1 10.1.1.200
+ cleanup
+}
+
function test_vxlan {
TYPE=vxlan
DEV_NS=vxlan00
@@ -239,6 +273,7 @@ function cleanup {
ip link del vxlan11
ip link del geneve11
ip link del erspan11
+ ip link del ip6erspan11
pkill tcpdump
pkill cat
set -ex
@@ -254,6 +289,8 @@ echo "Testing IP6GRETAP tunnel..."
test_ip6gretap
echo "Testing ERSPAN tunnel..."
test_erspan
+echo "Testing IP6ERSPAN tunnel..."
+test_ip6erspan
echo "Testing VXLAN tunnel..."
test_vxlan
echo "Testing GENEVE tunnel..."
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 0/2] add ip6erspan collect_md mode
From: William Tu @ 2017-12-05 23:15 UTC (permalink / raw)
To: netdev
Similar to erspan collect_md mode in ipv4, the first patch adds
support for ip6erspan collect metadata mode. The second patch
adds the test case using bpf_skb_[gs]et_tunnel_key helpers.
The corresponding iproute2 patch:
https://marc.info/?l=linux-netdev&m=151251545410047&w=2
William Tu (2):
ip6_gre: add ip6 erspan collect_md mode
samples/bpf: add ip6erspan sample code
net/ipv6/ip6_gre.c | 110 +++++++++++++++++++++++++++++++----------
samples/bpf/tcbpf2_kern.c | 58 ++++++++++++++++++++++
samples/bpf/test_tunnel_bpf.sh | 37 ++++++++++++++
3 files changed, 180 insertions(+), 25 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH v2 iproute2 net-next] gre6: add collect metadata support
From: William Tu @ 2017-12-05 23:10 UTC (permalink / raw)
To: netdev
The patch adds 'external' option to support collect metadata
gre6 tunnel. Example of L3 and L2 gre device:
bash:~# ip link add dev ip6gre123 type ip6gre external
bash:~# ip link add dev ip6gretap123 type ip6gretap external
Signed-off-by: William Tu <u9012063@gmail.com>
---
change in v2:
- remove "noexternal" in man page
---
ip/link_gre6.c | 55 ++++++++++++++++++++++++++++++++-------------------
man/man8/ip-link.8.in | 6 ++++++
2 files changed, 41 insertions(+), 20 deletions(-)
diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 0a82eaecf2cd..2cb46ca116d0 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -105,6 +105,7 @@ static int gre_parse_opt(struct link_util *lu, int argc, char **argv,
__u16 encapflags = TUNNEL_ENCAP_FLAG_CSUM6;
__u16 encapsport = 0;
__u16 encapdport = 0;
+ __u8 metadata = 0;
int len;
__u32 fwmark = 0;
__u32 erspan_idx = 0;
@@ -178,6 +179,9 @@ get_failed:
if (greinfo[IFLA_GRE_ENCAP_SPORT])
encapsport = rta_getattr_u16(greinfo[IFLA_GRE_ENCAP_SPORT]);
+ if (greinfo[IFLA_GRE_COLLECT_METADATA])
+ metadata = 1;
+
if (greinfo[IFLA_GRE_ENCAP_DPORT])
encapdport = rta_getattr_u16(greinfo[IFLA_GRE_ENCAP_DPORT]);
@@ -355,6 +359,8 @@ get_failed:
encapflags |= TUNNEL_ENCAP_FLAG_REMCSUM;
} else if (strcmp(*argv, "noencap-remcsum") == 0) {
encapflags &= ~TUNNEL_ENCAP_FLAG_REMCSUM;
+ } else if (strcmp(*argv, "external") == 0) {
+ metadata = 1;
} else if (strcmp(*argv, "fwmark") == 0) {
NEXT_ARG();
if (strcmp(*argv, "inherit") == 0) {
@@ -388,26 +394,30 @@ get_failed:
argc--; argv++;
}
- addattr32(n, 1024, IFLA_GRE_IKEY, ikey);
- addattr32(n, 1024, IFLA_GRE_OKEY, okey);
- addattr_l(n, 1024, IFLA_GRE_IFLAGS, &iflags, 2);
- addattr_l(n, 1024, IFLA_GRE_OFLAGS, &oflags, 2);
- addattr_l(n, 1024, IFLA_GRE_LOCAL, &laddr, sizeof(laddr));
- addattr_l(n, 1024, IFLA_GRE_REMOTE, &raddr, sizeof(raddr));
- if (link)
- addattr32(n, 1024, IFLA_GRE_LINK, link);
- addattr_l(n, 1024, IFLA_GRE_TTL, &hop_limit, 1);
- addattr_l(n, 1024, IFLA_GRE_ENCAP_LIMIT, &encap_limit, 1);
- addattr_l(n, 1024, IFLA_GRE_FLOWINFO, &flowinfo, 4);
- addattr32(n, 1024, IFLA_GRE_FLAGS, flags);
- addattr32(n, 1024, IFLA_GRE_FWMARK, fwmark);
- if (erspan_idx != 0)
- addattr32(n, 1024, IFLA_GRE_ERSPAN_INDEX, erspan_idx);
-
- addattr16(n, 1024, IFLA_GRE_ENCAP_TYPE, encaptype);
- addattr16(n, 1024, IFLA_GRE_ENCAP_FLAGS, encapflags);
- addattr16(n, 1024, IFLA_GRE_ENCAP_SPORT, htons(encapsport));
- addattr16(n, 1024, IFLA_GRE_ENCAP_DPORT, htons(encapdport));
+ if (!metadata) {
+ addattr32(n, 1024, IFLA_GRE_IKEY, ikey);
+ addattr32(n, 1024, IFLA_GRE_OKEY, okey);
+ addattr_l(n, 1024, IFLA_GRE_IFLAGS, &iflags, 2);
+ addattr_l(n, 1024, IFLA_GRE_OFLAGS, &oflags, 2);
+ addattr_l(n, 1024, IFLA_GRE_LOCAL, &laddr, sizeof(laddr));
+ addattr_l(n, 1024, IFLA_GRE_REMOTE, &raddr, sizeof(raddr));
+ if (link)
+ addattr32(n, 1024, IFLA_GRE_LINK, link);
+ addattr_l(n, 1024, IFLA_GRE_TTL, &hop_limit, 1);
+ addattr_l(n, 1024, IFLA_GRE_ENCAP_LIMIT, &encap_limit, 1);
+ addattr_l(n, 1024, IFLA_GRE_FLOWINFO, &flowinfo, 4);
+ addattr32(n, 1024, IFLA_GRE_FLAGS, flags);
+ addattr32(n, 1024, IFLA_GRE_FWMARK, fwmark);
+ if (erspan_idx != 0)
+ addattr32(n, 1024, IFLA_GRE_ERSPAN_INDEX, erspan_idx);
+
+ addattr16(n, 1024, IFLA_GRE_ENCAP_TYPE, encaptype);
+ addattr16(n, 1024, IFLA_GRE_ENCAP_FLAGS, encapflags);
+ addattr16(n, 1024, IFLA_GRE_ENCAP_SPORT, htons(encapsport));
+ addattr16(n, 1024, IFLA_GRE_ENCAP_DPORT, htons(encapdport));
+ } else {
+ addattr_l(n, 1024, IFLA_GRE_COLLECT_METADATA, NULL, 0);
+ }
return 0;
}
@@ -426,6 +436,11 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
if (!tb)
return;
+ if (tb[IFLA_GRE_COLLECT_METADATA]) {
+ print_bool(PRINT_ANY, "collect_metadata", "external", true);
+ return;
+ }
+
if (tb[IFLA_GRE_FLAGS])
flags = rta_getattr_u32(tb[IFLA_GRE_FLAGS]);
diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
index a6a10e577b1f..eb04f887c940 100644
--- a/man/man8/ip-link.8.in
+++ b/man/man8/ip-link.8.in
@@ -755,6 +755,8 @@ the following additional arguments are supported:
.BI "dscp inherit"
] [
.BI dev " PHYS_DEV "
+] [
+.RB external
]
.in +8
@@ -833,6 +835,10 @@ or
.IR 00 ".." ff
when tunneling non-IP packets. The default value is 00.
+.sp
+.RB external
+- make this tunnel externally controlled (or not, which is the default).
+
.in -8
.TP
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] dccp: CVE-2017-8824: use-after-free in DCCP code
From: David Miller @ 2017-12-05 23:09 UTC (permalink / raw)
To: simo.ghannam; +Cc: netdev
In-Reply-To: <5a270886.06aadf0a.60a1e.7631@mx.google.com>
From: simo.ghannam@gmail.com
Date: Tue, 5 Dec 2017 20:58:35 +0000
> From: Mohamed Ghannam <simo.ghannam@gmail.com>
>
> Whenever the sock object is in DCCP_CLOSED state,
> dccp_disconnect() must free dccps_hc_tx_ccid and
> dccps_hc_rx_ccid and set to NULL.
>
> Signed-off-by: Mohamed Ghannam <simo.ghannam@gmail.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [Patch net-next] net_sched: remove unused parameter from act cleanup ops
From: David Miller @ 2017-12-05 23:08 UTC (permalink / raw)
To: xiyou.wangcong; +Cc: netdev, jiri, jhs
In-Reply-To: <20171205205309.12106-1-xiyou.wangcong@gmail.com>
From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Tue, 5 Dec 2017 12:53:07 -0800
> No one actually uses it.
>
> Cc: Jiri Pirko <jiri@mellanox.com>
> Cc: Jamal Hadi Salim <jhs@mojatatu.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Nice cleanup, applied, thanks Cong.
^ permalink raw reply
* Re: [PATCH net] net: remove hlist_nulls_add_tail_rcu()
From: David Miller @ 2017-12-05 23:07 UTC (permalink / raw)
To: eric.dumazet; +Cc: kraigatgoog, edumazet, glider, netdev
In-Reply-To: <1512506756.25033.4.camel@gmail.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 05 Dec 2017 12:45:56 -0800
> From: Eric Dumazet <edumazet@google.com>
>
> Alexander Potapenko reported use of uninitialized memory [1]
>
> This happens when inserting a request socket into TCP ehash,
> in __sk_nulls_add_node_rcu(), since sk_reuseport is not initialized.
>
> Bug was added by commit d894ba18d4e4 ("soreuseport: fix ordering for
> mixed v4/v6 sockets")
>
> Note that d296ba60d8e2 ("soreuseport: Resolve merge conflict for v4/v6
> ordering fix") missed the opportunity to get rid of
> hlist_nulls_add_tail_rcu() :
>
> Both UDP sockets and TCP/DCCP listeners no longer use
> __sk_nulls_add_node_rcu() for their hash insertion.
>
> Since all other sockets have unique 4-tuple, the reuseport status
> has no special meaning, so we can always use hlist_nulls_add_head_rcu()
> for them and save few cycles/instructions.
>
> [1]
...
> Fixes: d894ba18d4e4 ("soreuseport: fix ordering for mixed v4/v6 sockets")
> Fixes: d296ba60d8e2 ("soreuseport: Resolve merge conflict for v4/v6 ordering fix")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Alexander Potapenko <glider@google.com>
> Acked-by: Craig Gallek <kraig@google.com>
I was just talking with Craig and Willem about this change the other
day, what a coincidence :-)
Applied and queued up for -stable, thanks Eric.
^ permalink raw reply
* Re: [PATCH net 0/2] net: qualcomm: rmnet: Fix leaks in failure scenarios
From: David Miller @ 2017-12-05 23:04 UTC (permalink / raw)
To: subashab; +Cc: netdev
In-Reply-To: <1512506478-9113-1-git-send-email-subashab@codeaurora.org>
From: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
Date: Tue, 5 Dec 2017 13:41:16 -0700
> Patch 1 fixes a leak in transmit path where a skb cannot be
> transmitted due to insufficient headroom to stamp the map header.
> Patch 2 fixes a leak in rmnet_newlink() failure because the
> rmnet endpoint was never freed
Series applied, thank you.
^ permalink raw reply
* Re: [PATCH v2 net-next] netlink: optimize err assignment
From: Stephen Hemminger @ 2017-12-05 23:03 UTC (permalink / raw)
To: Eric Dumazet; +Cc: yuan linyu, netdev, David S . Miller, yuan linyu
In-Reply-To: <1512314409.19682.51.camel@gmail.com>
On Sun, 03 Dec 2017 07:20:09 -0800
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Sun, 2017-12-03 at 21:10 +0800, yuan linyu wrote:
> > From: yuan linyu <Linyu.Yuan@alcatel-sbell.com.cn>
> >
> > Signed-off-by: yuan linyu <Linyu.Yuan@alcatel-sbell.com.cn>
> > ---
> > v2: fix kbuild test warning
> > ---
> > net/netlink/af_netlink.c | 52 ++++++++++++++++++++----------------
> > ------------
> > 1 file changed, 22 insertions(+), 30 deletions(-)
> >
>
> I see no reason why we should accept this code churn.
>
> This kind of change makes future fix backports harder.
>
I more worried about the possibility of latent bugs.
Humans aren't perfect at following all code paths
^ permalink raw reply
* Re: [PATCH net-next v2 0/5] net: dsa: use per-port upstream port
From: David Miller @ 2017-12-05 23:02 UTC (permalink / raw)
To: vivien.didelot; +Cc: netdev, linux-kernel, kernel, f.fainelli, andrew
In-Reply-To: <20171205203413.3342-1-vivien.didelot@savoirfairelinux.com>
From: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Date: Tue, 5 Dec 2017 15:34:08 -0500
> An upstream port is a local switch port used to reach a CPU port.
>
> DSA still considers a unique CPU port in the whole switch fabric and
> thus return a unique upstream port for a given switch. This is wrong in
> a multiple CPU ports environment.
>
> We are now switching to using the dedicated CPU port assigned to each
> port in order to get rid of the deprecated unique tree CPU port.
>
> This patchset makes the dsa_upstream_port() helper take a port argument
> and goes one step closer complete support for multiple CPU ports.
>
> Changes in v2:
> - reverse-christmas-tree-fy variables
Series applied, thanks Vivien.
^ permalink raw reply
* Re: [PATCH 1/2] net: macb: reduce scope of rx_fs_lock-protected regions
From: David Miller @ 2017-12-05 23:00 UTC (permalink / raw)
To: julia; +Cc: julia.lawall, rafalo, nicolas.ferre, netdev, linux-kernel,
kbuild-all
In-Reply-To: <20171205201711.GA18445@jcartwri.amer.corp.natinst.com>
From: Julia Cartwright <julia@ni.com>
Date: Tue, 5 Dec 2017 14:17:11 -0600
> While Julia Lawall's cocci-generated patch fixes the problem, the right
> solution is to obviate the problem altogether.
I already applied Julia's patch. And I hope that if you generated
this against current net-next you would have seen that.
So you'll need to redo this series and put the GFP_KERNEL back.
^ permalink raw reply
* Re: [PATCH net] netlink: Relax attr validation for fixed length types
From: David Miller @ 2017-12-05 22:58 UTC (permalink / raw)
To: dsahern; +Cc: netdev, johannes
In-Reply-To: <20171205195540.41822-1-dsahern@gmail.com>
From: David Ahern <dsahern@gmail.com>
Date: Tue, 5 Dec 2017 12:55:40 -0700
> Commit 28033ae4e0f5 ("net: netlink: Update attr validation to require
> exact length for some types") requires attributes using types NLA_U* and
> NLA_S* to have an exact length. This change is exposing bugs in various
> userspace commands that are sending attributes with an invalid length
> (e.g., attribute has type NLA_U8 and userspace sends NLA_U32). While
> the commands are clearly broken and need to be fixed, users are arguing
> that the sudden change in enforcement is breaking older commands on
> newer kernels for use cases that otherwise "worked".
>
> Relax the validation to print a warning mesage similar to what is done
> for messages containing extra bytes after parsing.
>
> Fixes: 28033ae4e0f5 ("net: netlink: Update attr validation to require exact length for some types")
> Signed-off-by: David Ahern <dsahern@gmail.com>
Johannes, please review.
> ---
> lib/nlattr.c | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/lib/nlattr.c b/lib/nlattr.c
> index 8bf78b4b78f0..6122662906c8 100644
> --- a/lib/nlattr.c
> +++ b/lib/nlattr.c
> @@ -28,8 +28,16 @@ static const u8 nla_attr_len[NLA_TYPE_MAX+1] = {
> };
>
> static const u8 nla_attr_minlen[NLA_TYPE_MAX+1] = {
> + [NLA_U8] = sizeof(u8),
> + [NLA_U16] = sizeof(u16),
> + [NLA_U32] = sizeof(u32),
> + [NLA_U64] = sizeof(u64),
> [NLA_MSECS] = sizeof(u64),
> [NLA_NESTED] = NLA_HDRLEN,
> + [NLA_S8] = sizeof(s8),
> + [NLA_S16] = sizeof(s16),
> + [NLA_S32] = sizeof(s32),
> + [NLA_S64] = sizeof(s64),
> };
>
> static int validate_nla_bitfield32(const struct nlattr *nla,
> @@ -70,10 +78,9 @@ static int validate_nla(const struct nlattr *nla, int maxtype,
> BUG_ON(pt->type > NLA_TYPE_MAX);
>
> /* for data types NLA_U* and NLA_S* require exact length */
> - if (nla_attr_len[pt->type]) {
> - if (attrlen != nla_attr_len[pt->type])
> - return -ERANGE;
> - return 0;
> + if (nla_attr_len[pt->type] && attrlen != nla_attr_len[pt->type]) {
> + pr_warn_ratelimited("netlink: '%s': attribute type %d has an invalid length.\n",
> + current->comm, type);
> }
>
> switch (pt->type) {
> --
> 2.11.0
>
^ permalink raw reply
* Re: [PATCH V11 3/5] printk: hash addresses printed with %p
From: Geert Uytterhoeven @ 2017-12-05 22:57 UTC (permalink / raw)
To: Tobin C. Harding
Cc: kernel-hardening, Linus Torvalds, Jason A. Donenfeld,
Theodore Ts'o, Kees Cook, Paolo Bonzini, Tycho Andersen,
Roberts, William C, Tejun Heo, Jordan Glover, Greg KH,
Petr Mladek, Joe Perches, Ian Campbell, Sergey Senozhatsky,
Catalin Marinas, Will Deacon, Steven Rostedt, Chris Fries,
Dave Weinstein
In-Reply-To: <20171205204402.GD11064@eros>
Hi Tobin,
On Tue, Dec 5, 2017 at 9:44 PM, Tobin C. Harding <me@tobin.cc> wrote:
> On Tue, Dec 05, 2017 at 09:20:57PM +0100, Geert Uytterhoeven wrote:
>> On Wed, Nov 29, 2017 at 3:05 AM, Tobin C. Harding <me@tobin.cc> wrote:
>> > Currently there exist approximately 14 000 places in the kernel where
>> > addresses are being printed using an unadorned %p. This potentially
>> > leaks sensitive information regarding the Kernel layout in memory. Many
>> > of these calls are stale, instead of fixing every call lets hash the
>> > address by default before printing. This will of course break some
>> > users, forcing code printing needed addresses to be updated.
>> >
>> > Code that _really_ needs the address will soon be able to use the new
>> > printk specifier %px to print the address.
>>
>> > --- a/lib/vsprintf.c
>> > +++ b/lib/vsprintf.c
>>
>> > +/* Maps a pointer to a 32 bit unique identifier. */
>> > +static char *ptr_to_id(char *buf, char *end, void *ptr, struct printf_spec spec)
>> > +{
>> > + unsigned long hashval;
>> > + const int default_width = 2 * sizeof(ptr);
>> > +
>> > + if (unlikely(!have_filled_random_ptr_key)) {
>> > + spec.field_width = default_width;
>> > + /* string length must be less than default_width */
>> > + return string(buf, end, "(ptrval)", spec);
>> > + }
>> > +
>> > +#ifdef CONFIG_64BIT
>> > + hashval = (unsigned long)siphash_1u64((u64)ptr, &ptr_key);
>> > + /*
>> > + * Mask off the first 32 bits, this makes explicit that we have
>> > + * modified the address (and 32 bits is plenty for a unique ID).
>> > + */
>> > + hashval = hashval & 0xffffffff;
>> > +#else
>> > + hashval = (unsigned long)siphash_1u32((u32)ptr, &ptr_key);
>> > +#endif
>>
>> Would it make sense to keep the 3 lowest bits of the address?
>>
>> Currently printed pointers no longer have any correlation with the actual
>> alignment in memory of the object, which is a typical cause of a class of bugs.
>
> We'd have to keep the lowest 4 since we are printing in hex, right? This
> is easy enough to add. I wasn't the architect behind the hashing but I
> can do up a patch and see if anyone who knows crypto objects.
Lowest 3 is good enough for all natural types, up to long long.
We may still receive complaints from people who care about seeing if
a pointer is cacheline-aligned or not. Fixing that may need up to 7 bits, I'm
afraid, which is a bit too much to give up.
Gr{oetje,eeting}s,
Geert
^ permalink raw reply
* [PATCH] netlink: Add netns check on taps
From: Kevin Cernekee @ 2017-12-05 22:46 UTC (permalink / raw)
To: davem; +Cc: johannes.berg, netdev, linux-kernel
Currently, a nlmon link inside a child namespace can observe systemwide
netlink activity. Filter the traffic so that in a non-init netns,
nlmon can only sniff netlink messages from its own netns.
Test case:
vpnns -- bash -c "ip link add nlmon0 type nlmon; \
ip link set nlmon0 up; \
tcpdump -i nlmon0 -q -w /tmp/nlmon.pcap -U" &
sudo ip xfrm state add src 10.1.1.1 dst 10.1.1.2 proto esp \
spi 0x1 mode transport \
auth sha1 0x6162633132330000000000000000000000000000 \
enc aes 0x00000000000000000000000000000000
grep abc123 /tmp/nlmon.pcap
Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
---
net/netlink/af_netlink.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index b9e0ee4..88381a2 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -253,6 +253,11 @@ static int __netlink_deliver_tap_skb(struct sk_buff *skb,
struct sock *sk = skb->sk;
int ret = -ENOMEM;
+ if (!net_eq(dev_net(dev), sock_net(sk)) &&
+ !net_eq(dev_net(dev), &init_net)) {
+ return 0;
+ }
+
dev_hold(dev);
if (is_vmalloc_addr(skb->head))
--
2.7.4
^ permalink raw reply related
* Re: [RFC] virtio-net: help live migrate SR-IOV devices
From: Stephen Hemminger @ 2017-12-05 22:41 UTC (permalink / raw)
To: Jakub Kicinski
Cc: achiad shochat, Alexander Duyck, Michael S. Tsirkin,
Hannes Frederic Sowa, Sridhar Samudrala, netdev, virtualization,
Achiad, Peter Waskiewicz Jr, Singhai, Anjali, Shannon Nelson,
Andy Gospodarek, Or Gerlitz
In-Reply-To: <20171205142928.00f5a51a@cakuba.netronome.com>
On Tue, 5 Dec 2017 14:29:28 -0800
Jakub Kicinski <kubakici@wp.pl> wrote:
> On Tue, 5 Dec 2017 11:59:17 +0200, achiad shochat wrote:
> > >>>> I second Jacob - having a netdev of one device driver enslave a netdev
> > >>>> of another device driver is an awkward a-symmetric model.
> > >>>> Regardless of whether they share the same backend device.
> > >>>> Only I am not sure the Linux Bond is the right choice.
> > >>>> e.g one may well want to use the virtio device also when the
> > >>>> pass-through device is available, e.g for multicasts, east-west
> > >>>> traffic, etc.
> > >>>> I'm not sure the Linux Bond fits that functionality.
> > >>>> And, as I hear in this thread, it is hard to make it work out of the box.
> > >>>> So I think the right thing would be to write a new dedicated module
> > >>>> for this purpose.
> > >
> > > This part I can sort of agree with. What if we were to look at
> > > providing a way to somehow advertise that the two devices were meant
> > > to be boded for virtualization purposes? For now lets call it a
> > > "virt-bond". Basically we could look at providing a means for virtio
> > > and VF drivers to advertise that they want this sort of bond. Then it
> > > would just be a matter of providing some sort of side channel to
> > > indicate where you want things like multicast/broadcast/east-west
> > > traffic to go.
> >
> > I like this approach.
>
> +1 on a separate driver, just enslaving devices to virtio may break
> existing setups. If people are bonding from user space today, if they
> update their kernel it may surprise them how things get auto-mangled.
>
> Is what Alex is suggesting a separate PV device that says "I would
> like to be a bond of those two interfaces"? That would make the HV
> intent explicit and kernel decisions more understandable.
So far, in my experience it still works.
As long as the kernel slaving happens first, it will work.
The attempt to bond an already slaved device will fail and no scripts seem
to check the error return.
^ permalink raw reply
* Re: [PATCH v2 3/3] ethtool: Add ETHTOOL_RESET support via --reset command
From: Scott Branden @ 2017-12-05 22:31 UTC (permalink / raw)
To: Andrew Lunn
Cc: John W. Linville, BCM Kernel Feedback, Steve Lin, Michael Chan,
netdev, Paul Greenwalt, Stephen Hemminger
In-Reply-To: <20171205222640.GA25752@lunn.ch>
Hi Andrew,
On 17-12-05 02:26 PM, Andrew Lunn wrote:
> On Tue, Dec 05, 2017 at 12:53:23PM -0800, Scott Branden wrote:
>> Add ETHTOOL_RESET support via --reset command.
>>
>> ie. ethtool --reset DEVNAME <flagname(s)>
>>
>> flagnames currently match the ETH_RESET_xxx names:
>> mgmt,irq,dma,filter,offload,mac,phy,ram,dedicated,all
Yes, I missed adding ap to the commit message here.
> [Snip]
>
>> +.B ethtool \-\-reset
>> +.I devname
>> +.BN flags
>> +.RB [ mgmt ]
>> +.RB [ irq ]
>> +.RB [ dma ]
>> +.RB [ filter ]
>> +.RB [ offload ]
>> +.RB [ mac ]
>> +.RB [ phy ]
>> +.RB [ ram ]
>> +.RB [ ap ]
>> +.RB [ dedicated ]
>> +.RB [ all ]
> Hi Scott
>
> Just a nick pick. You don't list ap above, which is kind of why you
> are doing this, if i remember correctly.
Yes - I added ap to v2 of this patch but didn't add it to the commit
message. Will update in v3.
>
> Andrew
^ permalink raw reply
* [PATCHv3 2/2] userns: control capabilities of some user namespaces
From: Mahesh Bandewar @ 2017-12-05 22:31 UTC (permalink / raw)
To: LKML, Netdev
Cc: Kernel-hardening, Linux API, Kees Cook, Serge Hallyn,
Eric W . Biederman, Eric Dumazet, David Miller, Mahesh Bandewar,
Mahesh Bandewar
From: Mahesh Bandewar <maheshb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
With this new notion of "controlled" user-namespaces, the controlled
user-namespaces are marked at the time of their creation while the
capabilities of processes that belong to them are controlled using the
global mask.
Init-user-ns is always uncontrolled and a process that has SYS_ADMIN
that belongs to uncontrolled user-ns can create another (child) user-
namespace that is uncontrolled. Any other process (that either does
not have SYS_ADMIN or belongs to a controlled user-ns) can only
create a user-ns that is controlled.
global-capability-whitelist (controlled_userns_caps_whitelist) is used
at the capability check-time and keeps the semantics for the processes
that belong to uncontrolled user-ns as it is. Processes that belong to
controlled user-ns however are subjected to different checks-
(a) if the capability in question is controlled and process belongs
to controlled user-ns, then it's always denied.
(b) if the capability in question is NOT controlled then fall back
to the traditional check.
Acked-by: Serge Hallyn <serge-A9i7LUbDfNHQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Mahesh Bandewar <maheshb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
---
v3:
Rebase
v2:
Don't recalculate user-ns flags for every setns() call.
v1:
Initial submission.
include/linux/capability.h | 4 ++++
include/linux/user_namespace.h | 25 +++++++++++++++++++++++++
kernel/capability.c | 5 +++++
kernel/user_namespace.c | 4 ++++
security/commoncap.c | 8 ++++++++
5 files changed, 46 insertions(+)
diff --git a/include/linux/capability.h b/include/linux/capability.h
index 7d79a4689625..383f31f066f0 100644
--- a/include/linux/capability.h
+++ b/include/linux/capability.h
@@ -251,6 +251,10 @@ extern bool ptracer_capable(struct task_struct *tsk, struct user_namespace *ns);
extern int get_vfs_caps_from_disk(const struct dentry *dentry, struct cpu_vfs_cap_data *cpu_caps);
int proc_douserns_caps_whitelist(struct ctl_table *table, int write,
void __user *buff, size_t *lenp, loff_t *ppos);
+/* Controlled capability is capability that is missing from the capability-mask
+ * controlled_userns_caps_whitelist controlled via sysctl.
+ */
+bool is_capability_controlled(int cap);
extern int cap_convert_nscap(struct dentry *dentry, void **ivalue, size_t size);
diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
index d6b74b91096b..a5c48684b317 100644
--- a/include/linux/user_namespace.h
+++ b/include/linux/user_namespace.h
@@ -32,6 +32,7 @@ struct uid_gid_map { /* 64 bytes -- 1 cache line */
};
#define USERNS_SETGROUPS_ALLOWED 1UL
+#define USERNS_CONTROLLED 2UL
#define USERNS_INIT_FLAGS USERNS_SETGROUPS_ALLOWED
@@ -112,6 +113,21 @@ static inline void put_user_ns(struct user_namespace *ns)
__put_user_ns(ns);
}
+/* Controlled user-ns is the one that is created by a process that does not
+ * have CAP_SYS_ADMIN (or descended from such an user-ns).
+ * For more details please see the sysctl description of
+ * controlled_userns_caps_whitelist.
+ */
+static inline bool is_user_ns_controlled(const struct user_namespace *ns)
+{
+ return ns->flags & USERNS_CONTROLLED;
+}
+
+static inline void mark_user_ns_controlled(struct user_namespace *ns)
+{
+ ns->flags |= USERNS_CONTROLLED;
+}
+
struct seq_operations;
extern const struct seq_operations proc_uid_seq_operations;
extern const struct seq_operations proc_gid_seq_operations;
@@ -170,6 +186,15 @@ static inline struct ns_common *ns_get_owner(struct ns_common *ns)
{
return ERR_PTR(-EPERM);
}
+
+static inline bool is_user_ns_controlled(const struct user_namespace *ns)
+{
+ return false;
+}
+
+static inline void mark_user_ns_controlled(struct user_namespace *ns)
+{
+}
#endif
#endif /* _LINUX_USER_H */
diff --git a/kernel/capability.c b/kernel/capability.c
index 4a859b7d4902..bffe249922de 100644
--- a/kernel/capability.c
+++ b/kernel/capability.c
@@ -511,6 +511,11 @@ bool ptracer_capable(struct task_struct *tsk, struct user_namespace *ns)
}
/* Controlled-userns capabilities routines */
+bool is_capability_controlled(int cap)
+{
+ return !cap_raised(controlled_userns_caps_whitelist, cap);
+}
+
#ifdef CONFIG_SYSCTL
int proc_douserns_caps_whitelist(struct ctl_table *table, int write,
void __user *buff, size_t *lenp, loff_t *ppos)
diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index 246d4d4ce5c7..ca0556d466b6 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -141,6 +141,10 @@ int create_user_ns(struct cred *new)
goto fail_keyring;
set_cred_user_ns(new, ns);
+ if (!ns_capable(parent_ns, CAP_SYS_ADMIN) ||
+ is_user_ns_controlled(parent_ns))
+ mark_user_ns_controlled(ns);
+
return 0;
fail_keyring:
#ifdef CONFIG_PERSISTENT_KEYRINGS
diff --git a/security/commoncap.c b/security/commoncap.c
index 4f8e09340956..5454e9c03ee8 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -73,6 +73,14 @@ int cap_capable(const struct cred *cred, struct user_namespace *targ_ns,
{
struct user_namespace *ns = targ_ns;
+ /* If the capability is controlled and user-ns that process
+ * belongs-to is 'controlled' then return EPERM and no need
+ * to check the user-ns hierarchy.
+ */
+ if (is_user_ns_controlled(cred->user_ns) &&
+ is_capability_controlled(cap))
+ return -EPERM;
+
/* See if cred has the capability in the target user namespace
* by examining the target user namespace and all of the target
* user namespace's parents.
--
2.15.0.531.g2ccb3012c9-goog
^ permalink raw reply related
* [PATCHv3 0/2] capability controlled user-namespaces
From: Mahesh Bandewar @ 2017-12-05 22:30 UTC (permalink / raw)
To: LKML, Netdev
Cc: Kernel-hardening, Linux API, Kees Cook, Serge Hallyn,
Eric W . Biederman, Eric Dumazet, David Miller, Mahesh Bandewar,
Mahesh Bandewar
From: Mahesh Bandewar <maheshb@google.com>
TL;DR version
-------------
Creating a sandbox environment with namespaces is challenging
considering what these sandboxed processes can engage into. e.g.
CVE-2017-6074, CVE-2017-7184, CVE-2017-7308 etc. just to name few.
Current form of user-namespaces, however, if changed a bit can allow
us to create a sandbox environment without locking down user-
namespaces.
Detailed version
----------------
Problem
-------
User-namespaces in the current form have increased the attack surface as
any process can acquire capabilities which are not available to them (by
default) by performing combination of clone()/unshare()/setns() syscalls.
#define _GNU_SOURCE
#include <stdio.h>
#include <sched.h>
#include <netinet/in.h>
int main(int ac, char **av)
{
int sock = -1;
printf("Attempting to open RAW socket before unshare()...\n");
sock = socket(AF_INET6, SOCK_RAW, IPPROTO_RAW);
if (sock < 0) {
perror("socket() SOCK_RAW failed: ");
} else {
printf("Successfully opened RAW-Sock before unshare().\n");
close(sock);
sock = -1;
}
if (unshare(CLONE_NEWUSER | CLONE_NEWNET) < 0) {
perror("unshare() failed: ");
return 1;
}
printf("Attempting to open RAW socket after unshare()...\n");
sock = socket(AF_INET6, SOCK_RAW, IPPROTO_RAW);
if (sock < 0) {
perror("socket() SOCK_RAW failed: ");
} else {
printf("Successfully opened RAW-Sock after unshare().\n");
close(sock);
sock = -1;
}
return 0;
}
The above example shows how easy it is to acquire NET_RAW capabilities
and once acquired, these processes could take benefit of above mentioned
or similar issues discovered/undiscovered with malicious intent. Note
that this is just an example and the problem/solution is not limited
to NET_RAW capability *only*.
The easiest fix one can apply here is to lock-down user-namespaces which
many of the distros do (i.e. don't allow users to create user namespaces),
but unfortunately that prevents everyone from using them.
Approach
--------
Introduce a notion of 'controlled' user-namespaces. Every process on
the host is allowed to create user-namespaces (governed by the limit
imposed by per-ns sysctl) however, mark user-namespaces created by
sandboxed processes as 'controlled'. Use this 'mark' at the time of
capability check in conjunction with a global capability whitelist.
If the capability is not whitelisted, processes that belong to
controlled user-namespaces will not be allowed.
Once a user-ns is marked as 'controlled'; all its child user-
namespaces are marked as 'controlled' too.
A global whitelist is list of capabilities governed by the
sysctl which is available to (privileged) user in init-ns to modify
while it's applicable to all controlled user-namespaces on the host.
Marking user-namespaces controlled without modifying the whitelist is
equivalent of the current behavior. The default value of whitelist includes
all capabilities so that the compatibility is maintained. However it gives
admins fine-grained ability to control various capabilities system wide
without locking down user-namespaces.
Please see individual patches in this series.
Mahesh Bandewar (2):
capability: introduce sysctl for controlled user-ns capability whitelist
userns: control capabilities of some user namespaces
Documentation/sysctl/kernel.txt | 21 +++++++++++++++++
include/linux/capability.h | 7 ++++++
include/linux/user_namespace.h | 25 ++++++++++++++++++++
kernel/capability.c | 52 +++++++++++++++++++++++++++++++++++++++++
kernel/sysctl.c | 5 ++++
kernel/user_namespace.c | 4 ++++
security/commoncap.c | 8 +++++++
7 files changed, 122 insertions(+)
--
2.15.0.531.g2ccb3012c9-goog
^ 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