* Re: tg3 pxe weirdness
From: Berend De Schouwer @ 2017-09-27 10:05 UTC (permalink / raw)
To: Siva Reddy Kallam; +Cc: Linux Netdev List
In-Reply-To: <CAMet4B6mZ8SyCT7W2j4OBEUDiy3ZupYKWXBFti0nXoxm51_6kg@mail.gmail.com>
On Mon, 2017-09-25 at 15:11 +0530, Siva Reddy Kallam wrote:
> On Fri, Sep 22, 2017 at 9:04 PM, Berend De Schouwer
> <berend.de.schouwer@gmail.com> wrote:
> > On Fri, 2017-09-22 at 11:51 +0530, Siva Reddy Kallam wrote:
> > >
> > >
> > > Can you please share below details?
> > > 1) Model and Manufacturer of the system
> > > 2) Linux distro/kernel used?
> >
> > 4.13.3 gets a little further, but after some more data is
> > transferred
> > the tg3 driver still crashes. This is unfortunately before I've
> > got a
> > writeable filesystem.
> >
> > The last line is:
> > tg3 0000:01:00.0: tg3_stop_block timed out, ofs=4c00 enable_bit=2
> >
> > I've got some ideas to get the full dmesg.
> >
> > As with the other kernels it works OK on 1Gbps, but not slower
> > switches.
>
> I am suspecting with link aware mode, the clock speed could be slow
> and boot code does not
> complete within the expected time with lower link speeds. So,
> Providing a patch to override clock.
> Can you please try with attached debug patch and provide us the
> feedback with 100M link?
> If it solves this issue, we will work on proper changes.
This does work on 4.13.3 and PXE for me.
I've tested on 1 Gbps, 100 Mbps and 10 Mbps. I've done some
preliminary testing (eg. large file copies.)
^ permalink raw reply
* Re: [PATCH v6 05/11] dt-bindings: net: dwmac-sun8i: update documentation about integrated PHY
From: Maxime Ripard @ 2017-09-27 10:15 UTC (permalink / raw)
To: Corentin Labbe
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
wens-jdAy2FN1RRM, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
catalin.marinas-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8,
peppe.cavallaro-qxv4g6HH51o, alexandre.torgue-qxv4g6HH51o,
andrew-g2DYL2Zd6BY, f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
frowand.list-Re5JQEeQqe8AvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20170927073414.17361-6-clabbe.montjoie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 421 bytes --]
Hi,
On Wed, Sep 27, 2017 at 07:34:08AM +0000, Corentin Labbe wrote:
> This patch add documentation about the MDIO switch used on sun8i-h3-emac
> for integrated PHY.
>
> Signed-off-by: Corentin Labbe <clabbe.montjoie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
This should be squashed with patch 1.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [PATCH net-next 1/6] net: dsa: Fix SWITCHDEV_ATTR_ID_PORT_PARENT_ID
From: Sergei Shtylyov @ 2017-09-27 10:15 UTC (permalink / raw)
To: Andrew Lunn, David Miller; +Cc: Vivien Didelot, netdev
In-Reply-To: <1506464764-12699-2-git-send-email-andrew@lunn.ch>
Hello!
On 9/27/2017 1:25 AM, Andrew Lunn wrote:
> SWITCHDEV_ATTR_ID_PORT_PARENT_ID is used by the software bridge when
> determining which ports to flood a packet out. If the packet
> originated from a switch, it assumes the switch has already flooded
> the packet out the switches ports, so the bridge should not flood the
> packet itself out switch ports. Ports on the same switch are expected
> to return the same parent ID when SWITCHDEV_ATTR_ID_PORT_PARENT_ID is
> called.
>
> DSA gets this wrong with clusters of switches. As far as the software
> bridge is concerned, the cluster is all one switch. A packet from any
> switch in the cluster can be assumed to of been flooded as needed out
s/of/have/?
> all ports of the cluster, not just the switch it originated
> from. Hence all ports of a cluster should return the same parent. The
> old implementation did not, each switch in the cluster had its own ID.
>
> Also wrong was that the ID was not unique if multiple DSA instances
> are in operation.
>
> Use the tree ID as the parent ID, which is the same for all switches
> in a cluster and unique across switch clusters.
>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
> v2: Swap from MAC address to dst->tree
[...]
MBR, Sergei
^ permalink raw reply
* Re: [PATCH v6 06/11] ARM: dts: sunxi: h3/h5: represent the mdio switch used by sun8i-h3-emac
From: Maxime Ripard @ 2017-09-27 10:16 UTC (permalink / raw)
To: Corentin Labbe
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
wens-jdAy2FN1RRM, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
catalin.marinas-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8,
peppe.cavallaro-qxv4g6HH51o, alexandre.torgue-qxv4g6HH51o,
andrew-g2DYL2Zd6BY, f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
frowand.list-Re5JQEeQqe8AvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20170927073414.17361-7-clabbe.montjoie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1816 bytes --]
On Wed, Sep 27, 2017 at 07:34:09AM +0000, Corentin Labbe wrote:
> Since dwmac-sun8i could use either an integrated PHY or an external PHY
> (which could be at same MDIO address), we need to represent this selection
> by a MDIO switch.
>
> Signed-off-by: Corentin Labbe <clabbe.montjoie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> arch/arm/boot/dts/sunxi-h3-h5.dtsi | 31 +++++++++++++++++++++++++------
> 1 file changed, 25 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> index 3b7d953429a6..a8e9b8f378ba 100644
> --- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> +++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> @@ -422,14 +422,33 @@
> #size-cells = <0>;
> status = "disabled";
>
> - mdio: mdio {
> + mdio0: mdio {
> #address-cells = <1>;
> #size-cells = <0>;
> - int_mii_phy: ethernet-phy@1 {
> - compatible = "ethernet-phy-ieee802.3-c22";
> - reg = <1>;
> - clocks = <&ccu CLK_BUS_EPHY>;
> - resets = <&ccu RST_BUS_EPHY>;
> + compatible = "snps,dwmac-mdio";
> +
> + mdio-mux {
> + compatible = "mdio-mux";
> + #address-cells = <1>;
> + #size-cells = <0>;
Newline
> + /* Only one MDIO is usable at the time */
> + internal_mdio: mdio@1 {
> + reg = <1>;
> + #address-cells = <1>;
> + #size-cells = <0>;
Newline
> + int_mii_phy: ethernet-phy@1 {
> + compatible = "ethernet-phy-ieee802.3-c22";
> + reg = <1>;
> + clocks = <&ccu CLK_BUS_EPHY>;
> + resets = <&ccu RST_BUS_EPHY>;
> + phy-is-integrated;
> + };
> + };
Newline
> + mdio: mdio@2 {
This is quite confusing. Why not call the label external_mdio?
Thanks
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: tg3 pxe weirdness
From: Siva Reddy Kallam @ 2017-09-27 10:31 UTC (permalink / raw)
To: Berend De Schouwer; +Cc: Linux Netdev List
On Wed, Sep 27, 2017 at 3:35 PM, Berend De Schouwer
<berend.de.schouwer@gmail.com> wrote:
> On Mon, 2017-09-25 at 15:11 +0530, Siva Reddy Kallam wrote:
>> On Fri, Sep 22, 2017 at 9:04 PM, Berend De Schouwer
>> <berend.de.schouwer@gmail.com> wrote:
>> > On Fri, 2017-09-22 at 11:51 +0530, Siva Reddy Kallam wrote:
>> > >
>> > >
>> > > Can you please share below details?
>> > > 1) Model and Manufacturer of the system
>> > > 2) Linux distro/kernel used?
>> >
>> > 4.13.3 gets a little further, but after some more data is
>> > transferred
>> > the tg3 driver still crashes. This is unfortunately before I've
>> > got a
>> > writeable filesystem.
>> >
>> > The last line is:
>> > tg3 0000:01:00.0: tg3_stop_block timed out, ofs=4c00 enable_bit=2
>> >
>> > I've got some ideas to get the full dmesg.
>> >
>> > As with the other kernels it works OK on 1Gbps, but not slower
>> > switches.
>>
>> I am suspecting with link aware mode, the clock speed could be slow
>> and boot code does not
>> complete within the expected time with lower link speeds. So,
>> Providing a patch to override clock.
>> Can you please try with attached debug patch and provide us the
>> feedback with 100M link?
>> If it solves this issue, we will work on proper changes.
>
> This does work on 4.13.3 and PXE for me.
>
> I've tested on 1 Gbps, 100 Mbps and 10 Mbps. I've done some
> preliminary testing (eg. large file copies.)
Good. We will work on required changes and upstream proper patch after
sanity test with multiple speeds.
^ permalink raw reply
* Re: [PATCH net-next 2/2] tools: bpf: add bpftool
From: Jesper Dangaard Brouer @ 2017-09-27 10:45 UTC (permalink / raw)
To: Jakub Kicinski
Cc: brouer, Alexei Starovoitov, netdev, daniel, davem, hannes,
dsahern, oss-drivers, linux-doc@vger.kernel.org
In-Reply-To: <20170927000208.4396dfb7@cakuba>
On Wed, 27 Sep 2017 00:02:08 +0100
Jakub Kicinski <jakub.kicinski@netronome.com> wrote:
> On Tue, 26 Sep 2017 15:24:06 -0700, Alexei Starovoitov wrote:
> > On Tue, Sep 26, 2017 at 08:35:22AM -0700, Jakub Kicinski wrote:
> > > Add a simple tool for querying and updating BPF objects on the system.
> > >
> > > Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> > > Reviewed-by: Simon Horman <simon.horman@netronome.com>
[...]
> > > tools/bpf/Makefile | 18 +-
> > > tools/bpf/bpftool/Makefile | 80 +++++
> > > tools/bpf/bpftool/common.c | 214 ++++++++++++
> > > tools/bpf/bpftool/jit_disasm.c | 83 +++++
> > > tools/bpf/bpftool/main.c | 212 ++++++++++++
> > > tools/bpf/bpftool/main.h | 99 ++++++
> > > tools/bpf/bpftool/map.c | 742 +++++++++++++++++++++++++++++++++++++++++
> > > tools/bpf/bpftool/prog.c | 392 ++++++++++++++++++++++
> > > 8 files changed, 1837 insertions(+), 3 deletions(-)
> > ...
> > > +static int do_help(int argc, char **argv)
> > > +{
> > > + fprintf(stderr,
> > > + "Usage: %s %s show [MAP]\n"
> > > + " %s %s dump MAP\n"
> > > + " %s %s update MAP key BYTES value VALUE [UPDATE_FLAGS]\n"
> > > + " %s %s lookup MAP key BYTES\n"
> > > + " %s %s getnext MAP [key BYTES]\n"
> > > + " %s %s delete MAP key BYTES\n"
> > > + " %s %s pin MAP FILE\n"
> > > + " %s %s help\n"
> > > + "\n"
> > > + " MAP := { id MAP_ID | pinned FILE }\n"
> > > + " " HELP_SPEC_PROGRAM "\n"
> > > + " VALUE := { BYTES | MAP | PROG }\n"
> > > + " UPDATE_FLAGS := { any | exist | noexist }\n"
> > > + "",
> >
> > overall looks good to me, but still difficult to grasp how to use it.
> > Can you add README with example usage and expected output?
>
> I have a README on GitHub, but I was thinking about perhaps writing a
> proper man page? Do you prefer one over the other?
I would prefer adding a README.rst file, in RST-format, as the rest of
the kernel documentation is moving in that direction[1] (your github
version is in README.md format). A man page will always be
out-of-sync, and even out-of-sync on different distros.
See[1]: https://www.kernel.org/doc/html/latest/
And then I would find some place in Documentation/admin-guide/ and
include the README.rst file, so it shows up at [1].
RST have an include method like:
.. include:: ../../tools/bpf/bpftool/README.rst
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* [PATCH 4/4] net: stmmac: fixing DMA reset sleep and timeout values
From: Emiliano Ingrassia @ 2017-09-27 10:46 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue, netdev, linux-amlogic
In-Reply-To: <cover.1506507688.git.ingrassia@epigenesys.com>
This patch fixes the sleep and timeout values used during
DMA reset, which were inverted in a previous patch.
Fixes: 8a70aeca80c2 ("net: stmmac: Use readl_poll_timeout")
Signed-off-by: Emiliano Ingrassia <ingrassia@epigenesys.com>
---
drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c b/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c
index 67af0bdd7f10..7516ca210855 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c
@@ -34,7 +34,7 @@ int dwmac_dma_reset(void __iomem *ioaddr)
err = readl_poll_timeout(ioaddr + DMA_BUS_MODE, value,
!(value & DMA_BUS_MODE_SFT_RESET),
- 100000, 10000);
+ 10000, 100000);
if (err)
return -EBUSY;
--
2.14.1
^ permalink raw reply related
* Re: [PATCH net-next 2/2] tools: bpf: add bpftool
From: Daniel Borkmann @ 2017-09-27 10:55 UTC (permalink / raw)
To: Jesper Dangaard Brouer, Jakub Kicinski
Cc: Alexei Starovoitov, netdev, davem, hannes, dsahern, oss-drivers,
linux-doc@vger.kernel.org
In-Reply-To: <20170927124511.650870bc@redhat.com>
On 09/27/2017 12:45 PM, Jesper Dangaard Brouer wrote:
> On Wed, 27 Sep 2017 00:02:08 +0100
> Jakub Kicinski <jakub.kicinski@netronome.com> wrote:
>> On Tue, 26 Sep 2017 15:24:06 -0700, Alexei Starovoitov wrote:
>>> On Tue, Sep 26, 2017 at 08:35:22AM -0700, Jakub Kicinski wrote:
>>>> Add a simple tool for querying and updating BPF objects on the system.
>>>>
>>>> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>>>> Reviewed-by: Simon Horman <simon.horman@netronome.com>
> [...]
>>>> tools/bpf/Makefile | 18 +-
>>>> tools/bpf/bpftool/Makefile | 80 +++++
>>>> tools/bpf/bpftool/common.c | 214 ++++++++++++
>>>> tools/bpf/bpftool/jit_disasm.c | 83 +++++
>>>> tools/bpf/bpftool/main.c | 212 ++++++++++++
>>>> tools/bpf/bpftool/main.h | 99 ++++++
>>>> tools/bpf/bpftool/map.c | 742 +++++++++++++++++++++++++++++++++++++++++
>>>> tools/bpf/bpftool/prog.c | 392 ++++++++++++++++++++++
>>>> 8 files changed, 1837 insertions(+), 3 deletions(-)
>>> ...
>>>> +static int do_help(int argc, char **argv)
>>>> +{
>>>> + fprintf(stderr,
>>>> + "Usage: %s %s show [MAP]\n"
>>>> + " %s %s dump MAP\n"
>>>> + " %s %s update MAP key BYTES value VALUE [UPDATE_FLAGS]\n"
>>>> + " %s %s lookup MAP key BYTES\n"
>>>> + " %s %s getnext MAP [key BYTES]\n"
>>>> + " %s %s delete MAP key BYTES\n"
>>>> + " %s %s pin MAP FILE\n"
>>>> + " %s %s help\n"
>>>> + "\n"
>>>> + " MAP := { id MAP_ID | pinned FILE }\n"
>>>> + " " HELP_SPEC_PROGRAM "\n"
>>>> + " VALUE := { BYTES | MAP | PROG }\n"
>>>> + " UPDATE_FLAGS := { any | exist | noexist }\n"
>>>> + "",
>>>
>>> overall looks good to me, but still difficult to grasp how to use it.
>>> Can you add README with example usage and expected output?
>>
>> I have a README on GitHub, but I was thinking about perhaps writing a
>> proper man page? Do you prefer one over the other?
>
> I would prefer adding a README.rst file, in RST-format, as the rest of
> the kernel documentation is moving in that direction[1] (your github
> version is in README.md format). A man page will always be
> out-of-sync, and even out-of-sync on different distros.
>
> See[1]: https://www.kernel.org/doc/html/latest/
>
> And then I would find some place in Documentation/admin-guide/ and
> include the README.rst file, so it shows up at [1].
>
> RST have an include method like:
>
> .. include:: ../../tools/bpf/bpftool/README.rst
Agree, to have a README.rst with a description/howto in bpftool/
seems like a good idea to me, too.
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH net-next 2/2] tools: bpf: add bpftool
From: Jakub Kicinski @ 2017-09-27 10:57 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Alexei Starovoitov, netdev, daniel, davem, hannes, dsahern,
oss-drivers, linux-doc@vger.kernel.org
In-Reply-To: <20170927124511.650870bc@redhat.com>
On Wed, 27 Sep 2017 12:45:11 +0200, Jesper Dangaard Brouer wrote:
> On Wed, 27 Sep 2017 00:02:08 +0100
> Jakub Kicinski <jakub.kicinski@netronome.com> wrote:
>
> > On Tue, 26 Sep 2017 15:24:06 -0700, Alexei Starovoitov wrote:
> > > On Tue, Sep 26, 2017 at 08:35:22AM -0700, Jakub Kicinski wrote:
> > > > Add a simple tool for querying and updating BPF objects on the system.
> > > >
> > > > Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> > > > Reviewed-by: Simon Horman <simon.horman@netronome.com>
> [...]
> > > > tools/bpf/Makefile | 18 +-
> > > > tools/bpf/bpftool/Makefile | 80 +++++
> > > > tools/bpf/bpftool/common.c | 214 ++++++++++++
> > > > tools/bpf/bpftool/jit_disasm.c | 83 +++++
> > > > tools/bpf/bpftool/main.c | 212 ++++++++++++
> > > > tools/bpf/bpftool/main.h | 99 ++++++
> > > > tools/bpf/bpftool/map.c | 742 +++++++++++++++++++++++++++++++++++++++++
> > > > tools/bpf/bpftool/prog.c | 392 ++++++++++++++++++++++
> > > > 8 files changed, 1837 insertions(+), 3 deletions(-)
> > > ...
> > > > +static int do_help(int argc, char **argv)
> > > > +{
> > > > + fprintf(stderr,
> > > > + "Usage: %s %s show [MAP]\n"
> > > > + " %s %s dump MAP\n"
> > > > + " %s %s update MAP key BYTES value VALUE [UPDATE_FLAGS]\n"
> > > > + " %s %s lookup MAP key BYTES\n"
> > > > + " %s %s getnext MAP [key BYTES]\n"
> > > > + " %s %s delete MAP key BYTES\n"
> > > > + " %s %s pin MAP FILE\n"
> > > > + " %s %s help\n"
> > > > + "\n"
> > > > + " MAP := { id MAP_ID | pinned FILE }\n"
> > > > + " " HELP_SPEC_PROGRAM "\n"
> > > > + " VALUE := { BYTES | MAP | PROG }\n"
> > > > + " UPDATE_FLAGS := { any | exist | noexist }\n"
> > > > + "",
> > >
> > > overall looks good to me, but still difficult to grasp how to use it.
> > > Can you add README with example usage and expected output?
> >
> > I have a README on GitHub, but I was thinking about perhaps writing a
> > proper man page? Do you prefer one over the other?
>
> I would prefer adding a README.rst file, in RST-format, as the rest of
> the kernel documentation is moving in that direction[1] (your github
> version is in README.md format). A man page will always be
> out-of-sync, and even out-of-sync on different distros.
>
> See[1]: https://www.kernel.org/doc/html/latest/
>
> And then I would find some place in Documentation/admin-guide/ and
> include the README.rst file, so it shows up at [1].
>
> RST have an include method like:
>
> .. include:: ../../tools/bpf/bpftool/README.rst
Can the docs in new format be rendered into a man page? Call me old
fashioned but I think we should provide some form of a man page.. :)
^ permalink raw reply
* Re: [PATCH v2 net-next 2/2] net/sched: allow flower to match tunnel options
From: Jiri Pirko @ 2017-09-27 11:08 UTC (permalink / raw)
To: Simon Horman
Cc: David Miller, Jiri Pirko, Jamal Hadi Salim, Cong Wang, netdev,
oss-drivers
In-Reply-To: <20170927092732.GC25449@vergenet.net>
Wed, Sep 27, 2017 at 11:27:33AM CEST, simon.horman@netronome.com wrote:
>On Wed, Sep 27, 2017 at 11:10:05AM +0200, Jiri Pirko wrote:
>> Wed, Sep 27, 2017 at 10:16:34AM CEST, simon.horman@netronome.com wrote:
>> >Allow matching on options in tunnel headers.
>> >This makes use of existing tunnel metadata support.
>> >
>> >Options are a bytestring of up to 256 bytes.
>> >Tunnel implementations may support less or more options,
>> >or no options at all.
>> >
>> >e.g.
>> > # ip link add name geneve0 type geneve dstport 0 external
>> > # tc qdisc add dev geneve0 ingress
>> > # tc filter add dev geneve0 protocol ip parent ffff: \
>> > flower \
>> > enc_src_ip 10.0.99.192 \
>> > enc_dst_ip 10.0.99.193 \
>> > enc_key_id 11 \
>> > enc_opts 0102800100800020/fffffffffffffff0 \
>> > ip_proto udp \
>> > action mirred egress redirect dev eth1
>> >
>> >Signed-off-by: Simon Horman <simon.horman@netronome.com>
>> >Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>> >
>> >---
>> >v2
>> >* Correct example which was incorrectly described setting rather
>> > than matching tunnel options
>> >---
>> > include/net/flow_dissector.h | 13 +++++++++++++
>> > include/uapi/linux/pkt_cls.h | 3 +++
>> > net/sched/cls_flower.c | 35 ++++++++++++++++++++++++++++++++++-
>> > 3 files changed, 50 insertions(+), 1 deletion(-)
>> >
>> >diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
>> >index fc3dce730a6b..43f98bf0b349 100644
>> >--- a/include/net/flow_dissector.h
>> >+++ b/include/net/flow_dissector.h
>> >@@ -183,6 +183,18 @@ struct flow_dissector_key_ip {
>> > __u8 ttl;
>> > };
>> >
>> >+/**
>> >+ * struct flow_dissector_key_enc_opts:
>> >+ * @data: data
>> >+ * @len: len
>> >+ */
>> >+struct flow_dissector_key_enc_opts {
>> >+ u8 data[256]; /* Using IP_TUNNEL_OPTS_MAX is desired here
>> >+ * but seems difficult to #include
>> >+ */
>> >+ u8 len;
>> >+};
>> >+
>> > enum flow_dissector_key_id {
>> > FLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */
>> > FLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */
>> >@@ -205,6 +217,7 @@ enum flow_dissector_key_id {
>> > FLOW_DISSECTOR_KEY_MPLS, /* struct flow_dissector_key_mpls */
>> > FLOW_DISSECTOR_KEY_TCP, /* struct flow_dissector_key_tcp */
>> > FLOW_DISSECTOR_KEY_IP, /* struct flow_dissector_key_ip */
>> >+ FLOW_DISSECTOR_KEY_ENC_OPTS, /* struct flow_dissector_key_enc_opts */
>>
>> I don't see the actual dissection implementation. Where is it?
>> Did you test the patchset?
>
>Yes, I did test it. But it is also possible something went astray along the
>way and I will retest.
>
>I think that the code you are looking for is in
>fl_classify() in this patch.
The dissection should be done in the flow_dissector. That's the whole
point in having it generic. You should move it there.
^ permalink raw reply
* Re: tc H/W offload issue with vxlan tunnels [was: nfp: flower vxlan tunnel offload]
From: Jiri Pirko @ 2017-09-27 11:11 UTC (permalink / raw)
To: Paolo Abeni
Cc: Or Gerlitz, Jiri Benc, Simon Horman, David Miller, Jakub Kicinski,
Linux Netdev List, oss-drivers, John Hurley, Paul Blakey,
Jiri Pirko, Roi Dayan
In-Reply-To: <1506505618.2867.34.camel@redhat.com>
Wed, Sep 27, 2017 at 11:46:58AM CEST, pabeni@redhat.com wrote:
>On Wed, 2017-09-27 at 11:17 +0200, Jiri Pirko wrote:
>> Wed, Sep 27, 2017 at 10:29:35AM CEST, pabeni@redhat.com wrote:
>> > So it looks like the H/W offload hook will still be called with the
>> > same arguments in both case, and 'bad' rule will still be pushed to the
>> > H/W as the driver itself has no way to distinct between the two
>> > scenarios.
>>
>> Why "bad"?
>
>Such rule is coped differently by the SW and the HW data path.
>
>a rule like:
>
>tc filter add dev eth0 protocol ip parent ffff: flower \
> enc_key_id 102 enc_dst_port 4789 src_ip 3.4.5.6 skip_hw \
> action action mirred redirect eth0_vf_1
>
>will match 0 packets, while:
>
>tc filter add dev eth0 protocol ip parent ffff: flower \
> enc_key_id 102 enc_dst_port 4789 src_ip 3.4.5.6 skip_sw \
> action action mirred redirect eth0_vf_1
>
>[just flipped 'skip_sw' and 'skip_hw' ]
>will match the vxlan-tunneled packets. I understand that one of the
>design goal for the h/w offload path is being consistent with the sw
>one, but that does not hold in the above scenario.
Sure, the consistency is important. Howcome "skip_hw" won't match and
"skip_sw" will match? What's different?
>
>> Regarding the distinction, driver knows if user add a rule directly to
>> the eth0, or if the eth0 is egress device in the action. Those are 2
>> separete driver entrypoints - of course, talking about code with my
>> changes.
>
>ok, but than each driver should catch the scenario "rule with tunnel
>match over non tunnel device" and cope with them properly - never match
>it - why don't simply avoiding pushing such rules to the H/W ?
>
>Cheers,
>
>Paolo
^ permalink raw reply
* [RFT] lan78xx: FIX use-after-free in lan78xx_write_reg
From: Arvind Yadav @ 2017-09-27 11:18 UTC (permalink / raw)
To: davem, woojung.huh, UNGLinuxDriver, netdev, andreyknvl, kcc,
dvyukov, Nisar.Sayed
Cc: linux-usb, linux-kernel, syzkaller
We are not releasing 'buf' memory on failure or disconnect a device.
Adding 'u8 *buf' as part of 'lan78xx_net' structure to make proper
handle for 'buf'.
Now releasing 'buf' memory on failure. It's allocate first in
lan78xx_probe() and it should be freed last in lan78xx_disconnect().
Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
drivers/net/usb/lan78xx.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index b99a7fb..e653982 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -402,6 +402,7 @@ struct lan78xx_net {
struct statstage stats;
struct irq_domain_data domain_data;
+ u8 *buf;
};
/* define external phy id */
@@ -3470,6 +3471,9 @@ static void lan78xx_disconnect(struct usb_interface *intf)
usb_scuttle_anchored_urbs(&dev->deferred);
+ kfree(dev->buf);
+ dev->buf = NULL;
+
lan78xx_unbind(dev, intf);
usb_kill_urb(dev->urb_intr);
@@ -3520,7 +3524,6 @@ static int lan78xx_probe(struct usb_interface *intf,
int ret;
unsigned maxp;
unsigned period;
- u8 *buf = NULL;
udev = interface_to_usbdev(intf);
udev = usb_get_dev(udev);
@@ -3588,16 +3591,15 @@ static int lan78xx_probe(struct usb_interface *intf,
period = dev->ep_intr->desc.bInterval;
maxp = usb_maxpacket(dev->udev, dev->pipe_intr, 0);
- buf = kmalloc(maxp, GFP_KERNEL);
- if (buf) {
+ dev->buf = kmalloc(maxp, GFP_KERNEL);
+ if (dev->buf) {
dev->urb_intr = usb_alloc_urb(0, GFP_KERNEL);
if (!dev->urb_intr) {
ret = -ENOMEM;
- kfree(buf);
goto out3;
} else {
usb_fill_int_urb(dev->urb_intr, dev->udev,
- dev->pipe_intr, buf, maxp,
+ dev->pipe_intr, dev->buf, maxp,
intr_complete, dev, period);
}
}
@@ -3626,6 +3628,8 @@ static int lan78xx_probe(struct usb_interface *intf,
return 0;
out3:
+ kfree(dev->buf);
+ dev->buf = NULL;
lan78xx_unbind(dev, intf);
out2:
free_netdev(netdev);
--
1.9.1
^ permalink raw reply related
* Re: [PATCH net-next 2/2] tools: bpf: add bpftool
From: Jesper Dangaard Brouer @ 2017-09-27 11:19 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Alexei Starovoitov, netdev, daniel, davem, hannes, dsahern,
oss-drivers, linux-doc@vger.kernel.org, brouer
In-Reply-To: <20170927035742.5c9ee8c3@cakuba>
On Wed, 27 Sep 2017 03:57:42 -0700
Jakub Kicinski <jakub.kicinski@netronome.com> wrote:
> On Wed, 27 Sep 2017 12:45:11 +0200, Jesper Dangaard Brouer wrote:
> > On Wed, 27 Sep 2017 00:02:08 +0100
> > Jakub Kicinski <jakub.kicinski@netronome.com> wrote:
> >
> > > On Tue, 26 Sep 2017 15:24:06 -0700, Alexei Starovoitov wrote:
> > > > On Tue, Sep 26, 2017 at 08:35:22AM -0700, Jakub Kicinski wrote:
> > > > > Add a simple tool for querying and updating BPF objects on the system.
> > > > >
> > > > > Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> > > > > Reviewed-by: Simon Horman <simon.horman@netronome.com>
> > [...]
> > > > > tools/bpf/Makefile | 18 +-
> > > > > tools/bpf/bpftool/Makefile | 80 +++++
> > > > > tools/bpf/bpftool/common.c | 214 ++++++++++++
> > > > > tools/bpf/bpftool/jit_disasm.c | 83 +++++
> > > > > tools/bpf/bpftool/main.c | 212 ++++++++++++
> > > > > tools/bpf/bpftool/main.h | 99 ++++++
> > > > > tools/bpf/bpftool/map.c | 742 +++++++++++++++++++++++++++++++++++++++++
> > > > > tools/bpf/bpftool/prog.c | 392 ++++++++++++++++++++++
> > > > > 8 files changed, 1837 insertions(+), 3 deletions(-)
> > > > ...
> > > > > +static int do_help(int argc, char **argv)
> > > > > +{
> > > > > + fprintf(stderr,
> > > > > + "Usage: %s %s show [MAP]\n"
> > > > > + " %s %s dump MAP\n"
> > > > > + " %s %s update MAP key BYTES value VALUE [UPDATE_FLAGS]\n"
> > > > > + " %s %s lookup MAP key BYTES\n"
> > > > > + " %s %s getnext MAP [key BYTES]\n"
> > > > > + " %s %s delete MAP key BYTES\n"
> > > > > + " %s %s pin MAP FILE\n"
> > > > > + " %s %s help\n"
> > > > > + "\n"
> > > > > + " MAP := { id MAP_ID | pinned FILE }\n"
> > > > > + " " HELP_SPEC_PROGRAM "\n"
> > > > > + " VALUE := { BYTES | MAP | PROG }\n"
> > > > > + " UPDATE_FLAGS := { any | exist | noexist }\n"
> > > > > + "",
> > > >
> > > > overall looks good to me, but still difficult to grasp how to use it.
> > > > Can you add README with example usage and expected output?
> > >
> > > I have a README on GitHub, but I was thinking about perhaps writing a
> > > proper man page? Do you prefer one over the other?
> >
> > I would prefer adding a README.rst file, in RST-format, as the rest of
> > the kernel documentation is moving in that direction[1] (your github
> > version is in README.md format). A man page will always be
> > out-of-sync, and even out-of-sync on different distros.
> >
> > See[1]: https://www.kernel.org/doc/html/latest/
> >
> > And then I would find some place in Documentation/admin-guide/ and
> > include the README.rst file, so it shows up at [1].
> >
> > RST have an include method like:
> >
> > .. include:: ../../tools/bpf/bpftool/README.rst
>
> Can the docs in new format be rendered into a man page? Call me old
> fashioned but I think we should provide some form of a man page.. :)
Yes, simply create the man page like:
rst2man README.rst README.man
You can add this to your local makefile.
The standard sphinx build can also generate man-pages, but it have been
removed from the kernel makefile targets:
Documentation targets:
Linux kernel internal documentation in different formats from ReST:
htmldocs - HTML
latexdocs - LaTeX
pdfdocs - PDF
epubdocs - EPUB
xmldocs - XML
linkcheckdocs - check for broken external links (will connect to external hosts)
cleandocs - clean all generated files
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH v2 02/16] thunderbolt: Add support for XDomain properties
From: Mika Westerberg @ 2017-09-27 11:32 UTC (permalink / raw)
To: David Miller
Cc: gregkh, andreas.noever, michael.jamet, yehezkel.bernat,
amir.jer.levy, Mario.Limonciello, lukas, andriy.shevchenko,
andrew, linux-kernel, netdev
In-Reply-To: <20170926.213354.305351416790211828.davem@davemloft.net>
On Tue, Sep 26, 2017 at 09:33:54PM -0700, David Miller wrote:
> From: Mika Westerberg <mika.westerberg@linux.intel.com>
> Date: Mon, 25 Sep 2017 14:07:24 +0300
>
> > +struct tb_property_entry {
> > + u32 key_hi;
> > + u32 key_lo;
> > + u16 length;
> > + u8 reserved;
> > + u8 type;
> > + u32 value;
> > +} __packed;
> > +
> > +struct tb_property_rootdir_entry {
> > + u32 magic;
> > + u32 length;
> > + struct tb_property_entry entries[];
> > +} __packed;
> > +
> > +struct tb_property_dir_entry {
> > + u32 uuid[4];
> > + struct tb_property_entry entries[];
> > +} __packed;
>
> There is no apparent need for __packed here, and __packed should be
> avoided unless absolutely necessary as it pessimizes the code
> significantly on some architectures.
>
> Please remove __packed from these datastructures unless you can
> prove it is absolutely needed and, in such case, please document
> in a comment why that requirement exists. Because from the layout
> of these types, everything will be packed in just fine without
> __packed.
I will thanks.
Just for my education, is there some rule which tells when __packed is
to be used? For example the above structures are all 32-bit aligned but
how about something like:
struct foo {
u32 value1;
u8 value2;
};
If the on-wire format requires such structures I assume __packed
is needed here?
Thanks!
^ permalink raw reply
* Re: [PATCH net] i40e: Fix limit imprecise of the number of MAC/VLAN that can be added for VFs
From: Sergei Shtylyov @ 2017-09-27 11:33 UTC (permalink / raw)
To: w00273186, davem, jeffrey.t.kirsher; +Cc: netdev, intel-wired-lan, caihe
In-Reply-To: <1506495497-10736-1-git-send-email-wangyunjian@huawei.com>
Hello!
On 9/27/2017 9:58 AM, w00273186 wrote:
> From: Yunjian Wang <wangyunjian@huawei.com>
>
> Now it don't limit the number of MAC/VLAN strictly. When there is more
Doesn't.
> elements in the virtchnl MAC/VLAN list, it can still add successfully.
>
> Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
[...]
MBR, Sergei
^ permalink raw reply
* Re: [PATCH net-next 2/2] tools: bpf: add bpftool
From: Markus Heiser @ 2017-09-27 11:49 UTC (permalink / raw)
To: Jesper Dangaard Brouer, Jonathan Corbet
Cc: Jakub Kicinski, Alexei Starovoitov, netdev, daniel, davem, hannes,
dsahern, oss-drivers, Linux Doc Mailing List
In-Reply-To: <20170927131906.286b1e25@redhat.com>
> Am 27.09.2017 um 13:19 schrieb Jesper Dangaard Brouer <brouer@redhat.com>:
[...]
>>> I would prefer adding a README.rst file, in RST-format, as the rest of
>>> the kernel documentation is moving in that direction[1] (your github
>>> version is in README.md format). A man page will always be
>>> out-of-sync, and even out-of-sync on different distros.
>>>
>>> See[1]: https://www.kernel.org/doc/html/latest/
>>>
>>> And then I would find some place in Documentation/admin-guide/ and
>>> include the README.rst file, so it shows up at [1].
>>>
>>> RST have an include method like:
>>>
>>> .. include:: ../../tools/bpf/bpftool/README.rst
>>
>> Can the docs in new format be rendered into a man page? Call me old
>> fashioned but I think we should provide some form of a man page.. :)
>
> Yes, simply create the man page like:
>
> rst2man README.rst README.man
> You can add this to your local makefile.
Hm ... this will work only for simple reST files.
FYI: Sphinx is build up on top of docutils extending the reST markup. Tools
like rst2man are from docutils and do not cover the markup extensions
from Sphinx. Since Sphinx has its own builder for man pages, generating
man pages is not the problem ...
> The standard sphinx build can also generate man-pages, but it have been
> removed from the kernel makefile targets:
>
> Documentation targets:
> Linux kernel internal documentation in different formats from ReST:
> htmldocs - HTML
> latexdocs - LaTeX
> pdfdocs - PDF
> epubdocs - EPUB
> xmldocs - XML
> linkcheckdocs - check for broken external links (will connect to external hosts)
> cleandocs - clean all generated files
The reason why we have no man page target is, that we have not yet
evaluated a concept, how we can manage the 'build man pages' task in
the kernel build.
Beside what you find in the kernel-source tree I'am working on such
a concept (adapted man page builder for the Kernel) [1].
IMO: Even if we merge the builder from [1] or implement something different,
we have to touch the current kernel-doc parser. I guess that and the absence
of a handy concept is the reason why we are dangling with a 'man' target.
Anyway, these are my estimations, it might be better if we hear what
Jon think about a man page builder (concept).
[1] https://return42.github.io/linuxdoc/linuxdoc-howto/man-pages.html
-- Markus --
^ permalink raw reply
* Re: [PATCH v2 net-next 2/2] net/sched: allow flower to match tunnel options
From: Simon Horman @ 2017-09-27 11:54 UTC (permalink / raw)
To: Jiri Pirko
Cc: David Miller, Jiri Pirko, Jamal Hadi Salim, Cong Wang, netdev,
oss-drivers
In-Reply-To: <20170927110822.GD1944@nanopsycho.orion>
On Wed, Sep 27, 2017 at 01:08:22PM +0200, Jiri Pirko wrote:
> Wed, Sep 27, 2017 at 11:27:33AM CEST, simon.horman@netronome.com wrote:
> >On Wed, Sep 27, 2017 at 11:10:05AM +0200, Jiri Pirko wrote:
> >> Wed, Sep 27, 2017 at 10:16:34AM CEST, simon.horman@netronome.com wrote:
> >> >Allow matching on options in tunnel headers.
> >> >This makes use of existing tunnel metadata support.
> >> >
> >> >Options are a bytestring of up to 256 bytes.
> >> >Tunnel implementations may support less or more options,
> >> >or no options at all.
> >> >
> >> >e.g.
> >> > # ip link add name geneve0 type geneve dstport 0 external
> >> > # tc qdisc add dev geneve0 ingress
> >> > # tc filter add dev geneve0 protocol ip parent ffff: \
> >> > flower \
> >> > enc_src_ip 10.0.99.192 \
> >> > enc_dst_ip 10.0.99.193 \
> >> > enc_key_id 11 \
> >> > enc_opts 0102800100800020/fffffffffffffff0 \
> >> > ip_proto udp \
> >> > action mirred egress redirect dev eth1
> >> >
> >> >Signed-off-by: Simon Horman <simon.horman@netronome.com>
> >> >Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> >> >
> >> >---
> >> >v2
> >> >* Correct example which was incorrectly described setting rather
> >> > than matching tunnel options
> >> >---
> >> > include/net/flow_dissector.h | 13 +++++++++++++
> >> > include/uapi/linux/pkt_cls.h | 3 +++
> >> > net/sched/cls_flower.c | 35 ++++++++++++++++++++++++++++++++++-
> >> > 3 files changed, 50 insertions(+), 1 deletion(-)
> >> >
> >> >diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
> >> >index fc3dce730a6b..43f98bf0b349 100644
> >> >--- a/include/net/flow_dissector.h
> >> >+++ b/include/net/flow_dissector.h
> >> >@@ -183,6 +183,18 @@ struct flow_dissector_key_ip {
> >> > __u8 ttl;
> >> > };
> >> >
> >> >+/**
> >> >+ * struct flow_dissector_key_enc_opts:
> >> >+ * @data: data
> >> >+ * @len: len
> >> >+ */
> >> >+struct flow_dissector_key_enc_opts {
> >> >+ u8 data[256]; /* Using IP_TUNNEL_OPTS_MAX is desired here
> >> >+ * but seems difficult to #include
> >> >+ */
> >> >+ u8 len;
> >> >+};
> >> >+
> >> > enum flow_dissector_key_id {
> >> > FLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */
> >> > FLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */
> >> >@@ -205,6 +217,7 @@ enum flow_dissector_key_id {
> >> > FLOW_DISSECTOR_KEY_MPLS, /* struct flow_dissector_key_mpls */
> >> > FLOW_DISSECTOR_KEY_TCP, /* struct flow_dissector_key_tcp */
> >> > FLOW_DISSECTOR_KEY_IP, /* struct flow_dissector_key_ip */
> >> >+ FLOW_DISSECTOR_KEY_ENC_OPTS, /* struct flow_dissector_key_enc_opts */
> >>
> >> I don't see the actual dissection implementation. Where is it?
> >> Did you test the patchset?
> >
> >Yes, I did test it. But it is also possible something went astray along the
> >way and I will retest.
> >
> >I think that the code you are looking for is in
> >fl_classify() in this patch.
>
> The dissection should be done in the flow_dissector. That's the whole
> point in having it generic. You should move it there.
>
Thanks, will do.
^ permalink raw reply
* Re: [RFT] lan78xx: FIX use-after-free in lan78xx_write_reg
From: Andrey Konovalov @ 2017-09-27 12:06 UTC (permalink / raw)
To: Arvind Yadav
Cc: David S. Miller, Woojung Huh, Microchip Linux Driver Support,
netdev, Kostya Serebryany, Dmitry Vyukov, Nisar.Sayed, USB list,
LKML, syzkaller
In-Reply-To: <d7710b468c6c1b9cf3b3fe2fc986118a2cd2639a.1506510873.git.arvind.yadav.cs@gmail.com>
On Wed, Sep 27, 2017 at 1:18 PM, Arvind Yadav <arvind.yadav.cs@gmail.com> wrote:
> We are not releasing 'buf' memory on failure or disconnect a device.
>
> Adding 'u8 *buf' as part of 'lan78xx_net' structure to make proper
> handle for 'buf'.
> Now releasing 'buf' memory on failure. It's allocate first in
> lan78xx_probe() and it should be freed last in lan78xx_disconnect().
>
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
Hi Arvind,
I've tried your patch and still see a crash.
Thanks!
lan78xx 1-1:90.0 (unnamed net_device) (uninitialized): can't register MDIO bus
lan78xx: probe of 1-1:90.0 failed with error -5
usb 1-1: USB disconnect, device number 2
==================================================================
BUG: KASAN: use-after-free in lan78xx_write_reg.isra.21+0x1a8/0x1b0
Read of size 8 at addr ffff880063dfac40 by task kworker/1:1/1152
CPU: 1 PID: 1152 Comm: kworker/1:1 Not tainted
4.14.0-rc2-42660-g24b7bd59eec0-dirty #274
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Workqueue: events lan78xx_deferred_multicast_write
Call Trace:
__dump_stack lib/dump_stack.c:16
dump_stack+0x292/0x395 lib/dump_stack.c:52
print_address_description+0x78/0x280 mm/kasan/report.c:252
kasan_report_error mm/kasan/report.c:351
kasan_report+0x23d/0x350 mm/kasan/report.c:409
__asan_report_load8_noabort+0x19/0x20 mm/kasan/report.c:430
lan78xx_write_reg.isra.21+0x1a8/0x1b0 drivers/net/usb/lan78xx.c:459
lan78xx_deferred_multicast_write+0x10d/0x1a0 drivers/net/usb/lan78xx.c:1043
process_one_work+0xc7f/0x1db0 kernel/workqueue.c:2119
worker_thread+0x221/0x1850 kernel/workqueue.c:2253
kthread+0x3a1/0x470 kernel/kthread.c:231
ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431
Allocated by task 1848:
save_stack_trace+0x1b/0x20 arch/x86/kernel/stacktrace.c:59
save_stack+0x43/0xd0 mm/kasan/kasan.c:447
set_track mm/kasan/kasan.c:459
kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:551
__kmalloc_node+0x192/0x480 mm/slub.c:3827
kmalloc_node ./include/linux/slab.h:535
kvmalloc_node+0x69/0xd0 mm/util.c:397
kvmalloc ./include/linux/mm.h:529
kvzalloc ./include/linux/mm.h:537
alloc_netdev_mqs+0x173/0xea0 net/core/dev.c:8023
alloc_etherdev_mqs+0x38/0x40 net/ethernet/eth.c:391
lan78xx_probe+0x13a/0x3020 drivers/net/usb/lan78xx.c:3545
usb_probe_interface+0x35d/0x8e0 drivers/usb/core/driver.c:361
really_probe drivers/base/dd.c:413
driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
__device_attach_driver+0x230/0x290 drivers/base/dd.c:653
bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463
__device_attach+0x26e/0x3d0 drivers/base/dd.c:710
device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523
device_add+0xd0b/0x1660 drivers/base/core.c:1835
usb_set_configuration+0x104e/0x1870 drivers/usb/core/message.c:1932
generic_probe+0x73/0xe0 drivers/usb/core/generic.c:174
usb_probe_device+0xaf/0xe0 drivers/usb/core/driver.c:266
really_probe drivers/base/dd.c:413
driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
__device_attach_driver+0x230/0x290 drivers/base/dd.c:653
bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463
__device_attach+0x26e/0x3d0 drivers/base/dd.c:710
device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523
device_add+0xd0b/0x1660 drivers/base/core.c:1835
usb_new_device+0x7b8/0x1020 drivers/usb/core/hub.c:2457
hub_port_connect drivers/usb/core/hub.c:4903
hub_port_connect_change drivers/usb/core/hub.c:5009
port_event drivers/usb/core/hub.c:5115
hub_event+0x194d/0x3740 drivers/usb/core/hub.c:5195
process_one_work+0xc7f/0x1db0 kernel/workqueue.c:2119
worker_thread+0x221/0x1850 kernel/workqueue.c:2253
kthread+0x3a1/0x470 kernel/kthread.c:231
ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431
Freed by task 1848:
save_stack_trace+0x1b/0x20 arch/x86/kernel/stacktrace.c:59
save_stack+0x43/0xd0 mm/kasan/kasan.c:447
set_track mm/kasan/kasan.c:459
kasan_slab_free+0x72/0xc0 mm/kasan/kasan.c:524
slab_free_hook mm/slub.c:1390
slab_free_freelist_hook mm/slub.c:1412
slab_free mm/slub.c:2988
kfree+0xf6/0x2f0 mm/slub.c:3919
kvfree+0x3b/0x60 mm/util.c:416
netdev_freemem net/core/dev.c:7975
free_netdev+0x304/0x3c0 net/core/dev.c:8137
lan78xx_probe+0x21a4/0x3020 drivers/net/usb/lan78xx.c:3649
usb_probe_interface+0x35d/0x8e0 drivers/usb/core/driver.c:361
really_probe drivers/base/dd.c:413
driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
__device_attach_driver+0x230/0x290 drivers/base/dd.c:653
bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463
__device_attach+0x26e/0x3d0 drivers/base/dd.c:710
device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523
device_add+0xd0b/0x1660 drivers/base/core.c:1835
usb_set_configuration+0x104e/0x1870 drivers/usb/core/message.c:1932
generic_probe+0x73/0xe0 drivers/usb/core/generic.c:174
usb_probe_device+0xaf/0xe0 drivers/usb/core/driver.c:266
really_probe drivers/base/dd.c:413
driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
__device_attach_driver+0x230/0x290 drivers/base/dd.c:653
bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463
__device_attach+0x26e/0x3d0 drivers/base/dd.c:710
device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523
device_add+0xd0b/0x1660 drivers/base/core.c:1835
usb_new_device+0x7b8/0x1020 drivers/usb/core/hub.c:2457
hub_port_connect drivers/usb/core/hub.c:4903
hub_port_connect_change drivers/usb/core/hub.c:5009
port_event drivers/usb/core/hub.c:5115
hub_event+0x194d/0x3740 drivers/usb/core/hub.c:5195
process_one_work+0xc7f/0x1db0 kernel/workqueue.c:2119
worker_thread+0x221/0x1850 kernel/workqueue.c:2253
kthread+0x3a1/0x470 kernel/kthread.c:231
ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431
The buggy address belongs to the object at ffff880063dfa100
which belongs to the cache kmalloc-8192 of size 8192
The buggy address is located 2880 bytes inside of
8192-byte region [ffff880063dfa100, ffff880063dfc100)
The buggy address belongs to the page:
page:ffffea00018f7e00 count:1 mapcount:0 mapping: (null)
index:0x0 compound_mapcount: 0
flags: 0x100000000008100(slab|head)
raw: 0100000000008100 0000000000000000 0000000000000000 0000000100030003
raw: ffffea0001afd800 0000000300000003 ffff88006c402a80 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff880063dfab00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff880063dfab80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>ffff880063dfac00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff880063dfac80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff880063dfad00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================
> ---
> drivers/net/usb/lan78xx.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
> index b99a7fb..e653982 100644
> --- a/drivers/net/usb/lan78xx.c
> +++ b/drivers/net/usb/lan78xx.c
> @@ -402,6 +402,7 @@ struct lan78xx_net {
> struct statstage stats;
>
> struct irq_domain_data domain_data;
> + u8 *buf;
> };
>
> /* define external phy id */
> @@ -3470,6 +3471,9 @@ static void lan78xx_disconnect(struct usb_interface *intf)
>
> usb_scuttle_anchored_urbs(&dev->deferred);
>
> + kfree(dev->buf);
> + dev->buf = NULL;
> +
> lan78xx_unbind(dev, intf);
>
> usb_kill_urb(dev->urb_intr);
> @@ -3520,7 +3524,6 @@ static int lan78xx_probe(struct usb_interface *intf,
> int ret;
> unsigned maxp;
> unsigned period;
> - u8 *buf = NULL;
>
> udev = interface_to_usbdev(intf);
> udev = usb_get_dev(udev);
> @@ -3588,16 +3591,15 @@ static int lan78xx_probe(struct usb_interface *intf,
> period = dev->ep_intr->desc.bInterval;
>
> maxp = usb_maxpacket(dev->udev, dev->pipe_intr, 0);
> - buf = kmalloc(maxp, GFP_KERNEL);
> - if (buf) {
> + dev->buf = kmalloc(maxp, GFP_KERNEL);
> + if (dev->buf) {
> dev->urb_intr = usb_alloc_urb(0, GFP_KERNEL);
> if (!dev->urb_intr) {
> ret = -ENOMEM;
> - kfree(buf);
> goto out3;
> } else {
> usb_fill_int_urb(dev->urb_intr, dev->udev,
> - dev->pipe_intr, buf, maxp,
> + dev->pipe_intr, dev->buf, maxp,
> intr_complete, dev, period);
> }
> }
> @@ -3626,6 +3628,8 @@ static int lan78xx_probe(struct usb_interface *intf,
> return 0;
>
> out3:
> + kfree(dev->buf);
> + dev->buf = NULL;
> lan78xx_unbind(dev, intf);
> out2:
> free_netdev(netdev);
> --
> 1.9.1
>
^ permalink raw reply
* Re: [RFT] lan78xx: FIX use-after-free in lan78xx_write_reg
From: Andrey Konovalov @ 2017-09-27 12:12 UTC (permalink / raw)
To: Arvind Yadav
Cc: David S. Miller, Woojung Huh, Microchip Linux Driver Support,
netdev, Kostya Serebryany, Dmitry Vyukov, Nisar.Sayed, USB list,
LKML, syzkaller
In-Reply-To: <CAAeHK+y1pc98H4xcVC79yBdfAD3212VxUYzjvFGx9bumwvH_KQ@mail.gmail.com>
On Wed, Sep 27, 2017 at 2:06 PM, Andrey Konovalov <andreyknvl@google.com> wrote:
> On Wed, Sep 27, 2017 at 1:18 PM, Arvind Yadav <arvind.yadav.cs@gmail.com> wrote:
>> We are not releasing 'buf' memory on failure or disconnect a device.
>>
>> Adding 'u8 *buf' as part of 'lan78xx_net' structure to make proper
>> handle for 'buf'.
>> Now releasing 'buf' memory on failure. It's allocate first in
>> lan78xx_probe() and it should be freed last in lan78xx_disconnect().
>>
>> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
>
> Hi Arvind,
>
> I've tried your patch and still see a crash.
>
> Thanks!
>
> lan78xx 1-1:90.0 (unnamed net_device) (uninitialized): can't register MDIO bus
> lan78xx: probe of 1-1:90.0 failed with error -5
> usb 1-1: USB disconnect, device number 2
> ==================================================================
> BUG: KASAN: use-after-free in lan78xx_write_reg.isra.21+0x1a8/0x1b0
> Read of size 8 at addr ffff880063dfac40 by task kworker/1:1/1152
It seems that the bug is caused by the fact that the
lan78xx_deferred_multicast_write() work is not shut down on
lan78xx_probe() failure.
>
> CPU: 1 PID: 1152 Comm: kworker/1:1 Not tainted
> 4.14.0-rc2-42660-g24b7bd59eec0-dirty #274
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> Workqueue: events lan78xx_deferred_multicast_write
> Call Trace:
> __dump_stack lib/dump_stack.c:16
> dump_stack+0x292/0x395 lib/dump_stack.c:52
> print_address_description+0x78/0x280 mm/kasan/report.c:252
> kasan_report_error mm/kasan/report.c:351
> kasan_report+0x23d/0x350 mm/kasan/report.c:409
> __asan_report_load8_noabort+0x19/0x20 mm/kasan/report.c:430
> lan78xx_write_reg.isra.21+0x1a8/0x1b0 drivers/net/usb/lan78xx.c:459
> lan78xx_deferred_multicast_write+0x10d/0x1a0 drivers/net/usb/lan78xx.c:1043
> process_one_work+0xc7f/0x1db0 kernel/workqueue.c:2119
> worker_thread+0x221/0x1850 kernel/workqueue.c:2253
> kthread+0x3a1/0x470 kernel/kthread.c:231
> ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431
>
> Allocated by task 1848:
> save_stack_trace+0x1b/0x20 arch/x86/kernel/stacktrace.c:59
> save_stack+0x43/0xd0 mm/kasan/kasan.c:447
> set_track mm/kasan/kasan.c:459
> kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:551
> __kmalloc_node+0x192/0x480 mm/slub.c:3827
> kmalloc_node ./include/linux/slab.h:535
> kvmalloc_node+0x69/0xd0 mm/util.c:397
> kvmalloc ./include/linux/mm.h:529
> kvzalloc ./include/linux/mm.h:537
> alloc_netdev_mqs+0x173/0xea0 net/core/dev.c:8023
> alloc_etherdev_mqs+0x38/0x40 net/ethernet/eth.c:391
> lan78xx_probe+0x13a/0x3020 drivers/net/usb/lan78xx.c:3545
> usb_probe_interface+0x35d/0x8e0 drivers/usb/core/driver.c:361
> really_probe drivers/base/dd.c:413
> driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
> __device_attach_driver+0x230/0x290 drivers/base/dd.c:653
> bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463
> __device_attach+0x26e/0x3d0 drivers/base/dd.c:710
> device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
> bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523
> device_add+0xd0b/0x1660 drivers/base/core.c:1835
> usb_set_configuration+0x104e/0x1870 drivers/usb/core/message.c:1932
> generic_probe+0x73/0xe0 drivers/usb/core/generic.c:174
> usb_probe_device+0xaf/0xe0 drivers/usb/core/driver.c:266
> really_probe drivers/base/dd.c:413
> driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
> __device_attach_driver+0x230/0x290 drivers/base/dd.c:653
> bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463
> __device_attach+0x26e/0x3d0 drivers/base/dd.c:710
> device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
> bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523
> device_add+0xd0b/0x1660 drivers/base/core.c:1835
> usb_new_device+0x7b8/0x1020 drivers/usb/core/hub.c:2457
> hub_port_connect drivers/usb/core/hub.c:4903
> hub_port_connect_change drivers/usb/core/hub.c:5009
> port_event drivers/usb/core/hub.c:5115
> hub_event+0x194d/0x3740 drivers/usb/core/hub.c:5195
> process_one_work+0xc7f/0x1db0 kernel/workqueue.c:2119
> worker_thread+0x221/0x1850 kernel/workqueue.c:2253
> kthread+0x3a1/0x470 kernel/kthread.c:231
> ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431
>
> Freed by task 1848:
> save_stack_trace+0x1b/0x20 arch/x86/kernel/stacktrace.c:59
> save_stack+0x43/0xd0 mm/kasan/kasan.c:447
> set_track mm/kasan/kasan.c:459
> kasan_slab_free+0x72/0xc0 mm/kasan/kasan.c:524
> slab_free_hook mm/slub.c:1390
> slab_free_freelist_hook mm/slub.c:1412
> slab_free mm/slub.c:2988
> kfree+0xf6/0x2f0 mm/slub.c:3919
> kvfree+0x3b/0x60 mm/util.c:416
> netdev_freemem net/core/dev.c:7975
> free_netdev+0x304/0x3c0 net/core/dev.c:8137
> lan78xx_probe+0x21a4/0x3020 drivers/net/usb/lan78xx.c:3649
> usb_probe_interface+0x35d/0x8e0 drivers/usb/core/driver.c:361
> really_probe drivers/base/dd.c:413
> driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
> __device_attach_driver+0x230/0x290 drivers/base/dd.c:653
> bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463
> __device_attach+0x26e/0x3d0 drivers/base/dd.c:710
> device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
> bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523
> device_add+0xd0b/0x1660 drivers/base/core.c:1835
> usb_set_configuration+0x104e/0x1870 drivers/usb/core/message.c:1932
> generic_probe+0x73/0xe0 drivers/usb/core/generic.c:174
> usb_probe_device+0xaf/0xe0 drivers/usb/core/driver.c:266
> really_probe drivers/base/dd.c:413
> driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
> __device_attach_driver+0x230/0x290 drivers/base/dd.c:653
> bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463
> __device_attach+0x26e/0x3d0 drivers/base/dd.c:710
> device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
> bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523
> device_add+0xd0b/0x1660 drivers/base/core.c:1835
> usb_new_device+0x7b8/0x1020 drivers/usb/core/hub.c:2457
> hub_port_connect drivers/usb/core/hub.c:4903
> hub_port_connect_change drivers/usb/core/hub.c:5009
> port_event drivers/usb/core/hub.c:5115
> hub_event+0x194d/0x3740 drivers/usb/core/hub.c:5195
> process_one_work+0xc7f/0x1db0 kernel/workqueue.c:2119
> worker_thread+0x221/0x1850 kernel/workqueue.c:2253
> kthread+0x3a1/0x470 kernel/kthread.c:231
> ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431
>
> The buggy address belongs to the object at ffff880063dfa100
> which belongs to the cache kmalloc-8192 of size 8192
> The buggy address is located 2880 bytes inside of
> 8192-byte region [ffff880063dfa100, ffff880063dfc100)
> The buggy address belongs to the page:
> page:ffffea00018f7e00 count:1 mapcount:0 mapping: (null)
> index:0x0 compound_mapcount: 0
> flags: 0x100000000008100(slab|head)
> raw: 0100000000008100 0000000000000000 0000000000000000 0000000100030003
> raw: ffffea0001afd800 0000000300000003 ffff88006c402a80 0000000000000000
> page dumped because: kasan: bad access detected
>
> Memory state around the buggy address:
> ffff880063dfab00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ffff880063dfab80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>>ffff880063dfac00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ^
> ffff880063dfac80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ffff880063dfad00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ==================================================================
>
>> ---
>> drivers/net/usb/lan78xx.c | 14 +++++++++-----
>> 1 file changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
>> index b99a7fb..e653982 100644
>> --- a/drivers/net/usb/lan78xx.c
>> +++ b/drivers/net/usb/lan78xx.c
>> @@ -402,6 +402,7 @@ struct lan78xx_net {
>> struct statstage stats;
>>
>> struct irq_domain_data domain_data;
>> + u8 *buf;
>> };
>>
>> /* define external phy id */
>> @@ -3470,6 +3471,9 @@ static void lan78xx_disconnect(struct usb_interface *intf)
>>
>> usb_scuttle_anchored_urbs(&dev->deferred);
>>
>> + kfree(dev->buf);
>> + dev->buf = NULL;
>> +
>> lan78xx_unbind(dev, intf);
>>
>> usb_kill_urb(dev->urb_intr);
>> @@ -3520,7 +3524,6 @@ static int lan78xx_probe(struct usb_interface *intf,
>> int ret;
>> unsigned maxp;
>> unsigned period;
>> - u8 *buf = NULL;
>>
>> udev = interface_to_usbdev(intf);
>> udev = usb_get_dev(udev);
>> @@ -3588,16 +3591,15 @@ static int lan78xx_probe(struct usb_interface *intf,
>> period = dev->ep_intr->desc.bInterval;
>>
>> maxp = usb_maxpacket(dev->udev, dev->pipe_intr, 0);
>> - buf = kmalloc(maxp, GFP_KERNEL);
>> - if (buf) {
>> + dev->buf = kmalloc(maxp, GFP_KERNEL);
>> + if (dev->buf) {
>> dev->urb_intr = usb_alloc_urb(0, GFP_KERNEL);
>> if (!dev->urb_intr) {
>> ret = -ENOMEM;
>> - kfree(buf);
>> goto out3;
>> } else {
>> usb_fill_int_urb(dev->urb_intr, dev->udev,
>> - dev->pipe_intr, buf, maxp,
>> + dev->pipe_intr, dev->buf, maxp,
>> intr_complete, dev, period);
>> }
>> }
>> @@ -3626,6 +3628,8 @@ static int lan78xx_probe(struct usb_interface *intf,
>> return 0;
>>
>> out3:
>> + kfree(dev->buf);
>> + dev->buf = NULL;
>> lan78xx_unbind(dev, intf);
>> out2:
>> free_netdev(netdev);
>> --
>> 1.9.1
>>
^ permalink raw reply
* [bug][tipc] Too short struct tipc_subscr passed to tipc_subscrb_rcv_cb()
From: Alexander Potapenko @ 2017-09-27 12:15 UTC (permalink / raw)
To: David Miller, ying.xue, jon.maloy; +Cc: Networking, syzkaller
[-- Attachment #1: Type: text/plain, Size: 3208 bytes --]
Hi all,
I've spotted the following bug in TIPC server code while fuzzing it
with KMSAN and syzkaller:
==================================================================
BUG: KMSAN: use of uninitialized memory in tipc_subscrb_rcv_cb+0x14a/0xc20
CPU: 2 PID: 29 Comm: kworker/u8:1 Not tainted 4.13.0+ #3150
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Workqueue: tipc_rcv tipc_recv_work
Call Trace:
__dump_stack lib/dump_stack.c:16
dump_stack+0x172/0x1c0 lib/dump_stack.c:52
kmsan_report+0x145/0x3d0 mm/kmsan/kmsan.c:943
__msan_warning_32+0x66/0xb0 mm/kmsan/kmsan_instr.c:477
htohl net/tipc/subscr.c:66
tipc_subscrb_rcv_cb+0x14a/0xc20 net/tipc/subscr.c:333
tipc_receive_from_sock+0x369/0x540 net/tipc/server.c:273
tipc_recv_work+0xbe/0x1c0 net/tipc/server.c:546
process_one_work+0xf9e/0x1ad0 kernel/workqueue.c:2097
worker_thread+0xefd/0x2090 kernel/workqueue.c:2231
kthread+0x499/0x620 kernel/kthread.c:232
ret_from_fork+0x22/0x30 arch/x86/entry/entry_64.S:425
origin:
save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:59
kmsan_save_stack_with_flags mm/kmsan/kmsan.c:302
kmsan_internal_poison_shadow+0xb3/0x1b0 mm/kmsan/kmsan.c:198
kmsan_kmalloc+0x80/0xe0 mm/kmsan/kmsan.c:337
kmem_cache_alloc+0x1e2/0x220 mm/slub.c:2756
tipc_receive_from_sock+0xff/0x540 net/tipc/server.c:257
tipc_recv_work+0xbe/0x1c0 net/tipc/server.c:546
process_one_work+0xf9e/0x1ad0 kernel/workqueue.c:2097
worker_thread+0xefd/0x2090 kernel/workqueue.c:2231
kthread+0x499/0x620 kernel/kthread.c:232
ret_from_fork+0x22/0x30 arch/x86/entry/entry_64.S:425
==================================================================
(see the reproducer attached).
Here the user sends 16 bytes via sendmsg(), which the kernel attempts
to treat as a struct tipc_subscr. Because the actual size of a struct
is bigger, we end up touching uninitialized fields.
What is the best way to fix this?
I think it's too late to check for |len| in tipc_subscrb_rcv_cb()
(http://elixir.free-electrons.com/linux/latest/source/net/tipc/subscr.c#L320),
because we'll probably need to call tipc_subscrp_cancel(s, subscriber)
in order to cancel the subscription, which is a bad idea since |s| is
invalid.
We could check that ret >= sizeof(struct tipc_subscr) in
tipc_receive_from_sock()
(http://elixir.free-electrons.com/linux/v4.8/source/net/tipc/server.c#L273),
but this function is agnostic to the type of the data received by the
callback, so we'd better not hardcode the size there.
Shall we add an integer field (or yet another callback) to tipc_server, maybe?
On a related note, something is wrong with the
tipc_receive_from_sock()'s return value.
First, the user appears to receive a positive value (16 in our case)
regardless of what tipc_receive_from_sock() returns.
Second, the branch at
http://elixir.free-electrons.com/linux/v4.8/source/net/tipc/server.c#L288
is never taken.
WBR,
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
[-- Attachment #2: tipc_subscrb_rcv_cb-kmsan.c --]
[-- Type: text/x-csrc, Size: 733 bytes --]
// autogenerated by syzkaller (http://github.com/google/syzkaller)
#include <arpa/inet.h>
#include <linux/tipc.h>
#include <string.h>
int main()
{
int sock = socket(AF_TIPC, SOCK_SEQPACKET, 0);
struct sockaddr_tipc addr;
memset(&addr, 0, sizeof(addr));
addr.family = AF_TIPC;
addr.addrtype = TIPC_ADDR_NAME;
addr.addr.name.name.type = 1;
addr.addr.name.name.instance = 1;
struct tipc_subscr subscr;
memset(&subscr, 0, sizeof(subscr));
struct iovec iov;
iov.iov_base = &subscr;
iov.iov_len = sizeof(subscr) - 12;
struct msghdr hdr;
memset(&hdr, 0, sizeof(hdr));
hdr.msg_name = &addr;
hdr.msg_namelen = sizeof(addr);
hdr.msg_iov = &iov;
hdr.msg_iovlen = 1;
sendmsg(sock, &hdr, 0);
return 0;
}
^ permalink raw reply
* [PATCH v2] tun: bail out from tun_get_user() if the skb is empty
From: Alexander Potapenko @ 2017-09-27 12:16 UTC (permalink / raw)
To: davem, edumazet; +Cc: dvyukov, syzkaller, netdev, linux-kernel
KMSAN (https://github.com/google/kmsan) reported accessing uninitialized
skb->data[0] in the case the skb is empty (i.e. skb->len is 0):
================================================
BUG: KMSAN: use of uninitialized memory in tun_get_user+0x19ba/0x3770
CPU: 0 PID: 3051 Comm: probe Not tainted 4.13.0+ #3140
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Call Trace:
...
__msan_warning_32+0x66/0xb0 mm/kmsan/kmsan_instr.c:477
tun_get_user+0x19ba/0x3770 drivers/net/tun.c:1301
tun_chr_write_iter+0x19f/0x300 drivers/net/tun.c:1365
call_write_iter ./include/linux/fs.h:1743
new_sync_write fs/read_write.c:457
__vfs_write+0x6c3/0x7f0 fs/read_write.c:470
vfs_write+0x3e4/0x770 fs/read_write.c:518
SYSC_write+0x12f/0x2b0 fs/read_write.c:565
SyS_write+0x55/0x80 fs/read_write.c:557
do_syscall_64+0x242/0x330 arch/x86/entry/common.c:284
entry_SYSCALL64_slow_path+0x25/0x25 arch/x86/entry/entry_64.S:245
...
origin:
...
kmsan_poison_shadow+0x6e/0xc0 mm/kmsan/kmsan.c:211
slab_alloc_node mm/slub.c:2732
__kmalloc_node_track_caller+0x351/0x370 mm/slub.c:4351
__kmalloc_reserve net/core/skbuff.c:138
__alloc_skb+0x26a/0x810 net/core/skbuff.c:231
alloc_skb ./include/linux/skbuff.h:903
alloc_skb_with_frags+0x1d7/0xc80 net/core/skbuff.c:4756
sock_alloc_send_pskb+0xabf/0xfe0 net/core/sock.c:2037
tun_alloc_skb drivers/net/tun.c:1144
tun_get_user+0x9a8/0x3770 drivers/net/tun.c:1274
tun_chr_write_iter+0x19f/0x300 drivers/net/tun.c:1365
call_write_iter ./include/linux/fs.h:1743
new_sync_write fs/read_write.c:457
__vfs_write+0x6c3/0x7f0 fs/read_write.c:470
vfs_write+0x3e4/0x770 fs/read_write.c:518
SYSC_write+0x12f/0x2b0 fs/read_write.c:565
SyS_write+0x55/0x80 fs/read_write.c:557
do_syscall_64+0x242/0x330 arch/x86/entry/common.c:284
return_from_SYSCALL_64+0x0/0x6a arch/x86/entry/entry_64.S:245
================================================
Make sure tun_get_user() doesn't touch skb->data[0] unless there is
actual data.
C reproducer below:
==========================
// autogenerated by syzkaller (http://github.com/google/syzkaller)
#define _GNU_SOURCE
#include <fcntl.h>
#include <linux/if_tun.h>
#include <netinet/ip.h>
#include <net/if.h>
#include <string.h>
#include <sys/ioctl.h>
int main()
{
int sock = socket(PF_INET, SOCK_STREAM, IPPROTO_IP);
int tun_fd = open("/dev/net/tun", O_RDWR);
struct ifreq req;
memset(&req, 0, sizeof(struct ifreq));
strcpy((char*)&req.ifr_name, "gre0");
req.ifr_flags = IFF_UP | IFF_MULTICAST;
ioctl(tun_fd, TUNSETIFF, &req);
ioctl(sock, SIOCSIFFLAGS, "gre0");
write(tun_fd, "hi", 0);
return 0;
}
==========================
Signed-off-by: Alexander Potapenko <glider@google.com>
---
v2: free the skb
---
drivers/net/tun.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 3c9985f29950..0d60fd4ada9e 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1496,6 +1496,11 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
switch (tun->flags & TUN_TYPE_MASK) {
case IFF_TUN:
if (tun->flags & IFF_NO_PI) {
+ if (!skb->len) {
+ this_cpu_inc(tun->pcpu_stats->rx_dropped);
+ kfree_skb(skb);
+ return -EINVAL;
+ }
switch (skb->data[0] & 0xf0) {
case 0x40:
pi.proto = htons(ETH_P_IP);
--
2.14.1.821.g8fa685d3b7-goog
^ permalink raw reply related
* [PATCH] xfrm: don't call xfrm_policy_cache_flush under xfrm_state_lock
From: Artem Savkov @ 2017-09-27 12:25 UTC (permalink / raw)
To: Florian Westphal
Cc: Steffen Klassert, Herbert Xu, David S . Miller, netdev,
linux-kernel, Artem Savkov
I might be wrong but it doesn't look like xfrm_state_lock is required
for xfrm_policy_cache_flush and calling it under this lock triggers both
"sleeping function called from invalid context" and "possible circular
locking dependency detected" warnings on flush.
Fixes: ec30d78c14a8 xfrm: add xdst pcpu cache
Signed-off-by: Artem Savkov <asavkov@redhat.com>
---
net/xfrm/xfrm_state.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 0dab1cd79ce4..12213477cd3a 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -732,12 +732,12 @@ int xfrm_state_flush(struct net *net, u8 proto, bool task_valid)
}
}
}
+out:
+ spin_unlock_bh(&net->xfrm.xfrm_state_lock);
if (cnt) {
err = 0;
xfrm_policy_cache_flush();
}
-out:
- spin_unlock_bh(&net->xfrm.xfrm_state_lock);
return err;
}
EXPORT_SYMBOL(xfrm_state_flush);
--
2.13.5
^ permalink raw reply related
* Re: [PATCH] xfrm: don't call xfrm_policy_cache_flush under xfrm_state_lock
From: Florian Westphal @ 2017-09-27 12:31 UTC (permalink / raw)
To: Artem Savkov
Cc: Florian Westphal, Steffen Klassert, Herbert Xu, David S . Miller,
netdev, linux-kernel
In-Reply-To: <20170927122537.14235-1-asavkov@redhat.com>
Artem Savkov <asavkov@redhat.com> wrote:
> I might be wrong but it doesn't look like xfrm_state_lock is required
> for xfrm_policy_cache_flush and calling it under this lock triggers both
> "sleeping function called from invalid context" and "possible circular
> locking dependency detected" warnings on flush.
>
> Fixes: ec30d78c14a8 xfrm: add xdst pcpu cache
> Signed-off-by: Artem Savkov <asavkov@redhat.com>
You're right, its not needed (and wrong).
Acked-by: Florian Westphal <fw@strlen.de>
^ permalink raw reply
* Re: tc H/W offload issue with vxlan tunnels [was: nfp: flower vxlan tunnel offload]
From: Paolo Abeni @ 2017-09-27 12:31 UTC (permalink / raw)
To: Jiri Pirko
Cc: Or Gerlitz, Jiri Benc, Simon Horman, David Miller, Jakub Kicinski,
Linux Netdev List, oss-drivers, John Hurley, Paul Blakey,
Jiri Pirko, Roi Dayan
In-Reply-To: <20170927111150.GE1944@nanopsycho.orion>
On Wed, 2017-09-27 at 13:11 +0200, Jiri Pirko wrote:
> Wed, Sep 27, 2017 at 11:46:58AM CEST, pabeni@redhat.com wrote:
> > On Wed, 2017-09-27 at 11:17 +0200, Jiri Pirko wrote:
> > > Wed, Sep 27, 2017 at 10:29:35AM CEST, pabeni@redhat.com wrote:
> > > > So it looks like the H/W offload hook will still be called with the
> > > > same arguments in both case, and 'bad' rule will still be pushed to the
> > > > H/W as the driver itself has no way to distinct between the two
> > > > scenarios.
> > >
> > > Why "bad"?
> >
> > Such rule is coped differently by the SW and the HW data path.
> >
> > a rule like:
> >
> > tc filter add dev eth0 protocol ip parent ffff: flower \
> > enc_key_id 102 enc_dst_port 4789 src_ip 3.4.5.6 skip_hw \
> > action action mirred redirect eth0_vf_1
> >
> > will match 0 packets, while:
> >
> > tc filter add dev eth0 protocol ip parent ffff: flower \
> > enc_key_id 102 enc_dst_port 4789 src_ip 3.4.5.6 skip_sw \
> > action action mirred redirect eth0_vf_1
> >
> > [just flipped 'skip_sw' and 'skip_hw' ]
> > will match the vxlan-tunneled packets. I understand that one of the
> > design goal for the h/w offload path is being consistent with the sw
> > one, but that does not hold in the above scenario.
>
> Sure, the consistency is important. Howcome "skip_hw" won't match and
> "skip_sw" will match? What's different?
For the SW datapath, we need a metadata based/lwt tunnel to collect the
tunnel information. eth0 is not a such device and does not provide the
metadata. Any match on tunnel based field will fail - correctly.
When the HW datapath is used, the underlaying NIC is programmed exactly
as done when we replace eth0 with vxlan0. The programmed flow matches
vxlan encapsulated traffic.
Cheers,
Paolo
^ permalink raw reply
* [PATCH] netlink: do not proceed if dump's start() errs
From: Jason A. Donenfeld @ 2017-09-27 12:39 UTC (permalink / raw)
To: davem, johannes.berg, netdev, linux-kernel; +Cc: Jason A. Donenfeld, stable
Drivers that use the start method for netlink dumping rely on dumpit not
being called if start fails. For example, ila_xlat.c allocates memory
and assigns it to cb->args[0] in its start() function. It might fail to
do that and return -ENOMEM instead. However, even when returning an
error, dumpit will be called, which, in the example above, quickly
dereferences the memory in cb->args[0], which will OOPS the kernel. This
is but one example of how this goes wrong.
Since start() has always been a function with an int return type, it
therefore makes sense to use it properly, rather than ignoring it. This
patch thus returns early and does not call dumpit() when start() fails.
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: stable@vger.kernel.org
---
net/netlink/af_netlink.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 327807731b44..be179876227d 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -2270,8 +2270,11 @@ int __netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
mutex_unlock(nlk->cb_mutex);
- if (cb->start)
- cb->start(cb);
+ if (cb->start) {
+ ret = cb->start(cb);
+ if (ret)
+ return ret;
+ }
ret = netlink_dump(sk);
sock_put(sk);
--
2.14.1
^ 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