* Re: [PATCH v2 0/4] can: mcp251x: Make use of device properties
From: Andy Shevchenko @ 2019-09-03 13:02 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: Wolfgang Grandegger, linux-can, David S. Miller, netdev
In-Reply-To: <9ba9d56b-c045-74d3-9693-a9a959ffb675@pengutronix.de>
On Tue, Sep 03, 2019 at 02:50:05PM +0200, Marc Kleine-Budde wrote:
> On 9/3/19 2:42 PM, Andy Shevchenko wrote:
> > The purpose of this series is to simplify driver by switching to use device
> > properties. In particular it allows to drop legacy platform data.
> >
> > Patch 1 switches driver to use devm_clk_get_optional() API.
> >
> > Patch 2 unifies getting the driver data independently of the table which
> > provides it.
> >
> > Patch 3 drops extra check for regulator presence by switch to use an already
> > present wrapper.
> >
> > And patch 4 gets rid of legacy platform data.
> >
> > Changelog v2:
> > - add patch 4 to get rid of legacy platform data
>
> Sorry for not telling, your v1 series has already been applied and it's
> included in the latest pull request. Can you please rebase your patches
> on top of:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git/tag/?h=linux-can-next-for-5.4-20190903
Thank you!
Basically first 3 patches didn't change. Consider patch 4 only. Should I resend
it separately as v3?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH net] ipmr: remove cache_resolve_queue_len
From: Hangbin Liu @ 2019-09-03 12:55 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: netdev, Phil Karn, Sukumar Gopalakrishnan, David S . Miller,
Alexey Kuznetsov, Hideaki YOSHIFUJI
In-Reply-To: <e0261582-78ce-038f-ed4c-c2694fb70250@cumulusnetworks.com>
Hi Nikolay,
Thanks for the feedback, see comments below.
On Tue, Sep 03, 2019 at 12:15:34PM +0300, Nikolay Aleksandrov wrote:
> On 03/09/2019 11:43, Hangbin Liu wrote:
> > This is a re-post of previous patch wrote by David Miller[1].
> >
> > Phil Karn reported[2] that on busy networks with lots of unresolved
> > multicast routing entries, the creation of new multicast group routes
> > can be extremely slow and unreliable.
> >
> > The reason is we hard-coded multicast route entries with unresolved source
> > addresses(cache_resolve_queue_len) to 10. If some multicast route never
> > resolves and the unresolved source addresses increased, there will
> > be no ability to create new multicast route cache.
> >
> > To resolve this issue, we need either add a sysctl entry to make the
> > cache_resolve_queue_len configurable, or just remove cache_resolve_queue_len
> > directly, as we already have the socket receive queue limits of mrouted
> > socket, pointed by David.
> >
> > From my side, I'd perfer to remove the cache_resolve_queue_len instead
> > of creating two more(IPv4 and IPv6 version) sysctl entry.
> >
> > [1] https://lkml.org/lkml/2018/7/22/11
> > [2] https://lkml.org/lkml/2018/7/21/343
> >
> > Reported-by: Phil Karn <karn@ka9q.net>
> > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> > ---
> > include/linux/mroute_base.h | 2 --
> > net/ipv4/ipmr.c | 27 ++++++++++++++++++---------
> > net/ipv6/ip6mr.c | 10 +++-------
> > 3 files changed, 21 insertions(+), 18 deletions(-)
> >
>
> Hi,
> IMO this is definitely net-next material. A few more comments below.
I thoug this is a bug fix. But it looks more suitable to net-next as you said.
>
> Note that hosts will automatically have this limit lifted to about 270
> entries with current defaults, some might be surprised if they have
> higher limits set and could be left with queues allowing for thousands
> of entries in a linked list.
I think this is just a cache list and should be expired soon. The cache
creation would also failed if there is no buffer.
But if you like, I can write a patch with sysctl parameter.
>
> > +static int queue_count(struct mr_table *mrt)
> > +{
> > + struct list_head *pos;
> > + int count = 0;
> > +
> > + list_for_each(pos, &mrt->mfc_unres_queue)
> > + count++;
> > + return count;
> > +}
>
> I don't think we hold the mfc_unres_lock here while walking
> the unresolved list below in ipmr_fill_table().
ah, yes, I will fix this.
Thanks
Hangbin
^ permalink raw reply
* Re: [PATCH v2 0/4] can: mcp251x: Make use of device properties
From: Marc Kleine-Budde @ 2019-09-03 12:50 UTC (permalink / raw)
To: Andy Shevchenko, Wolfgang Grandegger, linux-can, David S. Miller,
netdev
In-Reply-To: <20190903124259.60920-1-andriy.shevchenko@linux.intel.com>
[-- Attachment #1.1: Type: text/plain, Size: 1152 bytes --]
On 9/3/19 2:42 PM, Andy Shevchenko wrote:
> The purpose of this series is to simplify driver by switching to use device
> properties. In particular it allows to drop legacy platform data.
>
> Patch 1 switches driver to use devm_clk_get_optional() API.
>
> Patch 2 unifies getting the driver data independently of the table which
> provides it.
>
> Patch 3 drops extra check for regulator presence by switch to use an already
> present wrapper.
>
> And patch 4 gets rid of legacy platform data.
>
> Changelog v2:
> - add patch 4 to get rid of legacy platform data
Sorry for not telling, your v1 series has already been applied and it's
included in the latest pull request. Can you please rebase your patches
on top of:
https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git/tag/?h=linux-can-next-for-5.4-20190903
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH v2 1/4] can: mcp251x: Use devm_clk_get_optional() to get the input clock
From: Andy Shevchenko @ 2019-09-03 12:42 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde, linux-can,
David S. Miller, netdev
Cc: Andy Shevchenko
In-Reply-To: <20190903124259.60920-1-andriy.shevchenko@linux.intel.com>
Simplify the code which fetches the input clock by using
devm_clk_get_optional(). This comes with a small functional change: previously
all errors were ignored when platform data is present. Now all errors are
treated as errors. If no input clock is present devm_clk_get_optional() will
return NULL instead of an error which matches the behavior of the old code.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/net/can/spi/mcp251x.c | 30 ++++++++++++------------------
1 file changed, 12 insertions(+), 18 deletions(-)
diff --git a/drivers/net/can/spi/mcp251x.c b/drivers/net/can/spi/mcp251x.c
index 58992fd61cb9..e04b578f2b1f 100644
--- a/drivers/net/can/spi/mcp251x.c
+++ b/drivers/net/can/spi/mcp251x.c
@@ -1014,15 +1014,13 @@ static int mcp251x_can_probe(struct spi_device *spi)
struct clk *clk;
int freq, ret;
- clk = devm_clk_get(&spi->dev, NULL);
- if (IS_ERR(clk)) {
- if (pdata)
- freq = pdata->oscillator_frequency;
- else
- return PTR_ERR(clk);
- } else {
- freq = clk_get_rate(clk);
- }
+ clk = devm_clk_get_optional(&spi->dev, NULL);
+ if (IS_ERR(clk))
+ return PTR_ERR(clk);
+
+ freq = clk_get_rate(clk);
+ if (freq == 0 && pdata)
+ freq = pdata->oscillator_frequency;
/* Sanity check */
if (freq < 1000000 || freq > 25000000)
@@ -1033,11 +1031,9 @@ static int mcp251x_can_probe(struct spi_device *spi)
if (!net)
return -ENOMEM;
- if (!IS_ERR(clk)) {
- ret = clk_prepare_enable(clk);
- if (ret)
- goto out_free;
- }
+ ret = clk_prepare_enable(clk);
+ if (ret)
+ goto out_free;
net->netdev_ops = &mcp251x_netdev_ops;
net->flags |= IFF_ECHO;
@@ -1122,8 +1118,7 @@ static int mcp251x_can_probe(struct spi_device *spi)
mcp251x_power_enable(priv->power, 0);
out_clk:
- if (!IS_ERR(clk))
- clk_disable_unprepare(clk);
+ clk_disable_unprepare(clk);
out_free:
free_candev(net);
@@ -1141,8 +1136,7 @@ static int mcp251x_can_remove(struct spi_device *spi)
mcp251x_power_enable(priv->power, 0);
- if (!IS_ERR(priv->clk))
- clk_disable_unprepare(priv->clk);
+ clk_disable_unprepare(priv->clk);
free_candev(net);
--
2.23.0.rc1
^ permalink raw reply related
* [PATCH v2 3/4] can: mcp251x: Call wrapper instead of regulator_disable()
From: Andy Shevchenko @ 2019-09-03 12:42 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde, linux-can,
David S. Miller, netdev
Cc: Andy Shevchenko
In-Reply-To: <20190903124259.60920-1-andriy.shevchenko@linux.intel.com>
There is no need to check for regulator presence in the ->suspend()
since a wrapper does it for us. Due to this we may unconditionally set
AFTER_SUSPEND_POWER flag.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/net/can/spi/mcp251x.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/can/spi/mcp251x.c b/drivers/net/can/spi/mcp251x.c
index 0b7e743ca0a0..6ee0ea51399a 100644
--- a/drivers/net/can/spi/mcp251x.c
+++ b/drivers/net/can/spi/mcp251x.c
@@ -1162,10 +1162,8 @@ static int __maybe_unused mcp251x_can_suspend(struct device *dev)
priv->after_suspend = AFTER_SUSPEND_DOWN;
}
- if (!IS_ERR_OR_NULL(priv->power)) {
- regulator_disable(priv->power);
- priv->after_suspend |= AFTER_SUSPEND_POWER;
- }
+ mcp251x_power_enable(priv->power, 0);
+ priv->after_suspend |= AFTER_SUSPEND_POWER;
return 0;
}
--
2.23.0.rc1
^ permalink raw reply related
* [PATCH v2 4/4] can: mcp251x: Get rid of legacy platform data
From: Andy Shevchenko @ 2019-09-03 12:42 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde, linux-can,
David S. Miller, netdev
Cc: Andy Shevchenko, Daniel Mack, Haojian Zhuang, Robert Jarzmik,
Russell King
In-Reply-To: <20190903124259.60920-1-andriy.shevchenko@linux.intel.com>
Instead of using legacy platform data, switch to use device properties.
For clock frequency we are using well established clock-frequency property.
Users, two for now, are also converted here.
Cc: Daniel Mack <daniel@zonque.org>
Cc: Haojian Zhuang <haojian.zhuang@gmail.com>
Cc: Robert Jarzmik <robert.jarzmik@free.fr>
Cc: Russell King <linux@armlinux.org.uk>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
arch/arm/mach-pxa/icontrol.c | 9 +++++----
arch/arm/mach-pxa/zeus.c | 9 +++++----
drivers/net/can/spi/mcp251x.c | 19 ++++++++-----------
include/linux/can/platform/mcp251x.h | 22 ----------------------
4 files changed, 18 insertions(+), 41 deletions(-)
delete mode 100644 include/linux/can/platform/mcp251x.h
diff --git a/arch/arm/mach-pxa/icontrol.c b/arch/arm/mach-pxa/icontrol.c
index 865b10344ea2..aa4ccb9bb1c1 100644
--- a/arch/arm/mach-pxa/icontrol.c
+++ b/arch/arm/mach-pxa/icontrol.c
@@ -12,6 +12,7 @@
#include <linux/irq.h>
#include <linux/platform_device.h>
+#include <linux/property.h>
#include <linux/gpio.h>
#include <asm/mach-types.h>
@@ -22,7 +23,6 @@
#include <linux/spi/spi.h>
#include <linux/spi/pxa2xx_spi.h>
-#include <linux/can/platform/mcp251x.h>
#include <linux/regulator/machine.h>
#include "generic.h"
@@ -69,8 +69,9 @@ static struct pxa2xx_spi_chip mcp251x_chip_info4 = {
.gpio_cs = ICONTROL_MCP251x_nCS4
};
-static struct mcp251x_platform_data mcp251x_info = {
- .oscillator_frequency = 16E6,
+static const struct property_entry mcp251x_properties = {
+ PROPERTY_ENTRY_U32("clock-frequency", 16000000),
+ {}
};
static struct spi_board_info mcp251x_board_info[] = {
@@ -79,7 +80,7 @@ static struct spi_board_info mcp251x_board_info[] = {
.max_speed_hz = 6500000,
.bus_num = 3,
.chip_select = 0,
- .platform_data = &mcp251x_info,
+ .properties = &mcp251x_properties,
.controller_data = &mcp251x_chip_info1,
.irq = PXA_GPIO_TO_IRQ(ICONTROL_MCP251x_nIRQ1)
},
diff --git a/arch/arm/mach-pxa/zeus.c b/arch/arm/mach-pxa/zeus.c
index da113c8eefbf..645500ef427a 100644
--- a/arch/arm/mach-pxa/zeus.c
+++ b/arch/arm/mach-pxa/zeus.c
@@ -13,6 +13,7 @@
#include <linux/leds.h>
#include <linux/irq.h>
#include <linux/pm.h>
+#include <linux/property.h>
#include <linux/gpio.h>
#include <linux/gpio/machine.h>
#include <linux/serial_8250.h>
@@ -27,7 +28,6 @@
#include <linux/platform_data/i2c-pxa.h>
#include <linux/platform_data/pca953x.h>
#include <linux/apm-emulation.h>
-#include <linux/can/platform/mcp251x.h>
#include <linux/regulator/fixed.h>
#include <linux/regulator/machine.h>
@@ -428,14 +428,15 @@ static struct gpiod_lookup_table can_regulator_gpiod_table = {
},
};
-static struct mcp251x_platform_data zeus_mcp2515_pdata = {
- .oscillator_frequency = 16*1000*1000,
+static const struct property_entry mcp251x_properties = {
+ PROPERTY_ENTRY_U32("clock-frequency", 16000000),
+ {}
};
static struct spi_board_info zeus_spi_board_info[] = {
[0] = {
.modalias = "mcp2515",
- .platform_data = &zeus_mcp2515_pdata,
+ .properties = &mcp251x_properties,
.irq = PXA_GPIO_TO_IRQ(ZEUS_CAN_GPIO),
.max_speed_hz = 1*1000*1000,
.bus_num = 3,
diff --git a/drivers/net/can/spi/mcp251x.c b/drivers/net/can/spi/mcp251x.c
index 6ee0ea51399a..3a4d7089dc7c 100644
--- a/drivers/net/can/spi/mcp251x.c
+++ b/drivers/net/can/spi/mcp251x.c
@@ -20,29 +20,26 @@
*
* Your platform definition file should specify something like:
*
- * static struct mcp251x_platform_data mcp251x_info = {
- * .oscillator_frequency = 8000000,
+ * static const struct property_entry mpc251x_properties[] = {
+ * PROPERTY_ENTRY_U32("clock-frequency", 8000000),
+ * {}
* };
*
* static struct spi_board_info spi_board_info[] = {
* {
* .modalias = "mcp2510",
* // "mcp2515" or "mcp25625" depending on your controller
- * .platform_data = &mcp251x_info,
+ * .properties = &mcp251x_properties,
* .irq = IRQ_EINT13,
* .max_speed_hz = 2*1000*1000,
* .chip_select = 2,
* },
* };
- *
- * Please see mcp251x.h for a description of the fields in
- * struct mcp251x_platform_data.
*/
#include <linux/can/core.h>
#include <linux/can/dev.h>
#include <linux/can/led.h>
-#include <linux/can/platform/mcp251x.h>
#include <linux/clk.h>
#include <linux/completion.h>
#include <linux/delay.h>
@@ -1006,19 +1003,19 @@ MODULE_DEVICE_TABLE(spi, mcp251x_id_table);
static int mcp251x_can_probe(struct spi_device *spi)
{
const void *match = device_get_match_data(&spi->dev);
- struct mcp251x_platform_data *pdata = dev_get_platdata(&spi->dev);
struct net_device *net;
struct mcp251x_priv *priv;
struct clk *clk;
- int freq, ret;
+ u32 freq;
+ int ret;
clk = devm_clk_get_optional(&spi->dev, NULL);
if (IS_ERR(clk))
return PTR_ERR(clk);
freq = clk_get_rate(clk);
- if (freq == 0 && pdata)
- freq = pdata->oscillator_frequency;
+ if (freq == 0)
+ device_property_read_u32(&spi->dev, "clock-frequency", &freq);
/* Sanity check */
if (freq < 1000000 || freq > 25000000)
diff --git a/include/linux/can/platform/mcp251x.h b/include/linux/can/platform/mcp251x.h
deleted file mode 100644
index 9e5ac27fb6c1..000000000000
--- a/include/linux/can/platform/mcp251x.h
+++ /dev/null
@@ -1,22 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef _CAN_PLATFORM_MCP251X_H
-#define _CAN_PLATFORM_MCP251X_H
-
-/*
- *
- * CAN bus driver for Microchip 251x CAN Controller with SPI Interface
- *
- */
-
-#include <linux/spi/spi.h>
-
-/*
- * struct mcp251x_platform_data - MCP251X SPI CAN controller platform data
- * @oscillator_frequency: - oscillator frequency in Hz
- */
-
-struct mcp251x_platform_data {
- unsigned long oscillator_frequency;
-};
-
-#endif /* !_CAN_PLATFORM_MCP251X_H */
--
2.23.0.rc1
^ permalink raw reply related
* [PATCH v2 2/4] can: mcp251x: Make use of device property API
From: Andy Shevchenko @ 2019-09-03 12:42 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde, linux-can,
David S. Miller, netdev
Cc: Andy Shevchenko
In-Reply-To: <20190903124259.60920-1-andriy.shevchenko@linux.intel.com>
Make use of device property API in this driver so that both OF based
system and ACPI based system can use this driver.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/net/can/spi/mcp251x.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/net/can/spi/mcp251x.c b/drivers/net/can/spi/mcp251x.c
index e04b578f2b1f..0b7e743ca0a0 100644
--- a/drivers/net/can/spi/mcp251x.c
+++ b/drivers/net/can/spi/mcp251x.c
@@ -53,8 +53,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/netdevice.h>
-#include <linux/of.h>
-#include <linux/of_device.h>
+#include <linux/property.h>
#include <linux/platform_device.h>
#include <linux/slab.h>
#include <linux/spi/spi.h>
@@ -914,7 +913,7 @@ static int mcp251x_open(struct net_device *net)
priv->tx_skb = NULL;
priv->tx_len = 0;
- if (!spi->dev.of_node)
+ if (!dev_fwnode(&spi->dev))
flags = IRQF_TRIGGER_FALLING;
ret = request_threaded_irq(spi->irq, NULL, mcp251x_can_ist,
@@ -1006,8 +1005,7 @@ MODULE_DEVICE_TABLE(spi, mcp251x_id_table);
static int mcp251x_can_probe(struct spi_device *spi)
{
- const struct of_device_id *of_id = of_match_device(mcp251x_of_match,
- &spi->dev);
+ const void *match = device_get_match_data(&spi->dev);
struct mcp251x_platform_data *pdata = dev_get_platdata(&spi->dev);
struct net_device *net;
struct mcp251x_priv *priv;
@@ -1044,8 +1042,8 @@ static int mcp251x_can_probe(struct spi_device *spi)
priv->can.clock.freq = freq / 2;
priv->can.ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES |
CAN_CTRLMODE_LOOPBACK | CAN_CTRLMODE_LISTENONLY;
- if (of_id)
- priv->model = (enum mcp251x_model)of_id->data;
+ if (match)
+ priv->model = (enum mcp251x_model)match;
else
priv->model = spi_get_device_id(spi)->driver_data;
priv->net = net;
--
2.23.0.rc1
^ permalink raw reply related
* [PATCH v2 0/4] can: mcp251x: Make use of device properties
From: Andy Shevchenko @ 2019-09-03 12:42 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde, linux-can,
David S. Miller, netdev
Cc: Andy Shevchenko
The purpose of this series is to simplify driver by switching to use device
properties. In particular it allows to drop legacy platform data.
Patch 1 switches driver to use devm_clk_get_optional() API.
Patch 2 unifies getting the driver data independently of the table which
provides it.
Patch 3 drops extra check for regulator presence by switch to use an already
present wrapper.
And patch 4 gets rid of legacy platform data.
Changelog v2:
- add patch 4 to get rid of legacy platform data
Andy Shevchenko (4):
can: mcp251x: Use devm_clk_get_optional() to get the input clock
can: mcp251x: Make use of device property API
can: mcp251x: Call wrapper instead of regulator_disable()
can: mcp251x: Get rid of legacy platform data
arch/arm/mach-pxa/icontrol.c | 9 ++--
arch/arm/mach-pxa/zeus.c | 9 ++--
drivers/net/can/spi/mcp251x.c | 65 +++++++++++-----------------
include/linux/can/platform/mcp251x.h | 22 ----------
4 files changed, 36 insertions(+), 69 deletions(-)
delete mode 100644 include/linux/can/platform/mcp251x.h
--
2.23.0.rc1
^ permalink raw reply
* Re: [PATCH] net-ipv6: fix excessive RTF_ADDRCONF flag on ::1/128 local route (and others)
From: Maciej Żenczykowski @ 2019-09-03 12:17 UTC (permalink / raw)
To: Lorenzo Colitti; +Cc: David Ahern, David S . Miller, Linux NetDev
In-Reply-To: <CAKD1Yr2ykCyEiUyY4R+hYoZ+eWGjbE78wtSf2=_ZjLpCyp0n-Q@mail.gmail.com>
Well, if you look at the commit my commit is fixing, ie.
commit c7a1ce397adacaf5d4bb2eab0a738b5f80dc3e43
then you'll see this in the commit description:
"- dst_nocount is handled by the RTF_ADDRCONF flag"
and the patch diff itself is from
"f6i->fib6_flags = RTF_UP | RTF_NONEXTHOP;
f6i->dst_nocount = true;"
to
" .fc_flags = RTF_UP | RTF_ADDRCONF | RTF_NONEXTHOP,"
(and RTF_ANYCAST or RTF_LOCAL is later or'ed in in both versions of the code)
so I'm pretty sure that patch adds ADDRCONF unconditionally to that
function, and my commit unconditionally removes it.
Perhaps since then the call graph has changed???
Unfortunately I'm already in Europe on ancient ipv6 free networks and
since the office move my ipv6 lab is still not up (something about the
newly wired office jacks being blue which means corp, instead of any
other colour for a lab...) so I don't actually have an easy way to
test ipv6 slaac behaviour. :-(
On Tue, Sep 3, 2019 at 6:58 AM Lorenzo Colitti <lorenzo@google.com> wrote:
>
> On Tue, Sep 3, 2019 at 11:18 AM David Ahern <dsahern@gmail.com> wrote:
> > addrconf_f6i_alloc is used for addresses added by userspace
> > (ipv6_add_addr) and anycast. ie., from what I can see it is not used for RAs
>
> Isn't ipv6_add_addr called by addrconf_prefix_rcv_add_addr, which is
> called by addrconf_prefix_rcv, which is called by
> ndisc_router_discovery? That is what happens when we process an RA;
> AFAICS manual configuration is inet6_addr_add, not ipv6_add_addr.
>
> Maciej, with this patch, do SLAAC addresses still have RTF_ADDRCONF?
> Per my previous message, my assumption would be no, but I might be
> misreading the code.
^ permalink raw reply
* KASAN: use-after-free Read in atusb_disconnect
From: syzbot @ 2019-09-03 12:08 UTC (permalink / raw)
To: alex.aring, andreyknvl, davem, linux-kernel, linux-usb,
linux-wpan, netdev, stefan, syzkaller-bugs
Hello,
syzbot found the following crash on:
HEAD commit: eea39f24 usb-fuzzer: main usb gadget fuzzer driver
git tree: https://github.com/google/kasan.git usb-fuzzer
console output: https://syzkaller.appspot.com/x/log.txt?x=15c4eba6600000
kernel config: https://syzkaller.appspot.com/x/.config?x=d0c62209eedfd54e
dashboard link: https://syzkaller.appspot.com/bug?extid=f4509a9138a1472e7e80
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=15486ab6600000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=15777f22600000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+f4509a9138a1472e7e80@syzkaller.appspotmail.com
usb 1-1: USB disconnect, device number 2
==================================================================
BUG: KASAN: use-after-free in atusb_disconnect+0x17f/0x1c0
drivers/net/ieee802154/atusb.c:1143
Read of size 8 at addr ffff8881d53eee28 by task kworker/1:2/83
CPU: 1 PID: 83 Comm: kworker/1:2 Not tainted 5.3.0-rc5+ #28
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Workqueue: usb_hub_wq hub_event
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0xca/0x13e lib/dump_stack.c:113
print_address_description+0x6a/0x32c mm/kasan/report.c:351
__kasan_report.cold+0x1a/0x33 mm/kasan/report.c:482
kasan_report+0xe/0x12 mm/kasan/common.c:612
atusb_disconnect+0x17f/0x1c0 drivers/net/ieee802154/atusb.c:1143
usb_unbind_interface+0x1bd/0x8a0 drivers/usb/core/driver.c:423
__device_release_driver drivers/base/dd.c:1134 [inline]
device_release_driver_internal+0x42f/0x500 drivers/base/dd.c:1165
bus_remove_device+0x2dc/0x4a0 drivers/base/bus.c:556
device_del+0x420/0xb10 drivers/base/core.c:2339
usb_disable_device+0x211/0x690 drivers/usb/core/message.c:1237
usb_disconnect+0x284/0x8d0 drivers/usb/core/hub.c:2199
hub_port_connect drivers/usb/core/hub.c:4949 [inline]
hub_port_connect_change drivers/usb/core/hub.c:5213 [inline]
port_event drivers/usb/core/hub.c:5359 [inline]
hub_event+0x1454/0x3640 drivers/usb/core/hub.c:5441
process_one_work+0x92b/0x1530 kernel/workqueue.c:2269
worker_thread+0x96/0xe20 kernel/workqueue.c:2415
kthread+0x318/0x420 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
Allocated by task 12:
save_stack+0x1b/0x80 mm/kasan/common.c:69
set_track mm/kasan/common.c:77 [inline]
__kasan_kmalloc mm/kasan/common.c:487 [inline]
__kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:460
kmalloc include/linux/slab.h:557 [inline]
kzalloc include/linux/slab.h:748 [inline]
wpan_phy_new+0x22/0x290 net/ieee802154/core.c:109
ieee802154_alloc_hw+0x11d/0x750 net/mac802154/main.c:77
atusb_probe+0x9b/0xfa2 drivers/net/ieee802154/atusb.c:1023
usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361
really_probe+0x281/0x6d0 drivers/base/dd.c:548
driver_probe_device+0x101/0x1b0 drivers/base/dd.c:721
__device_attach_driver+0x1c2/0x220 drivers/base/dd.c:828
bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:454
__device_attach+0x217/0x360 drivers/base/dd.c:894
bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
device_add+0xae6/0x16f0 drivers/base/core.c:2165
usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023
generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210
usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266
really_probe+0x281/0x6d0 drivers/base/dd.c:548
driver_probe_device+0x101/0x1b0 drivers/base/dd.c:721
__device_attach_driver+0x1c2/0x220 drivers/base/dd.c:828
bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:454
__device_attach+0x217/0x360 drivers/base/dd.c:894
bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
device_add+0xae6/0x16f0 drivers/base/core.c:2165
usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536
hub_port_connect drivers/usb/core/hub.c:5098 [inline]
hub_port_connect_change drivers/usb/core/hub.c:5213 [inline]
port_event drivers/usb/core/hub.c:5359 [inline]
hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441
process_one_work+0x92b/0x1530 kernel/workqueue.c:2269
worker_thread+0x96/0xe20 kernel/workqueue.c:2415
kthread+0x318/0x420 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
Freed by task 83:
save_stack+0x1b/0x80 mm/kasan/common.c:69
set_track mm/kasan/common.c:77 [inline]
__kasan_slab_free+0x130/0x180 mm/kasan/common.c:449
slab_free_hook mm/slub.c:1423 [inline]
slab_free_freelist_hook mm/slub.c:1474 [inline]
slab_free mm/slub.c:3016 [inline]
kfree+0xe4/0x2f0 mm/slub.c:3957
device_release+0x71/0x200 drivers/base/core.c:1064
kobject_cleanup lib/kobject.c:693 [inline]
kobject_release lib/kobject.c:722 [inline]
kref_put include/linux/kref.h:65 [inline]
kobject_put+0x171/0x280 lib/kobject.c:739
put_device+0x1b/0x30 drivers/base/core.c:2264
atusb_disconnect+0x117/0x1c0 drivers/net/ieee802154/atusb.c:1140
usb_unbind_interface+0x1bd/0x8a0 drivers/usb/core/driver.c:423
__device_release_driver drivers/base/dd.c:1134 [inline]
device_release_driver_internal+0x42f/0x500 drivers/base/dd.c:1165
bus_remove_device+0x2dc/0x4a0 drivers/base/bus.c:556
device_del+0x420/0xb10 drivers/base/core.c:2339
usb_disable_device+0x211/0x690 drivers/usb/core/message.c:1237
usb_disconnect+0x284/0x8d0 drivers/usb/core/hub.c:2199
hub_port_connect drivers/usb/core/hub.c:4949 [inline]
hub_port_connect_change drivers/usb/core/hub.c:5213 [inline]
port_event drivers/usb/core/hub.c:5359 [inline]
hub_event+0x1454/0x3640 drivers/usb/core/hub.c:5441
process_one_work+0x92b/0x1530 kernel/workqueue.c:2269
worker_thread+0x96/0xe20 kernel/workqueue.c:2415
kthread+0x318/0x420 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
The buggy address belongs to the object at ffff8881d53ee600
which belongs to the cache kmalloc-4k of size 4096
The buggy address is located 2088 bytes inside of
4096-byte region [ffff8881d53ee600, ffff8881d53ef600)
The buggy address belongs to the page:
page:ffffea000754fa00 refcount:1 mapcount:0 mapping:ffff8881da00c280
index:0x0 compound_mapcount: 0
flags: 0x200000000010200(slab|head)
raw: 0200000000010200 0000000000000000 0000000600000001 ffff8881da00c280
raw: 0000000000000000 0000000000070007 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff8881d53eed00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8881d53eed80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ffff8881d53eee00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff8881d53eee80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8881d53eef00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* WARNING in hso_free_net_device
From: syzbot @ 2019-09-03 12:08 UTC (permalink / raw)
To: alexios.zavras, andreyknvl, benquike, davem, gregkh, linux-kernel,
linux-usb, mathias.payer, netdev, rfontana, syzkaller-bugs, tglx
Hello,
syzbot found the following crash on:
HEAD commit: eea39f24 usb-fuzzer: main usb gadget fuzzer driver
git tree: https://github.com/google/kasan.git usb-fuzzer
console output: https://syzkaller.appspot.com/x/log.txt?x=15f17e61600000
kernel config: https://syzkaller.appspot.com/x/.config?x=d0c62209eedfd54e
dashboard link: https://syzkaller.appspot.com/bug?extid=44d53c7255bb1aea22d2
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=10ffdd12600000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=15a738fe600000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+44d53c7255bb1aea22d2@syzkaller.appspotmail.com
usb 1-1: config 0 has no interface number 0
usb 1-1: New USB device found, idVendor=0af0, idProduct=d257,
bcdDevice=4e.87
usb 1-1: New USB device strings: Mfr=0, Product=0, SerialNumber=0
usb 1-1: config 0 descriptor??
hso 1-1:0.15: Can't find BULK IN endpoint
------------[ cut here ]------------
WARNING: CPU: 1 PID: 83 at net/core/dev.c:8167
rollback_registered_many.cold+0x41/0x1bc net/core/dev.c:8167
Kernel panic - not syncing: panic_on_warn set ...
CPU: 1 PID: 83 Comm: kworker/1:2 Not tainted 5.3.0-rc5+ #28
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Workqueue: usb_hub_wq hub_event
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0xca/0x13e lib/dump_stack.c:113
panic+0x2a3/0x6da kernel/panic.c:219
__warn.cold+0x20/0x4a kernel/panic.c:576
report_bug+0x262/0x2a0 lib/bug.c:186
fixup_bug arch/x86/kernel/traps.c:179 [inline]
fixup_bug arch/x86/kernel/traps.c:174 [inline]
do_error_trap+0x12b/0x1e0 arch/x86/kernel/traps.c:272
do_invalid_op+0x32/0x40 arch/x86/kernel/traps.c:291
invalid_op+0x23/0x30 arch/x86/entry/entry_64.S:1028
RIP: 0010:rollback_registered_many.cold+0x41/0x1bc net/core/dev.c:8167
Code: ff e8 c7 26 90 fc 48 c7 c7 40 ec 63 86 e8 24 c8 7a fc 0f 0b e9 93 be
ff ff e8 af 26 90 fc 48 c7 c7 40 ec 63 86 e8 0c c8 7a fc <0f> 0b 4c 89 e7
e8 f9 12 34 fd 31 ff 41 89 c4 89 c6 e8 bd 27 90 fc
RSP: 0018:ffff8881d934f088 EFLAGS: 00010282
RAX: 0000000000000024 RBX: ffff8881d2ad4400 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff81288cfd RDI: ffffed103b269e03
RBP: ffff8881d934f1b8 R08: 0000000000000024 R09: fffffbfff11ad794
R10: fffffbfff11ad793 R11: ffffffff88d6bc9f R12: ffff8881d2ad4470
R13: ffff8881d934f148 R14: dffffc0000000000 R15: 0000000000000000
rollback_registered+0xf2/0x1c0 net/core/dev.c:8243
unregister_netdevice_queue net/core/dev.c:9290 [inline]
unregister_netdevice_queue+0x1d7/0x2b0 net/core/dev.c:9283
unregister_netdevice include/linux/netdevice.h:2631 [inline]
unregister_netdev+0x18/0x20 net/core/dev.c:9331
hso_free_net_device+0xff/0x300 drivers/net/usb/hso.c:2366
hso_create_net_device+0x76d/0x9c0 drivers/net/usb/hso.c:2554
hso_probe+0x28d/0x1a46 drivers/net/usb/hso.c:2931
usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361
really_probe+0x281/0x6d0 drivers/base/dd.c:548
driver_probe_device+0x101/0x1b0 drivers/base/dd.c:721
__device_attach_driver+0x1c2/0x220 drivers/base/dd.c:828
bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:454
__device_attach+0x217/0x360 drivers/base/dd.c:894
bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
device_add+0xae6/0x16f0 drivers/base/core.c:2165
usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023
generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210
usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266
really_probe+0x281/0x6d0 drivers/base/dd.c:548
driver_probe_device+0x101/0x1b0 drivers/base/dd.c:721
__device_attach_driver+0x1c2/0x220 drivers/base/dd.c:828
bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:454
__device_attach+0x217/0x360 drivers/base/dd.c:894
bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
device_add+0xae6/0x16f0 drivers/base/core.c:2165
usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536
hub_port_connect drivers/usb/core/hub.c:5098 [inline]
hub_port_connect_change drivers/usb/core/hub.c:5213 [inline]
port_event drivers/usb/core/hub.c:5359 [inline]
hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441
process_one_work+0x92b/0x1530 kernel/workqueue.c:2269
worker_thread+0x96/0xe20 kernel/workqueue.c:2415
kthread+0x318/0x420 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
Kernel Offset: disabled
Rebooting in 86400 seconds..
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* KASAN: use-after-free Read in rsi_rx_done_handler
From: syzbot @ 2019-09-03 12:08 UTC (permalink / raw)
To: amitkarwar, andreyknvl, davem, kvalo, linux-kernel, linux-usb,
linux-wireless, netdev, siva8118, syzkaller-bugs
Hello,
syzbot found the following crash on:
HEAD commit: eea39f24 usb-fuzzer: main usb gadget fuzzer driver
git tree: https://github.com/google/kasan.git usb-fuzzer
console output: https://syzkaller.appspot.com/x/log.txt?x=1031c5f2600000
kernel config: https://syzkaller.appspot.com/x/.config?x=d0c62209eedfd54e
dashboard link: https://syzkaller.appspot.com/bug?extid=b563b7f8dbe8223a51e8
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
Unfortunately, I don't have any reproducer for this crash yet.
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+b563b7f8dbe8223a51e8@syzkaller.appspotmail.com
==================================================================
BUG: KASAN: use-after-free in rsi_rx_done_handler+0x3ba/0x490
drivers/net/wireless/rsi/rsi_91x_usb.c:267
Read of size 8 at addr ffff8881cebc0fe8 by task kworker/0:2/102
CPU: 0 PID: 102 Comm: kworker/0:2 Not tainted 5.3.0-rc5+ #28
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Workqueue: usb_hub_wq hub_event
Call Trace:
<IRQ>
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0xca/0x13e lib/dump_stack.c:113
print_address_description+0x6a/0x32c mm/kasan/report.c:351
__kasan_report.cold+0x1a/0x33 mm/kasan/report.c:482
kasan_report+0xe/0x12 mm/kasan/common.c:612
rsi_rx_done_handler+0x3ba/0x490 drivers/net/wireless/rsi/rsi_91x_usb.c:267
__usb_hcd_giveback_urb+0x1f2/0x470 drivers/usb/core/hcd.c:1657
usb_hcd_giveback_urb+0x368/0x420 drivers/usb/core/hcd.c:1722
dummy_timer+0x120f/0x2fa2 drivers/usb/gadget/udc/dummy_hcd.c:1965
call_timer_fn+0x179/0x650 kernel/time/timer.c:1322
expire_timers kernel/time/timer.c:1366 [inline]
__run_timers kernel/time/timer.c:1685 [inline]
__run_timers kernel/time/timer.c:1653 [inline]
run_timer_softirq+0x5cc/0x14b0 kernel/time/timer.c:1698
__do_softirq+0x221/0x912 kernel/softirq.c:292
invoke_softirq kernel/softirq.c:373 [inline]
irq_exit+0x178/0x1a0 kernel/softirq.c:413
exiting_irq arch/x86/include/asm/apic.h:537 [inline]
smp_apic_timer_interrupt+0x12f/0x500 arch/x86/kernel/apic/apic.c:1095
apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:830
</IRQ>
RIP: 0010:arch_local_irq_restore arch/x86/include/asm/irqflags.h:85 [inline]
RIP: 0010:console_unlock+0xa2a/0xc40 kernel/printk/printk.c:2471
Code: 00 89 ee 48 c7 c7 20 89 d3 86 e8 61 ad 03 00 65 ff 0d 72 b5 d9 7e e9
db f9 ff ff e8 80 a0 15 00 e8 2b ca 1a 00 ff 74 24 30 9d <e9> 18 fe ff ff
e8 6c a0 15 00 48 8d 7d 08 48 89 f8 48 c1 e8 03 42
RSP: 0018:ffff8881d5077200 EFLAGS: 00000216 ORIG_RAX: ffffffffffffff13
RAX: 0000000000000007 RBX: 0000000000000200 RCX: 0000000000000006
RDX: 0000000000000000 RSI: 0000000000000008 RDI: ffff8881d5068844
RBP: 0000000000000000 R08: ffff8881d5068000 R09: fffffbfff11ad791
R10: fffffbfff11ad790 R11: ffffffff88d6bc87 R12: 000000000000004d
R13: dffffc0000000000 R14: ffffffff829090d0 R15: ffffffff87077430
vprintk_emit+0x171/0x3e0 kernel/printk/printk.c:1986
vprintk_func+0x75/0x113 kernel/printk/printk_safe.c:386
printk+0xba/0xed kernel/printk/printk.c:2046
really_probe.cold+0x69/0x1f6 drivers/base/dd.c:628
driver_probe_device+0x101/0x1b0 drivers/base/dd.c:721
__device_attach_driver+0x1c2/0x220 drivers/base/dd.c:828
bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:454
__device_attach+0x217/0x360 drivers/base/dd.c:894
bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
device_add+0xae6/0x16f0 drivers/base/core.c:2165
usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023
generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210
usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266
really_probe+0x281/0x6d0 drivers/base/dd.c:548
driver_probe_device+0x101/0x1b0 drivers/base/dd.c:721
__device_attach_driver+0x1c2/0x220 drivers/base/dd.c:828
bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:454
__device_attach+0x217/0x360 drivers/base/dd.c:894
bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
device_add+0xae6/0x16f0 drivers/base/core.c:2165
usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536
hub_port_connect drivers/usb/core/hub.c:5098 [inline]
hub_port_connect_change drivers/usb/core/hub.c:5213 [inline]
port_event drivers/usb/core/hub.c:5359 [inline]
hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441
process_one_work+0x92b/0x1530 kernel/workqueue.c:2269
worker_thread+0x96/0xe20 kernel/workqueue.c:2415
kthread+0x318/0x420 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
Allocated by task 102:
save_stack+0x1b/0x80 mm/kasan/common.c:69
set_track mm/kasan/common.c:77 [inline]
__kasan_kmalloc mm/kasan/common.c:487 [inline]
__kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:460
kmalloc include/linux/slab.h:552 [inline]
kzalloc include/linux/slab.h:748 [inline]
rsi_init_usb_interface drivers/net/wireless/rsi/rsi_91x_usb.c:599 [inline]
rsi_probe+0x11a/0x15a0 drivers/net/wireless/rsi/rsi_91x_usb.c:780
usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361
really_probe+0x281/0x6d0 drivers/base/dd.c:548
driver_probe_device+0x101/0x1b0 drivers/base/dd.c:721
__device_attach_driver+0x1c2/0x220 drivers/base/dd.c:828
bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:454
__device_attach+0x217/0x360 drivers/base/dd.c:894
bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
device_add+0xae6/0x16f0 drivers/base/core.c:2165
usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023
generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210
usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266
really_probe+0x281/0x6d0 drivers/base/dd.c:548
driver_probe_device+0x101/0x1b0 drivers/base/dd.c:721
__device_attach_driver+0x1c2/0x220 drivers/base/dd.c:828
bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:454
__device_attach+0x217/0x360 drivers/base/dd.c:894
bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
device_add+0xae6/0x16f0 drivers/base/core.c:2165
usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536
hub_port_connect drivers/usb/core/hub.c:5098 [inline]
hub_port_connect_change drivers/usb/core/hub.c:5213 [inline]
port_event drivers/usb/core/hub.c:5359 [inline]
hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441
process_one_work+0x92b/0x1530 kernel/workqueue.c:2269
worker_thread+0x96/0xe20 kernel/workqueue.c:2415
kthread+0x318/0x420 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
Freed by task 102:
save_stack+0x1b/0x80 mm/kasan/common.c:69
set_track mm/kasan/common.c:77 [inline]
__kasan_slab_free+0x130/0x180 mm/kasan/common.c:449
slab_free_hook mm/slub.c:1423 [inline]
slab_free_freelist_hook mm/slub.c:1474 [inline]
slab_free mm/slub.c:3016 [inline]
kfree+0xe4/0x2f0 mm/slub.c:3957
rsi_91x_deinit+0x270/0x2f0 drivers/net/wireless/rsi/rsi_91x_main.c:407
rsi_probe+0xcec/0x15a0 drivers/net/wireless/rsi/rsi_91x_usb.c:834
usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361
really_probe+0x281/0x6d0 drivers/base/dd.c:548
driver_probe_device+0x101/0x1b0 drivers/base/dd.c:721
__device_attach_driver+0x1c2/0x220 drivers/base/dd.c:828
bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:454
__device_attach+0x217/0x360 drivers/base/dd.c:894
bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
device_add+0xae6/0x16f0 drivers/base/core.c:2165
usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023
generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210
usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266
really_probe+0x281/0x6d0 drivers/base/dd.c:548
driver_probe_device+0x101/0x1b0 drivers/base/dd.c:721
__device_attach_driver+0x1c2/0x220 drivers/base/dd.c:828
bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:454
__device_attach+0x217/0x360 drivers/base/dd.c:894
bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
device_add+0xae6/0x16f0 drivers/base/core.c:2165
usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536
hub_port_connect drivers/usb/core/hub.c:5098 [inline]
hub_port_connect_change drivers/usb/core/hub.c:5213 [inline]
port_event drivers/usb/core/hub.c:5359 [inline]
hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441
process_one_work+0x92b/0x1530 kernel/workqueue.c:2269
worker_thread+0x96/0xe20 kernel/workqueue.c:2415
kthread+0x318/0x420 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
The buggy address belongs to the object at ffff8881cebc0f00
which belongs to the cache kmalloc-512 of size 512
The buggy address is located 232 bytes inside of
512-byte region [ffff8881cebc0f00, ffff8881cebc1100)
The buggy address belongs to the page:
page:ffffea00073af000 refcount:1 mapcount:0 mapping:ffff8881da002500
index:0x0 compound_mapcount: 0
flags: 0x200000000010200(slab|head)
raw: 0200000000010200 0000000000000000 0000000b00000001 ffff8881da002500
raw: 0000000000000000 00000000800c000c 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff8881cebc0e80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff8881cebc0f00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ffff8881cebc0f80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff8881cebc1000: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8881cebc1080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
^ permalink raw reply
* Re: [Bridge] [PATCH v3 1/2] net: bridge: use mac_len in bridge forwarding
From: Toshiaki Makita @ 2019-09-03 11:37 UTC (permalink / raw)
To: Zahari Doychev, netdev
Cc: makita.toshiaki, jiri, nikolay, simon.horman, roopa, bridge, jhs,
dsahern, xiyou.wangcong, johannes, alexei.starovoitov
In-Reply-To: <20190902181000.25638-1-zahari.doychev@linux.com>
Hi Zahari,
Sorry for reviewing this late.
On 2019/09/03 3:09, Zahari Doychev wrote:
...
> @@ -466,13 +466,14 @@ static bool __allowed_ingress(const struct net_bridge *br,
> /* Tagged frame */
> if (skb->vlan_proto != br->vlan_proto) {
> /* Protocol-mismatch, empty out vlan_tci for new tag */
> - skb_push(skb, ETH_HLEN);
> + skb_push(skb, skb->mac_len);
> skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
> skb_vlan_tag_get(skb));
I think we should insert vlan at skb->data, i.e. mac_header + mac_len, while this
function inserts the tag at mac_header + ETH_HLEN which is not always the correct
offset.
> if (unlikely(!skb))
> return false;
>
> skb_pull(skb, ETH_HLEN);
Now skb->data is mac_header + ETH_HLEN which would be broken when mac_len is not
ETH_HLEN?
> + skb_reset_network_header(skb);
> skb_reset_mac_len(skb);
> *vid = 0;
> tagged = false;
>
Toshiaki Makita
^ permalink raw reply
* Re: [RFC v3] vhost: introduce mdev based hardware vhost backend
From: Michael S. Tsirkin @ 2019-09-03 11:26 UTC (permalink / raw)
To: Tiwei Bie
Cc: jasowang, alex.williamson, maxime.coquelin, linux-kernel, kvm,
virtualization, netdev, dan.daly, cunming.liang, zhihong.wang,
lingshan.zhu
In-Reply-To: <20190828053712.26106-1-tiwei.bie@intel.com>
On Wed, Aug 28, 2019 at 01:37:12PM +0800, Tiwei Bie wrote:
> Details about this can be found here:
>
> https://lwn.net/Articles/750770/
>
> What's new in this version
> ==========================
>
> There are three choices based on the discussion [1] in RFC v2:
>
> > #1. We expose a VFIO device, so we can reuse the VFIO container/group
> > based DMA API and potentially reuse a lot of VFIO code in QEMU.
> >
> > But in this case, we have two choices for the VFIO device interface
> > (i.e. the interface on top of VFIO device fd):
> >
> > A) we may invent a new vhost protocol (as demonstrated by the code
> > in this RFC) on VFIO device fd to make it work in VFIO's way,
> > i.e. regions and irqs.
> >
> > B) Or as you proposed, instead of inventing a new vhost protocol,
> > we can reuse most existing vhost ioctls on the VFIO device fd
> > directly. There should be no conflicts between the VFIO ioctls
> > (type is 0x3B) and VHOST ioctls (type is 0xAF) currently.
> >
> > #2. Instead of exposing a VFIO device, we may expose a VHOST device.
> > And we will introduce a new mdev driver vhost-mdev to do this.
> > It would be natural to reuse the existing kernel vhost interface
> > (ioctls) on it as much as possible. But we will need to invent
> > some APIs for DMA programming (reusing VHOST_SET_MEM_TABLE is a
> > choice, but it's too heavy and doesn't support vIOMMU by itself).
>
> This version is more like a quick PoC to try Jason's proposal on
> reusing vhost ioctls. And the second way (#1/B) in above three
> choices was chosen in this version to demonstrate the idea quickly.
>
> Now the userspace API looks like this:
>
> - VFIO's container/group based IOMMU API is used to do the
> DMA programming.
>
> - Vhost's existing ioctls are used to setup the device.
>
> And the device will report device_api as "vfio-vhost".
>
> Note that, there are dirty hacks in this version. If we decide to
> go this way, some refactoring in vhost.c/vhost.h may be needed.
>
> PS. The direct mapping of the notify registers isn't implemented
> in this version.
>
> [1] https://lkml.org/lkml/2019/7/9/101
>
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
....
> +long vhost_mdev_ioctl(struct mdev_device *mdev, unsigned int cmd,
> + unsigned long arg)
> +{
> + void __user *argp = (void __user *)arg;
> + struct vhost_mdev *vdpa;
> + unsigned long minsz;
> + int ret = 0;
> +
> + if (!mdev)
> + return -EINVAL;
> +
> + vdpa = mdev_get_drvdata(mdev);
> + if (!vdpa)
> + return -ENODEV;
> +
> + switch (cmd) {
> + case VFIO_DEVICE_GET_INFO:
> + {
> + struct vfio_device_info info;
> +
> + minsz = offsetofend(struct vfio_device_info, num_irqs);
> +
> + if (copy_from_user(&info, (void __user *)arg, minsz)) {
> + ret = -EFAULT;
> + break;
> + }
> +
> + if (info.argsz < minsz) {
> + ret = -EINVAL;
> + break;
> + }
> +
> + info.flags = VFIO_DEVICE_FLAGS_VHOST;
> + info.num_regions = 0;
> + info.num_irqs = 0;
> +
> + if (copy_to_user((void __user *)arg, &info, minsz)) {
> + ret = -EFAULT;
> + break;
> + }
> +
> + break;
> + }
> + case VFIO_DEVICE_GET_REGION_INFO:
> + case VFIO_DEVICE_GET_IRQ_INFO:
> + case VFIO_DEVICE_SET_IRQS:
> + case VFIO_DEVICE_RESET:
> + ret = -EINVAL;
> + break;
> +
> + case VHOST_MDEV_SET_STATE:
> + ret = vhost_set_state(vdpa, argp);
> + break;
> + case VHOST_GET_FEATURES:
> + ret = vhost_get_features(vdpa, argp);
> + break;
> + case VHOST_SET_FEATURES:
> + ret = vhost_set_features(vdpa, argp);
> + break;
> + case VHOST_GET_VRING_BASE:
> + ret = vhost_get_vring_base(vdpa, argp);
> + break;
> + default:
> + ret = vhost_dev_ioctl(&vdpa->dev, cmd, argp);
> + if (ret == -ENOIOCTLCMD)
> + ret = vhost_vring_ioctl(&vdpa->dev, cmd, argp);
> + }
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(vhost_mdev_ioctl);
I don't have a problem with this approach. A small question:
would it make sense to have two fds: send vhost ioctls
on one and vfio ioctls on another?
We can then pass vfio fd to the vhost fd with a
SET_BACKEND ioctl.
What do you think?
--
MST
^ permalink raw reply
* Re: [PATCH] net: 9p: Fix possible null-pointer dereferences in p9_cm_event_handler()
From: Dominique Martinet @ 2019-09-03 10:55 UTC (permalink / raw)
To: Jia-Ju Bai; +Cc: ericvh, lucho, davem, v9fs-developer, netdev, linux-kernel
In-Reply-To: <20190724125545.GA12982@nautica>
Jia-Ju,
Dominique Martinet wrote on Wed, Jul 24, 2019:
> Jia-Ju Bai wrote on Wed, Jul 24, 2019:
> > In p9_cm_event_handler(), there is an if statement on 260 to check
> > whether rdma is NULL, which indicates that rdma can be NULL.
> > If so, using rdma->xxx may cause a possible null-pointer dereference.
>
> The final dereference (complete(&rdma->cm_done) line 285) has been here
> from the start, so we would have seen crashes by now if rdma could be
> null at this point.
>
> Let's do it the other way around and remove the useless "if (rdma)" that
> has been here from day 1 instead ; I basically did the same with
> c->status a few months ago (from a coverity report)...
Did you get anywhere with this, or should I submit a new patch myself ?
In the later case I'll tag this as Reported-by you
Thanks,
--
Dominique
^ permalink raw reply
* Re: [PATCH v3] tun: fix use-after-free when register netdev failed
From: Jason Wang @ 2019-09-03 10:50 UTC (permalink / raw)
To: Yang Yingliang
Cc: David Miller, netdev, eric dumazet, xiyou wangcong, weiyongjun1
In-Reply-To: <5D6E17A7.1020102@huawei.com>
----- Original Message -----
>
>
> On 2019/9/3 14:06, Jason Wang wrote:
> >
> > On 2019/9/3 下午1:42, Yang Yingliang wrote:
> >>
> >>
> >> On 2019/9/3 11:03, Jason Wang wrote:
> >>>
> >>> On 2019/9/3 上午9:45, Yang Yingliang wrote:
> >>>>
> >>>>
> >>>> On 2019/9/2 13:32, Jason Wang wrote:
> >>>>>
> >>>>> On 2019/8/23 下午5:36, Yang Yingliang wrote:
> >>>>>>
> >>>>>>
> >>>>>> On 2019/8/23 11:05, Jason Wang wrote:
> >>>>>>> ----- Original Message -----
> >>>>>>>>
> >>>>>>>> On 2019/8/22 14:07, Yang Yingliang wrote:
> >>>>>>>>>
> >>>>>>>>> On 2019/8/22 10:13, Jason Wang wrote:
> >>>>>>>>>> On 2019/8/20 上午10:28, Jason Wang wrote:
> >>>>>>>>>>> On 2019/8/20 上午9:25, David Miller wrote:
> >>>>>>>>>>>> From: Yang Yingliang <yangyingliang@huawei.com>
> >>>>>>>>>>>> Date: Mon, 19 Aug 2019 21:31:19 +0800
> >>>>>>>>>>>>
> >>>>>>>>>>>>> Call tun_attach() after register_netdevice() to make sure
> >>>>>>>>>>>>> tfile->tun
> >>>>>>>>>>>>> is not published until the netdevice is registered. So the
> >>>>>>>>>>>>> read/write
> >>>>>>>>>>>>> thread can not use the tun pointer that may freed by
> >>>>>>>>>>>>> free_netdev().
> >>>>>>>>>>>>> (The tun and dev pointer are allocated by
> >>>>>>>>>>>>> alloc_netdev_mqs(), they
> >>>>>>>>>>>>> can
> >>>>>>>>>>>>> be freed by netdev_freemem().)
> >>>>>>>>>>>> register_netdevice() must always be the last operation in
> >>>>>>>>>>>> the order of
> >>>>>>>>>>>> network device setup.
> >>>>>>>>>>>>
> >>>>>>>>>>>> At the point register_netdevice() is called, the device is
> >>>>>>>>>>>> visible
> >>>>>>>>>>>> globally
> >>>>>>>>>>>> and therefore all of it's software state must be fully
> >>>>>>>>>>>> initialized and
> >>>>>>>>>>>> ready for us.
> >>>>>>>>>>>>
> >>>>>>>>>>>> You're going to have to find another solution to these
> >>>>>>>>>>>> problems.
> >>>>>>>>>>>
> >>>>>>>>>>> The device is loosely coupled with sockets/queues. Each side is
> >>>>>>>>>>> allowed to be go away without caring the other side. So in this
> >>>>>>>>>>> case, there's a small window that network stack think the
> >>>>>>>>>>> device has
> >>>>>>>>>>> one queue but actually not, the code can then safely drop them.
> >>>>>>>>>>> Maybe it's ok here with some comments?
> >>>>>>>>>>>
> >>>>>>>>>>> Or if not, we can try to hold the device before tun_attach
> >>>>>>>>>>> and drop
> >>>>>>>>>>> it after register_netdevice().
> >>>>>>>>>>
> >>>>>>>>>> Hi Yang:
> >>>>>>>>>>
> >>>>>>>>>> I think maybe we can try to hold refcnt instead of playing
> >>>>>>>>>> real num
> >>>>>>>>>> queues here. Do you want to post a V4?
> >>>>>>>>> I think the refcnt can prevent freeing the memory in this case.
> >>>>>>>>> When register_netdevice() failed, free_netdev() will be called
> >>>>>>>>> directly,
> >>>>>>>>> dev->pcpu_refcnt and dev are freed without checking refcnt of
> >>>>>>>>> dev.
> >>>>>>>> How about using patch-v1 that using a flag to check whether the
> >>>>>>>> device
> >>>>>>>> registered successfully.
> >>>>>>>>
> >>>>>>> As I said, it lacks sufficient locks or barriers. To be clear, I
> >>>>>>> meant
> >>>>>>> something like (compile-test only):
> >>>>>>>
> >>>>>>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> >>>>>>> index db16d7a13e00..e52678f9f049 100644
> >>>>>>> --- a/drivers/net/tun.c
> >>>>>>> +++ b/drivers/net/tun.c
> >>>>>>> @@ -2828,6 +2828,7 @@ static int tun_set_iff(struct net *net,
> >>>>>>> struct file *file, struct ifreq *ifr)
> >>>>>>> (ifr->ifr_flags & TUN_FEATURES);
> >>>>>>> INIT_LIST_HEAD(&tun->disabled);
> >>>>>>> + dev_hold(dev);
> >>>>>>> err = tun_attach(tun, file, false,
> >>>>>>> ifr->ifr_flags & IFF_NAPI,
> >>>>>>> ifr->ifr_flags & IFF_NAPI_FRAGS);
> >>>>>>> if (err < 0)
> >>>>>>> @@ -2836,6 +2837,7 @@ static int tun_set_iff(struct net *net,
> >>>>>>> struct file *file, struct ifreq *ifr)
> >>>>>>> err = register_netdevice(tun->dev);
> >>>>>>> if (err < 0)
> >>>>>>> goto err_detach;
> >>>>>>> + dev_put(dev);
> >>>>>>> }
> >>>>>>> netif_carrier_on(tun->dev);
> >>>>>>> @@ -2852,11 +2854,13 @@ static int tun_set_iff(struct net *net,
> >>>>>>> struct file *file, struct ifreq *ifr)
> >>>>>>> return 0;
> >>>>>>> err_detach:
> >>>>>>> + dev_put(dev);
> >>>>>>> tun_detach_all(dev);
> >>>>>>> /* register_netdevice() already called
> >>>>>>> tun_free_netdev() */
> >>>>>>> goto err_free_dev;
> >>>>>>> err_free_flow:
> >>>>>>> + dev_put(dev);
> >>>>>>> tun_flow_uninit(tun);
> >>>>>>> security_tun_dev_free_security(tun->security);
> >>>>>>> err_free_stat:
> >>>>>>>
> >>>>>>> What's your thought?
> >>>>>>
> >>>>>> The dev pointer are freed without checking the refcount in
> >>>>>> free_netdev() called by err_free_dev
> >>>>>>
> >>>>>> path, so I don't understand how the refcount protects this pointer.
> >>>>>>
> >>>>>
> >>>>> The refcount are guaranteed to be zero there, isn't it?
> >>>> No, it's not.
> >>>>
> >>>> err_free_dev:
> >>>> free_netdev(dev);
> >>>>
> >>>> void free_netdev(struct net_device *dev)
> >>>> {
> >>>> ...
> >>>> /* pcpu_refcnt can be freed without checking refcount */
> >>>> free_percpu(dev->pcpu_refcnt);
> >>>> dev->pcpu_refcnt = NULL;
> >>>>
> >>>> /* Compatibility with error handling in drivers */
> >>>> if (dev->reg_state == NETREG_UNINITIALIZED) {
> >>>> /* dev can be freed without checking refcount */
> >>>> netdev_freemem(dev);
> >>>> return;
> >>>> }
> >>>> ...
> >>>> }
> >>>
> >>>
> >>> Right, but what I meant is in my patch, when code reaches
> >>> free_netdev() the refcnt is zero. What did I miss?
> >> Yes, but it can't fix the UAF problem.
> >
> >
> > Well, it looks to me that the dev_put() in tun_put() won't release the
> > device in this case.
>
> The device is not released in tun_put().
> This is how the UAF occurs:
>
> CPUA CPUB
> tun_set_iff()
> alloc_netdev_mqs()
> tun_attach()
> tun_chr_read_iter()
> tun_get()
> tun_do_read()
> tun_ring_recv()
> register_netdevice() <-- inject error
> goto err_detach
> tun_detach_all() <-- set RCV_SHUTDOWN
> free_netdev() <-- called from
> err_free_dev path
> netdev_freemem() <-- free the memory
> without check refcount
> (In this path, the refcount cannot prevent
> freeing the memory of dev, and the memory
> will be used by dev_put() called by
> tun_chr_read_iter() on CPUB.)
> (Break from
> tun_ring_recv(),
> because RCV_SHUTDOWN
> is set)
> tun_put()
> dev_put() <-- use the
> memory freed by
> netdev_freemem()
>
>
My bad, thanks for the patience. Since all evil come from the
tfile->tun, how about delay the publishing of tfile->tun until the
success of registration to make sure dev_put() and dev_hold() work.
(Compile test only)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index db16d7a13e00..aab0be40d443 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -787,7 +787,8 @@ static void tun_detach_all(struct net_device *dev)
}
static int tun_attach(struct tun_struct *tun, struct file *file,
- bool skip_filter, bool napi, bool napi_frags)
+ bool skip_filter, bool napi, bool napi_frags,
+ bool publish_tun)
{
struct tun_file *tfile = file->private_data;
struct net_device *dev = tun->dev;
@@ -870,7 +871,8 @@ static int tun_attach(struct tun_struct *tun, struct file *file,
* initialized tfile; otherwise we risk using half-initialized
* object.
*/
- rcu_assign_pointer(tfile->tun, tun);
+ if (publish_tun)
+ rcu_assign_pointer(tfile->tun, tun);
rcu_assign_pointer(tun->tfiles[tun->numqueues], tfile);
tun->numqueues++;
tun_set_real_num_queues(tun);
@@ -2730,7 +2732,7 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
err = tun_attach(tun, file, ifr->ifr_flags & IFF_NOFILTER,
ifr->ifr_flags & IFF_NAPI,
- ifr->ifr_flags & IFF_NAPI_FRAGS);
+ ifr->ifr_flags & IFF_NAPI_FRAGS, true);
if (err < 0)
return err;
@@ -2829,13 +2831,17 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
INIT_LIST_HEAD(&tun->disabled);
err = tun_attach(tun, file, false, ifr->ifr_flags & IFF_NAPI,
- ifr->ifr_flags & IFF_NAPI_FRAGS);
+ ifr->ifr_flags & IFF_NAPI_FRAGS, false);
if (err < 0)
goto err_free_flow;
err = register_netdevice(tun->dev);
if (err < 0)
goto err_detach;
+ /* free_netdev() won't check refcnt, to aovid race
+ * with dev_put() we need publish tun after registration.
+ */
+ rcu_assign_pointer(tfile->tun, tun);
}
netif_carrier_on(tun->dev);
@@ -2978,7 +2984,7 @@ static int tun_set_queue(struct file *file, struct ifreq *ifr)
if (ret < 0)
goto unlock;
ret = tun_attach(tun, file, false, tun->flags & IFF_NAPI,
- tun->flags & IFF_NAPI_FRAGS);
+ tun->flags & IFF_NAPI_FRAGS, true);
} else if (ifr->ifr_flags & IFF_DETACH_QUEUE) {
tun = rtnl_dereference(tfile->tun);
if (!tun || !(tun->flags & IFF_MULTI_QUEUE) || tfile->detached)
--
2.18.1
> >
> > Thanks
> >
>
>
>
^ permalink raw reply related
* Re: [PATCH] net: sched: taprio: Fix potential integer overflow in taprio_set_picos_per_byte
From: Vladimir Oltean @ 2019-09-03 10:12 UTC (permalink / raw)
To: Eric Dumazet
Cc: Gustavo A. R. Silva, Jamal Hadi Salim, Cong Wang, Jiri Pirko,
David S. Miller, netdev, lkml
In-Reply-To: <cb7d53cd-3f1e-146b-c1ab-f11a584a7224@gmail.com>
On Tue, 3 Sep 2019 at 10:19, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
>
> On 9/3/19 3:08 AM, Gustavo A. R. Silva wrote:
> > Add suffix LL to constant 1000 in order to avoid a potential integer
> > overflow and give the compiler complete information about the proper
> > arithmetic to use. Notice that this constant is being used in a context
> > that expects an expression of type s64, but it's currently evaluated
> > using 32-bit arithmetic.
> >
> > Addresses-Coverity-ID: 1453459 ("Unintentional integer overflow")
> > Fixes: f04b514c0ce2 ("taprio: Set default link speed to 10 Mbps in taprio_set_picos_per_byte")
> > Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> > ---
> > net/sched/sch_taprio.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
> > index 8d8bc2ec5cd6..956f837436ea 100644
> > --- a/net/sched/sch_taprio.c
> > +++ b/net/sched/sch_taprio.c
> > @@ -966,7 +966,7 @@ static void taprio_set_picos_per_byte(struct net_device *dev,
> >
> > skip:
> > picos_per_byte = div64_s64(NSEC_PER_SEC * 1000LL * 8,
> > - speed * 1000 * 1000);
> > + speed * 1000LL * 1000);
> >
> > atomic64_set(&q->picos_per_byte, picos_per_byte);
> > netdev_dbg(dev, "taprio: set %s's picos_per_byte to: %lld, linkspeed: %d\n",
> >
>
> But, why even multiplying by 1,000,000 in the first place, this seems silly,
> a standard 32 bit divide could be used instead.
>
> ->
>
> diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
> index 8d8bc2ec5cd6281d811fd5d8a5c5211ebb0edd73..944b1af3215668e927d486b6c6c65c4599fb9539 100644
> --- a/net/sched/sch_taprio.c
> +++ b/net/sched/sch_taprio.c
> @@ -965,8 +965,7 @@ static void taprio_set_picos_per_byte(struct net_device *dev,
> speed = ecmd.base.speed;
>
> skip:
> - picos_per_byte = div64_s64(NSEC_PER_SEC * 1000LL * 8,
> - speed * 1000 * 1000);
> + picos_per_byte = (USEC_PER_SEC * 8) / speed;
>
> atomic64_set(&q->picos_per_byte, picos_per_byte);
> netdev_dbg(dev, "taprio: set %s's picos_per_byte to: %lld, linkspeed: %d\n",
>
>
>
Right. And while we're at it, there's still the potential
division-by-zero problem which I still don't know how to solve without
implementing a full-blown __ethtool_get_link_ksettings parser that
checks against all the possible outputs it can have under the "no
carrier" condition - see "[RFC PATCH 1/1] phylink: Set speed to
SPEED_UNKNOWN when there is no PHY connected" for details.
And there's also a third fix to be made: the netdev_dbg should be made
to print "speed" instead of "ecmd.base.speed".
Thanks,
-Vladimir
^ permalink raw reply
* [PATCH net] tipc: add NULL pointer check before calling kfree_rcu
From: Xin Long @ 2019-09-03 9:53 UTC (permalink / raw)
To: network dev; +Cc: davem, Jon Maloy, Ying Xue, tipc-discussion
Unlike kfree(p), kfree_rcu(p, rcu) won't do NULL pointer check. When
tipc_nametbl_remove_publ returns NULL, the panic below happens:
BUG: unable to handle kernel NULL pointer dereference at 0000000000000068
RIP: 0010:__call_rcu+0x1d/0x290
Call Trace:
<IRQ>
tipc_publ_notify+0xa9/0x170 [tipc]
tipc_node_write_unlock+0x8d/0x100 [tipc]
tipc_node_link_down+0xae/0x1d0 [tipc]
tipc_node_check_dest+0x3ea/0x8f0 [tipc]
? tipc_disc_rcv+0x2c7/0x430 [tipc]
tipc_disc_rcv+0x2c7/0x430 [tipc]
? tipc_rcv+0x6bb/0xf20 [tipc]
tipc_rcv+0x6bb/0xf20 [tipc]
? ip_route_input_slow+0x9cf/0xb10
tipc_udp_recv+0x195/0x1e0 [tipc]
? tipc_udp_is_known_peer+0x80/0x80 [tipc]
udp_queue_rcv_skb+0x180/0x460
udp_unicast_rcv_skb.isra.56+0x75/0x90
__udp4_lib_rcv+0x4ce/0xb90
ip_local_deliver_finish+0x11c/0x210
ip_local_deliver+0x6b/0xe0
? ip_rcv_finish+0xa9/0x410
ip_rcv+0x273/0x362
Fixes: 97ede29e80ee ("tipc: convert name table read-write lock to RCU")
Reported-by: Li Shuang <shuali@redhat.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/tipc/name_distr.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c
index 44abc8e..241ed22 100644
--- a/net/tipc/name_distr.c
+++ b/net/tipc/name_distr.c
@@ -223,7 +223,8 @@ static void tipc_publ_purge(struct net *net, struct publication *publ, u32 addr)
publ->key);
}
- kfree_rcu(p, rcu);
+ if (p)
+ kfree_rcu(p, rcu);
}
/**
--
2.1.0
^ permalink raw reply related
* pull-request: can-next 2019-09-03
From: Marc Kleine-Budde @ 2019-09-03 9:46 UTC (permalink / raw)
To: netdev; +Cc: davem, kernel, linux-can
[-- Attachment #1.1: Type: text/plain, Size: 3307 bytes --]
Hello David,
this is a pull request for net-next/master consisting of 15 patches.
The first patch is by Christer Beskow, targets the kvaser_pciefd driver
and fixes the PWM generator's frequency.
The next three patches are by Dan Murphy, the tcan4x5x is updated to use
a proper interrupts/interrupt-parent DT binding to specify the devices
IRQ line. Further the unneeded wake ups of the device is removed from
the driver.
A patch by me for the mcp25xx driver removes the deprecated board file
setup example. Three patches by Andy Shevchenko simplify clock handling,
update the driver from OF to device property API and simplify the
mcp251x_can_suspend() function.
The remaining 7 patches are by me and clean up checkpatch warnings in
the generic CAN device infrastructure.
regards,
Marc
---
The following changes since commit 67538eb5c00f08d7fe27f1bb703098b17302bdc0:
Merge branch 'mvpp2-per-cpu-buffers' (2019-09-02 12:07:46 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git tags/linux-can-next-for-5.4-20190903
for you to fetch changes up to 13ecee77fa810b21beaf3023e921525c55f88b04:
can: dev: can_dev_init(): convert from printk(KERN_INFO) to pr_info (2019-09-03 10:28:13 +0200)
----------------------------------------------------------------
linux-can-next-for-5.4-20190903
----------------------------------------------------------------
Andy Shevchenko (3):
can: mcp251x: Use devm_clk_get_optional() to get the input clock
can: mcp251x: Make use of device property API
can: mcp251x: Call wrapper instead of regulator_disable()
Christer Beskow (1):
can: kvaser_pciefd: the PWM generator is running at the bus frequency of the system.
Dan Murphy (3):
dt-bindings: can: tcan4x5x: Update binding to use interrupt property
can: tcan4x5x: Remove data-ready gpio interrupt
can: tcan4x5x: Remove checking the wake pin
Marc Kleine-Budde (8):
can: mcp251x: remove deprecated board file setup example
can: dev: convert block comments to network style comments
can: dev: avoid long lines
can: dev: remove unnecessary parentheses
can: dev: remove unnecessary blank line
can: dev: can_restart(): convert NULL pointer check
can: dev: can_dellink(): remove return at end of void function
can: dev: can_dev_init(): convert from printk(KERN_INFO) to pr_info
.../devicetree/bindings/net/can/tcan4x5x.txt | 7 +-
drivers/net/can/dev.c | 131 +++++++++------------
drivers/net/can/kvaser_pciefd.c | 6 +-
drivers/net/can/m_can/tcan4x5x.c | 24 +---
drivers/net/can/spi/mcp251x.c | 68 +++--------
include/linux/can/dev.h | 3 +-
include/linux/can/rx-offload.h | 13 +-
7 files changed, 97 insertions(+), 155 deletions(-)
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |-
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: how to search for the best route from userspace in netlink?
From: Dave Taht @ 2019-09-03 9:29 UTC (permalink / raw)
To: David Ahern; +Cc: Linux Kernel Network Developers
In-Reply-To: <3e8fd488-1bd1-3213-6329-6baf8935a446@gmail.com>
On Mon, Sep 2, 2019 at 8:13 PM David Ahern <dsahern@gmail.com> wrote:
>
> On 9/2/19 4:07 PM, Dave Taht wrote:
> > Windows has the "RtmGetMostSpecificDestination" call:
> > https://docs.microsoft.com/en-us/windows/win32/rras/search-for-the-best-route
> >
> > In particular, I wanted to search for the best route, AND pick up the
> > PMTU from that (if it existed)
> > for older UDP applications like dnssec[1] and newer ones like QUIC[2].
>
> RTM_GETROUTE with data for the route lookup. See iproute2 code as an
> example.
Yes. I really didn't describe my thinking very well. It's coping with
pmtu better
in the case of a more increasingly udp'd and tunneled internet. tcp
(being kernel based)
will do the probe and cache that attribute of the path, udp does not.
A udp based app with root privs could be setting it after figuring it
out, a userspace one cannot.
for more detail from server-al sides of that philosophical debate,
please see the links I posted
originally.
--
Dave Täht
CTO, TekLibre, LLC
http://www.teklibre.com
Tel: 1-831-205-9740
^ permalink raw reply
* RE: [PATCH v4] rtw88: pci: Move a mass of jobs in hw IRQ to soft IRQ
From: Tony Chuang @ 2019-09-03 9:18 UTC (permalink / raw)
To: Jian-Hong Pan, Kalle Valo
Cc: David S . Miller, linux-wireless@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
linux@endlessm.com
In-Reply-To: <CAPpJ_efAxQN4pRdpVmT5Pdkp-6Y-QVOQdJR4iY4A-PXZokLGtA@mail.gmail.com>
> From: Jian-Hong Pan [mailto:jian-hong@endlessm.com]
>
> >
> > Tony Chuang <yhchuang@realtek.com> writes:
> >
> > >> From: Jian-Hong Pan
> > >> Subject: [PATCH v4] rtw88: pci: Move a mass of jobs in hw IRQ to soft
> IRQ
> > >>
> > >> There is a mass of jobs between spin lock and unlock in the hardware
> > >> IRQ which will occupy much time originally. To make system work more
> > >> efficiently, this patch moves the jobs to the soft IRQ (bottom half) to
> > >> reduce the time in hardware IRQ.
> > >>
> > >> Signed-off-by: Jian-Hong Pan <jian-hong@endlessm.com>
> > >
> > > Now it works fine with MSI interrupt enabled.
> > >
> > > But this patch is conflicting with MSI interrupt patch.
> > > Is there a better way we can make Kalle apply them more smoothly?
> > > I can rebase them and submit both if you're OK.
>
> The rebase work is appreciated.
>
Rebased and sent. Please check it and see if I've done anything wrong :)
https://patchwork.kernel.org/cover/11127453/
Thanks,
Yan-Hsuan
^ permalink raw reply
* Re: [PATCH net] ipmr: remove cache_resolve_queue_len
From: Nikolay Aleksandrov @ 2019-09-03 9:15 UTC (permalink / raw)
To: Hangbin Liu, netdev
Cc: Phil Karn, Sukumar Gopalakrishnan, David S . Miller,
Alexey Kuznetsov, Hideaki YOSHIFUJI
In-Reply-To: <20190903084359.13310-1-liuhangbin@gmail.com>
On 03/09/2019 11:43, Hangbin Liu wrote:
> This is a re-post of previous patch wrote by David Miller[1].
>
> Phil Karn reported[2] that on busy networks with lots of unresolved
> multicast routing entries, the creation of new multicast group routes
> can be extremely slow and unreliable.
>
> The reason is we hard-coded multicast route entries with unresolved source
> addresses(cache_resolve_queue_len) to 10. If some multicast route never
> resolves and the unresolved source addresses increased, there will
> be no ability to create new multicast route cache.
>
> To resolve this issue, we need either add a sysctl entry to make the
> cache_resolve_queue_len configurable, or just remove cache_resolve_queue_len
> directly, as we already have the socket receive queue limits of mrouted
> socket, pointed by David.
>
> From my side, I'd perfer to remove the cache_resolve_queue_len instead
> of creating two more(IPv4 and IPv6 version) sysctl entry.
>
> [1] https://lkml.org/lkml/2018/7/22/11
> [2] https://lkml.org/lkml/2018/7/21/343
>
> Reported-by: Phil Karn <karn@ka9q.net>
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
> include/linux/mroute_base.h | 2 --
> net/ipv4/ipmr.c | 27 ++++++++++++++++++---------
> net/ipv6/ip6mr.c | 10 +++-------
> 3 files changed, 21 insertions(+), 18 deletions(-)
>
Hi,
IMO this is definitely net-next material. A few more comments below.
Note that hosts will automatically have this limit lifted to about 270
entries with current defaults, some might be surprised if they have
higher limits set and could be left with queues allowing for thousands
of entries in a linked list.
> diff --git a/include/linux/mroute_base.h b/include/linux/mroute_base.h
> index 34de06b426ef..50fb89533029 100644
> --- a/include/linux/mroute_base.h
> +++ b/include/linux/mroute_base.h
> @@ -235,7 +235,6 @@ struct mr_table_ops {
> * @mfc_hash: Hash table of all resolved routes for easy lookup
> * @mfc_cache_list: list of resovled routes for possible traversal
> * @maxvif: Identifier of highest value vif currently in use
> - * @cache_resolve_queue_len: current size of unresolved queue
> * @mroute_do_assert: Whether to inform userspace on wrong ingress
> * @mroute_do_pim: Whether to receive IGMP PIMv1
> * @mroute_reg_vif_num: PIM-device vif index
> @@ -252,7 +251,6 @@ struct mr_table {
> struct rhltable mfc_hash;
> struct list_head mfc_cache_list;
> int maxvif;
> - atomic_t cache_resolve_queue_len;
> bool mroute_do_assert;
> bool mroute_do_pim;
> bool mroute_do_wrvifwhole;
> diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
> index c07bc82cbbe9..6c5278b45afb 100644
> --- a/net/ipv4/ipmr.c
> +++ b/net/ipv4/ipmr.c
> @@ -744,8 +744,6 @@ static void ipmr_destroy_unres(struct mr_table *mrt, struct mfc_cache *c)
> struct sk_buff *skb;
> struct nlmsgerr *e;
>
> - atomic_dec(&mrt->cache_resolve_queue_len);
> -
> while ((skb = skb_dequeue(&c->_c.mfc_un.unres.unresolved))) {
> if (ip_hdr(skb)->version == 0) {
> struct nlmsghdr *nlh = skb_pull(skb,
> @@ -1133,9 +1131,11 @@ static int ipmr_cache_unresolved(struct mr_table *mrt, vifi_t vifi,
> }
>
> if (!found) {
> + bool was_empty;
> +
> /* Create a new entry if allowable */
> - if (atomic_read(&mrt->cache_resolve_queue_len) >= 10 ||
> - (c = ipmr_cache_alloc_unres()) == NULL) {
> + c = ipmr_cache_alloc_unres();
> + if (!c) {
> spin_unlock_bh(&mfc_unres_lock);
>
> kfree_skb(skb);
> @@ -1161,11 +1161,11 @@ static int ipmr_cache_unresolved(struct mr_table *mrt, vifi_t vifi,
> return err;
> }
>
> - atomic_inc(&mrt->cache_resolve_queue_len);
> + was_empty = list_empty(&mrt->mfc_unres_queue);
> list_add(&c->_c.list, &mrt->mfc_unres_queue);
> mroute_netlink_event(mrt, c, RTM_NEWROUTE);
>
> - if (atomic_read(&mrt->cache_resolve_queue_len) == 1)
> + if (was_empty)
> mod_timer(&mrt->ipmr_expire_timer,
> c->_c.mfc_un.unres.expires);
> }
> @@ -1272,7 +1272,6 @@ static int ipmr_mfc_add(struct net *net, struct mr_table *mrt,
> if (uc->mfc_origin == c->mfc_origin &&
> uc->mfc_mcastgrp == c->mfc_mcastgrp) {
> list_del(&_uc->list);
> - atomic_dec(&mrt->cache_resolve_queue_len);
> found = true;
> break;
> }
> @@ -1328,7 +1327,7 @@ static void mroute_clean_tables(struct mr_table *mrt, int flags)
> }
>
> if (flags & MRT_FLUSH_MFC) {
> - if (atomic_read(&mrt->cache_resolve_queue_len) != 0) {
> + if (!list_empty(&mrt->mfc_unres_queue)) {
> spin_lock_bh(&mfc_unres_lock);
> list_for_each_entry_safe(c, tmp, &mrt->mfc_unres_queue, list) {
> list_del(&c->list);
> @@ -2750,9 +2749,19 @@ static int ipmr_rtm_route(struct sk_buff *skb, struct nlmsghdr *nlh,
> return ipmr_mfc_delete(tbl, &mfcc, parent);
> }
>
> +static int queue_count(struct mr_table *mrt)
> +{
> + struct list_head *pos;
> + int count = 0;
> +
> + list_for_each(pos, &mrt->mfc_unres_queue)
> + count++;
> + return count;
> +}
I don't think we hold the mfc_unres_lock here while walking
the unresolved list below in ipmr_fill_table().
> +
> static bool ipmr_fill_table(struct mr_table *mrt, struct sk_buff *skb)
> {
> - u32 queue_len = atomic_read(&mrt->cache_resolve_queue_len);
> + u32 queue_len = queue_count(mrt);
>
> if (nla_put_u32(skb, IPMRA_TABLE_ID, mrt->id) ||
> nla_put_u32(skb, IPMRA_TABLE_CACHE_RES_QUEUE_LEN, queue_len) ||
> diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
> index e80d36c5073d..d02f0f903559 100644
> --- a/net/ipv6/ip6mr.c
> +++ b/net/ipv6/ip6mr.c
> @@ -768,8 +768,6 @@ static void ip6mr_destroy_unres(struct mr_table *mrt, struct mfc6_cache *c)
> struct net *net = read_pnet(&mrt->net);
> struct sk_buff *skb;
>
> - atomic_dec(&mrt->cache_resolve_queue_len);
> -
> while ((skb = skb_dequeue(&c->_c.mfc_un.unres.unresolved)) != NULL) {
> if (ipv6_hdr(skb)->version == 0) {
> struct nlmsghdr *nlh = skb_pull(skb,
> @@ -1148,8 +1146,8 @@ static int ip6mr_cache_unresolved(struct mr_table *mrt, mifi_t mifi,
> * Create a new entry if allowable
> */
>
> - if (atomic_read(&mrt->cache_resolve_queue_len) >= 10 ||
> - (c = ip6mr_cache_alloc_unres()) == NULL) {
> + c = ip6mr_cache_alloc_unres();
> + if (!c) {
> spin_unlock_bh(&mfc_unres_lock);
>
> kfree_skb(skb);
> @@ -1176,7 +1174,6 @@ static int ip6mr_cache_unresolved(struct mr_table *mrt, mifi_t mifi,
> return err;
> }
>
> - atomic_inc(&mrt->cache_resolve_queue_len);
> list_add(&c->_c.list, &mrt->mfc_unres_queue);
> mr6_netlink_event(mrt, c, RTM_NEWROUTE);
>
> @@ -1468,7 +1465,6 @@ static int ip6mr_mfc_add(struct net *net, struct mr_table *mrt,
> if (ipv6_addr_equal(&uc->mf6c_origin, &c->mf6c_origin) &&
> ipv6_addr_equal(&uc->mf6c_mcastgrp, &c->mf6c_mcastgrp)) {
> list_del(&_uc->list);
> - atomic_dec(&mrt->cache_resolve_queue_len);
> found = true;
> break;
> }
> @@ -1526,7 +1522,7 @@ static void mroute_clean_tables(struct mr_table *mrt, int flags)
> }
>
> if (flags & MRT6_FLUSH_MFC) {
> - if (atomic_read(&mrt->cache_resolve_queue_len) != 0) {
> + if (!list_empty(&mrt->mfc_unres_queue)) {
> spin_lock_bh(&mfc_unres_lock);
> list_for_each_entry_safe(c, tmp, &mrt->mfc_unres_queue, list) {
> list_del(&c->list);
>
^ permalink raw reply
* Re: [PATCH v5 04/17] MIPS: PCI: refactor ioc3 special handling
From: Paul Burton @ 2019-09-03 9:10 UTC (permalink / raw)
To: Thomas Bogendoerfer
Cc: Jonathan Corbet, Ralf Baechle, Paul Burton, James Hogan,
Dmitry Torokhov, Lee Jones, David S. Miller, Srinivas Kandagatla,
Alessandro Zummo, Alexandre Belloni, Greg Kroah-Hartman,
Jiri Slaby, Evgeniy Polyakov, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-mips@vger.kernel.org,
linux-input@vger.kernel.org, netdev@vger.kernel.org,
linux-rtc@vger.kernel.org, linux-serial@vger.kernel.org,
linux-mips@vger.kernel.org
In-Reply-To: <20190819163144.3478-5-tbogendoerfer@suse.de>
Hello,
Thomas Bogendoerfer wrote:
> Refactored code to only have one ioc3 special handling for read
> access and one for write access.
Applied to mips-next.
> commit 813cafc4109c
> https://git.kernel.org/mips/c/813cafc4109c
>
> Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>
> Signed-off-by: Paul Burton <paul.burton@mips.com>
Thanks,
Paul
[ This message was auto-generated; if you believe anything is incorrect
then please email paul.burton@mips.com to report it. ]
^ permalink raw reply
* RE: Proposal: r8152 firmware patching framework
From: Bambi Yeh @ 2019-09-03 9:01 UTC (permalink / raw)
To: Hayes Wang, Amber Chen, Prashant Malani
Cc: David Miller, netdev@vger.kernel.org, Ryankao, Jackc, Albertk,
marcochen@google.com, nic_swsd, Grant Grundler
In-Reply-To: <0835B3720019904CB8F7AA43166CEEB2F18DA7A9@RTITMBSVM03.realtek.com.tw>
Hi Prashant:
We will try to implement your requests.
Based on our experience, upstream reviewer often reject our modification if they have any concern.
Do you think you can talk to them about this idea and see if they will accept it or not?
Or if you can help on this after we submit it?
Also, Hayes is now updating our current upstream driver and it goes back and forth for a while.
So we will need some time to finish it and the target schedule to have your request done is in the end of this month.
Thank you very much.
Best Regards,
Bambi Yeh
-----Original Message-----
From: Hayes Wang <hayeswang@realtek.com>
Sent: Monday, September 2, 2019 2:31 PM
To: Amber Chen <amber.chen@realtek.com>; Prashant Malani <pmalani@chromium.org>
Cc: David Miller <davem@davemloft.net>; netdev@vger.kernel.org; Bambi Yeh <bambi.yeh@realtek.com>; Ryankao <ryankao@realtek.com>; Jackc <jackc@realtek.com>; Albertk <albertk@realtek.com>; marcochen@google.com; nic_swsd <nic_swsd@realtek.com>; Grant Grundler <grundler@chromium.org>
Subject: RE: Proposal: r8152 firmware patching framework
Prashant Malani <pmalani@chromium.org>
> >
> > (Adding a few more Realtek folks)
> >
> > Friendly ping. Any thoughts / feedback, Realtek folks (and others) ?
> >
> >> On Thu, Aug 29, 2019 at 11:40 AM Prashant Malani
> <pmalani@chromium.org> wrote:
> >>
> >> Hi,
> >>
> >> The r8152 driver source code distributed by Realtek (on
> >> www.realtek.com) contains firmware patches. This involves binary
> >> byte-arrays being written byte/word-wise to the hardware memory
> >> Example: grundler@chromium.org (cc-ed) has an experimental patch
> which
> >> includes the firmware patching code which was distributed with the
> >> Realtek source :
> >>
> https://chromium-review.googlesource.com/c/chromiumos/third_party/kern
> el
> /+/1417953
> >>
> >> It would be nice to have a way to incorporate these firmware fixes
> >> into the upstream code. Since having indecipherable byte-arrays is
> >> not possible upstream, I propose the following:
> >> - We use the assistance of Realtek to come up with a format which
> >> the firmware patch files can follow (this can be documented in the
> >> comments).
> >> - A real simple format could look like this:
> >> +
> >>
> <section1><size_in_bytes><address1><data1><address2><data2>...<address
> N
> ><dataN><section2>...
> >> + The driver would be able to understand how to
> >> parse each section (e.g is each data entry a byte or a word?)
> >>
> >> - We use request_firmware() to load the firmware, parse it and
> >> write the data to the relevant registers.
I plan to finish the patches which I am going to submit, first. Then, I could focus on this. However, I don't think I would start this quickly. There are many preparations and they would take me a lot of time.
Best Regards,
Hayes
^ permalink raw reply
* Re: [PATCH 4.14] tcp: fix tcp_rtx_queue_tail in case of empty retransmit queue
From: maowenan @ 2019-09-03 8:55 UTC (permalink / raw)
To: Tim Froidcoeur, eric.dumazet
Cc: David Miller, cpaasch@apple.com, jonathan.lemon@gmail.com,
stable@vger.kernel.org, gregkh@linuxfoundation.org,
matthieu.baerts@tessares.net, aprout@ll.mit.edu,
edumazet@google.com, jtl@netflix.com,
linux-kernel@vger.kernel.org, mkubecek@suse.cz,
ncardwell@google.com, sashal@kernel.org, ycheng@google.com,
netdev@vger.kernel.org
In-Reply-To: <CAOj+RUvXMaoVKzSeDab4oTn3p=-BJtuhgqwKDCUuhCQWHO7bgQ@mail.gmail.com>
On 2019/9/3 14:58, Tim Froidcoeur wrote:
> Hi,
>
> I also tried to reproduce this in a targeted way, and run into the
> same difficulty as you: satisfying the first condition “
> (sk->sk_wmem_queued >> 1) > limit “.
> I will not have bandwidth the coming days to try and reproduce it in
> this way. Maybe simply forcing a very small send buffer using sysctl
> net.ipv4.tcp_wmem might even do the trick?
>
> I suspect that the bug is easier to trigger with the MPTCP patch like
> I did originally, due to the way this patch manages the tcp subflow
> buffers (it can temporarily overfill the buffers, satisfying that
> first condition more often).
>
> another thing, the stacktrace you shared before seems caused by
> another issue (corrupted socket?), it will not be solved by the patch
> we submitted.
The trace shows zero window probe message can be BUG_ON in skb_queue_prev,
this is reproduced on our platform with syzkaller. It can be resolved by
your fix patch.
The thing I need to think is why the first condition can be satisfied?
Eric, Do you have any comments to reproduce it as the first condition
is hard to be true?
(sk->sk_wmem_queued >> 1) > limit
>
> kind regards,
>
> Tim
>
>
> On Tue, Sep 3, 2019 at 5:22 AM maowenan <maowenan@huawei.com> wrote:
>>
>> Hi Tim,
>>
>>
>>
>> I try to reproduce it with packetdrill or user application, but I can’t.
>>
>> The first condition “ (sk->sk_wmem_queued >> 1) > limit “ can’t be satisfied,
>>
>> This condition is to avoid tiny SO_SNDBUF values set by user.
>>
>> It also adds the some room due to the fact that tcp_sendmsg()
>>
>> and tcp_sendpage() might overshoot sk_wmem_queued by about one full
>>
>> TSO skb (64KB size).
>>
>>
>>
>> limit = sk->sk_sndbuf + 2 * SKB_TRUESIZE(GSO_MAX_SIZE);
>>
>> if (unlikely((sk->sk_wmem_queued >> 1) > limit &&
>>
>> skb != tcp_rtx_queue_head(sk) &&
>>
>> skb != tcp_rtx_queue_tail(sk))) {
>>
>> NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPWQUEUETOOBIG);
>>
>> return -ENOMEM;
>>
>> }
>>
>>
>>
>> Can you try to reproduce it with packetdrill or C socket application?
>>
>>
>
>
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox