Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] rtl8xxxu: add bluetooth co-existence support for single antenna
From: Kalle Valo @ 2019-09-06  7:17 UTC (permalink / raw)
  To: Chris Chiu
  Cc: Jes Sorensen, David Miller, linux-wireless, netdev, Linux Kernel,
	Linux Upstreaming Team
In-Reply-To: <CAB4CAwc5OBUWFThh__FedmG=fR-_1_GxUuiAb0J5yfU8c1aTfg@mail.gmail.com>

Chris Chiu <chiu@endlessm.com> writes:

> Gentle ping. Cheers.

Please edit your quotes. Including the full patch in quotes makes my use
of patchwork horrible:

https://patchwork.kernel.org/patch/11127227/

-- 
Kalle Valo

^ permalink raw reply

* Re: [iproute2, master 1/2] devlink: Print health reporter's dump time-stamp in a helper function
From: Aya Levin @ 2019-09-06  7:17 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev@vger.kernel.org, Jiri Pirko, Moshe Shemesh
In-Reply-To: <20190829162533.25606191@hermes.lan>



On 8/30/2019 2:25 AM, Stephen Hemminger wrote:
> On Thu, 22 Aug 2019 14:05:41 +0300
> Aya Levin <ayal@mellanox.com> wrote:
> 
>> Add pr_out_dump_reporter prefix to the helper function's name and
>> encapsulate the print in it.
>>
>> Fixes: 2f1242efe9d0 ("devlink: Add devlink health show command")
>> Signed-off-by: Aya Levin <ayal@mellanox.com>
>> Acked-by: Jiri Pirko <jiri@mellanox.com>
> 
> 
> Looks fine, but devlink needs to be converted from doing JSON
> printing its own way and use common iproute2 libraries.
Sorry for the late response.
You are correct, it is in our plans to complete a full transition to 
common iproute2 helpers in the following weeks.
I got your point, and will not submit more patches adding new print 
functions using the current API and will wait for submission after the 
transition to the common API.
This patch will be re-submitted after the transition.
> 

^ permalink raw reply

* Re: [PATCH] net: Remove the source address setting in connect() for UDP
From: David Miller @ 2019-09-06  7:13 UTC (permalink / raw)
  To: enkechen; +Cc: kuznet, yoshfuji, netdev, linux-kernel, xe-linux-external
In-Reply-To: <20190906025437.613-1-enkechen@cisco.com>

From: Enke Chen <enkechen@cisco.com>
Date: Thu,  5 Sep 2019 19:54:37 -0700

> The connect() system call for a UDP socket is for setting the destination
> address and port. But the current code mistakenly sets the source address
> for the socket as well. Remove the source address setting in connect() for
> UDP in this patch.

Do you have any idea how many decades of precedence this behavior has and
therefore how much you potentially will break userspace?

This boat has sailed a long time ago I'm afraid.

^ permalink raw reply

* Re: [PATCH V2 4/4] crypto: Add Xilinx AES driver
From: Corentin Labbe @ 2019-09-06  7:04 UTC (permalink / raw)
  To: Kalyani Akula
  Cc: herbert@gondor.apana.org.au, kstewart@linuxfoundation.org,
	gregkh@linuxfoundation.org, tglx@linutronix.de,
	pombredanne@nexb.com, linux-crypto@vger.kernel.org,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	Sarat Chand Savitala
In-Reply-To: <BN7PR02MB512445C31936CED70F02D15AAFB80@BN7PR02MB5124.namprd02.prod.outlook.com>

On Wed, Sep 04, 2019 at 05:40:22PM +0000, Kalyani Akula wrote:
> Hi Corentin,
> 
> Thanks for the review comments.
> Please find my response/queries inline.
> 
> > -----Original Message-----
> > From: Corentin Labbe <clabbe.montjoie@gmail.com>
> > Sent: Monday, September 2, 2019 12:29 PM
> > To: Kalyani Akula <kalyania@xilinx.com>
> > Cc: herbert@gondor.apana.org.au; kstewart@linuxfoundation.org;
> > gregkh@linuxfoundation.org; tglx@linutronix.de; pombredanne@nexb.com;
> > linux-crypto@vger.kernel.org; linux-kernel@vger.kernel.org;
> > netdev@vger.kernel.org; Kalyani Akula <kalyania@xilinx.com>
> > Subject: Re: [PATCH V2 4/4] crypto: Add Xilinx AES driver
> > 
> > On Sun, Sep 01, 2019 at 07:24:58PM +0530, Kalyani Akula wrote:
> > > This patch adds AES driver support for the Xilinx ZynqMP SoC.
> > >
> > > Signed-off-by: Kalyani Akula <kalyani.akula@xilinx.com>
> > > ---
> > 
> > Hello
> > 
> > I have some comment below
> > 
> > >  drivers/crypto/Kconfig          |  11 ++
> > >  drivers/crypto/Makefile         |   1 +
> > >  drivers/crypto/zynqmp-aes-gcm.c | 297
> > > ++++++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 309 insertions(+)
> > >  create mode 100644 drivers/crypto/zynqmp-aes-gcm.c
> > >
> > > diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig index
> > > 603413f..a0d058a 100644
> > > --- a/drivers/crypto/Kconfig
> > > +++ b/drivers/crypto/Kconfig
> > > @@ -677,6 +677,17 @@ config CRYPTO_DEV_ROCKCHIP
> > >  	  This driver interfaces with the hardware crypto accelerator.
> > >  	  Supporting cbc/ecb chainmode, and aes/des/des3_ede cipher mode.
> > >
> > > +config CRYPTO_DEV_ZYNQMP_AES
> > > +	tristate "Support for Xilinx ZynqMP AES hw accelerator"
> > > +	depends on ARCH_ZYNQMP || COMPILE_TEST
> > > +	select CRYPTO_AES
> > > +	select CRYPTO_SKCIPHER
> > > +	help
> > > +	  Xilinx ZynqMP has AES-GCM engine used for symmetric key
> > > +	  encryption and decryption. This driver interfaces with AES hw
> > > +	  accelerator. Select this if you want to use the ZynqMP module
> > > +	  for AES algorithms.
> > > +
> > >  config CRYPTO_DEV_MEDIATEK
> > >  	tristate "MediaTek's EIP97 Cryptographic Engine driver"
> > >  	depends on (ARM && ARCH_MEDIATEK) || COMPILE_TEST diff --git
> > > a/drivers/crypto/Makefile b/drivers/crypto/Makefile index
> > > afc4753..c99663a 100644
> > > --- a/drivers/crypto/Makefile
> > > +++ b/drivers/crypto/Makefile
> > > @@ -48,3 +48,4 @@ obj-$(CONFIG_CRYPTO_DEV_BCM_SPU) += bcm/
> > >  obj-$(CONFIG_CRYPTO_DEV_SAFEXCEL) += inside-secure/
> > >  obj-$(CONFIG_CRYPTO_DEV_ARTPEC6) += axis/  obj-y += hisilicon/
> > > +obj-$(CONFIG_CRYPTO_DEV_ZYNQMP_AES) += zynqmp-aes-gcm.o
> > > diff --git a/drivers/crypto/zynqmp-aes-gcm.c
> > > b/drivers/crypto/zynqmp-aes-gcm.c new file mode 100644 index
> > > 0000000..d65f038
> > > --- /dev/null
> > > +++ b/drivers/crypto/zynqmp-aes-gcm.c
> > > @@ -0,0 +1,297 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Xilinx ZynqMP AES Driver.
> > > + * Copyright (c) 2019 Xilinx Inc.
> > > + */
> > > +
> > > +#include <crypto/aes.h>
> > > +#include <crypto/scatterwalk.h>
> > > +#include <linux/kernel.h>
> > > +#include <linux/module.h>
> > > +#include <linux/of_device.h>
> > > +#include <linux/scatterlist.h>
> > > +#include <linux/firmware/xlnx-zynqmp.h>
> > > +
> > > +#define ZYNQMP_AES_IV_SIZE			12
> > > +#define ZYNQMP_AES_GCM_SIZE			16
> > > +#define ZYNQMP_AES_KEY_SIZE			32
> > > +
> > > +#define ZYNQMP_AES_DECRYPT			0
> > > +#define ZYNQMP_AES_ENCRYPT			1
> > > +
> > > +#define ZYNQMP_AES_KUP_KEY			0
> > > +#define ZYNQMP_AES_DEVICE_KEY			1
> > > +#define ZYNQMP_AES_PUF_KEY			2
> > > +
> > > +#define ZYNQMP_AES_GCM_TAG_MISMATCH_ERR		0x01
> > > +#define ZYNQMP_AES_SIZE_ERR			0x06
> > > +#define ZYNQMP_AES_WRONG_KEY_SRC_ERR		0x13
> > > +#define ZYNQMP_AES_PUF_NOT_PROGRAMMED		0xE300
> > > +
> > > +#define ZYNQMP_AES_BLOCKSIZE			0x04
> > > +
> > > +static const struct zynqmp_eemi_ops *eemi_ops; struct zynqmp_aes_dev
> > > +*aes_dd;
> > 
> > I still think that using a global variable for storing device driver data is bad.
> 
> I think storing the list of dd's would solve up the issue with global variable, but there is only one AES instance here.
> Please suggest
> 

