* Re: [PATCH 1/2] bnxt_en: Use refcount_t for refcount
From: Michael Chan @ 2019-07-31 17:48 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Chuhong Yuan, David S . Miller, Network Development, linux-kernel
In-Reply-To: <CA+FuTScqD4bMpm6n13ETFVEvSKnk_rRUzspzs9HB6B5Un101Dw@mail.gmail.com>
On Wed, Jul 31, 2019 at 9:06 AM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> On Wed, Jul 31, 2019 at 8:22 AM Chuhong Yuan <hslester96@gmail.com> wrote:
> >
> > refcount_t is better for reference counters since its
> > implementation can prevent overflows.
> > So convert atomic_t ref counters to refcount_t.
> >
> > Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
> > ---
> > drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 8 ++++----
> > drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h | 2 +-
> > 2 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
> > index fc77caf0a076..eb7ed34639e2 100644
> > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
> > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
> > @@ -49,7 +49,7 @@ static int bnxt_register_dev(struct bnxt_en_dev *edev, int ulp_id,
> > return -ENOMEM;
> > }
> >
> > - atomic_set(&ulp->ref_count, 0);
> > + refcount_set(&ulp->ref_count, 0);
>
> One feature of refcount_t is that it warns on refcount_inc from 0 to
> detect possible use-after_free. It appears that that can trigger here?
>
I think that's right. We need to change the driver to start counting
from 1 instead of 0 if we convert to refcount.
^ permalink raw reply
* Re: [PATCH v2 bpf-next 02/12] libbpf: implement BPF CO-RE offset relocation algorithm
From: Andrii Nakryiko @ 2019-07-31 17:18 UTC (permalink / raw)
To: Song Liu
Cc: Andrii Nakryiko, bpf, netdev@vger.kernel.org, Alexei Starovoitov,
daniel@iogearbox.net, Yonghong Song, Kernel Team
In-Reply-To: <4D2E1082-5013-4A50-B75D-AB88FDCAAC52@fb.com>
On Wed, Jul 31, 2019 at 1:30 AM Song Liu <songliubraving@fb.com> wrote:
>
>
>
> > On Jul 30, 2019, at 11:52 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> >
> > On Tue, Jul 30, 2019 at 10:19 PM Song Liu <songliubraving@fb.com> wrote:
> >>
> >>
> >>
> >>> On Jul 30, 2019, at 6:00 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> >>>
> >>> On Tue, Jul 30, 2019 at 5:39 PM Song Liu <songliubraving@fb.com> wrote:
> >>>>
> >>>>
> >>>>
> >>>>> On Jul 30, 2019, at 12:53 PM, Andrii Nakryiko <andriin@fb.com> wrote:
> >>>>>
> >>>>> This patch implements the core logic for BPF CO-RE offsets relocations.
> >>>>> Every instruction that needs to be relocated has corresponding
> >>>>> bpf_offset_reloc as part of BTF.ext. Relocations are performed by trying
> >>>>> to match recorded "local" relocation spec against potentially many
> >>>>> compatible "target" types, creating corresponding spec. Details of the
> >>>>> algorithm are noted in corresponding comments in the code.
> >>>>>
> >>>>> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
>
> [...]
>
> >>>>
> >>>
> >>> I just picked the most succinct and non-repetitive form. It's
> >>> immediately apparent which type it's implicitly converted to, so I
> >>> felt there is no need to repeat it. Also, just (void *) is much
> >>> shorter. :)
> >>
> >> _All_ other code in btf.c converts the pointer to the target type.
> >
> > Most in libbpf.c doesn't, though. Also, I try to preserve pointer
> > constness for uses that don't modify BTF types (pretty much all of
> > them in libbpf), so it becomes really verbose, despite extremely short
> > variable names:
> >
> > const struct btf_member *m = (const struct btf_member *)(t + 1);
>
> I don't think being verbose is a big problem here. Overusing
Problem is too big and strong word to describe this :). It hurts
readability and will often quite artificially force either wrapping
the line or unnecessarily splitting declaration and assignment. Void *
on the other hand is short and usually is in the same line as target
type declaration, if not, you'll have to find local variable
declaration to double-check type, if you are unsure.
Using (void *) + implicit cast to target pointer type is not
unprecedented in libbpf:
$ rg ' = \((const )?struct \w+ \*\)' tools/lib/bpf/ | wc -l
52
$ rg ' = \((const )?void \*\)' tools/lib/bpf/ | wc -l
35
52 vs 35 is majority overall, but not by a landslide.
> (void *) feels like a bigger problem.
Why do you feel it's a problem? void * conveys that we have a piece of
memory that we will need to reinterpret as some concrete pointer type.
That's what we are doing, skipping btf_type and then interpreting
memory after common btf_type prefix is some other type, depending on
actual BTF kind. I don't think void * is misleading in any way.
In any case, if you still feel strongly about this after all my
arguments, please let me know and I will convert them in this patch
set. It's not like I'm opposed to use duplicate type names (though it
does feel sort of Java-like before it got limited type inference),
it's just in practice it leads to unnecessarily verbose code which
doesn't really improve anything.
>
> >
> > Add one or two levels of nestedness and you are wrapping this line.
> >
> >> In some cases, it is not apparent which type it is converted to,
> >> for example:
> >>
> >> + m = (void *)(targ_type + 1);
> >>
> >> I would suggest we do implicit conversion whenever possible.
> >
> > Implicit conversion (`m = targ_type + 1;`) is a compilation error,
> > that won't work.
>
> I misused "implicit" here. I actually meant to say
>
> m = ((const struct btf_member *)(t + 1);
Ah, so you meant explicit, yep. It's either `void *` or `const struct
something *` then.
>
>
>
^ permalink raw reply
* Re: [PATCH bpf-next 1/9] selftests/bpf: prevent headers to be compiled as C code
From: Andrii Nakryiko @ 2019-07-31 17:04 UTC (permalink / raw)
To: Ilya Leoshkevich
Cc: Stanislav Fomichev, Andrii Nakryiko, bpf, Networking,
Alexei Starovoitov, Daniel Borkmann, Kernel Team
In-Reply-To: <3D822EE0-C033-4192-B505-A30E5EC23BC3@linux.ibm.com>
On Wed, Jul 31, 2019 at 6:21 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> > Am 27.07.2019 um 20:53 schrieb Andrii Nakryiko <andrii.nakryiko@gmail.com>:
> >
> > On Fri, Jul 26, 2019 at 3:01 PM Stanislav Fomichev <sdf@fomichev.me> wrote:
> >>
> >> On 07/26, Andrii Nakryiko wrote:
> >>> On Fri, Jul 26, 2019 at 2:21 PM Stanislav Fomichev <sdf@fomichev.me> wrote:
> >>>>
> >>>> On 07/26, Andrii Nakryiko wrote:
> >>>>> Apprently listing header as a normal dependency for a binary output
> >>>>> makes it go through compilation as if it was C code. This currently
> >>>>> works without a problem, but in subsequent commits causes problems for
> >>>>> differently generated test.h for test_progs. Marking those headers as
> >>>>> order-only dependency solves the issue.
> >>>> Are you sure it will not result in a situation where
> >>>> test_progs/test_maps is not regenerated if tests.h is updated.
> >>>>
> >>>> If I read the following doc correctly, order deps make sense for
> >>>> directories only:
> >>>> https://www.gnu.org/software/make/manual/html_node/Prerequisite-Types.html
> >>>>
> >>>> Can you maybe double check it with:
> >>>> * make
> >>>> * add new prog_tests/test_something.c
> >>>> * make
> >>>> to see if the binary is regenerated with test_something.c?
> >>>
> >>> Yeah, tested that, it triggers test_progs rebuild.
> >>>
> >>> Ordering is still preserved, because test.h is dependency of
> >>> test_progs.c, which is dependency of test_progs binary, so that's why
> >>> it works.
> >>>
> >>> As to why .h file is compiled as C file, I have no idea and ideally
> >>> that should be fixed somehow.
> >> I guess that's because it's a prerequisite and we have a target that
> >> puts all prerequisites when calling CC:
> >>
> >> test_progs: a.c b.c tests.h
> >> gcc a.c b.c tests.h -o test_progs
> >>
> >> So gcc compiles each input file.
> >
> > If that's really a definition of the rule, then it seems not exactly
> > correct. What if some of the prerequisites are some object files,
> > directories, etc. I'd say the rule should only include .c files. I'll
> > add it to my TODO list to go and check how all this is defined
> > somewhere, but for now I'm leaving everything as is in this patch.
> >
>
> I believe it’s an implicit built-in rule, which is defined by make
> itself here:
>
> https://git.savannah.gnu.org/cgit/make.git/tree/default.c?h=4.2.1#n131
>
> The observed behavior arises because these rules use $^ all over the
> place. My 2c is that it would be better to make the rule explicit,
> because it would cost just an extra line, but it would be immediately
> obvious why the code is correct w.r.t. rebuilds.
Thanks for pointing this out, Ilya! I agree, I'd rather have a simple
explicit rule in Makefile that having to guess where this is coming
from and why it doesn't work as you'd expect it to. If no one else
adds that first, I'll eventually get to this.
>
> Best regards,
> Ilya
^ permalink raw reply
* Re: [PATCH 1/2] arm64: Add support for function error injection
From: Leo Yan @ 2019-07-31 16:58 UTC (permalink / raw)
To: Will Deacon
Cc: Russell King, Oleg Nesterov, Catalin Marinas, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
Arnd Bergmann, linux-arm-kernel, linux-kernel, netdev, bpf,
Masami Hiramatsu, Justin He
In-Reply-To: <20190731160836.qmzlk3ndbahwhfmu@willie-the-truck>
Hi Will,
Thanks for reviewing.
On Wed, Jul 31, 2019 at 05:08:37PM +0100, Will Deacon wrote:
> On Tue, Jul 16, 2019 at 07:13:00PM +0800, Leo Yan wrote:
> > This patch implement regs_set_return_value() and
> > override_function_with_return() to support function error injection
> > for arm64.
> >
> > In the exception flow, arm64's general register x30 contains the value
> > for the link register; so we can just update pt_regs::pc with it rather
> > than redirecting execution to a dummy function that returns.
> >
> > This patch is heavily inspired by the commit 7cd01b08d35f ("powerpc:
> > Add support for function error injection").
> >
> > Signed-off-by: Leo Yan <leo.yan@linaro.org>
> > ---
> > arch/arm64/Kconfig | 1 +
> > arch/arm64/include/asm/error-injection.h | 13 +++++++++++++
> > arch/arm64/include/asm/ptrace.h | 5 +++++
> > arch/arm64/lib/Makefile | 2 ++
> > arch/arm64/lib/error-inject.c | 19 +++++++++++++++++++
> > 5 files changed, 40 insertions(+)
> > create mode 100644 arch/arm64/include/asm/error-injection.h
> > create mode 100644 arch/arm64/lib/error-inject.c
> >
> > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > index 697ea0510729..a6d9e622977d 100644
> > --- a/arch/arm64/Kconfig
> > +++ b/arch/arm64/Kconfig
> > @@ -142,6 +142,7 @@ config ARM64
> > select HAVE_EFFICIENT_UNALIGNED_ACCESS
> > select HAVE_FTRACE_MCOUNT_RECORD
> > select HAVE_FUNCTION_TRACER
> > + select HAVE_FUNCTION_ERROR_INJECTION
> > select HAVE_FUNCTION_GRAPH_TRACER
> > select HAVE_GCC_PLUGINS
> > select HAVE_HW_BREAKPOINT if PERF_EVENTS
> > diff --git a/arch/arm64/include/asm/error-injection.h b/arch/arm64/include/asm/error-injection.h
> > new file mode 100644
> > index 000000000000..da057e8ed224
> > --- /dev/null
> > +++ b/arch/arm64/include/asm/error-injection.h
> > @@ -0,0 +1,13 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +
> > +#ifndef __ASM_ERROR_INJECTION_H_
> > +#define __ASM_ERROR_INJECTION_H_
> > +
> > +#include <linux/compiler.h>
> > +#include <linux/linkage.h>
> > +#include <asm/ptrace.h>
> > +#include <asm-generic/error-injection.h>
> > +
> > +void override_function_with_return(struct pt_regs *regs);
> > +
> > +#endif /* __ASM_ERROR_INJECTION_H_ */
>
> Why isn't this prototype in the asm-generic header? Seems weird to have to
> duplicate it for each architecture.
Yeah. When I spin for new version patches, will try to refactor in
the asm-generic header.
> > diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
> > index dad858b6adc6..3aafbbe218a2 100644
> > --- a/arch/arm64/include/asm/ptrace.h
> > +++ b/arch/arm64/include/asm/ptrace.h
> > @@ -294,6 +294,11 @@ static inline unsigned long regs_return_value(struct pt_regs *regs)
> > return regs->regs[0];
> > }
> >
> > +static inline void regs_set_return_value(struct pt_regs *regs, unsigned long rc)
> > +{
> > + regs->regs[0] = rc;
> > +}
> > +
> > /**
> > * regs_get_kernel_argument() - get Nth function argument in kernel
> > * @regs: pt_regs of that context
> > diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile
> > index 33c2a4abda04..f182ccb0438e 100644
> > --- a/arch/arm64/lib/Makefile
> > +++ b/arch/arm64/lib/Makefile
> > @@ -33,3 +33,5 @@ UBSAN_SANITIZE_atomic_ll_sc.o := n
> > lib-$(CONFIG_ARCH_HAS_UACCESS_FLUSHCACHE) += uaccess_flushcache.o
> >
> > obj-$(CONFIG_CRC32) += crc32.o
> > +
> > +obj-$(CONFIG_FUNCTION_ERROR_INJECTION) += error-inject.o
> > diff --git a/arch/arm64/lib/error-inject.c b/arch/arm64/lib/error-inject.c
> > new file mode 100644
> > index 000000000000..35661c2de4b0
> > --- /dev/null
> > +++ b/arch/arm64/lib/error-inject.c
> > @@ -0,0 +1,19 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +#include <linux/error-injection.h>
> > +#include <linux/kprobes.h>
> > +
> > +void override_function_with_return(struct pt_regs *regs)
> > +{
> > + /*
> > + * 'regs' represents the state on entry of a predefined function in
> > + * the kernel/module and which is captured on a kprobe.
> > + *
> > + * 'regs->regs[30]' contains the the link register for the probed
>
> extra "the"
Will fix.
> > + * function and assign it to 'regs->pc', so when kprobe returns
> > + * back from exception it will override the end of probed function
> > + * and drirectly return to the predefined function's caller.
>
> directly
Will fix.
> > + */
> > + regs->pc = regs->regs[30];
>
> I suppose we could be all fancy and do:
>
> instruction_pointer_set(regs, procedure_link_pointer(regs));
>
> How about that?
Ah, good point. Will change to use the common APIs.
Thanks,
Leo Yan
^ permalink raw reply
* Re: next/master build: 221 builds: 11 failed, 210 passed, 13 errors, 1174 warnings (next-20190731)
From: David Miller @ 2019-07-31 16:41 UTC (permalink / raw)
To: natechancellor
Cc: gregkh, devel, andrew, f.fainelli, kernel-build-reports, netdev,
willy, broonie, linux-next, linux-arm-kernel, hkallweit1
In-Reply-To: <20190731163509.GA90028@archlinux-threadripper>
From: Nathan Chancellor <natechancellor@gmail.com>
Date: Wed, 31 Jul 2019 09:35:09 -0700
> In file included from ../drivers/net/phy/mdio-octeon.c:14:
> ../drivers/net/phy/mdio-cavium.h:111:36: error: implicit declaration of function 'writeq'; did you mean 'writeb'? [-Werror=implicit-function-declaration]
> 111 | #define oct_mdio_writeq(val, addr) writeq(val, (void *)addr)
One of the hi-lo, lo-hi writeq/readq headers has to be included.
^ permalink raw reply
* Re: [PATCH] net: sctp: Rename fallthrough label to unhandled
From: Joe Perches @ 2019-07-31 16:35 UTC (permalink / raw)
To: Neil Horman
Cc: Vlad Yasevich, Marcelo Ricardo Leitner, David S. Miller,
linux-sctp, netdev, linux-kernel
In-Reply-To: <20190731121646.GD9823@hmswarspite.think-freely.org>
On Wed, 2019-07-31 at 08:16 -0400, Neil Horman wrote:
> On Wed, Jul 31, 2019 at 04:32:43AM -0700, Joe Perches wrote:
> > On Wed, 2019-07-31 at 07:19 -0400, Neil Horman wrote:
> > > On Tue, Jul 30, 2019 at 10:04:37PM -0700, Joe Perches wrote:
> > > > fallthrough may become a pseudo reserved keyword so this only use of
> > > > fallthrough is better renamed to allow it.
> > > >
> > > > Signed-off-by: Joe Perches <joe@perches.com>
> > > Are you referring to the __attribute__((fallthrough)) statement that gcc
> > > supports? If so the compiler should by all rights be able to differentiate
> > > between a null statement attribute and a explicit goto and label without the
> > > need for renaming here. Or are you referring to something else?
> >
> > Hi.
> >
> > I sent after this a patch that adds
> >
> > # define fallthrough __attribute__((__fallthrough__))
> >
> > https://lore.kernel.org/patchwork/patch/1108577/
> >
> > So this rename is a prerequisite to adding this #define.
> >
> why not just define __fallthrough instead, like we do for all the other
> attributes we alias (i.e. __read_mostly, __protected_by, __unused, __exception,
> etc)
Because it's not as intelligible when used as a statement.
^ permalink raw reply
* Re: next/master build: 221 builds: 11 failed, 210 passed, 13 errors, 1174 warnings (next-20190731)
From: Nathan Chancellor @ 2019-07-31 16:35 UTC (permalink / raw)
To: Greg KH
Cc: David Miller, devel, andrew, f.fainelli, kernel-build-reports,
netdev, willy, broonie, linux-next, linux-arm-kernel, hkallweit1
In-Reply-To: <20190731160043.GA15520@kroah.com>
On Wed, Jul 31, 2019 at 06:00:43PM +0200, Greg KH wrote:
> On Wed, Jul 31, 2019 at 08:48:24AM -0700, David Miller wrote:
> > From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Date: Wed, 31 Jul 2019 13:35:22 +0200
> >
> > > On Wed, Jul 31, 2019 at 12:24:41PM +0100, Mark Brown wrote:
> > >> On Wed, Jul 31, 2019 at 04:07:41AM -0700, kernelci.org bot wrote:
> > >>
> > >> Today's -next fails to build an ARM allmodconfig due to:
> > >>
> > >> > allmodconfig (arm, gcc-8) ― FAIL, 1 error, 40 warnings, 0 section mismatches
> > >> >
> > >> > Errors:
> > >> > drivers/net/phy/mdio-cavium.h:111:36: error: implicit declaration of function 'writeq'; did you mean 'writel'? [-Werror=implicit-function-declaration]
> > >>
> > >> as a result of the changes that introduced:
> > >>
> > >> WARNING: unmet direct dependencies detected for MDIO_OCTEON
> > >> Depends on [n]: NETDEVICES [=y] && MDIO_DEVICE [=m] && MDIO_BUS [=m] && 64BIT && HAS_IOMEM [=y] && OF_MDIO [=m]
> > >> Selected by [m]:
> > >> - OCTEON_ETHERNET [=m] && STAGING [=y] && (CAVIUM_OCTEON_SOC && NETDEVICES [=y] || COMPILE_TEST [=y])
> > >>
> > >> which is triggered by the staging OCTEON_ETHERNET driver which misses a
> > >> 64BIT dependency but added COMPILE_TEST in 171a9bae68c72f2
> > >> (staging/octeon: Allow test build on !MIPS).
> > >
> > > A patch was posted for this, but it needs to go through the netdev tree
> > > as that's where the offending patches are coming from.
> >
> > I didn't catch that, was netdev CC:'d?
>
> Nope, just you :(
>
> I'll resend it now and cc: netdev.
>
> thanks,
>
> greg k-h
If it is this patch:
https://lore.kernel.org/netdev/20190731160219.GA2114@kroah.com/
It doesn't resolve that issue. I applied it and tested on next-20190731.
$ make -j$(nproc) -s ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- O=out distclean allyesconfig drivers/net/phy/
WARNING: unmet direct dependencies detected for MDIO_OCTEON
Depends on [n]: NETDEVICES [=y] && MDIO_DEVICE [=y] && MDIO_BUS [=y] && 64BIT && HAS_IOMEM [=y] && OF_MDIO [=y]
Selected by [y]:
- OCTEON_ETHERNET [=y] && STAGING [=y] && (CAVIUM_OCTEON_SOC || COMPILE_TEST [=y]) && NETDEVICES [=y]
../drivers/net/phy/mdio-octeon.c: In function 'octeon_mdiobus_probe':
../drivers/net/phy/mdio-octeon.c:48:3: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
48 | (u64)devm_ioremap(&pdev->dev, mdio_phys, regsize);
| ^
In file included from ../drivers/net/phy/mdio-octeon.c:14:
../drivers/net/phy/mdio-cavium.h:111:36: error: implicit declaration of function 'writeq'; did you mean 'writeb'? [-Werror=implicit-function-declaration]
111 | #define oct_mdio_writeq(val, addr) writeq(val, (void *)addr)
| ^~~~~~
../drivers/net/phy/mdio-octeon.c:56:2: note: in expansion of macro 'oct_mdio_writeq'
56 | oct_mdio_writeq(smi_en.u64, bus->register_base + SMI_EN);
| ^~~~~~~~~~~~~~~
../drivers/net/phy/mdio-cavium.h:111:48: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
111 | #define oct_mdio_writeq(val, addr) writeq(val, (void *)addr)
| ^
../drivers/net/phy/mdio-octeon.c:56:2: note: in expansion of macro 'oct_mdio_writeq'
56 | oct_mdio_writeq(smi_en.u64, bus->register_base + SMI_EN);
| ^~~~~~~~~~~~~~~
../drivers/net/phy/mdio-cavium.h:111:48: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
111 | #define oct_mdio_writeq(val, addr) writeq(val, (void *)addr)
| ^
../drivers/net/phy/mdio-octeon.c:77:2: note: in expansion of macro 'oct_mdio_writeq'
77 | oct_mdio_writeq(smi_en.u64, bus->register_base + SMI_EN);
| ^~~~~~~~~~~~~~~
../drivers/net/phy/mdio-octeon.c: In function 'octeon_mdiobus_remove':
../drivers/net/phy/mdio-cavium.h:111:48: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
111 | #define oct_mdio_writeq(val, addr) writeq(val, (void *)addr)
| ^
../drivers/net/phy/mdio-octeon.c:91:2: note: in expansion of macro 'oct_mdio_writeq'
91 | oct_mdio_writeq(smi_en.u64, bus->register_base + SMI_EN);
| ^~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
make[3]: *** [../scripts/Makefile.build:274: drivers/net/phy/mdio-octeon.o] Error 1
make[3]: *** Waiting for unfinished jobs....
make[2]: *** [../Makefile:1780: drivers/net/phy/] Error 2
make[1]: *** [/home/nathan/cbl/linux-next/Makefile:330: __build_one_by_one] Error 2
make: *** [Makefile:179: sub-make] Error 2
This is the diff that I came up with to solve the errors plus the casting
warnings but it doesn't feel proper to me. If you all feel otherwise, I
can draft up a formal commit message.
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 20f14c5fbb7e..ed2edf4b5b0e 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -159,7 +159,7 @@ config MDIO_MSCC_MIIM
config MDIO_OCTEON
tristate "Octeon and some ThunderX SOCs MDIO buses"
- depends on 64BIT
+ depends on 64BIT || COMPILE_TEST
depends on HAS_IOMEM && OF_MDIO
select MDIO_CAVIUM
help
diff --git a/drivers/net/phy/mdio-cavium.h b/drivers/net/phy/mdio-cavium.h
index ed5f9bb5448d..4b71b733edb4 100644
--- a/drivers/net/phy/mdio-cavium.h
+++ b/drivers/net/phy/mdio-cavium.h
@@ -108,8 +108,10 @@ static inline u64 oct_mdio_readq(u64 addr)
return cvmx_read_csr(addr);
}
#else
-#define oct_mdio_writeq(val, addr) writeq(val, (void *)addr)
-#define oct_mdio_readq(addr) readq((void *)addr)
+#include <linux/io-64-nonatomic-lo-hi.h>
+
+#define oct_mdio_writeq(val, addr) writeq(val, (void *)(uintptr_t)addr)
+#define oct_mdio_readq(addr) readq((void *)(uintptr_t)addr)
#endif
int cavium_mdiobus_read(struct mii_bus *bus, int phy_id, int regnum);
diff --git a/drivers/net/phy/mdio-octeon.c b/drivers/net/phy/mdio-octeon.c
index 8327382aa568..ab0d8ab588e4 100644
--- a/drivers/net/phy/mdio-octeon.c
+++ b/drivers/net/phy/mdio-octeon.c
@@ -45,7 +45,7 @@ static int octeon_mdiobus_probe(struct platform_device *pdev)
}
bus->register_base =
- (u64)devm_ioremap(&pdev->dev, mdio_phys, regsize);
+ (u64)(uintptr_t)devm_ioremap(&pdev->dev, mdio_phys, regsize);
if (!bus->register_base) {
dev_err(&pdev->dev, "dev_ioremap failed\n");
return -ENOMEM;
^ permalink raw reply related
* Re: [PATCH 1/2] arm64: Add support for function error injection
From: Will Deacon @ 2019-07-31 16:08 UTC (permalink / raw)
To: Leo Yan
Cc: Russell King, Oleg Nesterov, Catalin Marinas, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
Arnd Bergmann, linux-arm-kernel, linux-kernel, netdev, bpf,
Masami Hiramatsu, Justin He
In-Reply-To: <20190716111301.1855-2-leo.yan@linaro.org>
On Tue, Jul 16, 2019 at 07:13:00PM +0800, Leo Yan wrote:
> This patch implement regs_set_return_value() and
> override_function_with_return() to support function error injection
> for arm64.
>
> In the exception flow, arm64's general register x30 contains the value
> for the link register; so we can just update pt_regs::pc with it rather
> than redirecting execution to a dummy function that returns.
>
> This patch is heavily inspired by the commit 7cd01b08d35f ("powerpc:
> Add support for function error injection").
>
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
> arch/arm64/Kconfig | 1 +
> arch/arm64/include/asm/error-injection.h | 13 +++++++++++++
> arch/arm64/include/asm/ptrace.h | 5 +++++
> arch/arm64/lib/Makefile | 2 ++
> arch/arm64/lib/error-inject.c | 19 +++++++++++++++++++
> 5 files changed, 40 insertions(+)
> create mode 100644 arch/arm64/include/asm/error-injection.h
> create mode 100644 arch/arm64/lib/error-inject.c
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 697ea0510729..a6d9e622977d 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -142,6 +142,7 @@ config ARM64
> select HAVE_EFFICIENT_UNALIGNED_ACCESS
> select HAVE_FTRACE_MCOUNT_RECORD
> select HAVE_FUNCTION_TRACER
> + select HAVE_FUNCTION_ERROR_INJECTION
> select HAVE_FUNCTION_GRAPH_TRACER
> select HAVE_GCC_PLUGINS
> select HAVE_HW_BREAKPOINT if PERF_EVENTS
> diff --git a/arch/arm64/include/asm/error-injection.h b/arch/arm64/include/asm/error-injection.h
> new file mode 100644
> index 000000000000..da057e8ed224
> --- /dev/null
> +++ b/arch/arm64/include/asm/error-injection.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +
> +#ifndef __ASM_ERROR_INJECTION_H_
> +#define __ASM_ERROR_INJECTION_H_
> +
> +#include <linux/compiler.h>
> +#include <linux/linkage.h>
> +#include <asm/ptrace.h>
> +#include <asm-generic/error-injection.h>
> +
> +void override_function_with_return(struct pt_regs *regs);
> +
> +#endif /* __ASM_ERROR_INJECTION_H_ */
Why isn't this prototype in the asm-generic header? Seems weird to have to
duplicate it for each architecture.
> diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
> index dad858b6adc6..3aafbbe218a2 100644
> --- a/arch/arm64/include/asm/ptrace.h
> +++ b/arch/arm64/include/asm/ptrace.h
> @@ -294,6 +294,11 @@ static inline unsigned long regs_return_value(struct pt_regs *regs)
> return regs->regs[0];
> }
>
> +static inline void regs_set_return_value(struct pt_regs *regs, unsigned long rc)
> +{
> + regs->regs[0] = rc;
> +}
> +
> /**
> * regs_get_kernel_argument() - get Nth function argument in kernel
> * @regs: pt_regs of that context
> diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile
> index 33c2a4abda04..f182ccb0438e 100644
> --- a/arch/arm64/lib/Makefile
> +++ b/arch/arm64/lib/Makefile
> @@ -33,3 +33,5 @@ UBSAN_SANITIZE_atomic_ll_sc.o := n
> lib-$(CONFIG_ARCH_HAS_UACCESS_FLUSHCACHE) += uaccess_flushcache.o
>
> obj-$(CONFIG_CRC32) += crc32.o
> +
> +obj-$(CONFIG_FUNCTION_ERROR_INJECTION) += error-inject.o
> diff --git a/arch/arm64/lib/error-inject.c b/arch/arm64/lib/error-inject.c
> new file mode 100644
> index 000000000000..35661c2de4b0
> --- /dev/null
> +++ b/arch/arm64/lib/error-inject.c
> @@ -0,0 +1,19 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/error-injection.h>
> +#include <linux/kprobes.h>
> +
> +void override_function_with_return(struct pt_regs *regs)
> +{
> + /*
> + * 'regs' represents the state on entry of a predefined function in
> + * the kernel/module and which is captured on a kprobe.
> + *
> + * 'regs->regs[30]' contains the the link register for the probed
extra "the"
> + * function and assign it to 'regs->pc', so when kprobe returns
> + * back from exception it will override the end of probed function
> + * and drirectly return to the predefined function's caller.
directly
> + */
> + regs->pc = regs->regs[30];
I suppose we could be all fancy and do:
instruction_pointer_set(regs, procedure_link_pointer(regs));
How about that?
Will
^ permalink raw reply
* Re: [PATCH v2] staging/octeon: Fix build error without CONFIG_NETDEVICES
From: David Miller @ 2019-07-31 16:07 UTC (permalink / raw)
To: gregkh; +Cc: willy, netdev, devel, yuehaibing, linux-kernel
In-Reply-To: <20190731160219.GA2114@kroah.com>
From: Greg KH <gregkh@linuxfoundation.org>
Date: Wed, 31 Jul 2019 18:02:19 +0200
> From: YueHaibing <yuehaibing@huawei.com>
>
> While do COMPILE_TEST build without CONFIG_NETDEVICES,
> we get Kconfig warning:
>
> WARNING: unmet direct dependencies detected for PHYLIB
> Depends on [n]: NETDEVICES [=n]
> Selected by [y]:
> - OCTEON_ETHERNET [=y] && STAGING [=y] && (CAVIUM_OCTEON_SOC && NETDEVICES [=n] || COMPILE_TEST [=y])
>
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Reported-by: Mark Brown <broonie@kernel.org>
> Fixes: 171a9bae68c7 ("staging/octeon: Allow test build on !MIPS")
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Applied to net-next.
^ permalink raw reply
* Re: [PATCH 1/2] bnxt_en: Use refcount_t for refcount
From: Willem de Bruijn @ 2019-07-31 16:05 UTC (permalink / raw)
To: Chuhong Yuan
Cc: Michael Chan, David S . Miller, Network Development, linux-kernel
In-Reply-To: <20190731122203.948-1-hslester96@gmail.com>
On Wed, Jul 31, 2019 at 8:22 AM Chuhong Yuan <hslester96@gmail.com> wrote:
>
> refcount_t is better for reference counters since its
> implementation can prevent overflows.
> So convert atomic_t ref counters to refcount_t.
>
> Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
> ---
> drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 8 ++++----
> drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h | 2 +-
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
> index fc77caf0a076..eb7ed34639e2 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
> @@ -49,7 +49,7 @@ static int bnxt_register_dev(struct bnxt_en_dev *edev, int ulp_id,
> return -ENOMEM;
> }
>
> - atomic_set(&ulp->ref_count, 0);
> + refcount_set(&ulp->ref_count, 0);
One feature of refcount_t is that it warns on refcount_inc from 0 to
detect possible use-after_free. It appears that that can trigger here?
> ulp->handle = handle;
> rcu_assign_pointer(ulp->ulp_ops, ulp_ops);
>
> @@ -246,12 +246,12 @@ static int bnxt_send_msg(struct bnxt_en_dev *edev, int ulp_id,
>
> static void bnxt_ulp_get(struct bnxt_ulp *ulp)
> {
> - atomic_inc(&ulp->ref_count);
> + refcount_inc(&ulp->ref_count);
> }
^ permalink raw reply
* [PATCH net v3] ipvs: Improve robustness to the ipvs sysctl
From: hujunwei @ 2019-07-31 16:03 UTC (permalink / raw)
To: wensong, horms, pablo, kadlec, fw, davem, Julian Anastasov,
Florian Westphal
Cc: netdev@vger.kernel.org, lvs-devel, netfilter-devel, coreteam,
Mingfangsen, wangxiaogang3, xuhanbing
In-Reply-To: <4a0476d3-57a4-50e0-cae8-9dffc4f4d556@huawei.com>
From: Junwei Hu <hujunwei4@huawei.com>
The ipvs module parse the user buffer and save it to sysctl,
then check if the value is valid. invalid value occurs
over a period of time.
Here, I add a variable, struct ctl_table tmp, used to read
the value from the user buffer, and save only when it is valid.
I delete proc_do_sync_mode and use extra1/2 in table for the
proc_dointvec_minmax call.
Fixes: f73181c8288f ("ipvs: add support for sync threads")
Signed-off-by: Junwei Hu <hujunwei4@huawei.com>
Acked-by: Julian Anastasov <ja@ssi.bg>
---
V1->V2:
- delete proc_do_sync_mode and use proc_dointvec_minmax call.
V2->V3:
- update git version
---
net/netfilter/ipvs/ip_vs_ctl.c | 69 +++++++++++++++++-----------------
1 file changed, 35 insertions(+), 34 deletions(-)
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 060565e7d227..72189559a1cd 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -1737,12 +1737,18 @@ proc_do_defense_mode(struct ctl_table *table, int write,
int val = *valp;
int rc;
- rc = proc_dointvec(table, write, buffer, lenp, ppos);
+ struct ctl_table tmp = {
+ .data = &val,
+ .maxlen = sizeof(int),
+ .mode = table->mode,
+ };
+
+ rc = proc_dointvec(&tmp, write, buffer, lenp, ppos);
if (write && (*valp != val)) {
- if ((*valp < 0) || (*valp > 3)) {
- /* Restore the correct value */
- *valp = val;
+ if (val < 0 || val > 3) {
+ rc = -EINVAL;
} else {
+ *valp = val;
update_defense_level(ipvs);
}
}
@@ -1756,33 +1762,20 @@ proc_do_sync_threshold(struct ctl_table *table, int write,
int *valp = table->data;
int val[2];
int rc;
+ struct ctl_table tmp = {
+ .data = &val,
+ .maxlen = table->maxlen,
+ .mode = table->mode,
+ };
- /* backup the value first */
memcpy(val, valp, sizeof(val));
-
- rc = proc_dointvec(table, write, buffer, lenp, ppos);
- if (write && (valp[0] < 0 || valp[1] < 0 ||
- (valp[0] >= valp[1] && valp[1]))) {
- /* Restore the correct value */
- memcpy(valp, val, sizeof(val));
- }
- return rc;
-}
-
-static int
-proc_do_sync_mode(struct ctl_table *table, int write,
- void __user *buffer, size_t *lenp, loff_t *ppos)
-{
- int *valp = table->data;
- int val = *valp;
- int rc;
-
- rc = proc_dointvec(table, write, buffer, lenp, ppos);
- if (write && (*valp != val)) {
- if ((*valp < 0) || (*valp > 1)) {
- /* Restore the correct value */
- *valp = val;
- }
+ rc = proc_dointvec(&tmp, write, buffer, lenp, ppos);
+ if (write) {
+ if (val[0] < 0 || val[1] < 0 ||
+ (val[0] >= val[1] && val[1]))
+ rc = -EINVAL;
+ else
+ memcpy(valp, val, sizeof(val));
}
return rc;
}
@@ -1795,12 +1788,18 @@ proc_do_sync_ports(struct ctl_table *table, int write,
int val = *valp;
int rc;
- rc = proc_dointvec(table, write, buffer, lenp, ppos);
+ struct ctl_table tmp = {
+ .data = &val,
+ .maxlen = sizeof(int),
+ .mode = table->mode,
+ };
+
+ rc = proc_dointvec(&tmp, write, buffer, lenp, ppos);
if (write && (*valp != val)) {
- if (*valp < 1 || !is_power_of_2(*valp)) {
- /* Restore the correct value */
+ if (val < 1 || !is_power_of_2(val))
+ rc = -EINVAL;
+ else
*valp = val;
- }
}
return rc;
}
@@ -1860,7 +1859,9 @@ static struct ctl_table vs_vars[] = {
.procname = "sync_version",
.maxlen = sizeof(int),
.mode = 0644,
- .proc_handler = proc_do_sync_mode,
+ .proc_handler = proc_dointvec_minmax,
+ .extra1 = SYSCTL_ZERO,
+ .extra2 = SYSCTL_ONE,
},
{
.procname = "sync_ports",
--
2.21.GIT
^ permalink raw reply related
* [PATCH v2] staging/octeon: Fix build error without CONFIG_NETDEVICES
From: Greg KH @ 2019-07-31 16:02 UTC (permalink / raw)
To: willy, davem, netdev; +Cc: devel, YueHaibing, linux-kernel
From: YueHaibing <yuehaibing@huawei.com>
While do COMPILE_TEST build without CONFIG_NETDEVICES,
we get Kconfig warning:
WARNING: unmet direct dependencies detected for PHYLIB
Depends on [n]: NETDEVICES [=n]
Selected by [y]:
- OCTEON_ETHERNET [=y] && STAGING [=y] && (CAVIUM_OCTEON_SOC && NETDEVICES [=n] || COMPILE_TEST [=y])
Reported-by: Hulk Robot <hulkci@huawei.com>
Reported-by: Mark Brown <broonie@kernel.org>
Fixes: 171a9bae68c7 ("staging/octeon: Allow test build on !MIPS")
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
v2: cc: netdev and add another reported-by line.
drivers/staging/octeon/Kconfig | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/octeon/Kconfig b/drivers/staging/octeon/Kconfig
index 5b39946..5319909 100644
--- a/drivers/staging/octeon/Kconfig
+++ b/drivers/staging/octeon/Kconfig
@@ -1,7 +1,8 @@
# SPDX-License-Identifier: GPL-2.0
config OCTEON_ETHERNET
tristate "Cavium Networks Octeon Ethernet support"
- depends on CAVIUM_OCTEON_SOC && NETDEVICES || COMPILE_TEST
+ depends on CAVIUM_OCTEON_SOC || COMPILE_TEST
+ depends on NETDEVICES
select PHYLIB
select MDIO_OCTEON
help
--
2.7.4
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related
* Re: next/master build: 221 builds: 11 failed, 210 passed, 13 errors, 1174 warnings (next-20190731)
From: Greg KH @ 2019-07-31 16:00 UTC (permalink / raw)
To: David Miller
Cc: devel, andrew, f.fainelli, kernel-build-reports, netdev, willy,
broonie, linux-next, linux-arm-kernel, hkallweit1
In-Reply-To: <20190731.084824.2244928058443049.davem@davemloft.net>
On Wed, Jul 31, 2019 at 08:48:24AM -0700, David Miller wrote:
> From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Date: Wed, 31 Jul 2019 13:35:22 +0200
>
> > On Wed, Jul 31, 2019 at 12:24:41PM +0100, Mark Brown wrote:
> >> On Wed, Jul 31, 2019 at 04:07:41AM -0700, kernelci.org bot wrote:
> >>
> >> Today's -next fails to build an ARM allmodconfig due to:
> >>
> >> > allmodconfig (arm, gcc-8) ― FAIL, 1 error, 40 warnings, 0 section mismatches
> >> >
> >> > Errors:
> >> > drivers/net/phy/mdio-cavium.h:111:36: error: implicit declaration of function 'writeq'; did you mean 'writel'? [-Werror=implicit-function-declaration]
> >>
> >> as a result of the changes that introduced:
> >>
> >> WARNING: unmet direct dependencies detected for MDIO_OCTEON
> >> Depends on [n]: NETDEVICES [=y] && MDIO_DEVICE [=m] && MDIO_BUS [=m] && 64BIT && HAS_IOMEM [=y] && OF_MDIO [=m]
> >> Selected by [m]:
> >> - OCTEON_ETHERNET [=m] && STAGING [=y] && (CAVIUM_OCTEON_SOC && NETDEVICES [=y] || COMPILE_TEST [=y])
> >>
> >> which is triggered by the staging OCTEON_ETHERNET driver which misses a
> >> 64BIT dependency but added COMPILE_TEST in 171a9bae68c72f2
> >> (staging/octeon: Allow test build on !MIPS).
> >
> > A patch was posted for this, but it needs to go through the netdev tree
> > as that's where the offending patches are coming from.
>
> I didn't catch that, was netdev CC:'d?
Nope, just you :(
I'll resend it now and cc: netdev.
thanks,
greg k-h
^ permalink raw reply
* Re: pull-request: mac80211-next 2019-07-31
From: David Miller @ 2019-07-31 16:00 UTC (permalink / raw)
To: johannes; +Cc: netdev, linux-wireless
In-Reply-To: <20190731155057.23035-1-johannes@sipsolutions.net>
From: Johannes Berg <johannes@sipsolutions.net>
Date: Wed, 31 Jul 2019 17:50:56 +0200
> There's a fair number of changes here, so I thought I'd get them out.
> I've included two Intel driver cleanups because Luca is on vacation,
> I'm covering for him, and doing it all in one tree let me merge all
> of the patches at once (including mac80211 that depends on that);
> Kalle is aware.
>
> Also, though this isn't very interesting yet, I've started looking at
> weaning the wireless subsystem off the RTNL for all operations, as it
> can cause significant lock contention, especially with slow USB devices.
> The real patches for that are some way off, but one preparation here is
> to use generic netlink's parallel_ops=true, to avoid trading one place
> with contention for another in the future, and to avoid adding more
> genl_family_attrbuf() usage (since that's no longer possible with the
> parallel_ops setting).
>
> Please pull and let me know if there's any problem.
Pulled, thanks Johannes.
I'm sure people like Florian Westphal will also appreciate your RTNL
elimination work...
^ permalink raw reply
* Re: [PATCH net v2] ipvs: Improve robustness to the ipvs sysctl
From: hujunwei @ 2019-07-31 15:58 UTC (permalink / raw)
To: Julian Anastasov
Cc: wensong, horms, pablo, kadlec, Florian Westphal, davem,
netdev@vger.kernel.org, lvs-devel, netfilter-devel, coreteam,
Mingfangsen, wangxiaogang3, xuhanbing
In-Reply-To: <alpine.LFD.2.21.1907302226340.4897@ja.home.ssi.bg>
Hello, Julian
On 2019/7/31 3:29, Julian Anastasov wrote:
>
> Hello,
>
> On Tue, 30 Jul 2019, hujunwei wrote:
>
>> From: Junwei Hu <hujunwei4@huawei.com>
>>
>> The ipvs module parse the user buffer and save it to sysctl,
>> then check if the value is valid. invalid value occurs
>> over a period of time.
>> Here, I add a variable, struct ctl_table tmp, used to read
>> the value from the user buffer, and save only when it is valid.
>> I delete proc_do_sync_mode and use extra1/2 in table for the
>> proc_dointvec_minmax call.
>>
>> Fixes: f73181c8288f ("ipvs: add support for sync threads")
>> Signed-off-by: Junwei Hu <hujunwei4@huawei.com>
>
> Looks good to me, thanks!
>
> Acked-by: Julian Anastasov <ja@ssi.bg>
>
> BTW, why ip_vs_zero_all everywhere? Due to old git version?
>
I will update the git version and send the patch v3.
Regards,
Junwei
^ permalink raw reply
* Re: [PATCH net-next] net/tls: prevent skb_orphan() from leaking TLS plain text with offload
From: Willem de Bruijn @ 2019-07-31 15:57 UTC (permalink / raw)
To: Jakub Kicinski
Cc: David Miller, Network Development, oss-drivers, Eric Dumazet,
borisp, aviadye, davejwatson, John Fastabend, Daniel Borkmann,
Cong Wang, Florian Westphal, Alexei Starovoitov
In-Reply-To: <20190730211258.13748-1-jakub.kicinski@netronome.com>
On Tue, Jul 30, 2019 at 5:13 PM Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
>
> sk_validate_xmit_skb() and drivers depend on the sk member of
> struct sk_buff to identify segments requiring encryption.
> Any operation which removes or does not preserve the original TLS
> socket such as skb_orphan() or skb_clone() will cause clear text
> leaks.
>
> Make the TCP socket underlying an offloaded TLS connection
> mark all skbs as decrypted, if TLS TX is in offload mode.
> Then in sk_validate_xmit_skb() catch skbs which have no socket
> (or a socket with no validation) and decrypted flag set.
>
> Note that CONFIG_SOCK_VALIDATE_XMIT, CONFIG_TLS_DEVICE and
> sk->sk_validate_xmit_skb are slightly interchangeable right now,
> they all imply TLS offload. The new checks are guarded by
> CONFIG_TLS_DEVICE because that's the option guarding the
> sk_buff->decrypted member.
>
> Second, smaller issue with orphaning is that it breaks
> the guarantee that packets will be delivered to device
> queues in-order. All TLS offload drivers depend on that
> scheduling property. This means skb_orphan_partial()'s
> trick of preserving partial socket references will cause
> issues in the drivers. We need a full orphan, and as a
> result netem delay/throttling will cause all TLS offload
> skbs to be dropped.
>
> Reusing the sk_buff->decrypted flag also protects from
> leaking clear text when incoming, decrypted skb is redirected
> (e.g. by TC).
>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> ---
> I'm sending this for net-next because of lack of confidence
> in my own abilities. It should apply cleanly to net... :)
>
> Documentation/networking/tls-offload.rst | 9 --------
> include/net/sock.h | 28 +++++++++++++++++++++++-
> net/core/skbuff.c | 3 +++
> net/core/sock.c | 22 ++++++++++++++-----
> net/ipv4/tcp.c | 4 +++-
> net/ipv4/tcp_output.c | 3 +++
> net/tls/tls_device.c | 2 ++
> 7 files changed, 55 insertions(+), 16 deletions(-)
> Redirects leak clear text
> -------------------------
>
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 228db3998e46..90f3f552c789 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -814,6 +814,7 @@ enum sock_flags {
> SOCK_TXTIME,
> SOCK_XDP, /* XDP is attached */
> SOCK_TSTAMP_NEW, /* Indicates 64 bit timestamps always */
> + SOCK_CRYPTO_TX_PLAIN_TEXT, /* Generate skbs with decrypted flag set */
> };
>
> #define SK_FLAGS_TIMESTAMP ((1UL << SOCK_TIMESTAMP) | (1UL << SOCK_TIMESTAMPING_RX_SOFTWARE))
> @@ -2480,8 +2481,26 @@ static inline bool sk_fullsock(const struct sock *sk)
> return (1 << sk->sk_state) & ~(TCPF_TIME_WAIT | TCPF_NEW_SYN_RECV);
> }
>
> +static inline void sk_set_tx_crypto(const struct sock *sk, struct sk_buff *skb)
nit: skb_.. instead of sk_.. ?
> +{
> +#ifdef CONFIG_TLS_DEVICE
> + skb->decrypted = sock_flag(sk, SOCK_CRYPTO_TX_PLAIN_TEXT);
> +#endif
> +}
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 0b788df5a75b..9e92684479b9 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -3794,6 +3794,9 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
>
> skb_reserve(nskb, headroom);
> __skb_put(nskb, doffset);
> +#ifdef CONFIG_TLS_DEVICE
> + nskb->decrypted = head_skb->decrypted;
> +#endif
decrypted is between header_start and headers_end, so
__copy_skb_header just below should take care of this?
> diff --git a/net/core/sock.c b/net/core/sock.c
> index d57b0cc995a0..b0c10b518e65 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -1992,6 +1992,22 @@ void skb_set_owner_w(struct sk_buff *skb, struct sock *sk)
> }
> EXPORT_SYMBOL(skb_set_owner_w);
>
> +static bool can_skb_orphan_partial(const struct sk_buff *skb)
> +{
> +#ifdef CONFIG_TLS_DEVICE
> + /* Drivers depend on in-order delivery for crypto offload,
> + * partial orphan breaks out-of-order-OK logic.
> + */
> + if (skb->decrypted)
> + return false;
> +#endif
> +#ifdef CONFIG_INET
> + if (skb->destructor == tcp_wfree)
> + return true;
> +#endif
> + return skb->destructor == sock_wfree;
> +}
> +
Just insert the skb->decrypted check into skb_orphan_partial for less
code churn?
I also think that this is an independent concern from leaking plain
text, so perhaps could be a separate patch.
^ permalink raw reply
* Re: [PATCH v2] isdn: hfcsusb: Fix mISDN driver crash caused by transfer buffer on the stack
From: David Miller @ 2019-07-31 15:54 UTC (permalink / raw)
To: juliana.rodrigueiro; +Cc: isdn, netdev, thomas.jarosch
In-Reply-To: <2432092.e6oL0QVcq9@rocinante.m.i2n>
From: Juliana Rodrigueiro <juliana.rodrigueiro@intra2net.com>
Date: Wed, 31 Jul 2019 15:17:23 +0200
> Since linux 4.9 it is not possible to use buffers on the stack for DMA transfers.
>
> During usb probe the driver crashes with "transfer buffer is on stack" message.
>
> This fix k-allocates a buffer to be used on "read_reg_atomic", which is a macro
> that calls "usb_control_msg" under the hood.
>
> Kernel 4.19 backtrace:
...
> Signed-off-by: Juliana Rodrigueiro <juliana.rodrigueiro@intra2net.com>
Applied.
^ permalink raw reply
* Re: [PATCH] net: mediatek: Drop unneeded dependency on NET_VENDOR_MEDIATEK
From: David Miller @ 2019-07-31 15:53 UTC (permalink / raw)
To: geert+renesas
Cc: nbd, john, sean.wang, nelson.chang, matthias.bgg, netdev,
linux-arm-kernel, linux-mediatek
In-Reply-To: <20190731131202.16749-1-geert+renesas@glider.be>
From: Geert Uytterhoeven <geert+renesas@glider.be>
Date: Wed, 31 Jul 2019 15:12:02 +0200
> The whole block is protected by "if NET_VENDOR_MEDIATEK", so there is
> no need for individual driver config symbols to duplicate this
> dependency.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Applied.
^ permalink raw reply
* Re: pull-request: mac80211 2019-07-31
From: David Miller @ 2019-07-31 15:51 UTC (permalink / raw)
To: johannes; +Cc: netdev, linux-wireless
In-Reply-To: <20190731124933.19420-1-johannes@sipsolutions.net>
From: Johannes Berg <johannes@sipsolutions.net>
Date: Wed, 31 Jul 2019 14:49:32 +0200
> We have few fixes, most importantly probably the NETIF_F_LLTX revert,
> we thought we were now more layered like VLAN or such since we do all
> of the queue control internally, but it caused problems, evidently not.
>
> Please pull and let me know if there's any problem.
Pulled, thanks Johannes.
^ permalink raw reply
* pull-request: mac80211-next 2019-07-31
From: Johannes Berg @ 2019-07-31 15:50 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-wireless
Hi Dave,
There's a fair number of changes here, so I thought I'd get them out.
I've included two Intel driver cleanups because Luca is on vacation,
I'm covering for him, and doing it all in one tree let me merge all
of the patches at once (including mac80211 that depends on that);
Kalle is aware.
Also, though this isn't very interesting yet, I've started looking at
weaning the wireless subsystem off the RTNL for all operations, as it
can cause significant lock contention, especially with slow USB devices.
The real patches for that are some way off, but one preparation here is
to use generic netlink's parallel_ops=true, to avoid trading one place
with contention for another in the future, and to avoid adding more
genl_family_attrbuf() usage (since that's no longer possible with the
parallel_ops setting).
Please pull and let me know if there's any problem.
Thanks,
johannes
The following changes since commit 00c33afbf9dd06f77a2f15117cd4bdc2a54b51d7:
net: mvneta: use devm_platform_ioremap_resource() to simplify code (2019-07-25 17:28:11 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git tags/mac80211-next-for-davem-2019-07-31
for you to fetch changes up to f39b07fdfb688724fedabf5507e15eaf398f2500:
mac80211: HE STA disassoc due to QOS NULL not sent (2019-07-31 13:26:41 +0200)
----------------------------------------------------------------
We have a reasonably large number of changes:
* lots more HE (802.11ax) support, particularly things
relevant for the the AP side, but also mesh support
* debugfs cleanups from Greg
* some more work on extended key ID
* start using genl parallel_ops, as preparation for
weaning ourselves off RTNL and getting parallelism
* various other changes all over
----------------------------------------------------------------
Alexander Wetzel (3):
mac80211_hwsim: Extended Key ID API update
mac80211: Simplify Extended Key ID API
mac80211: AMPDU handling for rekeys with Extended Key ID
Ard Biesheuvel (1):
lib80211: use crypto API ccm(aes) transform for CCMP processing
Christophe JAILLET (1):
mac80211_hwsim: Fix a typo in the name of function 'mac80211_hswim_he_capab()'
Colin Ian King (1):
mac80211: add missing null return check from call to ieee80211_get_sband
Denis Kenzior (2):
nl80211: document uapi for CMD_FRAME_WAIT_CANCEL
nl80211: Include wiphy address setup in NEW_WIPHY
Emmanuel Grumbach (1):
mac80211: pass the vif to cancel_remain_on_channel
Erik Stromdahl (1):
mac80211: add tx dequeue function for process context
Greg Kroah-Hartman (4):
iwlwifi: dvm: no need to check return value of debugfs_create functions
iwlwifi: mvm: remove unused .remove_sta_debugfs callback
mac80211: remove unused and unneeded remove_sta_debugfs callback
cfg80211: no need to check return value of debugfs_create functions
Johannes Berg (6):
cfg80211: clean up cfg80211_inform_single_bss_frame_data()
cfg80211: don't parse MBSSID if transmitting BSS isn't created
cfg80211: give all multi-BSSID BSS entries the same timestamp
mac80211_hwsim: fill boottime_ns in netlink RX path
cfg80211: use parallel_ops for genl
nl80211: add strict start type
John Crispin (10):
mac80211: add support for parsing ADDBA_EXT IEs
mac80211: add xmit rate to struct ieee80211_tx_status
mac80211: propagate struct ieee80211_tx_status into ieee80211_tx_monitor()
mac80211: add struct ieee80211_tx_status support to ieee80211_add_tx_radiotap_header
mac80211: HE: add Spatial Reuse element parsing support
mac80211: fix ieee80211_he_oper_size() comment
mac80211: propagate HE operation info into bss_conf
mac80211: add support for the ADDBA extension element
cfg80211: add support for parsing OBBS_PD attributes
mac80211: allow setting spatial reuse parameters from bss_conf
Karthikeyan Periyasamy (1):
mac80211: reject zero MAC address in add station
Lorenzo Bianconi (1):
mac80211: add IEEE80211_KEY_FLAG_GENERATE_MMIE to ieee80211_key_flags
Michael Vassernis (1):
cfg80211: fix dfs channels remain DFS_AVAILABLE after ch_switch
Sergey Matyukevich (2):
cfg80211: refactor cfg80211_bss_update
cfg80211: fix duplicated scan entries after channel switch
Shay Bar (1):
mac80211: HE STA disassoc due to QOS NULL not sent
Sven Eckelmann (1):
mac80211: implement HE support for mesh
drivers/net/wireless/ath/ath10k/mac.c | 3 +-
drivers/net/wireless/ath/ath9k/main.c | 3 +-
drivers/net/wireless/intel/iwlwifi/dvm/rs.c | 29 +--
drivers/net/wireless/intel/iwlwifi/dvm/rs.h | 4 -
drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c | 3 +-
drivers/net/wireless/intel/iwlwifi/mvm/rs.c | 5 -
drivers/net/wireless/mac80211_hwsim.c | 20 +-
drivers/net/wireless/rsi/rsi_91x_mac80211.c | 3 +-
drivers/net/wireless/ti/wlcore/main.c | 3 +-
include/linux/ieee80211.h | 63 ++++-
include/net/cfg80211.h | 15 ++
include/net/mac80211.h | 53 ++++-
include/uapi/linux/nl80211.h | 31 ++-
net/mac80211/agg-rx.c | 72 +++++-
net/mac80211/cfg.c | 7 +-
net/mac80211/debugfs.c | 3 +-
net/mac80211/driver-ops.h | 8 +-
net/mac80211/he.c | 39 ++++
net/mac80211/ht.c | 2 +-
net/mac80211/ieee80211_i.h | 17 +-
net/mac80211/key.c | 16 +-
net/mac80211/main.c | 18 +-
net/mac80211/mesh.c | 62 +++++
net/mac80211/mesh.h | 4 +
net/mac80211/mesh_plink.c | 12 +-
net/mac80211/mlme.c | 7 +-
net/mac80211/offchannel.c | 5 +-
net/mac80211/rate.h | 9 -
net/mac80211/sta_info.c | 1 -
net/mac80211/status.c | 180 +++++++++++++--
net/mac80211/trace.h | 7 +-
net/mac80211/tx.c | 5 +-
net/mac80211/util.c | 60 +++++
net/mac80211/wpa.c | 6 +-
net/wireless/Kconfig | 2 +
net/wireless/core.c | 17 +-
net/wireless/core.h | 2 +
net/wireless/lib80211_crypt_ccmp.c | 197 +++++++---------
net/wireless/nl80211.c | 182 ++++++++++++---
net/wireless/scan.c | 269 ++++++++++++++--------
40 files changed, 1070 insertions(+), 374 deletions(-)
^ permalink raw reply
* Re: [PATCH 0/8] netfilter fixes for net
From: David Miller @ 2019-07-31 15:50 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel, netdev
In-Reply-To: <20190731115157.27020-1-pablo@netfilter.org>
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Wed, 31 Jul 2019 13:51:49 +0200
> The following patchset contains Netfilter fixes for your net tree:
...
> You can pull these changes from:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git
Pulled, thanks.
^ permalink raw reply
* Re: next/master build: 221 builds: 11 failed, 210 passed, 13 errors, 1174 warnings (next-20190731)
From: David Miller @ 2019-07-31 15:48 UTC (permalink / raw)
To: gregkh
Cc: broonie, andrew, f.fainelli, hkallweit1, willy, devel, netdev,
linux-next, linux-arm-kernel, kernel-build-reports
In-Reply-To: <20190731113522.GA3426@kroah.com>
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Date: Wed, 31 Jul 2019 13:35:22 +0200
> On Wed, Jul 31, 2019 at 12:24:41PM +0100, Mark Brown wrote:
>> On Wed, Jul 31, 2019 at 04:07:41AM -0700, kernelci.org bot wrote:
>>
>> Today's -next fails to build an ARM allmodconfig due to:
>>
>> > allmodconfig (arm, gcc-8) ― FAIL, 1 error, 40 warnings, 0 section mismatches
>> >
>> > Errors:
>> > drivers/net/phy/mdio-cavium.h:111:36: error: implicit declaration of function 'writeq'; did you mean 'writel'? [-Werror=implicit-function-declaration]
>>
>> as a result of the changes that introduced:
>>
>> WARNING: unmet direct dependencies detected for MDIO_OCTEON
>> Depends on [n]: NETDEVICES [=y] && MDIO_DEVICE [=m] && MDIO_BUS [=m] && 64BIT && HAS_IOMEM [=y] && OF_MDIO [=m]
>> Selected by [m]:
>> - OCTEON_ETHERNET [=m] && STAGING [=y] && (CAVIUM_OCTEON_SOC && NETDEVICES [=y] || COMPILE_TEST [=y])
>>
>> which is triggered by the staging OCTEON_ETHERNET driver which misses a
>> 64BIT dependency but added COMPILE_TEST in 171a9bae68c72f2
>> (staging/octeon: Allow test build on !MIPS).
>
> A patch was posted for this, but it needs to go through the netdev tree
> as that's where the offending patches are coming from.
I didn't catch that, was netdev CC:'d?
^ permalink raw reply
* Re: [PATCH net-next 0/2] mlxsw: Test coverage for DSCP leftover fix
From: David Miller @ 2019-07-31 15:47 UTC (permalink / raw)
To: petrm; +Cc: netdev, idosch
In-Reply-To: <cover.1564568595.git.petrm@mellanox.com>
From: Petr Machata <petrm@mellanox.com>
Date: Wed, 31 Jul 2019 10:30:25 +0000
> This patch set fixes some global scope pollution issues in the DSCP tests
> (in patch #1), and then proceeds (in patch #2) to add a new test for
> checking whether, after DSCP prioritization rules are removed from a port,
> DSCP rewrites consistently to zero, instead of the last removed rule still
> staying in effect.
Series applied.
^ permalink raw reply
* Re: [PATCH] myri10ge: remove unneeded variable
From: David Miller @ 2019-07-31 15:44 UTC (permalink / raw)
To: dingxiang; +Cc: christopher.lee, netdev, linux-kernel
In-Reply-To: <1564563226-13367-1-git-send-email-dingxiang@cmss.chinamobile.com>
From: Ding Xiang <dingxiang@cmss.chinamobile.com>
Date: Wed, 31 Jul 2019 16:53:46 +0800
> "error" is unneeded,just return 0
>
> Signed-off-by: Ding Xiang <dingxiang@cmss.chinamobile.com>
Applied to net-next.
^ permalink raw reply
* [PATCH net] net: dsa: mv88e6xxx: drop adjust_link to enabled phylink
From: Hubert Feurstein @ 2019-07-31 15:42 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: Hubert Feurstein, Andrew Lunn, Vivien Didelot, Florian Fainelli,
David S. Miller
We have to drop the adjust_link callback in order to finally migrate to
phylink.
Otherwise we get the following warning during startup:
"mv88e6xxx 2188000.ethernet-1:10: Using legacy PHYLIB callbacks. Please
migrate to PHYLINK!"
The warning is generated in the function dsa_port_link_register_of in
dsa/port.c:
int dsa_port_link_register_of(struct dsa_port *dp)
{
struct dsa_switch *ds = dp->ds;
if (!ds->ops->adjust_link)
return dsa_port_phylink_register(dp);
dev_warn(ds->dev,
"Using legacy PHYLIB callbacks. Please migrate to PHYLINK!\n");
[...]
}
Signed-off-by: Hubert Feurstein <h.feurstein@gmail.com>
---
drivers/net/dsa/mv88e6xxx/chip.c | 26 --------------------------
1 file changed, 26 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 366f70bfe055..37e8babd035f 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -27,7 +27,6 @@
#include <linux/platform_data/mv88e6xxx.h>
#include <linux/netdevice.h>
#include <linux/gpio/consumer.h>
-#include <linux/phy.h>
#include <linux/phylink.h>
#include <net/dsa.h>
@@ -482,30 +481,6 @@ static int mv88e6xxx_phy_is_internal(struct dsa_switch *ds, int port)
return port < chip->info->num_internal_phys;
}
-/* We expect the switch to perform auto negotiation if there is a real
- * phy. However, in the case of a fixed link phy, we force the port
- * settings from the fixed link settings.
- */
-static void mv88e6xxx_adjust_link(struct dsa_switch *ds, int port,
- struct phy_device *phydev)
-{
- struct mv88e6xxx_chip *chip = ds->priv;
- int err;
-
- if (!phy_is_pseudo_fixed_link(phydev) &&
- mv88e6xxx_phy_is_internal(ds, port))
- return;
-
- mv88e6xxx_reg_lock(chip);
- err = mv88e6xxx_port_setup_mac(chip, port, phydev->link, phydev->speed,
- phydev->duplex, phydev->pause,
- phydev->interface);
- mv88e6xxx_reg_unlock(chip);
-
- if (err && err != -EOPNOTSUPP)
- dev_err(ds->dev, "p%d: failed to configure MAC\n", port);
-}
-
static void mv88e6065_phylink_validate(struct mv88e6xxx_chip *chip, int port,
unsigned long *mask,
struct phylink_link_state *state)
@@ -4755,7 +4730,6 @@ static int mv88e6xxx_port_egress_floods(struct dsa_switch *ds, int port,
static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
.get_tag_protocol = mv88e6xxx_get_tag_protocol,
.setup = mv88e6xxx_setup,
- .adjust_link = mv88e6xxx_adjust_link,
.phylink_validate = mv88e6xxx_validate,
.phylink_mac_link_state = mv88e6xxx_link_state,
.phylink_mac_config = mv88e6xxx_mac_config,
--
2.22.0
^ permalink raw reply related
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