* Re: [RFC/fix] Re: libbpf build broken on musl libc (Alpine Linux)
From: Alexei Starovoitov @ 2018-09-18 0:52 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jakub Kicinski, Daniel Borkmann, Thomas Richter,
Hendrik Brueckner, Jiri Olsa, Namhyung Kim, linux-kernel, netdev
In-Reply-To: <20180917151636.GA21790@kernel.org>
On Mon, Sep 17, 2018 at 12:16:36PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Sep 13, 2018 at 11:56:31AM -0700, Alexei Starovoitov escreveu:
> > On Thu, Sep 13, 2018 at 03:32:40PM -0300, Arnaldo Carvalho de Melo wrote:
> > > Please do some testing with my perf/libbpf+str_error_r branch, it has
> > > two patches to get this fixed, the one I sent and a prep one making
> > > libbpf link against libapi.
>
> > > [acme@jouet perf]$ git log --oneline -2
> > > a7ab924b7fec (HEAD -> perf/urgent, acme.korg/perf/libbpf+str_error_r) tools lib bpf: Use str_error_r() to fix the build in Alpine Linux
> > > fb4a79e04c2b tools lib bpf: Build and link to tools/lib/api/
>
> > https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/commit/?h=perf/libbpf%2bstr_error_r&id=fb4a79e04c2b37ee873a3b31a3250925cf466fff
> > we cannot do this.
> > lib/api is GPL we cannot use it in LGPL library.
>
> So, look at this second attempt, are you ok with it?
>
> Builds with all the Alpine Linux test build containers and some more,
> still building with all the others:
>
> 1 alpine:3.4 : Ok gcc (Alpine 5.3.0) 5.3.0
> 2 alpine:3.5 : Ok gcc (Alpine 6.2.1) 6.2.1 20160822
> 3 alpine:3.6 : Ok gcc (Alpine 6.3.0) 6.3.0
> 4 alpine:3.7 : Ok gcc (Alpine 6.4.0) 6.4.0
> 5 alpine:3.8 : Ok gcc (Alpine 6.4.0) 6.4.0
> 6 alpine:edge : Ok gcc (Alpine 6.4.0) 6.4.0
> 7 amazonlinux:1 : Ok gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-28)
> 8 amazonlinux:2 : Ok gcc (GCC) 7.3.1 20180303 (Red Hat 7.3.1-5)
> 9 android-ndk:r12b-arm : Ok arm-linux-androideabi-gcc (GCC) 4.9.x 20150123 (prerelease)
> 10 android-ndk:r15c-arm : Ok arm-linux-androideabi-gcc (GCC) 4.9.x 20150123 (prerelease)
> 11 centos:5 : Ok gcc (GCC) 4.1.2 20080704 (Red Hat 4.1.2-55)
> 12 centos:6 : Ok gcc (GCC) 4.4.7 20120313 (Red Hat 4.4.7-23)
> 13 centos:7 : Ok gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-28)
> 14 debian:7 : Ok gcc (Debian 4.7.2-5) 4.7.2
> 15 debian:8 : Ok gcc (Debian 4.9.2-10+deb8u1) 4.9.2
> 16 debian:9 : Ok gcc (Debian 6.3.0-18+deb9u1) 6.3.0 20170516
> 17 debian:experimental : Ok gcc (Debian 8.2.0-4) 8.2.0
> 18 debian:experimental-x-arm64 : Ok aarch64-linux-gnu-gcc (Debian 8.2.0-4) 8.2.0
> 19 debian:experimental-x-mips : Ok mips-linux-gnu-gcc (Debian 8.2.0-4) 8.2.0
>
> commit 1ca0d8249e5bd335b1c33a33569e4ed94025128e
> Author: Arnaldo Carvalho de Melo <acme@redhat.com>
> Date: Fri Sep 14 16:47:14 2018 -0300
>
> tools lib bpf: Provide wrapper for strerror_r to build in !_GNU_SOURCE systems
>
> Same problem that got fixed in a similar fashion in tools/perf/ in
> c8b5f2c96d1b ("tools: Introduce str_error_r()"), fix it in the same
> way, licensing needs to be sorted out to libbpf to use libapi, so,
> for this simple case, just get the same wrapper in tools/lib/bpf.
>
> This makes libbpf and its users (bpftool, selftests, perf) to build
> again in Alpine Linux 3.[45678] and edge.
>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Hendrik Brueckner <brueckner@linux.ibm.com>
> Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Martin KaFai Lau <kafai@fb.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Quentin Monnet <quentin.monnet@netronome.com>
> Cc: Thomas Richter <tmricht@linux.ibm.com>
> Cc: Wang Nan <wangnan0@huawei.com>
> Cc: Yonghong Song <yhs@fb.com>
> Fixes: 1ce6a9fc1549 ("bpf: fix build error in libbpf with EXTRA_CFLAGS="-Wp, -D_FORTIFY_SOURCE=2 -O2"")
> Link: https://lkml.kernel.org/n/tip-i8ckf6s06e7tayw7xxhhhkux@git.kernel.org
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> diff --git a/tools/lib/bpf/Build b/tools/lib/bpf/Build
> index 13a861135127..6eb9bacd1948 100644
> --- a/tools/lib/bpf/Build
> +++ b/tools/lib/bpf/Build
> @@ -1 +1 @@
> -libbpf-y := libbpf.o bpf.o nlattr.o btf.o libbpf_errno.o
> +libbpf-y := libbpf.o bpf.o nlattr.o btf.o libbpf_errno.o str_error.o
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 2abd0f112627..bdb94939fd60 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -50,6 +50,7 @@
> #include "libbpf.h"
> #include "bpf.h"
> #include "btf.h"
> +#include "str_error.h"
>
> #ifndef EM_BPF
> #define EM_BPF 247
> @@ -469,7 +470,7 @@ static int bpf_object__elf_init(struct bpf_object *obj)
> obj->efile.fd = open(obj->path, O_RDONLY);
> if (obj->efile.fd < 0) {
> char errmsg[STRERR_BUFSIZE];
> - char *cp = strerror_r(errno, errmsg, sizeof(errmsg));
> + char *cp = str_error(errno, errmsg, sizeof(errmsg));
>
> pr_warning("failed to open %s: %s\n", obj->path, cp);
> return -errno;
> @@ -810,8 +811,7 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
> data->d_size, name, idx);
> if (err) {
> char errmsg[STRERR_BUFSIZE];
> - char *cp = strerror_r(-err, errmsg,
> - sizeof(errmsg));
> + char *cp = str_error(-err, errmsg, sizeof(errmsg));
>
> pr_warning("failed to alloc program %s (%s): %s",
> name, obj->path, cp);
> @@ -1140,7 +1140,7 @@ bpf_object__create_maps(struct bpf_object *obj)
>
> *pfd = bpf_create_map_xattr(&create_attr);
> if (*pfd < 0 && create_attr.btf_key_type_id) {
> - cp = strerror_r(errno, errmsg, sizeof(errmsg));
> + cp = str_error(errno, errmsg, sizeof(errmsg));
> pr_warning("Error in bpf_create_map_xattr(%s):%s(%d). Retrying without BTF.\n",
> map->name, cp, errno);
> create_attr.btf_fd = 0;
> @@ -1155,7 +1155,7 @@ bpf_object__create_maps(struct bpf_object *obj)
> size_t j;
>
> err = *pfd;
> - cp = strerror_r(errno, errmsg, sizeof(errmsg));
> + cp = str_error(errno, errmsg, sizeof(errmsg));
> pr_warning("failed to create map (name: '%s'): %s\n",
> map->name, cp);
> for (j = 0; j < i; j++)
> @@ -1339,7 +1339,7 @@ load_program(enum bpf_prog_type type, enum bpf_attach_type expected_attach_type,
> }
>
> ret = -LIBBPF_ERRNO__LOAD;
> - cp = strerror_r(errno, errmsg, sizeof(errmsg));
> + cp = str_error(errno, errmsg, sizeof(errmsg));
> pr_warning("load bpf program failed: %s\n", cp);
>
> if (log_buf && log_buf[0] != '\0') {
> @@ -1654,7 +1654,7 @@ static int check_path(const char *path)
>
> dir = dirname(dname);
> if (statfs(dir, &st_fs)) {
> - cp = strerror_r(errno, errmsg, sizeof(errmsg));
> + cp = str_error(errno, errmsg, sizeof(errmsg));
> pr_warning("failed to statfs %s: %s\n", dir, cp);
> err = -errno;
> }
> @@ -1690,7 +1690,7 @@ int bpf_program__pin_instance(struct bpf_program *prog, const char *path,
> }
>
> if (bpf_obj_pin(prog->instances.fds[instance], path)) {
> - cp = strerror_r(errno, errmsg, sizeof(errmsg));
> + cp = str_error(errno, errmsg, sizeof(errmsg));
> pr_warning("failed to pin program: %s\n", cp);
> return -errno;
> }
> @@ -1708,7 +1708,7 @@ static int make_dir(const char *path)
> err = -errno;
>
> if (err) {
> - cp = strerror_r(-err, errmsg, sizeof(errmsg));
> + cp = str_error(-err, errmsg, sizeof(errmsg));
> pr_warning("failed to mkdir %s: %s\n", path, cp);
> }
> return err;
> @@ -1770,7 +1770,7 @@ int bpf_map__pin(struct bpf_map *map, const char *path)
> }
>
> if (bpf_obj_pin(map->fd, path)) {
> - cp = strerror_r(errno, errmsg, sizeof(errmsg));
> + cp = str_error(errno, errmsg, sizeof(errmsg));
> pr_warning("failed to pin map: %s\n", cp);
> return -errno;
> }
> diff --git a/tools/lib/bpf/str_error.c b/tools/lib/bpf/str_error.c
> new file mode 100644
> index 000000000000..b8798114a357
> --- /dev/null
> +++ b/tools/lib/bpf/str_error.c
> @@ -0,0 +1,18 @@
> +// SPDX-License-Identifier: LGPL-2.1
> +#undef _GNU_SOURCE
> +#include <string.h>
> +#include <stdio.h>
> +#include "str_error.h"
> +
> +/*
> + * Wrapper to allow for building in non-GNU systems such as Alpine Linux's musl
> + * libc, while checking strerror_r() return to avoid having to check this in
> + * all places calling it.
> + */
> +char *str_error(int err, char *dst, int len)
> +{
> + int ret = strerror_r(err, dst, len);
> + if (ret)
> + snprintf(dst, len, "ERROR: strerror_r(%d)=%d", err, ret);
> + return dst;
> +}
> diff --git a/tools/lib/bpf/str_error.h b/tools/lib/bpf/str_error.h
> new file mode 100644
> index 000000000000..b9a22564ddc6
> --- /dev/null
> +++ b/tools/lib/bpf/str_error.h
> @@ -0,0 +1,6 @@
> +// SPDX-License-Identifier: GPL-2.1
LGPL-2.1 in the above?
The rest looks good to me.
Should we take it via bpf-next tree?
If you feel there is an urgency to fix musl build, we can take it via bpf tree too.
Jakub, thoughts? you've been messing with strerror last..
^ permalink raw reply
* [PATCH] docs: fix some broken documentation references
From: Mauro Carvalho Chehab @ 2018-09-17 19:02 UTC (permalink / raw)
To: linux-kernel
Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
Mauro Carvalho Chehab, Jonathan Corbet, Jan Kara,
Stephen Hemminger, David S. Miller, Michael Ellerman,
Greg Kroah-Hartman, Andrew Morton, Arnd Bergmann, linux-doc,
linux-ext4, bridge, netdev
Some documentation files received recent changes and are
pointing to wrong places.
Those references can easily fixed with the help of a
script:
$ ./scripts/documentation-file-ref-check --fix
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
---
Documentation/filesystems/dax.txt | 2 +-
Documentation/filesystems/ext2.txt | 2 +-
MAINTAINERS | 4 ++--
net/bridge/Kconfig | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/Documentation/filesystems/dax.txt b/Documentation/filesystems/dax.txt
index 70cb68bed2e8..bc393e0a22b8 100644
--- a/Documentation/filesystems/dax.txt
+++ b/Documentation/filesystems/dax.txt
@@ -75,7 +75,7 @@ exposure of uninitialized data through mmap.
These filesystems may be used for inspiration:
- ext2: see Documentation/filesystems/ext2.txt
-- ext4: see Documentation/filesystems/ext4.txt
+- ext4: see Documentation/filesystems/ext4/ext4.rst
- xfs: see Documentation/filesystems/xfs.txt
diff --git a/Documentation/filesystems/ext2.txt b/Documentation/filesystems/ext2.txt
index 81c0becab225..a45c9fc0747b 100644
--- a/Documentation/filesystems/ext2.txt
+++ b/Documentation/filesystems/ext2.txt
@@ -358,7 +358,7 @@ and are copied into the filesystem. If a transaction is incomplete at
the time of the crash, then there is no guarantee of consistency for
the blocks in that transaction so they are discarded (which means any
filesystem changes they represent are also lost).
-Check Documentation/filesystems/ext4.txt if you want to read more about
+Check Documentation/filesystems/ext4/ext4.rst if you want to read more about
ext4 and journaling.
References
diff --git a/MAINTAINERS b/MAINTAINERS
index 9989925f658d..078a4cf6d064 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -840,7 +840,7 @@ ANALOG DEVICES INC ADGS1408 DRIVER
M: Mircea Caprioru <mircea.caprioru@analog.com>
S: Supported
F: drivers/mux/adgs1408.c
-F: Documentation/devicetree/bindings/mux/adgs1408.txt
+F: Documentation/devicetree/bindings/mux/adi,adgs1408.txt
ANALOG DEVICES INC ADP5061 DRIVER
M: Stefan Popa <stefan.popa@analog.com>
@@ -5515,7 +5515,7 @@ W: http://ext4.wiki.kernel.org
Q: http://patchwork.ozlabs.org/project/linux-ext4/list/
T: git git://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git
S: Maintained
-F: Documentation/filesystems/ext4.txt
+F: Documentation/filesystems/ext4/ext4.rst
F: fs/ext4/
Extended Verification Module (EVM)
diff --git a/net/bridge/Kconfig b/net/bridge/Kconfig
index aa0d3b2f1bb7..3625d6ade45c 100644
--- a/net/bridge/Kconfig
+++ b/net/bridge/Kconfig
@@ -17,7 +17,7 @@ config BRIDGE
other third party bridge products.
In order to use the Ethernet bridge, you'll need the bridge
- configuration tools; see <file:Documentation/networking/bridge.txt>
+ configuration tools; see <file:Documentation/networking/bridge.rst>
for location. Please read the Bridge mini-HOWTO for more
information.
--
2.17.1
^ permalink raw reply related
* linux-next: manual merge of the net-next tree with the jc_docs tree
From: Stephen Rothwell @ 2018-09-18 0:14 UTC (permalink / raw)
To: David Miller, Networking, Jonathan Corbet
Cc: Linux-Next Mailing List, Linux Kernel Mailing List, Henrik Austad,
Tobin C. Harding
[-- Attachment #1: Type: text/plain, Size: 754 bytes --]
Hi all,
Today's linux-next merge of the net-next tree got a conflict in:
Documentation/networking/00-INDEX
between commit:
a7ddcea58ae2 ("Drop all 00-INDEX files from Documentation/")
from the jc_docs tree and commit:
a20625e49dde ("docs: net: Remove TCP congestion document")
from the net-next tree.
I fixed it up (I just removed the file) and can carry the fix as
necessary. This is now fixed as far as linux-next is concerned, but any
non trivial conflicts should be mentioned to your upstream maintainer
when your tree is submitted for merging. You may also want to consider
cooperating with the maintainer of the conflicting tree to minimise any
particularly complex conflicts.
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* linux-next: manual merge of the net-next tree with the net tree
From: Stephen Rothwell @ 2018-09-18 0:11 UTC (permalink / raw)
To: David Miller, Networking
Cc: Linux-Next Mailing List, Linux Kernel Mailing List,
Daniel Borkmann, Vakul Garg
[-- Attachment #1: Type: text/plain, Size: 3248 bytes --]
Hi all,
Today's linux-next merge of the net-next tree got a conflict in:
tools/testing/selftests/net/tls.c
between commit:
50c6b58a814d ("tls: fix currently broken MSG_PEEK behavior")
from the net tree and commit:
c2ad647c6442 ("selftests/tls: Add test for recv(PEEK) spanning across multiple records")
from the net-next tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
--
Cheers,
Stephen Rothwell
diff --cc tools/testing/selftests/net/tls.c
index 8fdfeafaf8c0,96fc6fe70293..000000000000
--- a/tools/testing/selftests/net/tls.c
+++ b/tools/testing/selftests/net/tls.c
@@@ -502,55 -502,28 +502,77 @@@ TEST_F(tls, recv_peek_multiple
EXPECT_EQ(memcmp(test_str, buf, send_len), 0);
}
+TEST_F(tls, recv_peek_multiple_records)
+{
+ char const *test_str = "test_read_peek_mult_recs";
+ char const *test_str_first = "test_read_peek";
+ char const *test_str_second = "_mult_recs";
+ int len;
+ char buf[64];
+
+ len = strlen(test_str_first);
+ EXPECT_EQ(send(self->fd, test_str_first, len, 0), len);
+
+ len = strlen(test_str_second) + 1;
+ EXPECT_EQ(send(self->fd, test_str_second, len, 0), len);
+
+ len = sizeof(buf);
+ memset(buf, 0, len);
+ EXPECT_NE(recv(self->cfd, buf, len, MSG_PEEK), -1);
+
+ /* MSG_PEEK can only peek into the current record. */
+ len = strlen(test_str_first) + 1;
+ EXPECT_EQ(memcmp(test_str_first, buf, len), 0);
+
+ len = sizeof(buf);
+ memset(buf, 0, len);
+ EXPECT_NE(recv(self->cfd, buf, len, 0), -1);
+
+ /* Non-MSG_PEEK will advance strparser (and therefore record)
+ * however.
+ */
+ len = strlen(test_str) + 1;
+ EXPECT_EQ(memcmp(test_str, buf, len), 0);
+
+ /* MSG_MORE will hold current record open, so later MSG_PEEK
+ * will see everything.
+ */
+ len = strlen(test_str_first);
+ EXPECT_EQ(send(self->fd, test_str_first, len, MSG_MORE), len);
+
+ len = strlen(test_str_second) + 1;
+ EXPECT_EQ(send(self->fd, test_str_second, len, 0), len);
+
+ len = sizeof(buf);
+ memset(buf, 0, len);
+ EXPECT_NE(recv(self->cfd, buf, len, MSG_PEEK), -1);
+
+ len = strlen(test_str) + 1;
+ EXPECT_EQ(memcmp(test_str, buf, len), 0);
+}
+
+ TEST_F(tls, recv_peek_large_buf_mult_recs)
+ {
+ char const *test_str = "test_read_peek_mult_recs";
+ char const *test_str_first = "test_read_peek";
+ char const *test_str_second = "_mult_recs";
+ int len;
+ char buf[64];
+
+ len = strlen(test_str_first);
+ EXPECT_EQ(send(self->fd, test_str_first, len, 0), len);
+
+ len = strlen(test_str_second) + 1;
+ EXPECT_EQ(send(self->fd, test_str_second, len, 0), len);
+
+ len = sizeof(buf);
+ memset(buf, 0, len);
+ EXPECT_NE(recv(self->cfd, buf, len, MSG_PEEK), -1);
+
+ len = strlen(test_str) + 1;
+ EXPECT_EQ(memcmp(test_str, buf, len), 0);
+ }
+
TEST_F(tls, pollin)
{
char const *test_str = "test_poll";
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH] net: apm: xgene: force XGene enet driver to re-balance IRQ usage
From: Al Stone @ 2018-09-17 23:35 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: Al Stone, Iyappan Subramanian, Keyur Chudgar, Quan Nguyen,
David S . Miller
When using the user-space command 'tuned-adm profile network-latency',
the XGene enet driver will cause a hang trying to enable IRQs while
the system is trying to tune itself to be more responsive to network
traffic; dmesg will even tell us that the enables/disables are not
in balance. With this fix, we force the driver to only enable_irq()
when there has been a previous disable_irq() -- i.e., force the number
of calls to each to be balanced. This allows the kernel to continue
operating.
In an ideal world, the XGene enet driver would be restructured to avoid
this problem (it seems to be an artifact of when additional packets
arrive and differences of opinion in how the NIC responds under those
circumstances, some of which is controlled by firmware). In the XGene2
driver, this is not an issue.
However, the XGene (aka Mustang) where this NIC is used is most likely
at the end of its useful life (APM which originally created the XGene
has completely morphed into a new company). It is unlikely the driver
restructuring that is needed will ever be done. There are, however,
a bunch of these machines out in the real world, and there are many
of us still using them daily (me, for example). So, while this patch
is not the ideal way to repair the NIC driver, it does work and allows
us to continue using these boxes for a while longer.
Cc: Iyappan Subramanian <isubramanian@apm.com>
Cc: Keyur Chudgar <kchudgar@apm.com>
Cc: Quan Nguyen <qnguyen@apm.com>
Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: Al Stone <ahs3@redhat.com>
---
drivers/net/ethernet/apm/xgene/xgene_enet_main.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_main.c b/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
index 3b889efddf78..90fb87f7e24e 100644
--- a/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
+++ b/drivers/net/ethernet/apm/xgene/xgene_enet_main.c
@@ -866,8 +866,11 @@ static int xgene_enet_napi(struct napi_struct *napi, const int budget)
processed = xgene_enet_process_ring(ring, budget);
if (processed != budget) {
+ struct irq_desc *desc = irq_to_desc(ring->irq);
+
napi_complete_done(napi, processed);
- enable_irq(ring->irq);
+ if (desc && desc->depth > 0)
+ enable_irq(ring->irq);
}
return processed;
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net] net/ipv4: defensive cipso option parsing
From: Nuernberger, Stefan @ 2018-09-17 18:02 UTC (permalink / raw)
To: paul@paul-moore.com
Cc: netdev@vger.kernel.org, Nuernberger, Stefan,
yujuan.qi@mediatek.com, Shah, Amit, stable@vger.kernel.org
In-Reply-To: <CAHC9VhQ+VKPFmx3R7Ty60KAJhiZwnc2-ZKRYG9w2NSAH7vgnoQ@mail.gmail.com>
On Mon, 2018-09-17 at 12:35 -0400, Paul Moore wrote:
> On Mon, Sep 17, 2018 at 11:12 AM Stefan Nuernberger <snu@amazon.com>
> wrote:
> >
> > commit 40413955ee26 ("Cipso: cipso_v4_optptr enter infinite loop")
> > fixed
> > a possible infinite loop in the IP option parsing of CIPSO. The fix
> > assumes that ip_options_compile filtered out all zero length
> > options and
> > that no other one-byte options beside IPOPT_END and IPOPT_NOOP
> > exist.
> > While this assumption currently holds true, add explicit checks for
> > zero
> > length and invalid length options to be safe for the future. Even
> > though
> > ip_options_compile should have validated the options, the
> > introduction of
> > new one-byte options can still confuse this code without the
> > additional
> > checks.
> >
> > Signed-off-by: Stefan Nuernberger <snu@amazon.com>
> > Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
> > Reviewed-by: Simon Veith <sveith@amazon.de>
> > Cc: stable@vger.kernel.org
> > ---
> > net/ipv4/cipso_ipv4.c | 10 ++++++++--
> > 1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
> > index 82178cc69c96..f291b57b8474 100644
> > --- a/net/ipv4/cipso_ipv4.c
> > +++ b/net/ipv4/cipso_ipv4.c
> > @@ -1512,7 +1512,7 @@ static int cipso_v4_parsetag_loc(const struct
> > cipso_v4_doi *doi_def,
> > *
> > * Description:
> > * Parse the packet's IP header looking for a CIPSO
> > option. Returns a pointer
> > - * to the start of the CIPSO option on success, NULL if one if not
> > found.
> > + * to the start of the CIPSO option on success, NULL if one is not
> > found.
> > *
> > */
> > unsigned char *cipso_v4_optptr(const struct sk_buff *skb)
> > @@ -1522,9 +1522,11 @@ unsigned char *cipso_v4_optptr(const struct
> > sk_buff *skb)
> > int optlen;
> > int taglen;
> >
> > - for (optlen = iph->ihl*4 - sizeof(struct iphdr); optlen >
> > 0; ) {
> > + for (optlen = iph->ihl*4 - sizeof(struct iphdr); optlen >
> > 1; ) {
> > switch (optptr[0]) {
> > case IPOPT_CIPSO:
> > + if (!optptr[1] || optptr[1] > optlen)
> > + return NULL;
> > return optptr;
> > case IPOPT_END:
> > return NULL;
> > @@ -1534,6 +1536,10 @@ unsigned char *cipso_v4_optptr(const struct
> > sk_buff *skb)
> > default:
> > taglen = optptr[1];
> > }
> > +
> > + if (!taglen || taglen > optlen)
> > + break;
> I tend to think that you reach a point where you simply need to trust
> that the stack is doing the right thing and that by the time you hit
> a
> certain point you can safely assume that the packet is well formed,
> but I'm not going to fight about that here.
>
> Regardless of the above, I don't like how you're doing the option
> length check twice in this code, that looks ugly to me, I think we
> can
> do better. How about something like this:
>
> for (...) {
> switch(optptr[0]) {
> case IPOPT_END:
> return NULL;
> case IPOPT_NOOP:
> taglen = 1;
> default:
> taglen = optptr[1];
> }
> if (taglen == 0 || taglen > optlen)
> return NULL;
> if (optptr[0] == IPOPT_CIPSO)
> return optptr;
> ....
> }
>
You're right, that looks much better. I sent around a new patch.
> >
> > optlen -= taglen;
> > optptr += taglen;
> > }
Amazon Development Center Germany GmbH
Berlin - Dresden - Aachen
main office: Krausenstr. 38, 10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B
^ permalink raw reply
* [bpf PATCH v2 3/3] bpf: test_maps, only support ESTABLISHED socks
From: John Fastabend @ 2018-09-17 17:59 UTC (permalink / raw)
To: edumazet, ast, daniel; +Cc: netdev
In-Reply-To: <20180917175801.3870.61111.stgit@john-Precision-Tower-5810>
Ensure that sockets added to a sock{map|hash} that is not in the
ESTABLISHED state is rejected.
Fixes: 1aa12bdf1bfb ("bpf: sockmap, add sock close() hook to remove socks")
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
tools/testing/selftests/bpf/test_maps.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
index 6f54f84..0f2090f 100644
--- a/tools/testing/selftests/bpf/test_maps.c
+++ b/tools/testing/selftests/bpf/test_maps.c
@@ -580,7 +580,11 @@ static void test_sockmap(int tasks, void *data)
/* Test update without programs */
for (i = 0; i < 6; i++) {
err = bpf_map_update_elem(fd, &i, &sfd[i], BPF_ANY);
- if (err) {
+ if (i < 2 && !err) {
+ printf("Allowed update sockmap '%i:%i' not in ESTABLISHED\n",
+ i, sfd[i]);
+ goto out_sockmap;
+ } else if (i > 1 && err) {
printf("Failed noprog update sockmap '%i:%i'\n",
i, sfd[i]);
goto out_sockmap;
@@ -741,7 +745,7 @@ static void test_sockmap(int tasks, void *data)
}
/* Test map update elem afterwards fd lives in fd and map_fd */
- for (i = 0; i < 6; i++) {
+ for (i = 2; i < 6; i++) {
err = bpf_map_update_elem(map_fd_rx, &i, &sfd[i], BPF_ANY);
if (err) {
printf("Failed map_fd_rx update sockmap %i '%i:%i'\n",
@@ -845,7 +849,7 @@ static void test_sockmap(int tasks, void *data)
}
/* Delete the elems without programs */
- for (i = 0; i < 6; i++) {
+ for (i = 2; i < 6; i++) {
err = bpf_map_delete_elem(fd, &i);
if (err) {
printf("Failed delete sockmap %i '%i:%i'\n",
^ permalink raw reply related
* [bpf PATCH v2 2/3] bpf: sockmap, fix transition through disconnect without close
From: John Fastabend @ 2018-09-17 17:59 UTC (permalink / raw)
To: edumazet, ast, daniel; +Cc: netdev
In-Reply-To: <20180917175801.3870.61111.stgit@john-Precision-Tower-5810>
It is possible (via shutdown()) for TCP socks to go trough TCP_CLOSE
state via tcp_disconnect() without actually calling tcp_close which
would then call our bpf_tcp_close() callback. Because of this a user
could disconnect a socket then put it in a LISTEN state which would
break our assumptions about sockets always being ESTABLISHED state.
To resolve this rely on the unhash hook, which is called in the
disconnect case, to remove the sock from the sockmap.
Reported-by: Eric Dumazet <edumazet@google.com>
Fixes: 1aa12bdf1bfb ("bpf: sockmap, add sock close() hook to remove socks")
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
kernel/bpf/sockmap.c | 71 +++++++++++++++++++++++++++++++++++++-------------
1 file changed, 52 insertions(+), 19 deletions(-)
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index 1f97b55..7deb362 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -132,6 +132,7 @@ struct smap_psock {
struct work_struct gc_work;
struct proto *sk_proto;
+ void (*save_unhash)(struct sock *sk);
void (*save_close)(struct sock *sk, long timeout);
void (*save_data_ready)(struct sock *sk);
void (*save_write_space)(struct sock *sk);
@@ -143,6 +144,7 @@ static int bpf_tcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
static int bpf_tcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t size);
static int bpf_tcp_sendpage(struct sock *sk, struct page *page,
int offset, size_t size, int flags);
+static void bpf_tcp_unhash(struct sock *sk);
static void bpf_tcp_close(struct sock *sk, long timeout);
static inline struct smap_psock *smap_psock_sk(const struct sock *sk)
@@ -184,6 +186,7 @@ static void build_protos(struct proto prot[SOCKMAP_NUM_CONFIGS],
struct proto *base)
{
prot[SOCKMAP_BASE] = *base;
+ prot[SOCKMAP_BASE].unhash = bpf_tcp_unhash;
prot[SOCKMAP_BASE].close = bpf_tcp_close;
prot[SOCKMAP_BASE].recvmsg = bpf_tcp_recvmsg;
prot[SOCKMAP_BASE].stream_memory_read = bpf_tcp_stream_read;
@@ -217,6 +220,7 @@ static int bpf_tcp_init(struct sock *sk)
return -EBUSY;
}
+ psock->save_unhash = sk->sk_prot->unhash;
psock->save_close = sk->sk_prot->close;
psock->sk_proto = sk->sk_prot;
@@ -305,30 +309,12 @@ static struct smap_psock_map_entry *psock_map_pop(struct sock *sk,
return e;
}
-static void bpf_tcp_close(struct sock *sk, long timeout)
+static void bpf_tcp_remove(struct sock *sk, struct smap_psock *psock)
{
- void (*close_fun)(struct sock *sk, long timeout);
struct smap_psock_map_entry *e;
struct sk_msg_buff *md, *mtmp;
- struct smap_psock *psock;
struct sock *osk;
- lock_sock(sk);
- rcu_read_lock();
- psock = smap_psock_sk(sk);
- if (unlikely(!psock)) {
- rcu_read_unlock();
- release_sock(sk);
- return sk->sk_prot->close(sk, timeout);
- }
-
- /* The psock may be destroyed anytime after exiting the RCU critial
- * section so by the time we use close_fun the psock may no longer
- * be valid. However, bpf_tcp_close is called with the sock lock
- * held so the close hook and sk are still valid.
- */
- close_fun = psock->save_close;
-
if (psock->cork) {
free_start_sg(psock->sock, psock->cork, true);
kfree(psock->cork);
@@ -379,6 +365,53 @@ static void bpf_tcp_close(struct sock *sk, long timeout)
kfree(e);
e = psock_map_pop(sk, psock);
}
+}
+
+static void bpf_tcp_unhash(struct sock *sk)
+{
+ void (*unhash_fun)(struct sock *sk);
+ struct smap_psock *psock;
+
+ rcu_read_lock();
+ psock = smap_psock_sk(sk);
+ if (unlikely(!psock)) {
+ rcu_read_unlock();
+ release_sock(sk);
+ return sk->sk_prot->unhash(sk);
+ }
+
+ /* The psock may be destroyed anytime after exiting the RCU critial
+ * section so by the time we use close_fun the psock may no longer
+ * be valid. However, bpf_tcp_close is called with the sock lock
+ * held so the close hook and sk are still valid.
+ */
+ unhash_fun = psock->save_unhash;
+ bpf_tcp_remove(sk, psock);
+ rcu_read_unlock();
+ unhash_fun(sk);
+}
+
+static void bpf_tcp_close(struct sock *sk, long timeout)
+{
+ void (*close_fun)(struct sock *sk, long timeout);
+ struct smap_psock *psock;
+
+ lock_sock(sk);
+ rcu_read_lock();
+ psock = smap_psock_sk(sk);
+ if (unlikely(!psock)) {
+ rcu_read_unlock();
+ release_sock(sk);
+ return sk->sk_prot->close(sk, timeout);
+ }
+
+ /* The psock may be destroyed anytime after exiting the RCU critial
+ * section so by the time we use close_fun the psock may no longer
+ * be valid. However, bpf_tcp_close is called with the sock lock
+ * held so the close hook and sk are still valid.
+ */
+ close_fun = psock->save_close;
+ bpf_tcp_remove(sk, psock);
rcu_read_unlock();
release_sock(sk);
close_fun(sk, timeout);
^ permalink raw reply related
* [bpf PATCH v2 1/3] bpf: sockmap only allow ESTABLISHED sock state
From: John Fastabend @ 2018-09-17 17:59 UTC (permalink / raw)
To: edumazet, ast, daniel; +Cc: netdev
In-Reply-To: <20180917175801.3870.61111.stgit@john-Precision-Tower-5810>
After this patch we only allow socks that are in ESTABLISHED state or
are being added via a sock_ops event that is transitioning into an
ESTABLISHED state. By allowing sock_ops events we allow users to
manage sockmaps directly from sock ops programs. The two supported
sock_ops ops are BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB and
BPF_SOCK_OPS_ACTIVE_ESTABLISHED_CB.
Similar to TLS ULP this ensures sk_user_data is correct.
Reported-by: Eric Dumazet <edumazet@google.com>
Fixes: 1aa12bdf1bfb ("bpf: sockmap, add sock close() hook to remove socks")
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
kernel/bpf/sockmap.c | 31 ++++++++++++++++++++++++++++++-
1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index 488ef96..1f97b55 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -2097,8 +2097,12 @@ static int sock_map_update_elem(struct bpf_map *map,
return -EINVAL;
}
+ /* ULPs are currently supported only for TCP sockets in ESTABLISHED
+ * state.
+ */
if (skops.sk->sk_type != SOCK_STREAM ||
- skops.sk->sk_protocol != IPPROTO_TCP) {
+ skops.sk->sk_protocol != IPPROTO_TCP ||
+ skops.sk->sk_state != TCP_ESTABLISHED) {
fput(socket->file);
return -EOPNOTSUPP;
}
@@ -2453,6 +2457,16 @@ static int sock_hash_update_elem(struct bpf_map *map,
return -EINVAL;
}
+ /* ULPs are currently supported only for TCP sockets in ESTABLISHED
+ * state.
+ */
+ if (skops.sk->sk_type != SOCK_STREAM ||
+ skops.sk->sk_protocol != IPPROTO_TCP ||
+ skops.sk->sk_state != TCP_ESTABLISHED) {
+ fput(socket->file);
+ return -EOPNOTSUPP;
+ }
+
lock_sock(skops.sk);
preempt_disable();
rcu_read_lock();
@@ -2543,10 +2557,22 @@ struct sock *__sock_hash_lookup_elem(struct bpf_map *map, void *key)
.map_check_btf = map_check_no_btf,
};
+static bool bpf_is_valid_sock_op(struct bpf_sock_ops_kern *ops)
+{
+ return ops->op == BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB ||
+ ops->op == BPF_SOCK_OPS_ACTIVE_ESTABLISHED_CB;
+}
BPF_CALL_4(bpf_sock_map_update, struct bpf_sock_ops_kern *, bpf_sock,
struct bpf_map *, map, void *, key, u64, flags)
{
WARN_ON_ONCE(!rcu_read_lock_held());
+
+ /* ULPs are currently supported only for TCP sockets in ESTABLISHED
+ * state. This checks that the sock ops triggering the update is
+ * one indicating we are (or will be soon) in an ESTABLISHED state.
+ */
+ if (!bpf_is_valid_sock_op(bpf_sock))
+ return -EOPNOTSUPP;
return sock_map_ctx_update_elem(bpf_sock, map, key, flags);
}
@@ -2565,6 +2591,9 @@ struct sock *__sock_hash_lookup_elem(struct bpf_map *map, void *key)
struct bpf_map *, map, void *, key, u64, flags)
{
WARN_ON_ONCE(!rcu_read_lock_held());
+
+ if (!bpf_is_valid_sock_op(bpf_sock))
+ return -EOPNOTSUPP;
return sock_hash_ctx_update_elem(bpf_sock, map, key, flags);
}
^ permalink raw reply related
* [bpf PATCH v2 0/3] bpf, sockmap ESTABLISHED state only
From: John Fastabend @ 2018-09-17 17:59 UTC (permalink / raw)
To: edumazet, ast, daniel; +Cc: netdev
Eric noted that using the close callback is not sufficient
to catch all transitions from ESTABLISHED state to a LISTEN
state. So this series does two things. First, only allow
adding socks in ESTABLISH state and second use unhash callback
to catch tcp_disconnect() transitions.
v2: Added check for ESTABLISH state in hash update sockmap as well.
Thanks,
John
---
John Fastabend (3):
bpf: sockmap only allow ESTABLISHED sock state
bpf: sockmap, fix transition through disconnect without close
bpf: test_maps, only support ESTABLISHED socks
kernel/bpf/sockmap.c | 102 +++++++++++++++++++++++++------
tools/testing/selftests/bpf/test_maps.c | 10 ++-
2 files changed, 89 insertions(+), 23 deletions(-)
^ permalink raw reply
* Re: [PATCH net-next] ravb: do not write 1 to reserved bits
From: Sergei Shtylyov @ 2018-09-17 17:58 UTC (permalink / raw)
To: Simon Horman, David Miller
Cc: Magnus Damm, netdev, linux-renesas-soc, Kazuya Mizuguchi
In-Reply-To: <20180917151911.25450-1-horms+renesas@verge.net.au>
On 09/17/2018 06:19 PM, Simon Horman wrote:
> From: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com>
>
> EtherAVB hardware requires 0 to be written to status register bits in
> order to clear them, however, care must be taken not to:
>
> 1. Clear other bits, by writing zero to them
> 2. Write one to reserved bits
>
> This patch corrects the ravb driver with respect to the second point above.
> This is done by defining reserved bit masks for the affected registers and,
> after auditing the code, ensure all sites that may write a one to a
> reserved bit use are suitably masked.
>
> Signed-off-by: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> ---
> v2 [Simon Horman]
> * Cover ravb_timestamp_interrupt() by this change
> * Use enum value rather than #define for reserved masks
> * Reword changelog
>
> v1 [Kazuya Mizuguchi]
> ---
> drivers/net/ethernet/renesas/ravb.h | 6 ++++++
> drivers/net/ethernet/renesas/ravb_main.c | 11 ++++++-----
> drivers/net/ethernet/renesas/ravb_ptp.c | 2 +-
> 3 files changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
> index 1470fc12282b..bca219edcf94 100644
> --- a/drivers/net/ethernet/renesas/ravb.h
> +++ b/drivers/net/ethernet/renesas/ravb.h
> @@ -428,6 +428,7 @@ enum EIS_BIT {
> EIS_CULF1 = 0x00000080,
> EIS_TFFF = 0x00000100,
> EIS_QFS = 0x00010000,
> + EIS_RESERVED = (u32)(GENMASK(31, 17) | GENMASK(15, 11)),
Are you sure those (u32) casts are necessary? Happily builds in both 32- and 64-bit
mode without them...
[...]
> @@ -528,6 +530,7 @@ enum RIS2_BIT {
> RIS2_QFF16 = 0x00010000,
> RIS2_QFF17 = 0x00020000,
> RIS2_RFFF = 0x80000000,
> + RIS2_RESERVED = (u32)GENMASK_ULL(30, 18),
Why GENMASK_ULL() suddenly? Doesn't seem needed at all...
[...]
> @@ -544,6 +547,8 @@ enum TIS_BIT {
> TIS_FTF1 = 0x00000002, /* Undocumented? */
> TIS_TFUF = 0x00000100,
> TIS_TFWF = 0x00000200,
> + TIS_RESERVED = (u32)(GENMASK_ULL(31, 20) | GENMASK_ULL(15, 12) | \
> + GENMASK_ULL(7, 4))
Same question.
[...]
MBR, Sergei
^ permalink raw reply
* Re: [PATCH net] net: phy: sfp: Prevent NULL deference of socket_ops
From: Russell King - ARM Linux @ 2018-09-17 23:13 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, Andrew Lunn, David S. Miller, open list
In-Reply-To: <3abb0233-4234-ca8c-31ac-8a6b9e123064@gmail.com>
On Mon, Sep 17, 2018 at 10:30:52AM -0700, Florian Fainelli wrote:
> On 09/15/2018 12:32 AM, Russell King - ARM Linux wrote:
> > On Fri, Sep 14, 2018 at 04:01:31PM -0700, Florian Fainelli wrote:
> >> In case we have specified a SFP and an I2C phandle in the Device Tree,
> >> but we somehow failed to look up the I2C adapter (e.g: the driver is not
> >> enabled), we will leave dangling socket_ops, and the sfp_bus will still
> >> have been registered. We can then observe NULL pointer dereferences
> >> while doing ethtool -m:
> >>
> >> [ 20.218613] Unable to handle kernel NULL pointer dereference at virtual address 00000008
> >> [ 20.226977] pgd = (ptrval)
> >> [ 20.229785] [00000008] *pgd=7c400831, *pte=00000000, *ppte=00000000
> >> [ 20.236274] Internal error: Oops: 17 [#1] SMP ARM
> >> [ 20.241122] Modules linked in:
> >> [ 20.244278] CPU: 0 PID: 1480 Comm: ethtool Not tainted 4.19.0-rc3 #138
> >> [ 20.251013] Hardware name: Broadcom Northstar Plus SoC
> >> [ 20.256316] PC is at sfp_get_module_info+0x8/0x10
> >> [ 20.261172] LR is at dev_ethtool+0x218c/0x2afc
> >>
> >> Specifically guard against that.
> >
> > The other alternative to this would be to only set ndev->sfp_bus when
> > the bus moves to registered state, which would probably be a saner
> > alternative than to add an additional layer of tests. IOW:
>
> Indeed, your patch works for me:
>
> Tested-by: Florian Fainelli <f.fainelli@gmail.com>
Thanks, I'll re-send it properly for davem tomorrow.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 13.8Mbps down 630kbps up
According to speedtest.net: 13Mbps down 490kbps up
^ permalink raw reply
* [bpf PATCH 3/3] bpf: test_maps, only support ESTABLISHED socks
From: John Fastabend @ 2018-09-17 17:32 UTC (permalink / raw)
To: edumazet, ast, daniel; +Cc: netdev
In-Reply-To: <20180917172946.21218.66049.stgit@john-Precision-Tower-5810>
Ensure that sockets added to a sock{map|hash} that is not in the
ESTABLISHED state is rejected.
Fixes: 1aa12bdf1bfb ("bpf: sockmap, add sock close() hook to remove socks")
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
tools/testing/selftests/bpf/test_maps.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
index 6f54f84..0f2090f 100644
--- a/tools/testing/selftests/bpf/test_maps.c
+++ b/tools/testing/selftests/bpf/test_maps.c
@@ -580,7 +580,11 @@ static void test_sockmap(int tasks, void *data)
/* Test update without programs */
for (i = 0; i < 6; i++) {
err = bpf_map_update_elem(fd, &i, &sfd[i], BPF_ANY);
- if (err) {
+ if (i < 2 && !err) {
+ printf("Allowed update sockmap '%i:%i' not in ESTABLISHED\n",
+ i, sfd[i]);
+ goto out_sockmap;
+ } else if (i > 1 && err) {
printf("Failed noprog update sockmap '%i:%i'\n",
i, sfd[i]);
goto out_sockmap;
@@ -741,7 +745,7 @@ static void test_sockmap(int tasks, void *data)
}
/* Test map update elem afterwards fd lives in fd and map_fd */
- for (i = 0; i < 6; i++) {
+ for (i = 2; i < 6; i++) {
err = bpf_map_update_elem(map_fd_rx, &i, &sfd[i], BPF_ANY);
if (err) {
printf("Failed map_fd_rx update sockmap %i '%i:%i'\n",
@@ -845,7 +849,7 @@ static void test_sockmap(int tasks, void *data)
}
/* Delete the elems without programs */
- for (i = 0; i < 6; i++) {
+ for (i = 2; i < 6; i++) {
err = bpf_map_delete_elem(fd, &i);
if (err) {
printf("Failed delete sockmap %i '%i:%i'\n",
^ permalink raw reply related
* [bpf PATCH 2/3] bpf: sockmap, fix transition through disconnect without close
From: John Fastabend @ 2018-09-17 17:31 UTC (permalink / raw)
To: edumazet, ast, daniel; +Cc: netdev
In-Reply-To: <20180917172946.21218.66049.stgit@john-Precision-Tower-5810>
It is possible (via shutdown()) for TCP socks to go trough TCP_CLOSE
state via tcp_disconnect() without actually calling tcp_close which
would then call our bpf_tcp_close() callback. Because of this a user
could disconnect a socket then put it in a LISTEN state which would
break our assumptions about sockets always being ESTABLISHED state.
To resolve this rely on the unhash hook, which is called in the
disconnect case, to remove the sock from the sockmap.
Reported-by: Eric Dumazet <edumazet@google.com>
Fixes: 1aa12bdf1bfb ("bpf: sockmap, add sock close() hook to remove socks")
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
kernel/bpf/sockmap.c | 71 +++++++++++++++++++++++++++++++++++++-------------
1 file changed, 52 insertions(+), 19 deletions(-)
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index 998b7bd..f6ab7f3 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -132,6 +132,7 @@ struct smap_psock {
struct work_struct gc_work;
struct proto *sk_proto;
+ void (*save_unhash)(struct sock *sk);
void (*save_close)(struct sock *sk, long timeout);
void (*save_data_ready)(struct sock *sk);
void (*save_write_space)(struct sock *sk);
@@ -143,6 +144,7 @@ static int bpf_tcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
static int bpf_tcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t size);
static int bpf_tcp_sendpage(struct sock *sk, struct page *page,
int offset, size_t size, int flags);
+static void bpf_tcp_unhash(struct sock *sk);
static void bpf_tcp_close(struct sock *sk, long timeout);
static inline struct smap_psock *smap_psock_sk(const struct sock *sk)
@@ -184,6 +186,7 @@ static void build_protos(struct proto prot[SOCKMAP_NUM_CONFIGS],
struct proto *base)
{
prot[SOCKMAP_BASE] = *base;
+ prot[SOCKMAP_BASE].unhash = bpf_tcp_unhash;
prot[SOCKMAP_BASE].close = bpf_tcp_close;
prot[SOCKMAP_BASE].recvmsg = bpf_tcp_recvmsg;
prot[SOCKMAP_BASE].stream_memory_read = bpf_tcp_stream_read;
@@ -217,6 +220,7 @@ static int bpf_tcp_init(struct sock *sk)
return -EBUSY;
}
+ psock->save_unhash = sk->sk_prot->unhash;
psock->save_close = sk->sk_prot->close;
psock->sk_proto = sk->sk_prot;
@@ -305,30 +309,12 @@ static struct smap_psock_map_entry *psock_map_pop(struct sock *sk,
return e;
}
-static void bpf_tcp_close(struct sock *sk, long timeout)
+static void bpf_tcp_remove(struct sock *sk, struct smap_psock *psock)
{
- void (*close_fun)(struct sock *sk, long timeout);
struct smap_psock_map_entry *e;
struct sk_msg_buff *md, *mtmp;
- struct smap_psock *psock;
struct sock *osk;
- lock_sock(sk);
- rcu_read_lock();
- psock = smap_psock_sk(sk);
- if (unlikely(!psock)) {
- rcu_read_unlock();
- release_sock(sk);
- return sk->sk_prot->close(sk, timeout);
- }
-
- /* The psock may be destroyed anytime after exiting the RCU critial
- * section so by the time we use close_fun the psock may no longer
- * be valid. However, bpf_tcp_close is called with the sock lock
- * held so the close hook and sk are still valid.
- */
- close_fun = psock->save_close;
-
if (psock->cork) {
free_start_sg(psock->sock, psock->cork, true);
kfree(psock->cork);
@@ -379,6 +365,53 @@ static void bpf_tcp_close(struct sock *sk, long timeout)
kfree(e);
e = psock_map_pop(sk, psock);
}
+}
+
+static void bpf_tcp_unhash(struct sock *sk)
+{
+ void (*unhash_fun)(struct sock *sk);
+ struct smap_psock *psock;
+
+ rcu_read_lock();
+ psock = smap_psock_sk(sk);
+ if (unlikely(!psock)) {
+ rcu_read_unlock();
+ release_sock(sk);
+ return sk->sk_prot->unhash(sk);
+ }
+
+ /* The psock may be destroyed anytime after exiting the RCU critial
+ * section so by the time we use close_fun the psock may no longer
+ * be valid. However, bpf_tcp_close is called with the sock lock
+ * held so the close hook and sk are still valid.
+ */
+ unhash_fun = psock->save_unhash;
+ bpf_tcp_remove(sk, psock);
+ rcu_read_unlock();
+ unhash_fun(sk);
+}
+
+static void bpf_tcp_close(struct sock *sk, long timeout)
+{
+ void (*close_fun)(struct sock *sk, long timeout);
+ struct smap_psock *psock;
+
+ lock_sock(sk);
+ rcu_read_lock();
+ psock = smap_psock_sk(sk);
+ if (unlikely(!psock)) {
+ rcu_read_unlock();
+ release_sock(sk);
+ return sk->sk_prot->close(sk, timeout);
+ }
+
+ /* The psock may be destroyed anytime after exiting the RCU critial
+ * section so by the time we use close_fun the psock may no longer
+ * be valid. However, bpf_tcp_close is called with the sock lock
+ * held so the close hook and sk are still valid.
+ */
+ close_fun = psock->save_close;
+ bpf_tcp_remove(sk, psock);
rcu_read_unlock();
release_sock(sk);
close_fun(sk, timeout);
^ permalink raw reply related
* [bpf PATCH 1/3] bpf: sockmap only allow ESTABLISHED sock state
From: John Fastabend @ 2018-09-17 17:31 UTC (permalink / raw)
To: edumazet, ast, daniel; +Cc: netdev
In-Reply-To: <20180917172946.21218.66049.stgit@john-Precision-Tower-5810>
After this patch we only allow socks that are in ESTABLISHED state or
are being added via a sock_ops event that is transitioning into an
ESTABLISHED state. By allowing sock_ops events we allow users to
manage sockmaps directly from sock ops programs. The two supported
sock_ops ops are BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB and
BPF_SOCK_OPS_ACTIVE_ESTABLISHED_CB.
Similar to TLS ULP this ensures sk_user_data is correct.
Reported-by: Eric Dumazet <edumazet@google.com>
Fixes: 1aa12bdf1bfb ("bpf: sockmap, add sock close() hook to remove socks")
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
kernel/bpf/sockmap.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index 488ef96..998b7bd 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -2097,8 +2097,12 @@ static int sock_map_update_elem(struct bpf_map *map,
return -EINVAL;
}
+ /* ULPs are currently supported only for TCP sockets in ESTABLISHED
+ * state.
+ */
if (skops.sk->sk_type != SOCK_STREAM ||
- skops.sk->sk_protocol != IPPROTO_TCP) {
+ skops.sk->sk_protocol != IPPROTO_TCP ||
+ skops.sk->sk_state != TCP_ESTABLISHED) {
fput(socket->file);
return -EOPNOTSUPP;
}
@@ -2543,10 +2547,22 @@ struct sock *__sock_hash_lookup_elem(struct bpf_map *map, void *key)
.map_check_btf = map_check_no_btf,
};
+static bool bpf_is_valid_sock_op(struct bpf_sock_ops_kern *ops)
+{
+ return ops->op == BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB ||
+ ops->op == BPF_SOCK_OPS_ACTIVE_ESTABLISHED_CB;
+}
BPF_CALL_4(bpf_sock_map_update, struct bpf_sock_ops_kern *, bpf_sock,
struct bpf_map *, map, void *, key, u64, flags)
{
WARN_ON_ONCE(!rcu_read_lock_held());
+
+ /* ULPs are currently supported only for TCP sockets in ESTABLISHED
+ * state. This checks that the sock ops triggering the update is
+ * one indicating we are (or will be soon) in an ESTABLISHED state.
+ */
+ if (!bpf_is_valid_sock_op(bpf_sock))
+ return -EOPNOTSUPP;
return sock_map_ctx_update_elem(bpf_sock, map, key, flags);
}
@@ -2565,6 +2581,9 @@ struct sock *__sock_hash_lookup_elem(struct bpf_map *map, void *key)
struct bpf_map *, map, void *, key, u64, flags)
{
WARN_ON_ONCE(!rcu_read_lock_held());
+
+ if (!bpf_is_valid_sock_op(bpf_sock))
+ return -EOPNOTSUPP;
return sock_hash_ctx_update_elem(bpf_sock, map, key, flags);
}
^ permalink raw reply related
* [bpf PATCH 0/3] bpf, sockmap ESTABLISHED state only
From: John Fastabend @ 2018-09-17 17:31 UTC (permalink / raw)
To: edumazet, ast, daniel; +Cc: netdev
Eric noted that using the close callback is not sufficient
to catch all transitions from ESTABLISHED state to a LISTEN
state. So this series does two things. First, only allow
adding socks in ESTABLISH state and second use unhash callback
to catch tcp_disconnect() transitions.
Thanks,
John
---
John Fastabend (3):
bpf: sockmap only allow ESTABLISHED sock state
bpf: sockmap, fix transition through disconnect without close
bpf: test_maps, only support ESTABLISHED socks
kernel/bpf/sockmap.c | 92 ++++++++++++++++++++++++-------
tools/testing/selftests/bpf/test_maps.c | 10 ++-
2 files changed, 79 insertions(+), 23 deletions(-)
^ permalink raw reply
* Re: [Patch net v2] net/ipv6: do not copy dst flags on rt init
From: David Ahern @ 2018-09-17 17:27 UTC (permalink / raw)
To: Peter Oskolkov, David Miller, netdev
In-Reply-To: <20180917172053.126170-1-posk@google.com>
On 9/17/18 10:20 AM, Peter Oskolkov wrote:
> DST_NOCOUNT in dst_entry::flags tracks whether the entry counts
> toward route cache size (net->ipv6.sysctl.ip6_rt_max_size).
>
> If the flag is NOT set, dst_ops::pcpuc_entries counter is incremented
> in dist_init() and decremented in dst_destroy().
>
> This flag is tied to allocation/deallocation of dst_entry and
> should not be copied from another dst/route. Otherwise it can happen
> that dst_ops::pcpuc_entries counter grows until no new routes can
> be allocated because the counter reached ip6_rt_max_size due to
> DST_NOCOUNT not set and thus no counter decrements on gc-ed routes.
>
> Fixes: 3b6761d18bc1 ("net/ipv6: Move dst flags to booleans in fib entries")
> Cc: David Ahern <dsahern@gmail.com>
> Acked-by: Wei Wang <weiwan@google.com>
> Signed-off-by: Peter Oskolkov <posk@google.com>
> ---
> net/ipv6/route.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 3eed045c65a5..480a79f47c52 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -946,8 +946,6 @@ static void ip6_rt_init_dst_reject(struct rt6_info *rt, struct fib6_info *ort)
>
> static void ip6_rt_init_dst(struct rt6_info *rt, struct fib6_info *ort)
> {
> - rt->dst.flags |= fib6_info_dst_flags(ort);
> -
> if (ort->fib6_flags & RTF_REJECT) {
> ip6_rt_init_dst_reject(rt, ort);
> return;
>
Reviewed-by: David Ahern <dsahern@gmail.com>
Thanks for the patch.
^ permalink raw reply
* Re: [PATCH net] net/ipv6: do not copy DST_NOCOUNT flag on rt init
From: Peter Oskolkov @ 2018-09-17 17:23 UTC (permalink / raw)
To: dsahern; +Cc: davem, netdev
In-Reply-To: <ecda87c9-4fdb-8967-2948-2fd138d330ec@gmail.com>
On Mon, Sep 17, 2018 at 9:59 AM David Ahern <dsahern@gmail.com> wrote:
>
> On 9/17/18 9:11 AM, Peter Oskolkov wrote:
> > On Thu, Sep 13, 2018 at 9:11 PM David Ahern <dsahern@gmail.com> wrote:
> >>
> >> On 9/13/18 1:38 PM, Peter Oskolkov wrote:
> >>
> >>> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> >>> index 3eed045c65a5..a3902f805305 100644
> >>> --- a/net/ipv6/route.c
> >>> +++ b/net/ipv6/route.c
> >>> @@ -946,7 +946,7 @@ static void ip6_rt_init_dst_reject(struct rt6_info *rt, struct fib6_info *ort)
> >>>
> >>> static void ip6_rt_init_dst(struct rt6_info *rt, struct fib6_info *ort)
> >>> {
> >>> - rt->dst.flags |= fib6_info_dst_flags(ort);
> >>> + rt->dst.flags |= fib6_info_dst_flags(ort) & ~DST_NOCOUNT;
> >>
> >> I think my mistake is setting dst.flags in ip6_rt_init_dst. Flags
> >> argument is passed to ip6_dst_alloc which is always invoked before
> >> ip6_rt_copy_init is called which is the only caller of ip6_rt_init_dst.
> >
> > ip6_rt_cache_alloc calls ip6_dst_alloc with zero as flags; and only
> > one flag is copied later (DST_HOST) outside of ip6_rt_init_dst().
> > If the flag assignment is completely removed from ip6_rt_init_dst(),
> > then DST_NOPOLICY flag will be lost.
> >
> > Which may be OK, but is more than what this patch tries to solve (do not
> > copy DST_NOCOUNT flag).
>
> In the 4.17 kernel (prior to the fib6_info change), ip6_rt_cache_alloc
> calls __ip6_dst_alloc with 0 for flags so this is correct. The mistake
> is ip6_rt_copy_init -> ip6_rt_init_dst -> fib6_info_dst_flags.
>
> I believe the right fix is to drop the 'rt->dst.flags |=
> fib6_info_dst_flags(ort);' from ip6_rt_init_dst.
OK, I sent a v2 with the assignment removed. Thanks for the review!
^ permalink raw reply
* [Patch net v2] net/ipv6: do not copy dst flags on rt init
From: Peter Oskolkov @ 2018-09-17 17:20 UTC (permalink / raw)
To: David Miller, netdev; +Cc: Peter Oskolkov, David Ahern
DST_NOCOUNT in dst_entry::flags tracks whether the entry counts
toward route cache size (net->ipv6.sysctl.ip6_rt_max_size).
If the flag is NOT set, dst_ops::pcpuc_entries counter is incremented
in dist_init() and decremented in dst_destroy().
This flag is tied to allocation/deallocation of dst_entry and
should not be copied from another dst/route. Otherwise it can happen
that dst_ops::pcpuc_entries counter grows until no new routes can
be allocated because the counter reached ip6_rt_max_size due to
DST_NOCOUNT not set and thus no counter decrements on gc-ed routes.
Fixes: 3b6761d18bc1 ("net/ipv6: Move dst flags to booleans in fib entries")
Cc: David Ahern <dsahern@gmail.com>
Acked-by: Wei Wang <weiwan@google.com>
Signed-off-by: Peter Oskolkov <posk@google.com>
---
net/ipv6/route.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 3eed045c65a5..480a79f47c52 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -946,8 +946,6 @@ static void ip6_rt_init_dst_reject(struct rt6_info *rt, struct fib6_info *ort)
static void ip6_rt_init_dst(struct rt6_info *rt, struct fib6_info *ort)
{
- rt->dst.flags |= fib6_info_dst_flags(ort);
-
if (ort->fib6_flags & RTF_REJECT) {
ip6_rt_init_dst_reject(rt, ort);
return;
--
2.19.0.397.gdd90340f6a-goog
^ permalink raw reply related
* [PATCH 4.14 121/126] ip: frags: fix crash in ip_do_fragment()
From: Greg Kroah-Hartman @ 2018-09-17 22:42 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable,
netdev@vger.kernel.org, stable@vger.kernel.org, edumazet@google.com, Taehee Yoo,
Eric Dumazet, David S. Miller, Taehee Yoo
In-Reply-To: <20180917211703.481236999@linuxfoundation.org>
4.14-stable review patch. If anyone has any objections, please let me know.
------------------
From: Taehee Yoo <ap420073@gmail.com>
commit 5d407b071dc369c26a38398326ee2be53651cfe4 upstream
A kernel crash occurrs when defragmented packet is fragmented
in ip_do_fragment().
In defragment routine, skb_orphan() is called and
skb->ip_defrag_offset is set. but skb->sk and
skb->ip_defrag_offset are same union member. so that
frag->sk is not NULL.
Hence crash occurrs in skb->sk check routine in ip_do_fragment() when
defragmented packet is fragmented.
test commands:
%iptables -t nat -I POSTROUTING -j MASQUERADE
%hping3 192.168.4.2 -s 1000 -p 2000 -d 60000
splat looks like:
[ 261.069429] kernel BUG at net/ipv4/ip_output.c:636!
[ 261.075753] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN PTI
[ 261.083854] CPU: 1 PID: 1349 Comm: hping3 Not tainted 4.19.0-rc2+ #3
[ 261.100977] RIP: 0010:ip_do_fragment+0x1613/0x2600
[ 261.106945] Code: e8 e2 38 e3 fe 4c 8b 44 24 18 48 8b 74 24 08 e9 92 f6 ff ff 80 3c 02 00 0f 85 da 07 00 00 48 8b b5 d0 00 00 00 e9 25 f6 ff ff <0f> 0b 0f 0b 44 8b 54 24 58 4c 8b 4c 24 18 4c 8b 5c 24 60 4c 8b 6c
[ 261.127015] RSP: 0018:ffff8801031cf2c0 EFLAGS: 00010202
[ 261.134156] RAX: 1ffff1002297537b RBX: ffffed0020639e6e RCX: 0000000000000004
[ 261.142156] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff880114ba9bd8
[ 261.150157] RBP: ffff880114ba8a40 R08: ffffed0022975395 R09: ffffed0022975395
[ 261.158157] R10: 0000000000000001 R11: ffffed0022975394 R12: ffff880114ba9ca4
[ 261.166159] R13: 0000000000000010 R14: ffff880114ba9bc0 R15: dffffc0000000000
[ 261.174169] FS: 00007fbae2199700(0000) GS:ffff88011b400000(0000) knlGS:0000000000000000
[ 261.183012] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 261.189013] CR2: 00005579244fe000 CR3: 0000000119bf4000 CR4: 00000000001006e0
[ 261.198158] Call Trace:
[ 261.199018] ? dst_output+0x180/0x180
[ 261.205011] ? save_trace+0x300/0x300
[ 261.209018] ? ip_copy_metadata+0xb00/0xb00
[ 261.213034] ? sched_clock_local+0xd4/0x140
[ 261.218158] ? kill_l4proto+0x120/0x120 [nf_conntrack]
[ 261.223014] ? rt_cpu_seq_stop+0x10/0x10
[ 261.227014] ? find_held_lock+0x39/0x1c0
[ 261.233008] ip_finish_output+0x51d/0xb50
[ 261.237006] ? ip_fragment.constprop.56+0x220/0x220
[ 261.243011] ? nf_ct_l4proto_register_one+0x5b0/0x5b0 [nf_conntrack]
[ 261.250152] ? rcu_is_watching+0x77/0x120
[ 261.255010] ? nf_nat_ipv4_out+0x1e/0x2b0 [nf_nat_ipv4]
[ 261.261033] ? nf_hook_slow+0xb1/0x160
[ 261.265007] ip_output+0x1c7/0x710
[ 261.269005] ? ip_mc_output+0x13f0/0x13f0
[ 261.273002] ? __local_bh_enable_ip+0xe9/0x1b0
[ 261.278152] ? ip_fragment.constprop.56+0x220/0x220
[ 261.282996] ? nf_hook_slow+0xb1/0x160
[ 261.287007] raw_sendmsg+0x21f9/0x4420
[ 261.291008] ? dst_output+0x180/0x180
[ 261.297003] ? sched_clock_cpu+0x126/0x170
[ 261.301003] ? find_held_lock+0x39/0x1c0
[ 261.306155] ? stop_critical_timings+0x420/0x420
[ 261.311004] ? check_flags.part.36+0x450/0x450
[ 261.315005] ? _raw_spin_unlock_irq+0x29/0x40
[ 261.320995] ? _raw_spin_unlock_irq+0x29/0x40
[ 261.326142] ? cyc2ns_read_end+0x10/0x10
[ 261.330139] ? raw_bind+0x280/0x280
[ 261.334138] ? sched_clock_cpu+0x126/0x170
[ 261.338995] ? check_flags.part.36+0x450/0x450
[ 261.342991] ? __lock_acquire+0x4500/0x4500
[ 261.348994] ? inet_sendmsg+0x11c/0x500
[ 261.352989] ? dst_output+0x180/0x180
[ 261.357012] inet_sendmsg+0x11c/0x500
[ ... ]
v2:
- clear skb->sk at reassembly routine.(Eric Dumarzet)
Fixes: fa0f527358bd ("ip: use rb trees for IP frag queue.")
Suggested-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
net/ipv4/ip_fragment.c | 1 +
net/ipv6/netfilter/nf_conntrack_reasm.c | 1 +
2 files changed, 2 insertions(+)
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -599,6 +599,7 @@ static int ip_frag_reasm(struct ipq *qp,
nextp = &fp->next;
fp->prev = NULL;
memset(&fp->rbnode, 0, sizeof(fp->rbnode));
+ fp->sk = NULL;
head->data_len += fp->len;
head->len += fp->len;
if (head->ip_summed != fp->ip_summed)
--- a/net/ipv6/netfilter/nf_conntrack_reasm.c
+++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
@@ -453,6 +453,7 @@ nf_ct_frag6_reasm(struct frag_queue *fq,
else if (head->ip_summed == CHECKSUM_COMPLETE)
head->csum = csum_add(head->csum, fp->csum);
head->truesize += fp->truesize;
+ fp->sk = NULL;
}
sub_frag_mem_limit(fq->q.net, head->truesize);
^ permalink raw reply
* [PATCH 4.14 118/126] ipv4: frags: precedence bug in ip_expire()
From: Greg Kroah-Hartman @ 2018-09-17 22:42 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable,
netdev@vger.kernel.org, stable@vger.kernel.org, edumazet@google.com, Dan Carpenter,
David S. Miller, Dan Carpenter
In-Reply-To: <20180917211703.481236999@linuxfoundation.org>
4.14-stable review patch. If anyone has any objections, please let me know.
------------------
From: Dan Carpenter <dan.carpenter@oracle.com>
We accidentally removed the parentheses here, but they are required
because '!' has higher precedence than '&'.
Fixes: fa0f527358bd ("ip: use rb trees for IP frag queue.")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit 70837ffe3085c9a91488b52ca13ac84424da1042)
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
net/ipv4/ip_fragment.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -154,7 +154,7 @@ static void ip_expire(struct timer_list
__IP_INC_STATS(net, IPSTATS_MIB_REASMFAILS);
__IP_INC_STATS(net, IPSTATS_MIB_REASMTIMEOUT);
- if (!qp->q.flags & INET_FRAG_FIRST_IN)
+ if (!(qp->q.flags & INET_FRAG_FIRST_IN))
goto out;
/* sk_buff::dev and sk_buff::rbnode are unionized. So we
^ permalink raw reply
* [PATCH 4.14 098/126] ipv6: export ip6 fragments sysctl to unprivileged users
From: Greg Kroah-Hartman @ 2018-09-17 22:42 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable,
netdev@vger.kernel.org, stable@vger.kernel.org, edumazet@google.com, Nikolay Borisov,
Eric Dumazet, David S. Miller, Nikolay Borisov
In-Reply-To: <20180917211703.481236999@linuxfoundation.org>
4.14-stable review patch. If anyone has any objections, please let me know.
------------------
From: Eric Dumazet <edumazet@google.com>
IPv4 was changed in commit 52a773d645e9 ("net: Export ip fragment
sysctl to unprivileged users")
The only sysctl that is not per-netns is not used :
ip6frag_secret_interval
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Nikolay Borisov <kernel@kyup.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit 18dcbe12fe9fca0ab825f7eff993060525ac2503)
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
net/ipv6/reassembly.c | 4 ----
1 file changed, 4 deletions(-)
--- a/net/ipv6/reassembly.c
+++ b/net/ipv6/reassembly.c
@@ -649,10 +649,6 @@ static int __net_init ip6_frags_ns_sysct
table[1].data = &net->ipv6.frags.low_thresh;
table[1].extra2 = &net->ipv6.frags.high_thresh;
table[2].data = &net->ipv6.frags.timeout;
-
- /* Don't export sysctls to unprivileged users */
- if (net->user_ns != &init_user_ns)
- table[0].procname = NULL;
}
hdr = register_net_sysctl(net, "net/ipv6", table);
^ permalink raw reply
* [PATCH 4.14 095/126] inet: frags: Convert timers to use timer_setup()
From: Greg Kroah-Hartman @ 2018-09-17 22:42 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable, Alexander Aring, Stefan Schmidt,
David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal, linux-wpan,
netdev, netfilter-devel, coreteam, Kees Cook
In-Reply-To: <20180917211703.481236999@linuxfoundation.org>
4.14-stable review patch. If anyone has any objections, please let me know.
------------------
From: Kees Cook <keescook@chromium.org>
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.
Cc: Alexander Aring <alex.aring@gmail.com>
Cc: Stefan Schmidt <stefan@osg.samsung.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
Cc: Florian Westphal <fw@strlen.de>
Cc: linux-wpan@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: netfilter-devel@vger.kernel.org
Cc: coreteam@netfilter.org
Signed-off-by: Kees Cook <keescook@chromium.org>
Acked-by: Stefan Schmidt <stefan@osg.samsung.com> # for ieee802154
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit 78802011fbe34331bdef6f2dfb1634011f0e4c32)
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
include/net/inet_frag.h | 2 +-
net/ieee802154/6lowpan/reassembly.c | 5 +++--
net/ipv4/inet_fragment.c | 4 ++--
net/ipv4/ip_fragment.c | 5 +++--
net/ipv6/netfilter/nf_conntrack_reasm.c | 5 +++--
net/ipv6/reassembly.c | 5 +++--
6 files changed, 15 insertions(+), 11 deletions(-)
--- a/include/net/inet_frag.h
+++ b/include/net/inet_frag.h
@@ -97,7 +97,7 @@ struct inet_frags {
void (*constructor)(struct inet_frag_queue *q,
const void *arg);
void (*destructor)(struct inet_frag_queue *);
- void (*frag_expire)(unsigned long data);
+ void (*frag_expire)(struct timer_list *t);
struct kmem_cache *frags_cachep;
const char *frags_cache_name;
};
--- a/net/ieee802154/6lowpan/reassembly.c
+++ b/net/ieee802154/6lowpan/reassembly.c
@@ -80,12 +80,13 @@ static void lowpan_frag_init(struct inet
fq->daddr = *arg->dst;
}
-static void lowpan_frag_expire(unsigned long data)
+static void lowpan_frag_expire(struct timer_list *t)
{
+ struct inet_frag_queue *frag = from_timer(frag, t, timer);
struct frag_queue *fq;
struct net *net;
- fq = container_of((struct inet_frag_queue *)data, struct frag_queue, q);
+ fq = container_of(frag, struct frag_queue, q);
net = container_of(fq->q.net, struct net, ieee802154_lowpan.frags);
spin_lock(&fq->q.lock);
--- a/net/ipv4/inet_fragment.c
+++ b/net/ipv4/inet_fragment.c
@@ -150,7 +150,7 @@ inet_evict_bucket(struct inet_frags *f,
spin_unlock(&hb->chain_lock);
hlist_for_each_entry_safe(fq, n, &expired, list_evictor)
- f->frag_expire((unsigned long) fq);
+ f->frag_expire(&fq->timer);
return evicted;
}
@@ -367,7 +367,7 @@ static struct inet_frag_queue *inet_frag
f->constructor(q, arg);
add_frag_mem_limit(nf, f->qsize);
- setup_timer(&q->timer, f->frag_expire, (unsigned long)q);
+ timer_setup(&q->timer, f->frag_expire, 0);
spin_lock_init(&q->lock);
refcount_set(&q->refcnt, 1);
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -191,12 +191,13 @@ static bool frag_expire_skip_icmp(u32 us
/*
* Oops, a fragment queue timed out. Kill it and send an ICMP reply.
*/
-static void ip_expire(unsigned long arg)
+static void ip_expire(struct timer_list *t)
{
+ struct inet_frag_queue *frag = from_timer(frag, t, timer);
struct ipq *qp;
struct net *net;
- qp = container_of((struct inet_frag_queue *) arg, struct ipq, q);
+ qp = container_of(frag, struct ipq, q);
net = container_of(qp->q.net, struct net, ipv4.frags);
rcu_read_lock();
--- a/net/ipv6/netfilter/nf_conntrack_reasm.c
+++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
@@ -169,12 +169,13 @@ static unsigned int nf_hashfn(const stru
return nf_hash_frag(nq->id, &nq->saddr, &nq->daddr);
}
-static void nf_ct_frag6_expire(unsigned long data)
+static void nf_ct_frag6_expire(struct timer_list *t)
{
+ struct inet_frag_queue *frag = from_timer(frag, t, timer);
struct frag_queue *fq;
struct net *net;
- fq = container_of((struct inet_frag_queue *)data, struct frag_queue, q);
+ fq = container_of(frag, struct frag_queue, q);
net = container_of(fq->q.net, struct net, nf_frag.frags);
ip6_expire_frag_queue(net, fq);
--- a/net/ipv6/reassembly.c
+++ b/net/ipv6/reassembly.c
@@ -169,12 +169,13 @@ out:
}
EXPORT_SYMBOL(ip6_expire_frag_queue);
-static void ip6_frag_expire(unsigned long data)
+static void ip6_frag_expire(struct timer_list *t)
{
+ struct inet_frag_queue *frag = from_timer(frag, t, timer);
struct frag_queue *fq;
struct net *net;
- fq = container_of((struct inet_frag_queue *)data, struct frag_queue, q);
+ fq = container_of(frag, struct frag_queue, q);
net = container_of(fq->q.net, struct net, ipv6.frags);
ip6_expire_frag_queue(net, fq);
^ permalink raw reply
* iproute2: fail to add fdb entries to ipv6 vxlan device
From: Lorenzo Bianconi @ 2018-09-17 17:13 UTC (permalink / raw)
To: netdev; +Cc: stephen, serhe.popovych
Hi all,
while working on IPv6 vlxan driver I figured out that with recent version
of iproute2 it is no longer possible to configure an IPv6 vxlan device without
endpoint info (local ip, remote ip or group ip) and later add entries in
the vxlan fdb. This issue can be triggered with the following reproducer:
$ip -6 link add vxlan0 type vxlan id 42 dev enp0s2 \
proxy nolearning l2miss l3miss
$bridge fdb add 46:47:1f:a7:1c:25 dev vxlan1 dst 2000::2
RTNETLINK answers: Address family not supported by protocol
Starting from commit 1e9b8072de2c ("iplink_vxlan: Get rid of inet_get_addr()")
the preferred_family is no longer taken into account if neither saddr or daddr
are provided and vxlan kernel module will use IPv4 as default remote inet
family neglecting the one provided by userspace.
I guess we can fix that issue in two ways:
1- add a new netlink attribute to vxlan driver in order to specify the
preferred_family to use
2- restore the behaviour introduced in commit 97d564b90ccb ("vxlan: use
preferred address family when neither group or remote is specified") with
the following patch
-- >8 --
diff --git a/ip/iplink_vxlan.c b/ip/iplink_vxlan.c
index 2bc253fc..831f39a2 100644
--- a/ip/iplink_vxlan.c
+++ b/ip/iplink_vxlan.c
@@ -82,6 +82,7 @@ static int vxlan_parse_opt(struct link_util *lu, int argc, char **argv,
__u64 attrs = 0;
bool set_op = (n->nlmsg_type == RTM_NEWLINK &&
!(n->nlmsg_flags & NLM_F_CREATE));
+ bool selected_family = false;
saddr.family = daddr.family = AF_UNSPEC;
@@ -356,12 +357,26 @@ static int vxlan_parse_opt(struct link_util *lu, int argc, char **argv,
int type = (saddr.family == AF_INET) ? IFLA_VXLAN_LOCAL
: IFLA_VXLAN_LOCAL6;
addattr_l(n, 1024, type, saddr.data, saddr.bytelen);
+ selected_family = true;
}
if (is_addrtype_inet(&daddr)) {
int type = (daddr.family == AF_INET) ? IFLA_VXLAN_GROUP
: IFLA_VXLAN_GROUP6;
addattr_l(n, 1024, type, daddr.data, daddr.bytelen);
+ selected_family = true;
+ }
+
+ if (!selected_family) {
+ if (preferred_family == AF_INET) {
+ get_addr(&daddr, "default", AF_INET);
+ addattr_l(n, 1024, IFLA_VXLAN_GROUP,
+ daddr.data, daddr.bytelen);
+ } else if (preferred_family == AF_INET6) {
+ get_addr(&daddr, "default", AF_INET6);
+ addattr_l(n, 1024, IFLA_VXLAN_GROUP6,
+ daddr.data, daddr.bytelen);
+ }
}
if (!set_op || VXLAN_ATTRSET(attrs, IFLA_VXLAN_LEARNING))
-- 8< --
What is the best way to proceed?
Regards,
Lorenzo
^ permalink raw reply related
* Re: [PATCH net] net/ipv6: do not copy DST_NOCOUNT flag on rt init
From: David Ahern @ 2018-09-17 16:59 UTC (permalink / raw)
To: Peter Oskolkov; +Cc: davem, netdev
In-Reply-To: <CAPNVh5f3_GPpvV6nequZ8JNBhMVVOpEsJQ_XRfTN-+eaMPOWpw@mail.gmail.com>
On 9/17/18 9:11 AM, Peter Oskolkov wrote:
> On Thu, Sep 13, 2018 at 9:11 PM David Ahern <dsahern@gmail.com> wrote:
>>
>> On 9/13/18 1:38 PM, Peter Oskolkov wrote:
>>
>>> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
>>> index 3eed045c65a5..a3902f805305 100644
>>> --- a/net/ipv6/route.c
>>> +++ b/net/ipv6/route.c
>>> @@ -946,7 +946,7 @@ static void ip6_rt_init_dst_reject(struct rt6_info *rt, struct fib6_info *ort)
>>>
>>> static void ip6_rt_init_dst(struct rt6_info *rt, struct fib6_info *ort)
>>> {
>>> - rt->dst.flags |= fib6_info_dst_flags(ort);
>>> + rt->dst.flags |= fib6_info_dst_flags(ort) & ~DST_NOCOUNT;
>>
>> I think my mistake is setting dst.flags in ip6_rt_init_dst. Flags
>> argument is passed to ip6_dst_alloc which is always invoked before
>> ip6_rt_copy_init is called which is the only caller of ip6_rt_init_dst.
>
> ip6_rt_cache_alloc calls ip6_dst_alloc with zero as flags; and only
> one flag is copied later (DST_HOST) outside of ip6_rt_init_dst().
> If the flag assignment is completely removed from ip6_rt_init_dst(),
> then DST_NOPOLICY flag will be lost.
>
> Which may be OK, but is more than what this patch tries to solve (do not
> copy DST_NOCOUNT flag).
In the 4.17 kernel (prior to the fib6_info change), ip6_rt_cache_alloc
calls __ip6_dst_alloc with 0 for flags so this is correct. The mistake
is ip6_rt_copy_init -> ip6_rt_init_dst -> fib6_info_dst_flags.
I believe the right fix is to drop the 'rt->dst.flags |=
fib6_info_dst_flags(ort);' from ip6_rt_init_dst.
^ 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