Look what I do for amlogic driver https://patchwork.kernel.org/patch/11059633/.
I store the device driver in the instatiation of a crypto template.

[...]
> > > +static int zynqmp_setkey_blk(struct crypto_tfm *tfm, const u8 *key,
> > > +			     unsigned int len)
> > > +{
> > > +	struct zynqmp_aes_op *op = crypto_tfm_ctx(tfm);
> > > +
> > > +	if (((len != 1) && (len !=  ZYNQMP_AES_KEY_SIZE)) || (!key))
> > 
> > typo, two space
> 
> Will fix in the next version
> 
> > 
> > > +		return -EINVAL;
> > > +
> > > +	if (len == 1) {
> > > +		op->keytype = *key;
> > > +
> > > +		if ((op->keytype < ZYNQMP_AES_KUP_KEY) ||
> > > +			(op->keytype > ZYNQMP_AES_PUF_KEY))
> > > +			return -EINVAL;
> > > +
> > > +	} else if (len == ZYNQMP_AES_KEY_SIZE) {
> > > +		op->keytype = ZYNQMP_AES_KUP_KEY;
> > > +		op->keylen = len;
> > > +		memcpy(op->key, key, len);
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > 
> > It seems your driver does not support AES keysize of 128/196, you need to
> > fallback in that case.
> 
> [Kalyani] In case of 128/196 keysize, returning the error would suffice ?
> Or still algorithm need to work ?
> If error is enough, it is taken care by this condition 
> if (((len != 1) && (len !=  ZYNQMP_AES_KEY_SIZE)) || (!key))

I think this problem just simply show us that your driver is not tested against crypto selftest.
I have tried to refuse 128/196 in my driver and I get:
alg: skcipher: cbc-aes-sun8i-ce setkey failed on test vector 0; expected_error=0, actual_error=-22, flags=0x1

So if your hardware lack 128/196 keys support, you must fallback to a software version.

Anyway please test your driver with crypto selftest enabled (and also CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y)

Regards

^ permalink raw reply

* Re: [PATCH net] net: gso: Fix skb_segment splat when splitting gso_size mangled skb having linear-headed frag_list
From: Shmulik Ladkani @ 2019-09-06  6:47 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: Daniel Borkmann, Eric Dumazet, eyal, netdev, Shmulik Ladkani,
	Alexander Duyck
In-Reply-To: <CAF=yD-J9Ax9f7BsGBFAaG=QU6CPVw6sSzBkZJOHRW-m6o49oyw@mail.gmail.com>

On Thu, 5 Sep 2019 17:51:20 -0400
Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote:

> On Thu, Sep 5, 2019 at 2:36 PM Shmulik Ladkani <shmulik@metanetworks.com> wrote:
> >
> > +       if (mss != GSO_BY_FRAGS &&
> > +           (skb_shinfo(head_skb)->gso_type & SKB_GSO_DODGY)) {
> > +               /* gso_size is untrusted.
> > +                *
> > +                * If head_skb has a frag_list with a linear non head_frag
> > +                * item, and head_skb's headlen does not fit requested
> > +                * gso_size, fall back to copying the skbs - by disabling sg.
> > +                *
> > +                * We assume checking the first frag suffices, i.e if either of
> > +                * the frags have non head_frag data, then the first frag is
> > +                * too.
> > +                */
> > +               if (list_skb && skb_headlen(list_skb) && !list_skb->head_frag &&
> > +                   (mss != skb_headlen(head_skb) - doffset)) {  
> 
> I thought the idea was to check skb_headlen(list_skb), as that is the
> cause of the problem. Is skb_headlen(head_skb) a good predictor of
> that? I can certainly imagine that it is, just not sure.

Yes, 'mss != skb_headlen(HEAD_SKB)' seems to be a very good predictor,
both for the test reproducer, and what's observered on a live system.

We *CANNOT* use 'mss != skb_headlen(LIST_SKB)' as the test condition.
The packet could have just a SINGLE frag_list member, and that member could
be a "small remainder" not reaching the full mss size - so we could hit
the test condition EVEN FOR NON gso_size mangled frag_list skbs -
which is not desired.

Also, is we test 'mss != skb_headlen(list_skb)' and execute 'sg=false'
ONLY IF 'list_skb' is *NOT* the last item, this is still bogus.
Imagine a gso_size mangled packet having just head_skb and a single
"small remainder" frag. This packet will hit the BUG_ON, as the
'sg=false' solution is now skipped according to the revised condition.

> Thanks for preparing the patch, and explaining the problem and
> solution clearly in the commit message. I'm pretty sure I'll have
> forgotten the finer details next time we have to look at this
> function again.

Indeed. Apparently I've been there myself few years back and forgot all
the gritty details :) see [0]

[0] https://patchwork.ozlabs.org/patch/661419/ 

^ permalink raw reply

* Re: [PATCH net] net: gso: Fix skb_segment splat when splitting gso_size mangled skb having linear-headed frag_list
From: Shmulik Ladkani @ 2019-09-06  6:20 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Daniel Borkmann, Eric Dumazet, Willem de Bruijn, eyal, netdev,
	Shmulik Ladkani
In-Reply-To: <CAKgT0Uf-OvKKycJz7aTZ93J=RdUuwd=SFS9C9pTngieDxe0uYQ@mail.gmail.com>

On Thu, 5 Sep 2019 14:49:44 -0700
Alexander Duyck <alexander.duyck@gmail.com> wrote:

> I would change the order of the tests you use here so that we can
> eliminate the possibility of needing to perform many tests for the
> more common cases. You could probably swap "list_skb" and "mss !=
> GSO_BY_FRAGS" since list_skb is more likely to be false for many of
> the common cases such as a standard TSO send from a socket. You might
> even consider moving the GSO_BY_FRAGS check toward the end of your
> checks since SCTP is the only protocol that I believe uses it and the
> likelihood of encountering it is much lower compared to other
> protocols.
> 
> You could probably test for !list_skb->head_frag before seeing if
> there is a headlen since many NICs would be generating frames using
> head_frag, so in the GRO case you mentioned above it could probably
> save you some effort on a number of NICs.
> 
> You might also consider moving this code up before we push the mac
> header back on and instead of setting sg to false you could just clear
> the NETIF_F_SG flag from features. It would save you from having to
> then remove doffset in your last check.

Thanks Alexander for the input. Will encorporate as much as possible
into a v2 patch.

BTW, I've strugged with myself regarding order of tests, and came
up with this suggestion, as my motivation was to have the tests order
tell a coherent logical story when read top-to-bottom left-to-right, FWIW.
For example, although 'mss != skb_headlen(head_skb)' could be tested
earlier, the condition by itself isn't meaningful unless we have an
existing frag_list and with a !head_frag.

Best
Shmulik

^ permalink raw reply

* Re: [BACKPORT 4.14.y v2 5/6] ppp: mppe: Revert "ppp: mppe: Add softdep to arc4"
From: Baolin Wang @ 2019-09-06  6:13 UTC (permalink / raw)
  To: Baolin Wang, # 3.4.x, paulus, linux-ppp, Networking,
	Arnd Bergmann, Orson Zhai, Vincent Guittot, LKML
In-Reply-To: <20190905161642.GA5659@google.com>

On Fri, 6 Sep 2019 at 00:16, Eric Biggers <ebiggers@google.com> wrote:
>
> On Thu, Sep 05, 2019 at 11:10:45AM +0800, Baolin Wang wrote:
> > From: Eric Biggers <ebiggers@google.com>
> >
> > [Upstream commit 25a09ce79639a8775244808c17282c491cff89cf]
> >
> > Commit 0e5a610b5ca5 ("ppp: mppe: switch to RC4 library interface"),
> > which was merged through the crypto tree for v5.3, changed ppp_mppe.c to
> > use the new arc4_crypt() library function rather than access RC4 through
> > the dynamic crypto_skcipher API.
> >
> > Meanwhile commit aad1dcc4f011 ("ppp: mppe: Add softdep to arc4") was
> > merged through the net tree and added a module soft-dependency on "arc4".
> >
> > The latter commit no longer makes sense because the code now uses the
> > "libarc4" module rather than "arc4", and also due to the direct use of
> > arc4_crypt(), no module soft-dependency is required.
> >
> > So revert the latter commit.
> >
> > Cc: Takashi Iwai <tiwai@suse.de>
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > Signed-off-by: David S. Miller <davem@davemloft.net>
> > Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> > ---
> >  drivers/net/ppp/ppp_mppe.c |    1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/net/ppp/ppp_mppe.c b/drivers/net/ppp/ppp_mppe.c
> > index d9eda7c..6c7fd98 100644
> > --- a/drivers/net/ppp/ppp_mppe.c
> > +++ b/drivers/net/ppp/ppp_mppe.c
> > @@ -63,7 +63,6 @@
> >  MODULE_DESCRIPTION("Point-to-Point Protocol Microsoft Point-to-Point Encryption support");
> >  MODULE_LICENSE("Dual BSD/GPL");
> >  MODULE_ALIAS("ppp-compress-" __stringify(CI_MPPE));
> > -MODULE_SOFTDEP("pre: arc4");
>
> Why is this being backported?  This revert was only needed because of a
> different patch that was merged in v5.3, as I explained in the commit message.

Sorry I missed this. I should remove this patch from our product kernel too.

-- 
Baolin Wang
Best Regards

^ permalink raw reply

* Re: [patch net-next] net: fib_notifier: move fib_notifier_ops from struct net into per-net struct
From: Jiri Pirko @ 2019-09-06  6:05 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, davem, idosch, dsahern, mlxsw
In-Reply-To: <bb24e9d5-24c6-d590-e490-be2226016288@gmail.com>

Fri, Sep 06, 2019 at 07:54:39AM CEST, eric.dumazet@gmail.com wrote:
>
>
>On 9/5/19 8:06 PM, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>> 
>> No need for fib_notifier_ops to be in struct net. It is used only by
>> fib_notifier as a private data. Use net_generic to introduce per-net
>> fib_notifier struct and move fib_notifier_ops there.
>> 
>>
>
>...
>
>>  static struct pernet_operations fib_notifier_net_ops = {
>>  	.init = fib_notifier_net_init,
>>  	.exit = fib_notifier_net_exit,
>> +	.id = &fib_notifier_net_id,
>> +	.size = sizeof(struct fib_notifier_net),
>>  };
>>  
>>  static int __init fib_notifier_init(void)
>> 
>
>Note that this will allocate a block of memory (in ops_init()) to hold this,
>plus a second one to hold the pointer to this block.
>
>Due to kmalloc() constraints, this block will use more memory.

I'm aware. But we have net_generic for exactly this purpose not to
pullute struct net.


>
>Not sure your patch is a win, since it makes things a bit more complex.
>
>Is it a preparation patch so that you can add later other fields in struct fib_notifier_net ?

Yes.


>

^ permalink raw reply

* Re: [patch net-next] net: fib_notifier: move fib_notifier_ops from struct net into per-net struct
From: Eric Dumazet @ 2019-09-06  5:54 UTC (permalink / raw)
  To: Jiri Pirko, netdev; +Cc: davem, idosch, dsahern, mlxsw
In-Reply-To: <20190905180656.4756-1-jiri@resnulli.us>



On 9/5/19 8:06 PM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
> 
> No need for fib_notifier_ops to be in struct net. It is used only by
> fib_notifier as a private data. Use net_generic to introduce per-net
> fib_notifier struct and move fib_notifier_ops there.
> 
>

...

>  static struct pernet_operations fib_notifier_net_ops = {
>  	.init = fib_notifier_net_init,
>  	.exit = fib_notifier_net_exit,
> +	.id = &fib_notifier_net_id,
> +	.size = sizeof(struct fib_notifier_net),
>  };
>  
>  static int __init fib_notifier_init(void)
> 

Note that this will allocate a block of memory (in ops_init()) to hold this,
plus a second one to hold the pointer to this block.

Due to kmalloc() constraints, this block will use more memory.

Not sure your patch is a win, since it makes things a bit more complex.

Is it a preparation patch so that you can add later other fields in struct fib_notifier_net ?


^ permalink raw reply

* Re: [PATCH v2 2/2] PTP: add support for one-shot output
From: Richard Cochran @ 2019-09-06  5:35 UTC (permalink / raw)
  To: Felipe Balbi; +Cc: Christopher S Hall, netdev, linux-kernel, davem
In-Reply-To: <87h85roy9p.fsf@gmail.com>

On Thu, Sep 05, 2019 at 01:03:46PM +0300, Felipe Balbi wrote:
> This a bit confusing, really. Specially when the comment right above
> those flags states:
> 
> /* PTP_xxx bits, for the flags field within the request structures. */

Agreed, it is confusing.  Go ahead and remove this comment.

> Seems like we will, at least, make it clear which flags are valid for
> which request structures.

Yes, please do make it as clear as you can.

Thanks,
Richard

^ permalink raw reply

* Re: [PATCH] net-ipv6: addrconf_f6i_alloc - fix non-null pointer check to !IS_ERR()
From: Eric Dumazet @ 2019-09-06  5:10 UTC (permalink / raw)
  To: Maciej Żenczykowski
  Cc: Maciej Żenczykowski, David S . Miller, netdev, David Ahern,
	Lorenzo Colitti
In-Reply-To: <20190906035637.47097-1-zenczykowski@gmail.com>

On Fri, Sep 6, 2019 at 5:56 AM Maciej Żenczykowski
<zenczykowski@gmail.com> wrote:
>
> From: Maciej Żenczykowski <maze@google.com>
>
> Fixes a stupid bug I recently introduced...
> ip6_route_info_create() returns an ERR_PTR(err) and not a NULL on error.
>
> Fixes: d55a2e374a94 ("net-ipv6: fix excessive RTF_ADDRCONF flag on ::1/128 local route (and others)'")
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Lorenzo Colitti <lorenzo@google.com>
> Cc: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Maciej Żenczykowski <maze@google.com>
> ---

Reported-by: syzbot <syzkaller@googlegroups.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>

Thanks.

^ permalink raw reply

* Need more information on "ifi_change" in "struct ifinfomsg"
From: dhan lin @ 2019-09-06  5:08 UTC (permalink / raw)
  To: netdev
In-Reply-To: <CAMvS6vYbphKKM4evbV6Vre7vaR8r+oJgZ8TuQU6VtBSjVqH7dA@mail.gmail.com>

Hi All,

There is a field called ifi_change in "struct ifinfomsg". man page for
rtnetlink says its for future use and should be always set to
0xFFFFFFFF.

But ive run some sample tests, to confirm the value is not as per man
pages explanation.
Its 0 most of the times and non-zero sometimes.

I've the following question,

Is ifi_change set only when there is a state change in interface values?
>>My application is not interested in processing the netlink messages without any state changes.
>>Can i add a BPF socket filter to drop all Netlink messages with ifi_change=0?


with regards,
dhanlin

^ permalink raw reply

* Re: [PATCH] net/skbuff: silence warnings under memory pressure
From: Sergey Senozhatsky @ 2019-09-06  4:32 UTC (permalink / raw)
  To: Qian Cai
  Cc: Sergey Senozhatsky, Steven Rostedt, Petr Mladek,
	Sergey Senozhatsky, Michal Hocko, Eric Dumazet, davem, netdev,
	linux-mm, linux-kernel
In-Reply-To: <1567699393.5576.96.camel@lca.pw>

On (09/05/19 12:03), Qian Cai wrote:
> > ---
> > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> > index cd51aa7d08a9..89cb47882254 100644
> > --- a/kernel/printk/printk.c
> > +++ b/kernel/printk/printk.c
> > @@ -2027,8 +2027,11 @@ asmlinkage int vprintk_emit(int facility, int level,
> >  	pending_output = (curr_log_seq != log_next_seq);
> >  	logbuf_unlock_irqrestore(flags);
> >  
> > +	if (!pending_output)
> > +		return printed_len;
> > +
> >  	/* If called from the scheduler, we can not call up(). */
> > -	if (!in_sched && pending_output) {
> > +	if (!in_sched) {
> >  		/*
> >  		 * Disable preemption to avoid being preempted while holding
> >  		 * console_sem which would prevent anyone from printing to
> > @@ -2043,10 +2046,11 @@ asmlinkage int vprintk_emit(int facility, int level,
> >  		if (console_trylock_spinning())
> >  			console_unlock();
> >  		preempt_enable();
> > -	}
> >  
> > -	if (pending_output)
> > +		wake_up_interruptible(&log_wait);
> > +	} else {
> >  		wake_up_klogd();
> > +	}
> >  	return printed_len;
> >  }
> >  EXPORT_SYMBOL(vprintk_emit);
> > ---

Qian Cai, any chance you can test that patch?

	-ss

^ permalink raw reply

* [PATCH] net-ipv6: addrconf_f6i_alloc - fix non-null pointer check to !IS_ERR()
From: Maciej Żenczykowski @ 2019-09-06  3:56 UTC (permalink / raw)
  To: Maciej Żenczykowski, David S . Miller
  Cc: netdev, David Ahern, Lorenzo Colitti, Eric Dumazet
In-Reply-To: <CANP3RGcbEP2N-CDQ6N649k0-cV4AhQeWqF-niz7EMPFOFpkU1w@mail.gmail.com>

From: Maciej Żenczykowski <maze@google.com>

Fixes a stupid bug I recently introduced...
ip6_route_info_create() returns an ERR_PTR(err) and not a NULL on error.

Fixes: d55a2e374a94 ("net-ipv6: fix excessive RTF_ADDRCONF flag on ::1/128 local route (and others)'")
Cc: David Ahern <dsahern@gmail.com>
Cc: Lorenzo Colitti <lorenzo@google.com>
Cc: Eric Dumazet <edumazet@google.com>
Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 net/ipv6/route.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 003562dd3395..2fb2b913214c 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -4383,7 +4383,7 @@ struct fib6_info *addrconf_f6i_alloc(struct net *net,
 	}
 
 	f6i = ip6_route_info_create(&cfg, gfp_flags, NULL);
-	if (f6i)
+	if (!IS_ERR(f6i))
 		f6i->dst_nocount = true;
 	return f6i;
 }
-- 
2.23.0.187.g17f5b7556c-goog


^ permalink raw reply related

* RE: [PATCH bpf-next] kbuild: replace BASH-specific ${@:2} with shift and ${@}
From: yamada.masahiro @ 2019-09-06  3:53 UTC (permalink / raw)
  To: andriin, bpf, netdev, ast, daniel; +Cc: andrii.nakryiko, kernel-team, sfr
In-Reply-To: <20190905175938.599455-1-andriin@fb.com>



> -----Original Message-----
> From: Andrii Nakryiko <andriin@fb.com>
> Sent: Friday, September 06, 2019 3:00 AM
> To: bpf@vger.kernel.org; netdev@vger.kernel.org; ast@fb.com;
> daniel@iogearbox.net
> Cc: andrii.nakryiko@gmail.com; kernel-team@fb.com; Andrii Nakryiko
> <andriin@fb.com>; Stephen Rothwell <sfr@canb.auug.org.au>; Yamada,
> Masahiro/山田 真弘 <yamada.masahiro@socionext.com>
> Subject: [PATCH bpf-next] kbuild: replace BASH-specific ${@:2} with shift
> and ${@}
> 
> ${@:2} is BASH-specific extension, which makes link-vmlinux.sh rely on
> BASH. Use shift and ${@} instead to fix this issue.
> 
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Fixes: 341dfcf8d78e ("btf: expose BTF info through sysfs")
> Cc: Stephen Rothwell <sfr@canb.auug.org.au>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>

Reviewed-by: Masahiro Yamada <yamada.masahiro@socionext.com>


^ permalink raw reply

* Re: [PATCH] net/skbuff: silence warnings under memory pressure
From: Sergey Senozhatsky @ 2019-09-06  3:39 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Sergey Senozhatsky, Qian Cai, Petr Mladek, Sergey Senozhatsky,
	Michal Hocko, Eric Dumazet, davem, netdev, linux-mm, linux-kernel
In-Reply-To: <20190905132334.52b13d95@oasis.local.home>

On (09/05/19 13:23), Steven Rostedt wrote:
> > I think we can queue significantly much less irq_work-s from printk().
> > 
> > Petr, Steven, what do you think?

[..]
> I mean, really, do we need to keep calling wake up if it
> probably never even executed?

I guess ratelimiting you are talking about ("if it probably never even
executed") would be to check if we have already called wake up on the
log_wait ->head. For that we need to, at least, take log_wait spin_lock
and check that ->head is still in TASK_INTERRUPTIBLE; which is (quite,
but not exactly) close to what wake_up_interruptible() does - it doesn't
wake up the same task twice, it bails out on `p->state & state' check.

Or did I miss something?

	-ss

^ permalink raw reply

* Re: [PATCH v2] net-ipv6: fix excessive RTF_ADDRCONF flag on ::1/128 local route (and others)
From: Maciej Żenczykowski @ 2019-09-06  3:31 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David S . Miller, Linux NetDev, David Ahern, Lorenzo Colitti
In-Reply-To: <98b8a95f-245a-0bdf-6a4c-c07a372d4d0f@gmail.com>

> Shouldn't it use
>
>         if (!IS_ERR(f6i))
>                 f6i->dst_nocount = true;
>
> ???

Yes, certainly.  I'll send a fix.

^ permalink raw reply

* [PATCH] net: Remove the source address setting in connect() for UDP
From: Enke Chen @ 2019-09-06  2:54 UTC (permalink / raw)
  To: David S . Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netdev
  Cc: enkechen, linux-kernel, xe-linux-external

The connect() system call for a UDP socket is for setting the destination
address and port. But the current code mistakenly sets the source address
for the socket as well. Remove the source address setting in connect() for
UDP in this patch.

Implications of the bug:

  - Packet drop:

    On a multi-homed device, an address assigned to any interface may
    qualify as a source address when originating a packet. If needed, the
    IP_PKTINFO option can be used to explicitly specify the source address.
    But with the source address being mistakenly set for the socket in
    connect(), a return packet (for the socket) destined to an interface
    address different from that source address would be wrongly dropped
    due to address mismatch.

    This can be reproduced easily. The dropped packets are shown in the
    following output by "netstat -s" for UDP:

          xxx packets to unknown port received

  - Source address selection:

    The source address, if unspecified via "bind()" or IP_PKTINFO, should
    be determined by routing at the time of packet origination, and not at
    the time when the connect() call is made. The difference matters as
    routing can change, e.g., by interface down/up events, and using a
    source address of an "down" interface is known to be problematic.

There is no backward compatibility issue here as the source address setting
in connect() is not needed anyway.

  - No impact on the source address selection when the source address
    is explicitly specified by "bind()", or by the "IP_PKTINFO" option.

  - In the case that the source address is not explicitly specified,
    the selection of the source address would be more accurate and
    reliable based on the up-to-date routing table.

Signed-off-by: Enke Chen <enkechen@cisco.com>
---
 net/ipv4/datagram.c |  7 -------
 net/ipv6/datagram.c | 15 +--------------
 2 files changed, 1 insertion(+), 21 deletions(-)

diff --git a/net/ipv4/datagram.c b/net/ipv4/datagram.c
index f915abff1350..4065808ec6c1 100644
--- a/net/ipv4/datagram.c
+++ b/net/ipv4/datagram.c
@@ -64,13 +64,6 @@ int __ip4_datagram_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len
 		err = -EACCES;
 		goto out;
 	}
-	if (!inet->inet_saddr)
-		inet->inet_saddr = fl4->saddr;	/* Update source address */
-	if (!inet->inet_rcv_saddr) {
-		inet->inet_rcv_saddr = fl4->saddr;
-		if (sk->sk_prot->rehash)
-			sk->sk_prot->rehash(sk);
-	}
 	inet->inet_daddr = fl4->daddr;
 	inet->inet_dport = usin->sin_port;
 	sk->sk_state = TCP_ESTABLISHED;
diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
index ecf440a4f593..80388cd50dc3 100644
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -197,19 +197,6 @@ int __ip6_datagram_connect(struct sock *sk, struct sockaddr *uaddr,
 			goto out;
 
 		ipv6_addr_set_v4mapped(inet->inet_daddr, &sk->sk_v6_daddr);
-
-		if (ipv6_addr_any(&np->saddr) ||
-		    ipv6_mapped_addr_any(&np->saddr))
-			ipv6_addr_set_v4mapped(inet->inet_saddr, &np->saddr);
-
-		if (ipv6_addr_any(&sk->sk_v6_rcv_saddr) ||
-		    ipv6_mapped_addr_any(&sk->sk_v6_rcv_saddr)) {
-			ipv6_addr_set_v4mapped(inet->inet_rcv_saddr,
-					       &sk->sk_v6_rcv_saddr);
-			if (sk->sk_prot->rehash)
-				sk->sk_prot->rehash(sk);
-		}
-
 		goto out;
 	}
 
@@ -247,7 +234,7 @@ int __ip6_datagram_connect(struct sock *sk, struct sockaddr *uaddr,
 	 *	destination cache for it.
 	 */
 
-	err = ip6_datagram_dst_update(sk, true);
+	err = ip6_datagram_dst_update(sk, false);
 	if (err) {
 		/* Restore the socket peer info, to keep it consistent with
 		 * the old socket state
-- 
2.19.1


^ permalink raw reply related

* Re: linux-next: build failure after merge of the net-next tree
From: Masahiro Yamada @ 2019-09-06  2:52 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Stephen Rothwell, David Miller, Networking,
	Linux Next Mailing List, Linux Kernel Mailing List,
	Andrii Nakryiko, Daniel Borkmann, Alexei Starovoitov
In-Reply-To: <CAEf4BzZLBV3o=t9+a4o4T7KZ_M04vddD0RMVs3s4JvDsvQ8onA@mail.gmail.com>

On Fri, Sep 6, 2019 at 4:26 AM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Tue, Sep 3, 2019 at 11:20 PM Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
> >
> > On Wed, Sep 4, 2019 at 3:00 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > >
> > > Hi all,
> > >
> > > After merging the net-next tree, today's linux-next build (arm
> > > multi_v7_defconfig) failed like this:
> > >
> > > scripts/link-vmlinux.sh: 74: Bad substitution
> > >
> > > Caused by commit
> > >
> > >   341dfcf8d78e ("btf: expose BTF info through sysfs")
> > >
> > > interacting with commit
> > >
> > >   1267f9d3047d ("kbuild: add $(BASH) to run scripts with bash-extension")
> > >
> > > from the kbuild tree.
> >
> >
> > I knew that they were using bash-extension
> > in the #!/bin/sh script.  :-D
> >
> > In fact, I wrote my patch in order to break their code
> > and  make btf people realize that they were doing wrong.
>
> Was there a specific reason to wait until this would break during
> Stephen's merge, instead of giving me a heads up (or just replying on
> original patch) and letting me fix it and save everyone's time and
> efforts?
>
> Either way, I've fixed the issue in
> https://patchwork.ozlabs.org/patch/1158620/ and will pay way more
> attention to BASH-specific features going forward (I found it pretty
> hard to verify stuff like this, unfortunately). But again, code review
> process is the best place to catch this and I really hope in the
> future we can keep this process productive. Thanks!

I could have pointed it out if I had noticed
it in the review process.

I actually noticed your patch by Stephen's
former email.  (i.e. when it appeared in linux-next)

(I try my best to check kbuild ML, and also search for
my name in LKML in case I am explicitly addressed,
but a large number of emails fall off my filter)

It was somewhat too late when I noticed it.
Of course, I still could email you afterward, or even send a patch to btf ML,
but I did not fix a particular instance of breakage
because there are already the same type of breakages in code base.

Then, I applied the all-or-nothing checker because I thought it was
the only way to address the root cause of the problems.

I admit I could have done the process better.
Sorry if I made people uncomfortable and waste time.

Thanks.




> >
> >
> >
> > > The change in the net-next tree turned link-vmlinux.sh into a bash script
> > > (I think).
> > >
> > > I have applied the following patch for today:
> >
> >
> > But, this is a temporary fix only for linux-next.
> >
> > scripts/link-vmlinux.sh does not need to use the
> > bash-extension ${@:2} in the first place.
> >
> > I hope btf people will write the correct code.
>
> I replaced ${@:2} with shift and ${@}, I hope that's a correct fix,
> but if you think it's not, please reply on the patch and let me know.
>
>
> >
> > Thanks.
> >
> >
> >
> >
> > > From: Stephen Rothwell <sfr@canb.auug.org.au>
> > > Date: Wed, 4 Sep 2019 15:43:41 +1000
> > > Subject: [PATCH] link-vmlinux.sh is now a bash script
> > >
> > > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > > ---
> > >  Makefile                | 4 ++--
> > >  scripts/link-vmlinux.sh | 2 +-
> > >  2 files changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/Makefile b/Makefile
> > > index ac97fb282d99..523d12c5cebe 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -1087,7 +1087,7 @@ ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink)
> > >
> > >  # Final link of vmlinux with optional arch pass after final link
> > >  cmd_link-vmlinux =                                                 \
> > > -       $(CONFIG_SHELL) $< $(LD) $(KBUILD_LDFLAGS) $(LDFLAGS_vmlinux) ;    \
> > > +       $(BASH) $< $(LD) $(KBUILD_LDFLAGS) $(LDFLAGS_vmlinux) ;    \
> > >         $(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
> > >
> > >  vmlinux: scripts/link-vmlinux.sh autoksyms_recursive $(vmlinux-deps) FORCE
> > > @@ -1403,7 +1403,7 @@ clean: rm-files := $(CLEAN_FILES)
> > >  PHONY += archclean vmlinuxclean
> > >
> > >  vmlinuxclean:
> > > -       $(Q)$(CONFIG_SHELL) $(srctree)/scripts/link-vmlinux.sh clean
> > > +       $(Q)$(BASH) $(srctree)/scripts/link-vmlinux.sh clean
> > >         $(Q)$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) clean)
> > >
> > >  clean: archclean vmlinuxclean
> > > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> > > index f7edb75f9806..ea1f8673869d 100755
> > > --- a/scripts/link-vmlinux.sh
> > > +++ b/scripts/link-vmlinux.sh
> > > @@ -1,4 +1,4 @@
> > > -#!/bin/sh
> > > +#!/bin/bash
> > >  # SPDX-License-Identifier: GPL-2.0
> > >  #
> > >  # link vmlinux
> > > --
> > > 2.23.0.rc1
> > >
> > > --
> > > Cheers,
> > > Stephen Rothwell
> >
> >
> >
> > --
> > Best Regards
> > Masahiro Yamada



--
Best Regards
Masahiro Yamada

^ permalink raw reply

* Re: [PATCH] net/skbuff: silence warnings under memory pressure
From: Sergey Senozhatsky @ 2019-09-06  2:50 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Qian Cai, Sergey Senozhatsky, Petr Mladek, Sergey Senozhatsky,
	Michal Hocko, Eric Dumazet, davem, netdev, linux-mm, linux-kernel
In-Reply-To: <20190905131413.0aa4e4f1@oasis.local.home>

On (09/05/19 13:14), Steven Rostedt wrote:
> > Hmm, from the article,
> > 
> > https://en.wikipedia.org/wiki/Universal_asynchronous_receiver-transmitter
> > 
> > "Since transmission of a single or multiple characters may take a long time
> > relative to CPU speeds, a UART maintains a flag showing busy status so that the
> > host system knows if there is at least one character in the transmit buffer or
> > shift register; "ready for next character(s)" may also be signaled with an
> > interrupt."
> 
> I'm pretty sure all serial consoles do a busy loop on the UART and not
> use interrupts to notify when it's available.

Yes. Besides, we call console drivers with local IRQs disabled.

	-ss

^ permalink raw reply

* Re: [PATCH] rtl8xxxu: add bluetooth co-existence support for single antenna
From: Chris Chiu @ 2019-09-06  2:44 UTC (permalink / raw)
  To: Jes Sorensen, Kalle Valo, David Miller
  Cc: linux-wireless, netdev, Linux Kernel, Linux Upstreaming Team
In-Reply-To: <20190903053735.85957-1-chiu@endlessm.com>

On Tue, Sep 3, 2019 at 1:37 PM Chris Chiu <chiu@endlessm.com> wrote:
>
> The RTL8723BU suffers the wifi disconnection problem while bluetooth
> device connected. While wifi is doing tx/rx, the bluetooth will scan
> without results. This is due to the wifi and bluetooth share the same
> single antenna for RF communication and they need to have a mechanism
> to collaborate.
>
> BT information is provided via the packet sent from co-processor to
> host (C2H). It contains the status of BT but the rtl8723bu_handle_c2h
> dose not really handle it. And there's no bluetooth coexistence
> mechanism to deal with it.
>
> This commit adds a workqueue to set the tdma configurations and
> coefficient table per the parsed bluetooth link status and given
> wifi connection state. The tdma/coef table comes from the vendor
> driver code of the RTL8192EU and RTL8723BU. However, this commit is
> only for single antenna scenario which RTL8192EU is default dual
> antenna. The rtl8xxxu_parse_rxdesc24 which invokes the handle_c2h
> is only for 8723b and 8192e so the mechanism is expected to work
> on both chips with single antenna. Note RTL8192EU dual antenna is
> not supported.
>
> Signed-off-by: Chris Chiu <chiu@endlessm.com>
> ---
>  .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h  |  37 +++
>  .../realtek/rtl8xxxu/rtl8xxxu_8723b.c         |   2 -
>  .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 243 +++++++++++++++++-
>  3 files changed, 275 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> index 582c2a346cec..22e95b11bfbb 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> @@ -1220,6 +1220,37 @@ enum ratr_table_mode_new {
>         RATEID_IDX_BGN_3SS = 14,
>  };
>
> +#define BT_INFO_8723B_1ANT_B_FTP               BIT(7)
> +#define BT_INFO_8723B_1ANT_B_A2DP              BIT(6)
> +#define BT_INFO_8723B_1ANT_B_HID               BIT(5)
> +#define BT_INFO_8723B_1ANT_B_SCO_BUSY          BIT(4)
> +#define BT_INFO_8723B_1ANT_B_ACL_BUSY          BIT(3)
> +#define BT_INFO_8723B_1ANT_B_INQ_PAGE          BIT(2)
> +#define BT_INFO_8723B_1ANT_B_SCO_ESCO          BIT(1)
> +#define BT_INFO_8723B_1ANT_B_CONNECTION        BIT(0)
> +
> +enum _BT_8723B_1ANT_STATUS {
> +       BT_8723B_1ANT_STATUS_NON_CONNECTED_IDLE      = 0x0,
> +       BT_8723B_1ANT_STATUS_CONNECTED_IDLE          = 0x1,
> +       BT_8723B_1ANT_STATUS_INQ_PAGE                = 0x2,
> +       BT_8723B_1ANT_STATUS_ACL_BUSY                = 0x3,
> +       BT_8723B_1ANT_STATUS_SCO_BUSY                = 0x4,
> +       BT_8723B_1ANT_STATUS_ACL_SCO_BUSY            = 0x5,
> +       BT_8723B_1ANT_STATUS_MAX
> +};
> +
> +struct rtl8xxxu_btcoex {
> +       u8      bt_status;
> +       bool    bt_busy;
> +       bool    has_sco;
> +       bool    has_a2dp;
> +       bool    has_hid;
> +       bool    has_pan;
> +       bool    hid_only;
> +       bool    a2dp_only;
> +       bool    c2h_bt_inquiry;
> +};
> +
>  #define RTL8XXXU_RATR_STA_INIT 0
>  #define RTL8XXXU_RATR_STA_HIGH 1
>  #define RTL8XXXU_RATR_STA_MID  2
> @@ -1340,6 +1371,10 @@ struct rtl8xxxu_priv {
>          */
>         struct ieee80211_vif *vif;
>         struct delayed_work ra_watchdog;
> +       struct work_struct c2hcmd_work;
> +       struct sk_buff_head c2hcmd_queue;
> +       spinlock_t c2hcmd_lock;
> +       struct rtl8xxxu_btcoex bt_coex;
>  };
>
>  struct rtl8xxxu_rx_urb {
> @@ -1486,6 +1521,8 @@ void rtl8xxxu_fill_txdesc_v2(struct ieee80211_hw *hw, struct ieee80211_hdr *hdr,
>                              struct rtl8xxxu_txdesc32 *tx_desc32, bool sgi,
>                              bool short_preamble, bool ampdu_enable,
>                              u32 rts_rate);
> +void rtl8723bu_set_ps_tdma(struct rtl8xxxu_priv *priv,
> +                          u8 arg1, u8 arg2, u8 arg3, u8 arg4, u8 arg5);
>
>  extern struct rtl8xxxu_fileops rtl8192cu_fops;
>  extern struct rtl8xxxu_fileops rtl8192eu_fops;
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
> index ceffe05bd65b..9ba661b3d767 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
> @@ -1580,9 +1580,7 @@ static void rtl8723b_enable_rf(struct rtl8xxxu_priv *priv)
>         /*
>          * Software control, antenna at WiFi side
>          */
> -#ifdef NEED_PS_TDMA
>         rtl8723bu_set_ps_tdma(priv, 0x08, 0x00, 0x00, 0x00, 0x00);
> -#endif
>
>         rtl8xxxu_write32(priv, REG_BT_COEX_TABLE1, 0x55555555);
>         rtl8xxxu_write32(priv, REG_BT_COEX_TABLE2, 0x55555555);
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index a6f358b9e447..4f72c2d14d44 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -3820,9 +3820,8 @@ void rtl8xxxu_power_off(struct rtl8xxxu_priv *priv)
>         rtl8xxxu_write8(priv, REG_RSV_CTRL, 0x0e);
>  }
>
> -#ifdef NEED_PS_TDMA
> -static void rtl8723bu_set_ps_tdma(struct rtl8xxxu_priv *priv,
> -                                 u8 arg1, u8 arg2, u8 arg3, u8 arg4, u8 arg5)
> +void rtl8723bu_set_ps_tdma(struct rtl8xxxu_priv *priv,
> +                          u8 arg1, u8 arg2, u8 arg3, u8 arg4, u8 arg5)
>  {
>         struct h2c_cmd h2c;
>
> @@ -3835,7 +3834,6 @@ static void rtl8723bu_set_ps_tdma(struct rtl8xxxu_priv *priv,
>         h2c.b_type_dma.data5 = arg5;
>         rtl8xxxu_gen2_h2c_cmd(priv, &h2c, sizeof(h2c.b_type_dma));
>  }
> -#endif
>
>  void rtl8xxxu_gen2_disable_rf(struct rtl8xxxu_priv *priv)
>  {
> @@ -5186,12 +5184,239 @@ static void rtl8xxxu_rx_urb_work(struct work_struct *work)
>         }
>  }
>
> +void rtl8723bu_set_coex_with_type(struct rtl8xxxu_priv *priv, u8 type)
> +{
> +       switch (type) {
> +       case 0:
> +               rtl8xxxu_write32(priv, REG_BT_COEX_TABLE1, 0x55555555);
> +               rtl8xxxu_write32(priv, REG_BT_COEX_TABLE2, 0x55555555);
> +               rtl8xxxu_write32(priv, REG_BT_COEX_TABLE3, 0x00ffffff);
> +               rtl8xxxu_write8(priv, REG_BT_COEX_TABLE4, 0x03);
> +               break;
> +       case 1:
> +       case 3:
> +               rtl8xxxu_write32(priv, REG_BT_COEX_TABLE1, 0x55555555);
> +               rtl8xxxu_write32(priv, REG_BT_COEX_TABLE2, 0x5a5a5a5a);
> +               rtl8xxxu_write32(priv, REG_BT_COEX_TABLE3, 0x00ffffff);
> +               rtl8xxxu_write8(priv, REG_BT_COEX_TABLE4, 0x03);
> +               break;
> +       case 2:
> +               rtl8xxxu_write32(priv, REG_BT_COEX_TABLE1, 0x5a5a5a5a);
> +               rtl8xxxu_write32(priv, REG_BT_COEX_TABLE2, 0x5a5a5a5a);
> +               rtl8xxxu_write32(priv, REG_BT_COEX_TABLE3, 0x00ffffff);
> +               rtl8xxxu_write8(priv, REG_BT_COEX_TABLE4, 0x03);
> +               break;
> +       case 4:
> +               rtl8xxxu_write32(priv, REG_BT_COEX_TABLE1, 0x5a5a5a5a);
> +               rtl8xxxu_write32(priv, REG_BT_COEX_TABLE2, 0xaaaa5a5a);
> +               rtl8xxxu_write32(priv, REG_BT_COEX_TABLE3, 0x00ffffff);
> +               rtl8xxxu_write8(priv, REG_BT_COEX_TABLE4, 0x03);
> +               break;
> +       case 5:
> +               rtl8xxxu_write32(priv, REG_BT_COEX_TABLE1, 0x5a5a5a5a);
> +               rtl8xxxu_write32(priv, REG_BT_COEX_TABLE2, 0xaa5a5a5a);
> +               rtl8xxxu_write32(priv, REG_BT_COEX_TABLE3, 0x00ffffff);
> +               rtl8xxxu_write8(priv, REG_BT_COEX_TABLE4, 0x03);
> +               break;
> +       case 6:
> +               rtl8xxxu_write32(priv, REG_BT_COEX_TABLE1, 0x55555555);
> +               rtl8xxxu_write32(priv, REG_BT_COEX_TABLE2, 0xaaaaaaaa);
> +               rtl8xxxu_write32(priv, REG_BT_COEX_TABLE3, 0x00ffffff);
> +               rtl8xxxu_write8(priv, REG_BT_COEX_TABLE4, 0x03);
> +               break;
> +       case 7:
> +               rtl8xxxu_write32(priv, REG_BT_COEX_TABLE1, 0xaaaaaaaa);
> +               rtl8xxxu_write32(priv, REG_BT_COEX_TABLE2, 0xaaaaaaaa);
> +               rtl8xxxu_write32(priv, REG_BT_COEX_TABLE3, 0x00ffffff);
> +               rtl8xxxu_write8(priv, REG_BT_COEX_TABLE4, 0x03);
> +               break;
> +       default:
> +               break;
> +       }
> +}
> +
> +void rtl8723bu_update_bt_link_info(struct rtl8xxxu_priv *priv, u8 bt_info)
> +{
> +       struct rtl8xxxu_btcoex *btcoex = &priv->bt_coex;
> +
> +       if (bt_info & BT_INFO_8723B_1ANT_B_INQ_PAGE)
> +               btcoex->c2h_bt_inquiry = true;
> +       else
> +               btcoex->c2h_bt_inquiry = false;
> +
> +       if (!(bt_info & BT_INFO_8723B_1ANT_B_CONNECTION)) {
> +               btcoex->bt_status = BT_8723B_1ANT_STATUS_NON_CONNECTED_IDLE;
> +               btcoex->has_sco = false;
> +               btcoex->has_hid = false;
> +               btcoex->has_pan = false;
> +               btcoex->has_a2dp = false;
> +       } else {
> +               if ((bt_info & 0x1f) == BT_INFO_8723B_1ANT_B_CONNECTION)
> +                       btcoex->bt_status = BT_8723B_1ANT_STATUS_CONNECTED_IDLE;
> +               else if ((bt_info & BT_INFO_8723B_1ANT_B_SCO_ESCO) ||
> +                        (bt_info & BT_INFO_8723B_1ANT_B_SCO_BUSY))
> +                       btcoex->bt_status = BT_8723B_1ANT_STATUS_SCO_BUSY;
> +               else if (bt_info & BT_INFO_8723B_1ANT_B_ACL_BUSY)
> +                       btcoex->bt_status = BT_8723B_1ANT_STATUS_ACL_BUSY;
> +               else
> +                       btcoex->bt_status = BT_8723B_1ANT_STATUS_MAX;
> +
> +               if (bt_info & BT_INFO_8723B_1ANT_B_FTP)
> +                       btcoex->has_pan = true;
> +               else
> +                       btcoex->has_pan = false;
> +
> +               if (bt_info & BT_INFO_8723B_1ANT_B_A2DP)
> +                       btcoex->has_a2dp = true;
> +               else
> +                       btcoex->has_a2dp = false;
> +
> +               if (bt_info & BT_INFO_8723B_1ANT_B_HID)
> +                       btcoex->has_hid = true;
> +               else
> +                       btcoex->has_hid = false;
> +
> +               if (bt_info & BT_INFO_8723B_1ANT_B_SCO_ESCO)
> +                       btcoex->has_sco = true;
> +               else
> +                       btcoex->has_sco = false;
> +       }
> +
> +       if (!btcoex->has_a2dp &&
> +           !btcoex->has_sco &&
> +           !btcoex->has_pan &&
> +           btcoex->has_hid)
> +               btcoex->hid_only = true;
> +       else
> +               btcoex->hid_only = false;
> +
> +       if (!btcoex->has_sco &&
> +           !btcoex->has_pan &&
> +           !btcoex->has_hid &&
> +           btcoex->has_a2dp)
> +               btcoex->has_a2dp = true;
> +       else
> +               btcoex->has_a2dp = false;
> +
> +       if (btcoex->bt_status == BT_8723B_1ANT_STATUS_SCO_BUSY ||
> +           btcoex->bt_status == BT_8723B_1ANT_STATUS_ACL_BUSY)
> +               btcoex->bt_busy = true;
> +       else
> +               btcoex->bt_busy = false;
> +}
> +
> +static void rtl8xxxu_c2hcmd_callback(struct work_struct *work)
> +{
> +       struct rtl8xxxu_priv *priv;
> +       struct rtl8723bu_c2h *c2h;
> +       struct ieee80211_vif *vif;
> +       struct device *dev;
> +       struct sk_buff *skb = NULL;
> +       unsigned long flags;
> +       int len;
> +       u8 bt_info = 0;
> +       struct rtl8xxxu_btcoex *btcoex;
> +
> +       priv = container_of(work, struct rtl8xxxu_priv, c2hcmd_work);
> +       vif = priv->vif;
> +       btcoex = &priv->bt_coex;
> +       dev = &priv->udev->dev;
> +
> +       if (priv->rf_paths > 1)
> +               goto out;
> +
> +       while (!skb_queue_empty(&priv->c2hcmd_queue)) {
> +               spin_lock_irqsave(&priv->c2hcmd_lock, flags);
> +               skb = __skb_dequeue(&priv->c2hcmd_queue);
> +               spin_unlock_irqrestore(&priv->c2hcmd_lock, flags);
> +
> +               c2h = (struct rtl8723bu_c2h *)skb->data;
> +               len = skb->len - 2;
> +
> +               switch (c2h->id) {
> +               case C2H_8723B_BT_INFO:
> +                       bt_info = c2h->bt_info.bt_info;
> +
> +                       rtl8723bu_update_bt_link_info(priv, bt_info);
> +
> +                       if (btcoex->c2h_bt_inquiry) {
> +                               if (vif && !vif->bss_conf.assoc) {
> +                                       rtl8723bu_set_ps_tdma(priv, 0x8, 0x0, 0x0, 0x0, 0x0);
> +                                       rtl8723bu_set_coex_with_type(priv, 0);
> +                               } else if (btcoex->has_sco ||
> +                                          btcoex->has_hid ||
> +                                          btcoex->has_a2dp) {
> +                                       rtl8723bu_set_ps_tdma(priv, 0x61, 0x35, 0x3, 0x11, 0x11);
> +                                       rtl8723bu_set_coex_with_type(priv, 4);
> +                               } else if (btcoex->has_pan) {
> +                                       rtl8723bu_set_ps_tdma(priv, 0x61, 0x3f, 0x3, 0x11, 0x11);
> +                                       rtl8723bu_set_coex_with_type(priv, 4);
> +                               } else {
> +                                       rtl8723bu_set_ps_tdma(priv, 0x8, 0x0, 0x0, 0x0, 0x0);
> +                                       rtl8723bu_set_coex_with_type(priv, 7);
> +                               }
> +
> +                               return;
> +                       }
> +
> +                       if (vif && vif->bss_conf.assoc) {
> +                               u32 val32 = 0;
> +                               u32 high_prio_tx = 0, high_prio_rx = 0;
> +
> +                               val32 = rtl8xxxu_read32(priv, 0x770);
> +                               high_prio_tx = val32 & 0x0000ffff;
> +                               high_prio_rx = (val32  & 0xffff0000) >> 16;
> +
> +                               if (btcoex->bt_busy) {
> +                                       if (btcoex->hid_only) {
> +                                               rtl8723bu_set_ps_tdma(priv, 0x61, 0x20, 0x3, 0x11, 0x11);
> +                                               rtl8723bu_set_coex_with_type(priv, 5);
> +                                       } else if (btcoex->a2dp_only) {
> +                                               rtl8723bu_set_ps_tdma(priv, 0x61, 0x35, 0x3, 0x11, 0x11);
> +                                               rtl8723bu_set_coex_with_type(priv, 4);
> +                                       } else if ((btcoex->has_a2dp &&
> +                                                   btcoex->has_pan) ||
> +                                                  (btcoex->has_hid &&
> +                                                   btcoex->has_a2dp &&
> +                                                   btcoex->has_pan)) {
> +                                               rtl8723bu_set_ps_tdma(priv, 0x51, 0x21, 0x3, 0x10, 0x10);
> +                                               rtl8723bu_set_coex_with_type(priv, 4);
> +                                       } else if (btcoex->has_hid &&
> +                                                btcoex->has_a2dp) {
> +                                               rtl8723bu_set_ps_tdma(priv, 0x51, 0x21, 0x3, 0x10, 0x10);
> +                                               rtl8723bu_set_coex_with_type(priv, 3);
> +                                       } else {
> +                                               rtl8723bu_set_ps_tdma(priv, 0x61, 0x35, 0x3, 0x11, 0x11);
> +                                               rtl8723bu_set_coex_with_type(priv, 4);
> +                                       }
> +                               } else {
> +                                       rtl8723bu_set_ps_tdma(priv, 0x8, 0x0, 0x0, 0x0, 0x0);
> +                                       if (high_prio_tx + high_prio_rx <= 60)
> +                                               rtl8723bu_set_coex_with_type(priv, 2);
> +                                       else
> +                                               rtl8723bu_set_coex_with_type(priv, 7);
> +                               }
> +                       } else {
> +                               rtl8723bu_set_ps_tdma(priv, 0x8, 0x0, 0x0, 0x0, 0x0);
> +                               rtl8723bu_set_coex_with_type(priv, 0);
> +                       }
> +                       break;
> +               default:
> +                       break;
> +               }
> +       }
> +
> +out:
> +       dev_kfree_skb(skb);
> +}
> +
>  static void rtl8723bu_handle_c2h(struct rtl8xxxu_priv *priv,
>                                  struct sk_buff *skb)
>  {
>         struct rtl8723bu_c2h *c2h = (struct rtl8723bu_c2h *)skb->data;
>         struct device *dev = &priv->udev->dev;
>         int len;
> +       unsigned long flags;
>
>         len = skb->len - 2;
>
> @@ -5229,6 +5454,12 @@ static void rtl8723bu_handle_c2h(struct rtl8xxxu_priv *priv,
>                                16, 1, c2h->raw.payload, len, false);
>                 break;
>         }
> +
> +       spin_lock_irqsave(&priv->c2hcmd_lock, flags);
> +       __skb_queue_tail(&priv->c2hcmd_queue, skb);
> +       spin_unlock_irqrestore(&priv->c2hcmd_lock, flags);
> +
> +       schedule_work(&priv->c2hcmd_work);
>  }
>
>  int rtl8xxxu_parse_rxdesc16(struct rtl8xxxu_priv *priv, struct sk_buff *skb)
> @@ -5353,7 +5584,6 @@ int rtl8xxxu_parse_rxdesc24(struct rtl8xxxu_priv *priv, struct sk_buff *skb)
>                 struct device *dev = &priv->udev->dev;
>                 dev_dbg(dev, "%s: C2H packet\n", __func__);
>                 rtl8723bu_handle_c2h(priv, skb);
> -               dev_kfree_skb(skb);
>                 return RX_TYPE_C2H;
>         }
>
> @@ -6272,6 +6502,9 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
>         spin_lock_init(&priv->rx_urb_lock);
>         INIT_WORK(&priv->rx_urb_wq, rtl8xxxu_rx_urb_work);
>         INIT_DELAYED_WORK(&priv->ra_watchdog, rtl8xxxu_watchdog_callback);
> +       spin_lock_init(&priv->c2hcmd_lock);
> +       INIT_WORK(&priv->c2hcmd_work, rtl8xxxu_c2hcmd_callback);
> +       skb_queue_head_init(&priv->c2hcmd_queue);
>
>         usb_set_intfdata(interface, hw);
>
> --
> 2.20.1
>

Gentle ping. Cheers.

Chris

^ permalink raw reply

* [PATCH net v2] isdn/capi: check message length in capi_write()
From: Eric Biggers @ 2019-09-06  2:36 UTC (permalink / raw)
  To: netdev, David Miller; +Cc: Karsten Keil, syzkaller-bugs
In-Reply-To: <20190901.114406.528788701327829265.davem@davemloft.net>

From: Eric Biggers <ebiggers@google.com>

syzbot reported:

    BUG: KMSAN: uninit-value in capi_write+0x791/0xa90 drivers/isdn/capi/capi.c:700
    CPU: 0 PID: 10025 Comm: syz-executor379 Not tainted 4.20.0-rc7+ #2
    Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
    Call Trace:
      __dump_stack lib/dump_stack.c:77 [inline]
      dump_stack+0x173/0x1d0 lib/dump_stack.c:113
      kmsan_report+0x12e/0x2a0 mm/kmsan/kmsan.c:613
      __msan_warning+0x82/0xf0 mm/kmsan/kmsan_instr.c:313
      capi_write+0x791/0xa90 drivers/isdn/capi/capi.c:700
      do_loop_readv_writev fs/read_write.c:703 [inline]
      do_iter_write+0x83e/0xd80 fs/read_write.c:961
      vfs_writev fs/read_write.c:1004 [inline]
      do_writev+0x397/0x840 fs/read_write.c:1039
      __do_sys_writev fs/read_write.c:1112 [inline]
      __se_sys_writev+0x9b/0xb0 fs/read_write.c:1109
      __x64_sys_writev+0x4a/0x70 fs/read_write.c:1109
      do_syscall_64+0xbc/0xf0 arch/x86/entry/common.c:291
      entry_SYSCALL_64_after_hwframe+0x63/0xe7
    [...]

The problem is that capi_write() is reading past the end of the message.
Fix it by checking the message's length in the needed places.

Reported-and-tested-by: syzbot+0849c524d9c634f5ae66@syzkaller.appspotmail.com
Signed-off-by: Eric Biggers <ebiggers@google.com>
---

Changed v1 => v2:

- Use CAPI_DATA_B3_REQ_LEN, and define and use a constant for
  CAPI_DISCONNECT_B3_RESP_LEN.

 drivers/isdn/capi/capi.c          | 10 +++++++++-
 include/uapi/linux/isdn/capicmd.h |  1 +
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index 3c3ad42f22bf..c92b405b7646 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -688,6 +688,9 @@ capi_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos
 	if (!cdev->ap.applid)
 		return -ENODEV;
 
+	if (count < CAPIMSG_BASELEN)
+		return -EINVAL;
+
 	skb = alloc_skb(count, GFP_USER);
 	if (!skb)
 		return -ENOMEM;
@@ -698,7 +701,8 @@ capi_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos
 	}
 	mlen = CAPIMSG_LEN(skb->data);
 	if (CAPIMSG_CMD(skb->data) == CAPI_DATA_B3_REQ) {
-		if ((size_t)(mlen + CAPIMSG_DATALEN(skb->data)) != count) {
+		if (count < CAPI_DATA_B3_REQ_LEN ||
+		    (size_t)(mlen + CAPIMSG_DATALEN(skb->data)) != count) {
 			kfree_skb(skb);
 			return -EINVAL;
 		}
@@ -711,6 +715,10 @@ capi_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos
 	CAPIMSG_SETAPPID(skb->data, cdev->ap.applid);
 
 	if (CAPIMSG_CMD(skb->data) == CAPI_DISCONNECT_B3_RESP) {
+		if (count < CAPI_DISCONNECT_B3_RESP_LEN) {
+			kfree_skb(skb);
+			return -EINVAL;
+		}
 		mutex_lock(&cdev->lock);
 		capincci_free(cdev, CAPIMSG_NCCI(skb->data));
 		mutex_unlock(&cdev->lock);
diff --git a/include/uapi/linux/isdn/capicmd.h b/include/uapi/linux/isdn/capicmd.h
index 4941628a4fb9..5ec88e7548a9 100644
--- a/include/uapi/linux/isdn/capicmd.h
+++ b/include/uapi/linux/isdn/capicmd.h
@@ -16,6 +16,7 @@
 #define CAPI_MSG_BASELEN		8
 #define CAPI_DATA_B3_REQ_LEN		(CAPI_MSG_BASELEN+4+4+2+2+2)
 #define CAPI_DATA_B3_RESP_LEN		(CAPI_MSG_BASELEN+4+2)
+#define CAPI_DISCONNECT_B3_RESP_LEN	(CAPI_MSG_BASELEN+4)
 
 /*----- CAPI commands -----*/
 #define CAPI_ALERT		    0x01
-- 
2.23.0


^ permalink raw reply related

* RE: [PATCH REPOST 1/2] can: flexcan: fix deadlock when using self wakeup
From: Joakim Zhang @ 2019-09-06  2:11 UTC (permalink / raw)
  To: Sean Nyekjaer, mkl@pengutronix.de, linux-can@vger.kernel.org
  Cc: wg@grandegger.com, netdev@vger.kernel.org, dl-linux-imx,
	Martin Hundebøll
In-Reply-To: <1655f342-7aaf-5e36-d141-d00eee84f3ec@geanix.com>


> -----Original Message-----
> From: Sean Nyekjaer <sean@geanix.com>
> Sent: 2019年9月5日 21:18
> To: Joakim Zhang <qiangqing.zhang@nxp.com>; mkl@pengutronix.de;
> linux-can@vger.kernel.org
> Cc: wg@grandegger.com; netdev@vger.kernel.org; dl-linux-imx
> <linux-imx@nxp.com>; Martin Hundebøll <martin@geanix.com>
> Subject: Re: [PATCH REPOST 1/2] can: flexcan: fix deadlock when using self
> wakeup
> 
> 
> 
> On 05/09/2019 09.10, Joakim Zhang wrote:
> > Hi Sean,
> >
> > Could you update lastest flexcan driver using linux-can-next/flexcan and then
> merge below two patches from linux-can/testing?
> > d0b53616716e (HEAD -> testing, origin/testing) can: flexcan: add LPSR
> > mode support for i.MX7D 803eb6bad65b can: flexcan: fix deadlock when
> > using self wakeup
> >
> > Best Regards,
> > Joakim Zhang
> 
> The testing branch have some UBI bugs, when suspending it crashes...
> So will have to leave this, until they are resolved :-)

I think you can find the base and cherry-pick all above flexcan related latest patches to your local stable tree.
I did this and test wakeup function on i.MX8QM/QXP before.

BTW, I think without CAN FD related patches, you still can test wakeup function on i.MX6/7 with this patch(may need fix merge conflicts)
This patch has been merged into our 4.19 kernel and passed the wakeup test for i.MX6/7/8. I went through the flexcan suspend/resume process again,
and not find logical issue. If you met wakeup failure, could you help to locate the problem more accurate. Failure log you provided last time, strange
and illogical.

Thanks a lot! 😊
 	
Best Regards,
Joakim Zhang
> /Sean

^ permalink raw reply

* Re: WARNING in hso_free_net_device
From: Hui Peng @ 2019-09-06  2:05 UTC (permalink / raw)
  To: Andrey Konovalov
  Cc: Stephen Hemminger, syzbot+44d53c7255bb1aea22d2, alexios.zavras,
	David S. Miller, Greg Kroah-Hartman, LKML, USB list,
	Mathias Payer, netdev, rfontana, syzkaller-bugs, Thomas Gleixner,
	Oliver Neukum
In-Reply-To: <CAAeHK+y3eQ7bXvo1tiAkwLCsFkbSU5B+6hsKbdEzkSXP2_Jyzg@mail.gmail.com>



On 9/5/2019 7:24 AM, Andrey Konovalov wrote:
> On Thu, Sep 5, 2019 at 4:20 AM Hui Peng <benquike@gmail.com> wrote:
>>
>> Can you guys have  a look at the attached patch?
> 
> Let's try it:
> 
> #syz test: https://github.com/google/kasan.git eea39f24
> 
> FYI: there are two more reports coming from this driver, which might
> (or might not) have the same root cause. One of them has a suggested
> fix by Oliver.
> 
> https://syzkaller.appspot.com/bug?extid=67b2bd0e34f952d0321e
> https://syzkaller.appspot.com/bug?extid=93f2f45b19519b289613
> 

I think they are different, though similar.
This one is resulted from unregistering a network device.
These 2 are resulted from unregistering a tty device.

>>
>> On 9/4/19 6:41 PM, Stephen Hemminger wrote:
>>> On Wed, 4 Sep 2019 16:27:50 -0400
>>> Hui Peng <benquike@gmail.com> wrote:
>>>
>>>> Hi, all:
>>>>
>>>> I looked at the bug a little.
>>>>
>>>> The issue is that in the error handling code, hso_free_net_device
>>>> unregisters
>>>>
>>>> the net_device (hso_net->net)  by calling unregister_netdev. In the
>>>> error handling code path,
>>>>
>>>> hso_net->net has not been registered yet.
>>>>
>>>> I think there are two ways to solve the issue:
>>>>
>>>> 1. fix it in drivers/net/usb/hso.c to avoiding unregistering the
>>>> net_device when it is still not registered
>>>>
>>>> 2. fix it in unregister_netdev. We can add a field in net_device to
>>>> record whether it is registered, and make unregister_netdev return if
>>>> the net_device is not registered yet.
>>>>
>>>> What do you guys think ?
>>> #1

^ permalink raw reply

* Re: [PATCH] Revert "net: get rid of an signed integer overflow in ip_idents_reserve()"
From: Shaokun Zhang @ 2019-09-06  1:25 UTC (permalink / raw)
  To: Eric Dumazet, netdev, linux-kernel
  Cc: Yang Guo, David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI,
	Jiri Pirko
In-Reply-To: <afa992f6-eb5e-8104-9a03-f992b184a6b6@gmail.com>

Hi Eric,

On 2019/7/26 17:58, Eric Dumazet wrote:
> 
> 
> On 7/26/19 11:17 AM, Shaokun Zhang wrote:
>> From: Yang Guo <guoyang2@huawei.com>
>>
>> There is an significant performance regression with the following
>> commit-id <adb03115f459>
>> ("net: get rid of an signed integer overflow in ip_idents_reserve()").
>>
>>
> 
> So, you jump around and took ownership of this issue, while some of us
> are already working on it ?
> 

Any update about this issue?

Thanks,
Shaokun

> Have you first checked that current UBSAN versions will not complain anymore ?
> 
> A revert adding back the original issue would be silly, performance of
> benchmarks is nice but secondary.
> 
> 
> 


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox