Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next v1] bonding: add an option to specify a delay between peer notifications
From: Jiri Pirko @ 2019-07-01  9:27 UTC (permalink / raw)
  To: Vincent Bernat
  Cc: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, David S. Miller,
	netdev
In-Reply-To: <20190630185931.18746-1-vincent@bernat.ch>

Sun, Jun 30, 2019 at 08:59:31PM CEST, vincent@bernat.ch wrote:

[...]


>+module_param(peer_notif_delay, int, 0);
>+MODULE_PARM_DESC(peer_notif_delay, "Delay between each peer notification on "
>+				   "failover event, in milliseconds");

No module options please. Use netlink. See bond_changelink() function.

[...]

^ permalink raw reply

* Re: [PATCH] ipv6_sockglue: Fix a missing-check bug in ip6_ra_control()
From: Gen Zhang @ 2019-07-01  9:06 UTC (permalink / raw)
  To: Jiri Slaby; +Cc: davem, kuznet, yoshfuji, netdev, linux-kernel
In-Reply-To: <1b5f82ae-31a7-db36-dc9d-efc46cda2af3@suse.cz>

On Mon, Jul 01, 2019 at 10:57:36AM +0200, Jiri Slaby wrote:
> On 24. 05. 19, 5:19, Gen Zhang wrote:
> > In function ip6_ra_control(), the pointer new_ra is allocated a memory 
> > space via kmalloc(). And it is used in the following codes. However, 
> > when there is a memory allocation error, kmalloc() fails. Thus null 
> > pointer dereference may happen. And it will cause the kernel to crash. 
> > Therefore, we should check the return value and handle the error.
> > 
> > Signed-off-by: Gen Zhang <blackgod016574@gmail.com>
> > 
> > ---
> > diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
> > index 40f21fe..0a3d035 100644
> > --- a/net/ipv6/ipv6_sockglue.c
> > +++ b/net/ipv6/ipv6_sockglue.c
> > @@ -68,6 +68,8 @@ int ip6_ra_control(struct sock *sk, int sel)
> >  		return -ENOPROTOOPT;
> >  
> >  	new_ra = (sel >= 0) ? kmalloc(sizeof(*new_ra), GFP_KERNEL) : NULL;
> > +	if (sel >= 0 && !new_ra)
> > +		return -ENOMEM;
> >  
> >  	write_lock_bh(&ip6_ra_lock);
> >  	for (rap = &ip6_ra_chain; (ra = *rap) != NULL; rap = &ra->next) {
> > 
> 
> Was this really an omission? There is (!new_ra) handling below the for loop:
>         if (!new_ra) {
>                 write_unlock_bh(&ip6_ra_lock);
>                 return -ENOBUFS;
>         }
> 
> It used to handle both (sel >= 0) and (sel == 0) cases and it used to
> return ENOBUFS in case of failure. For (sel >= 0) it also could at least
> return EADDRINUSE when a collision was found -- even if memory was
> exhausted.
> 
> In anyway, how could this lead to a pointer dereference? And why/how did
> this get a CVE number?
> 
> thanks,
> -- 
> js
> suse labs
This CVE is already disputed by other maintainers and marked *DISPUTED*
on the website.

Thanks
Gen

^ permalink raw reply

* Re: [PATCH v2 bpf-next 1/4] bpf: unprivileged BPF access via /dev/bpf
From: Song Liu @ 2019-07-01  9:03 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: linux-security@vger.kernel.org, Networking, bpf,
	Alexei Starovoitov, Daniel Borkmann, Kernel Team, Lorenz Bauer,
	Jann Horn, Greg KH, linux-abi@vger.kernel.org, kees@chromium.org
In-Reply-To: <CALCETrWBnH4Q43POU8cQ7YMjb9LioK28FDEQf7aHZbdf1eBZWg@mail.gmail.com>

Hi Andy,

Thanks for these detailed analysis. 

> On Jun 30, 2019, at 8:12 AM, Andy Lutomirski <luto@kernel.org> wrote:
> 
> On Fri, Jun 28, 2019 at 12:05 PM Song Liu <songliubraving@fb.com> wrote:
>> 
>> Hi Andy,
>> 
>>> On Jun 27, 2019, at 4:40 PM, Andy Lutomirski <luto@kernel.org> wrote:
>>> 
>>> On 6/27/19 1:19 PM, Song Liu wrote:
>>>> This patch introduce unprivileged BPF access. The access control is
>>>> achieved via device /dev/bpf. Users with write access to /dev/bpf are able
>>>> to call sys_bpf().
>>>> Two ioctl command are added to /dev/bpf:
>>>> The two commands enable/disable permission to call sys_bpf() for current
>>>> task. This permission is noted by bpf_permitted in task_struct. This
>>>> permission is inherited during clone(CLONE_THREAD).
>>>> Helper function bpf_capable() is added to check whether the task has got
>>>> permission via /dev/bpf.
>>> 
>>>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>>>> index 0e079b2298f8..79dc4d641cf3 100644
>>>> --- a/kernel/bpf/verifier.c
>>>> +++ b/kernel/bpf/verifier.c
>>>> @@ -9134,7 +9134,7 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr,
>>>>             env->insn_aux_data[i].orig_idx = i;
>>>>     env->prog = *prog;
>>>>     env->ops = bpf_verifier_ops[env->prog->type];
>>>> -    is_priv = capable(CAP_SYS_ADMIN);
>>>> +    is_priv = bpf_capable(CAP_SYS_ADMIN);
>>> 
>>> Huh?  This isn't a hardening measure -- the "is_priv" verifier mode allows straight-up leaks of private kernel state to user mode.
>>> 
>>> (For that matter, the pending lockdown stuff should possibly consider this a "confidentiality" issue.)
>>> 
>>> 
>>> I have a bigger issue with this patch, though: it's a really awkward way to pretend to have capabilities. For bpf, it seems like you could make this be a *real* capability without too much pain since there's only one syscall there.  Just find a way to pass an fd to /dev/bpf into the syscall.  If this means you need a new bpf_with_cap() syscall that takes an extra argument, so be it.  The old bpf() syscall can just translate to bpf_with_cap(..., -1).
>>> 
>>> For a while, I've considered a scheme I call "implicit rights".  There would be a directory in /dev called /dev/implicit_rights.  This would either be part of devtmpfs or a whole new filesystem -- it would *not* be any other filesystem.  The contents would be files that can't be read or written and exist only in memory. You create them with a privileged syscall.  Certain actions that are sensitive but not at the level of CAP_SYS_ADMIN (use of large-attack-surface bpf stuff, creation of user namespaces, profiling the kernel, etc) could require an "implicit right".  When you do them, if you don't have CAP_SYS_ADMIN, the kernel would do a path walk for, say, /dev/implicit_rights/bpf and, if the object exists, can be opened, and actually refers to the "bpf" rights object, then the action is allowed.  Otherwise it's denied.
>>> 
>>> This is extensible, and it doesn't require the rather ugly per-task state of whether it's enabled.
>>> 
>>> For things like creation of user namespaces, there's an existing API, and the default is that it works without privilege.  Switching it to an implicit right has the benefit of not requiring code changes to programs that already work as non-root.
>>> 
>>> But, for BPF in particular, this type of compatibility issue doesn't exist now.  You already can't use most eBPF functionality without privilege.  New bpf-using programs meant to run without privilege are *new*, so they can use a new improved API.  So, rather than adding this obnoxious ioctl, just make the API explicit, please.
>>> 
>>> Also, please cc: linux-abi next time.
>> 
>> Thanks for your inputs.
>> 
>> I think we need to clarify the use case here. In this case, we are NOT
>> thinking about creating new tools for unprivileged users. Instead, we
>> would like to use existing tools without root.
> 
> I read patch 4, and I interpret it very differently.  Patches 2-4 are
> creating a new version of libbpf and a new version of bpftool.  Given
> this, I see no real justification for adding a new in-kernel per-task
> state instead of just pushing the complexity into libbpf.

I am not sure whether we are on the same page. Let me try an example, 
say we have application A, which calls sys_bpf(). 

Before the series: we have to run A with root; 
After the series:  we add a special user with access to /dev/bpf, and 
                   run A with this special user. 

If we look at the whole system, I would say we are more secure after 
the series. 

I am not trying to make an extreme example here, because this use case
is the motivation here. 

To stay safe, we have to properly manage the permission of /dev/bpf. 
This is just like we need to properly manage access to /etc/sudoers and 
/dev/mem. 

Does this make sense? 

Thanks,
Song


^ permalink raw reply

* Re: [PATCH] xfrm: use list_for_each_entry_safe in xfrm_policy_flush
From: Florian Westphal @ 2019-07-01  9:03 UTC (permalink / raw)
  To: Li RongQing; +Cc: netdev
In-Reply-To: <1561969747-8629-1-git-send-email-lirongqing@baidu.com>

Li RongQing <lirongqing@baidu.com> wrote:
> The iterated pol maybe be freed since it is not protected
> by RCU or spinlock when put it, lead to UAF, so use _safe
> function to iterate over it against removal
> 
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> ---
>  net/xfrm/xfrm_policy.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
> index 3235562f6588..87d770dab1f5 100644
> --- a/net/xfrm/xfrm_policy.c
> +++ b/net/xfrm/xfrm_policy.c
> @@ -1772,7 +1772,7 @@ xfrm_policy_flush_secctx_check(struct net *net, u8 type, bool task_valid)
>  int xfrm_policy_flush(struct net *net, u8 type, bool task_valid)
>  {
>  	int dir, err = 0, cnt = 0;
> -	struct xfrm_policy *pol;
> +	struct xfrm_policy *pol, *tmp;
>  
>  	spin_lock_bh(&net->xfrm.xfrm_policy_lock);
>  
> @@ -1781,7 +1781,7 @@ int xfrm_policy_flush(struct net *net, u8 type, bool task_valid)
>  		goto out;
>  
>  again:
> -	list_for_each_entry(pol, &net->xfrm.policy_all, walk.all) {
> +	list_for_each_entry_safe(pol, tmp, &net->xfrm.policy_all, walk.all) {
>  		dir = xfrm_policy_id2dir(pol->index);
>  		if (pol->walk.dead ||
>  		    dir >= XFRM_POLICY_MAX ||

This function drops the lock, but after re-acquire jumps to the 'again'
label, so I do not see the UAF as the entire loop gets restarted.

^ permalink raw reply

* [PATCH] sis900: add ethtool tests (link, eeprom)
From: Sergej Benilov @ 2019-07-01  9:03 UTC (permalink / raw)
  To: venza, netdev; +Cc: Sergej Benilov

Add tests for ethtool: link test, EEPROM read test.
Correct a few typos, too.

Signed-off-by: Sergej Benilov <sergej.benilov@googlemail.com>
---
 drivers/net/ethernet/sis/sis900.c | 78 +++++++++++++++++++++++++++++--
 1 file changed, 74 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/sis/sis900.c b/drivers/net/ethernet/sis/sis900.c
index 9b036c857b1d..a781bce23ec8 100644
--- a/drivers/net/ethernet/sis/sis900.c
+++ b/drivers/net/ethernet/sis/sis900.c
@@ -262,7 +262,7 @@ static int sis900_get_mac_addr(struct pci_dev *pci_dev,
 	/* check to see if we have sane EEPROM */
 	signature = (u16) read_eeprom(ioaddr, EEPROMSignature);
 	if (signature == 0xffff || signature == 0x0000) {
-		printk (KERN_WARNING "%s: Error EERPOM read %x\n",
+		printk (KERN_WARNING "%s: Error EEPROM read %x\n",
 			pci_name(pci_dev), signature);
 		return 0;
 	}
@@ -359,9 +359,9 @@ static int sis635_get_mac_addr(struct pci_dev *pci_dev,
  *
  *	SiS962 or SiS963 model, use EEPROM to store MAC address. And EEPROM
  *	is shared by
- *	LAN and 1394. When access EEPROM, send EEREQ signal to hardware first
- *	and wait for EEGNT. If EEGNT is ON, EEPROM is permitted to be access
- *	by LAN, otherwise is not. After MAC address is read from EEPROM, send
+ *	LAN and 1394. When accessing EEPROM, send EEREQ signal to hardware first
+ *	and wait for EEGNT. If EEGNT is ON, EEPROM is permitted to be accessed
+ *	by LAN, otherwise it is not. After MAC address is read from EEPROM, send
  *	EEDONE signal to refuse EEPROM access by LAN.
  *	The EEPROM map of SiS962 or SiS963 is different to SiS900.
  *	The signature field in SiS962 or SiS963 spec is meaningless.
@@ -2122,6 +2122,73 @@ static void sis900_get_wol(struct net_device *net_dev, struct ethtool_wolinfo *w
 	wol->supported = (WAKE_PHY | WAKE_MAGIC);
 }
 
+static const char sis900_gstrings_test[][ETH_GSTRING_LEN] = {
+	"Link test     (on/offline)",
+	"EEPROM read test   (on/offline)",
+};
+#define SIS900_TEST_LEN	ARRAY_SIZE(sis900_gstrings_test)
+
+static int sis900_eeprom_readtest(struct net_device *net_dev)
+{
+	struct sis900_private *sis_priv = netdev_priv(net_dev);
+	void __iomem *ioaddr = sis_priv->ioaddr;
+	int wait, ret = -EAGAIN;
+	u16 signature;
+
+	if (sis_priv->chipset_rev == SIS96x_900_REV) {
+	sw32(mear, EEREQ);
+	for (wait = 0; wait < 2000; wait++) {
+		if (sr32(mear) & EEGNT) {
+		 signature = (u16) read_eeprom(ioaddr, EEPROMSignature);
+    	 ret = 0;
+		 break;
+		}
+		udelay(1);
+	  }		
+	sw32(mear, EEDONE);
+	}
+	else {		
+		signature = (u16) read_eeprom(ioaddr, EEPROMSignature);
+		if (signature != 0xffff && signature != 0x0000)
+			ret = 0;
+	}
+	return ret;
+}
+
+static void sis900_diag_test(struct net_device *netdev,
+	struct ethtool_test *test, u64 *data)
+{
+	struct sis900_private *nic = netdev_priv(netdev);
+	int i;
+
+	memset(data, 0, SIS900_TEST_LEN * sizeof(u64));
+	data[0] = !mii_link_ok(&nic->mii_info);
+    data[1] = sis900_eeprom_readtest(netdev);
+	for (i = 0; i < SIS900_TEST_LEN; i++)
+		test->flags |= data[i] ? ETH_TEST_FL_FAILED : 0;
+
+	msleep_interruptible(4 * 1000);
+}
+
+static int sis900_get_sset_count(struct net_device *netdev, int sset)
+{
+	switch (sset) {
+	case ETH_SS_TEST:
+		return SIS900_TEST_LEN;
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static void sis900_get_strings(struct net_device *netdev, u32 stringset, u8 *data)
+{
+	switch (stringset) {
+	case ETH_SS_TEST:
+		memcpy(data, *sis900_gstrings_test, sizeof(sis900_gstrings_test));
+		break;
+	}
+}
+
 static const struct ethtool_ops sis900_ethtool_ops = {
 	.get_drvinfo 	= sis900_get_drvinfo,
 	.get_msglevel	= sis900_get_msglevel,
@@ -2132,6 +2199,9 @@ static const struct ethtool_ops sis900_ethtool_ops = {
 	.set_wol	= sis900_set_wol,
 	.get_link_ksettings = sis900_get_link_ksettings,
 	.set_link_ksettings = sis900_set_link_ksettings,
+	.self_test		= sis900_diag_test,
+	.get_strings	= sis900_get_strings,
+	.get_sset_count		= sis900_get_sset_count,
 };
 
 /**
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH] ipv6_sockglue: Fix a missing-check bug in ip6_ra_control()
From: Jiri Slaby @ 2019-07-01  8:57 UTC (permalink / raw)
  To: Gen Zhang, davem, kuznet, yoshfuji; +Cc: netdev, linux-kernel
In-Reply-To: <20190524031946.GA6463@zhanggen-UX430UQ>

On 24. 05. 19, 5:19, Gen Zhang wrote:
> In function ip6_ra_control(), the pointer new_ra is allocated a memory 
> space via kmalloc(). And it is used in the following codes. However, 
> when there is a memory allocation error, kmalloc() fails. Thus null 
> pointer dereference may happen. And it will cause the kernel to crash. 
> Therefore, we should check the return value and handle the error.
> 
> Signed-off-by: Gen Zhang <blackgod016574@gmail.com>
> 
> ---
> diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
> index 40f21fe..0a3d035 100644
> --- a/net/ipv6/ipv6_sockglue.c
> +++ b/net/ipv6/ipv6_sockglue.c
> @@ -68,6 +68,8 @@ int ip6_ra_control(struct sock *sk, int sel)
>  		return -ENOPROTOOPT;
>  
>  	new_ra = (sel >= 0) ? kmalloc(sizeof(*new_ra), GFP_KERNEL) : NULL;
> +	if (sel >= 0 && !new_ra)
> +		return -ENOMEM;
>  
>  	write_lock_bh(&ip6_ra_lock);
>  	for (rap = &ip6_ra_chain; (ra = *rap) != NULL; rap = &ra->next) {
> 

Was this really an omission? There is (!new_ra) handling below the for loop:
        if (!new_ra) {
                write_unlock_bh(&ip6_ra_lock);
                return -ENOBUFS;
        }

It used to handle both (sel >= 0) and (sel == 0) cases and it used to
return ENOBUFS in case of failure. For (sel >= 0) it also could at least
return EADDRINUSE when a collision was found -- even if memory was
exhausted.

In anyway, how could this lead to a pointer dereference? And why/how did
this get a CVE number?

thanks,
-- 
js
suse labs

^ permalink raw reply

* Re: use exact allocation for dma coherent memory
From: Christoph Hellwig @ 2019-07-01  8:48 UTC (permalink / raw)
  To: Maarten Lankhorst, Maxime Ripard, Sean Paul, David Airlie,
	Daniel Vetter, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Ian Abbott, H Hartley Sweeten
  Cc: devel, linux-s390, Intel Linux Wireless, linux-rdma, netdev,
	intel-gfx, linux-wireless, linux-kernel, dri-devel, linux-mm,
	iommu, moderated list:ARM PORT, linux-media
In-Reply-To: <20190614134726.3827-1-hch@lst.de>

On Fri, Jun 14, 2019 at 03:47:10PM +0200, Christoph Hellwig wrote:
> Switching to a slightly cleaned up alloc_pages_exact is pretty easy,
> but it turns out that because we didn't filter valid gfp_t flags
> on the DMA allocator, a bunch of drivers were passing __GFP_COMP
> to it, which is rather bogus in too many ways to explain.  Arm has
> been filtering it for a while, but this series instead tries to fix
> the drivers and warn when __GFP_COMP is passed, which makes it much
> larger than just adding the functionality.

Dear driver maintainers,

can you look over the patches touching your drivers, please?  I'd
like to get as much as possible of the driver patches into this
merge window, so that it can you through your maintainer trees.

^ permalink raw reply

* [PATCH net] Documentation/networking: fix default_ttl typo in mpls-sysctl
From: Hangbin Liu @ 2019-07-01  8:45 UTC (permalink / raw)
  To: netdev; +Cc: David Miller, Hangbin Liu

default_ttl should be integer instead of bool

Reported-by: Ying Xu <yinxu@redhat.com>
Fixes: a59166e47086 ("mpls: allow TTL propagation from IP packets to be configured")
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 Documentation/networking/mpls-sysctl.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/networking/mpls-sysctl.txt b/Documentation/networking/mpls-sysctl.txt
index 2f24a1912a48..025cc9b96992 100644
--- a/Documentation/networking/mpls-sysctl.txt
+++ b/Documentation/networking/mpls-sysctl.txt
@@ -30,7 +30,7 @@ ip_ttl_propagate - BOOL
 	0 - disabled / RFC 3443 [Short] Pipe Model
 	1 - enabled / RFC 3443 Uniform Model (default)
 
-default_ttl - BOOL
+default_ttl - INTEGER
 	Default TTL value to use for MPLS packets where it cannot be
 	propagated from an IP header, either because one isn't present
 	or ip_ttl_propagate has been disabled.
-- 
2.19.2


^ permalink raw reply related

* Re: [PATCH rdma-next v4 06/17] RDMA/counter: Add "auto" configuration mode support
From: Leon Romanovsky @ 2019-07-01  8:42 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Doug Ledford, RDMA mailing list, Majd Dibbiny, Mark Zhang,
	Saeed Mahameed, linux-netdev
In-Reply-To: <20190630004048.GB7173@mellanox.com>

On Sun, Jun 30, 2019 at 12:40:54AM +0000, Jason Gunthorpe wrote:
> On Tue, Jun 18, 2019 at 08:26:14PM +0300, Leon Romanovsky wrote:
>
> > +static void __rdma_counter_dealloc(struct rdma_counter *counter)
> > +{
> > +	mutex_lock(&counter->lock);
> > +	counter->device->ops.counter_dealloc(counter);
> > +	mutex_unlock(&counter->lock);
> > +}
>
> Does this lock do anything? The kref is 0 at this point, so no other
> thread can have a pointer to this lock.

Yes, it is leftover from atomic_read implementation.

>
> > +
> > +static void rdma_counter_dealloc(struct rdma_counter *counter)
> > +{
> > +	if (!counter)
> > +		return;
>
> Counter is never NULL.

Ohh, right, I'll clean some code near rdma_counter_dealloc/__rdma_counter_dealloc.

Thanks

>
> Jason

^ permalink raw reply

* [PATCH] xfrm: use list_for_each_entry_safe in xfrm_policy_flush
From: Li RongQing @ 2019-07-01  8:29 UTC (permalink / raw)
  To: netdev

The iterated pol maybe be freed since it is not protected
by RCU or spinlock when put it, lead to UAF, so use _safe
function to iterate over it against removal

Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
 net/xfrm/xfrm_policy.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 3235562f6588..87d770dab1f5 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -1772,7 +1772,7 @@ xfrm_policy_flush_secctx_check(struct net *net, u8 type, bool task_valid)
 int xfrm_policy_flush(struct net *net, u8 type, bool task_valid)
 {
 	int dir, err = 0, cnt = 0;
-	struct xfrm_policy *pol;
+	struct xfrm_policy *pol, *tmp;
 
 	spin_lock_bh(&net->xfrm.xfrm_policy_lock);
 
@@ -1781,7 +1781,7 @@ int xfrm_policy_flush(struct net *net, u8 type, bool task_valid)
 		goto out;
 
 again:
-	list_for_each_entry(pol, &net->xfrm.policy_all, walk.all) {
+	list_for_each_entry_safe(pol, tmp, &net->xfrm.policy_all, walk.all) {
 		dir = xfrm_policy_id2dir(pol->index);
 		if (pol->walk.dead ||
 		    dir >= XFRM_POLICY_MAX ||
-- 
2.16.2


^ permalink raw reply related

* Re: [PATCH] rtl8xxxu: Fix wifi low signal strength issue of RTL8723BU
From: Daniel Drake @ 2019-07-01  8:27 UTC (permalink / raw)
  To: Chris Chiu
  Cc: Jes Sorensen, Kalle Valo, David Miller, linux-wireless, netdev,
	Linux Kernel, Linux Upstreaming Team, Larry Finger
In-Reply-To: <20190627095247.8792-1-chiu@endlessm.com>

Hi Chris,

On Thu, Jun 27, 2019 at 5:53 PM Chris Chiu <chiu@endlessm.com> wrote:
> The WiFi tx power of RTL8723BU is extremely low after booting. So
> the WiFi scan gives very limited AP list and it always fails to
> connect to the selected AP. This module only supports 1x1 antenna
> and the antenna is switched to bluetooth due to some incorrect
> register settings.
>
> This commit hand over the antenna control to PTA, the wifi signal
> will be back to normal and the bluetooth scan can also work at the
> same time. However, the btcoexist still needs to be handled under
> different circumstances. If there's a BT connection established,
> the wifi still fails to connect until disconneting the BT.
>
> Signed-off-by: Chris Chiu <chiu@endlessm.com>

Really nice work finding this!

I know that after this change, you plan to bring over the btcoexist
code from the vendor driver (or at least the minimum required code)
for a more complete fix, but I'm curious how you found these magic
register values and how they compare to the values used by the vendor
driver with btcoexist?

What's PTA? A type of firmware-implemented btcoexist that works for
scanning but doesn't work when a BT connection is actually
established?

> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
> index 3adb1d3d47ac..6c3c70d93ac1 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
> @@ -1525,7 +1525,7 @@ static void rtl8723b_enable_rf(struct rtl8xxxu_priv *priv)
>         /*
>          * WLAN action by PTA
>          */
> -       rtl8xxxu_write8(priv, REG_WLAN_ACT_CONTROL_8723B, 0x04);
> +       rtl8xxxu_write8(priv, REG_WLAN_ACT_CONTROL_8723B, 0x0c);

The comment above this still says "WLAN action by PTA" and the vendor
driver has:
        //set wlan_act control by PTA
        pBtCoexist->fBtcWrite1Byte(pBtCoexist, 0x76e, 0x4);

but then also:
            //set wlan_act control by PTA
            pBtCoexist->fBtcWrite1Byte(pBtCoexist, 0x76e, 0xc);

So this change seems to be at least consistent with ambiguity of the
vendor driver, do you have any understanding of the extra bit that is
now set here?

It's not easy to follow the code flow of the vendor driver to see what
actually happens, have you checked that, does it end up using the 0xc
value?

> -        * 0x280, 0x00, 0x200, 0x80 - not clear
> +        * Different settings per different antenna position.
> +        * Antenna switch to BT: 0x280, 0x00 (inverse)
> +        * Antenna switch to WiFi: 0x0, 0x280 (inverse)
> +        * Antenna controlled by PTA: 0x200, 0x80 (inverse)
>          */
> -       rtl8xxxu_write32(priv, REG_S0S1_PATH_SWITCH, 0x00);
> +       rtl8xxxu_write32(priv, REG_S0S1_PATH_SWITCH, 0x80);

I don't quite follow the comment here. Why are there 2 values listed
for each possibility, what do you mean by inverse? You say the
register settings were incorrect, but the previous value was 0x00
which you now document as "antenna switch to wifi" which sounds like
it was already correct?

Which value does the vendor driver use?

>         /*
>          * Software control, antenna at WiFi side
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index 8136e268b4e6..87b2179a769e 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -3891,12 +3891,13 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
>
>         /* Check if MAC is already powered on */
>         val8 = rtl8xxxu_read8(priv, REG_CR);
> +       val16 = rtl8xxxu_read16(priv, REG_SYS_CLKR);
>
>         /*
>          * Fix 92DU-VC S3 hang with the reason is that secondary mac is not
>          * initialized. First MAC returns 0xea, second MAC returns 0x00
>          */
> -       if (val8 == 0xea)
> +       if (val8 == 0xea || !(val16 & BIT(11)))
>                 macpower = false;
>         else
>                 macpower = true;

At a glance I can't see which code this corresponds to in the vendor
driver, can you point that out?

Thanks
Daniel

^ permalink raw reply

* kernel panic: corrupted stack end in dput
From: syzbot @ 2019-07-01  8:27 UTC (permalink / raw)
  To: linux-fsdevel, linux-kernel, netdev, syzkaller-bugs, viro

Hello,

syzbot found the following crash on:

HEAD commit:    7b75e49d net: dsa: mv88e6xxx: wait after reset deactivation
git tree:       net
console output: https://syzkaller.appspot.com/x/log.txt?x=10f51b13a00000
kernel config:  https://syzkaller.appspot.com/x/.config?x=e7c31a94f66cc0aa
dashboard link: https://syzkaller.appspot.com/bug?extid=d88a977731a9888db7ba
compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=130f49bda00000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+d88a977731a9888db7ba@syzkaller.appspotmail.com

Kernel panic - not syncing: corrupted stack end detected inside scheduler
CPU: 1 PID: 8936 Comm: syz-executor.3 Not tainted 5.2.0-rc6+ #69
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+0x172/0x1f0 lib/dump_stack.c:113
  panic+0x2cb/0x744 kernel/panic.c:219
  schedule_debug kernel/sched/core.c:3272 [inline]
  __schedule+0x155d/0x1560 kernel/sched/core.c:3381
  preempt_schedule_notrace kernel/sched/core.c:3664 [inline]
  preempt_schedule_notrace+0xa0/0x130 kernel/sched/core.c:3635
  ___preempt_schedule_notrace+0x16/0x2f
  rcu_is_watching+0x23/0x30 kernel/rcu/tree.c:873
  rcu_read_lock include/linux/rcupdate.h:594 [inline]
  dput+0x41e/0x690 fs/dcache.c:845
  __fput+0x424/0x890 fs/file_table.c:293
  ____fput+0x16/0x20 fs/file_table.c:313
  task_work_run+0x145/0x1c0 kernel/task_work.c:113
  tracehook_notify_resume include/linux/tracehook.h:185 [inline]
  exit_to_usermode_loop+0x273/0x2c0 arch/x86/entry/common.c:168
  prepare_exit_to_usermode arch/x86/entry/common.c:199 [inline]
  syscall_return_slowpath arch/x86/entry/common.c:279 [inline]
  do_syscall_64+0x58e/0x680 arch/x86/entry/common.c:304
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x413201
Code: 75 14 b8 03 00 00 00 0f 05 48 3d 01 f0 ff ff 0f 83 04 1b 00 00 c3 48  
83 ec 08 e8 0a fc ff ff 48 89 04 24 b8 03 00 00 00 0f 05 <48> 8b 3c 24 48  
89 c2 e8 53 fc ff ff 48 89 d0 48 83 c4 08 48 3d 01
RSP: 002b:00007ffcee6f8e20 EFLAGS: 00000293 ORIG_RAX: 0000000000000003
RAX: 0000000000000000 RBX: 0000000000000006 RCX: 0000000000413201
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000005
RBP: 0000000000000001 R08: ffffffffffffffff R09: ffffffffffffffff
R10: 00007ffcee6f8f00 R11: 0000000000000293 R12: 000000000075c9a0
R13: 000000000075c9a0 R14: 00000000000127ed R15: ffffffffffffffff
Shutting down cpus with NMI
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

* Re: [PATCH v3 7/7] Input: add IOC3 serio driver
From: Dmitry Torokhov @ 2019-07-01  8:19 UTC (permalink / raw)
  To: Thomas Bogendoerfer
  Cc: Ralf Baechle, Paul Burton, James Hogan, Lee Jones,
	David S. Miller, Srinivas Kandagatla, Alessandro Zummo,
	Alexandre Belloni, Greg Kroah-Hartman, Jiri Slaby, linux-mips,
	linux-kernel, linux-input, netdev, linux-rtc, linux-serial
In-Reply-To: <20190613170636.6647-8-tbogendoerfer@suse.de>

Hi Thomas,

On Thu, Jun 13, 2019 at 07:06:33PM +0200, Thomas Bogendoerfer wrote:
> This patch adds a platform driver for supporting keyboard and mouse
> interface of SGI IOC3 chips.
> 
> Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>
> ---
>  drivers/input/serio/Kconfig   |  10 +++
>  drivers/input/serio/Makefile  |   1 +
>  drivers/input/serio/ioc3kbd.c | 158 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 169 insertions(+)
>  create mode 100644 drivers/input/serio/ioc3kbd.c
> 
> diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
> index f3e18f8ef9ca..373a1646019e 100644
> --- a/drivers/input/serio/Kconfig
> +++ b/drivers/input/serio/Kconfig
> @@ -165,6 +165,16 @@ config SERIO_MACEPS2
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called maceps2.
>  
> +config SERIO_SGI_IOC3
> +	tristate "SGI IOC3 PS/2 controller"
> +	depends on SGI_MFD_IOC3
> +	help
> +	  Say Y here if you have an SGI Onyx2, SGI Octane or IOC3 PCI card
> +	  and you want to attach and use a keyboard, mouse, or both.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called ioc3kbd.
> +
>  config SERIO_LIBPS2
>  	tristate "PS/2 driver library"
>  	depends on SERIO_I8042 || SERIO_I8042=n
> diff --git a/drivers/input/serio/Makefile b/drivers/input/serio/Makefile
> index 67950a5ccb3f..6d97bad7b844 100644
> --- a/drivers/input/serio/Makefile
> +++ b/drivers/input/serio/Makefile
> @@ -20,6 +20,7 @@ obj-$(CONFIG_HIL_MLC)		+= hp_sdc_mlc.o hil_mlc.o
>  obj-$(CONFIG_SERIO_PCIPS2)	+= pcips2.o
>  obj-$(CONFIG_SERIO_PS2MULT)	+= ps2mult.o
>  obj-$(CONFIG_SERIO_MACEPS2)	+= maceps2.o
> +obj-$(CONFIG_SERIO_SGI_IOC3)	+= ioc3kbd.o
>  obj-$(CONFIG_SERIO_LIBPS2)	+= libps2.o
>  obj-$(CONFIG_SERIO_RAW)		+= serio_raw.o
>  obj-$(CONFIG_SERIO_AMS_DELTA)	+= ams_delta_serio.o
> diff --git a/drivers/input/serio/ioc3kbd.c b/drivers/input/serio/ioc3kbd.c
> new file mode 100644
> index 000000000000..26fcf57465d6
> --- /dev/null
> +++ b/drivers/input/serio/ioc3kbd.c
> @@ -0,0 +1,158 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * SGI IOC3 PS/2 controller driver for linux
> + *
> + * Copyright (C) 2019 Thomas Bogendoerfer <tbogendoerfer@suse.de>
> + *
> + * Based on code Copyright (C) 2005 Stanislaw Skowronek <skylark@unaligned.org>
> + *               Copyright (C) 2009 Johannes Dickgreber <tanzy@gmx.de>
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/serio.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +
> +#include <asm/sn/ioc3.h>
> +
> +struct ioc3kbd_data {
> +	struct ioc3_serioregs __iomem *regs;
> +	struct serio *kbd, *aux;
> +};
> +
> +static int ioc3kbd_write(struct serio *dev, u8 val)
> +{
> +	struct ioc3kbd_data *d = dev->port_data;
> +	unsigned long timeout = 0;
> +	u32 mask;
> +
> +	mask = (dev == d->aux) ? KM_CSR_M_WRT_PEND : KM_CSR_K_WRT_PEND;
> +	while ((readl(&d->regs->km_csr) & mask) && (timeout < 1000)) {
> +		udelay(100);
> +		timeout++;
> +	}
> +
> +	if (timeout >= 1000)
> +		return -1;

-ETIMEDOUT?

> +
> +	writel(val, dev == d->aux ?  &d->regs->m_wd : &d->regs->k_wd);

Nit: there are 2 spaces after ?, only one is needed.

> +
> +	return 0;
> +}
> +
> +static irqreturn_t ioc3kbd_intr(int itq, void *dev_id)
> +{
> +	struct ioc3kbd_data *d = dev_id;
> +	u32 data_k, data_m;
> +
> +	data_k = readl(&d->regs->k_rd);
> +	data_m = readl(&d->regs->m_rd);
> +
> +	if (data_k & KM_RD_VALID_0)
> +		serio_interrupt(d->kbd,
> +		(data_k >> KM_RD_DATA_0_SHIFT) & 0xff, 0);

This is weird formatting, you need one more tab here.

> +	if (data_k & KM_RD_VALID_1)
> +		serio_interrupt(d->kbd,
> +		(data_k >> KM_RD_DATA_1_SHIFT) & 0xff, 0);
> +	if (data_k & KM_RD_VALID_2)
> +		serio_interrupt(d->kbd,
> +		(data_k >> KM_RD_DATA_2_SHIFT) & 0xff, 0);
> +	if (data_m & KM_RD_VALID_0)
> +		serio_interrupt(d->aux,
> +		(data_m >> KM_RD_DATA_0_SHIFT) & 0xff, 0);
> +	if (data_m & KM_RD_VALID_1)
> +		serio_interrupt(d->aux,
> +		(data_m >> KM_RD_DATA_1_SHIFT) & 0xff, 0);
> +	if (data_m & KM_RD_VALID_2)
> +		serio_interrupt(d->aux,
> +		(data_m >> KM_RD_DATA_2_SHIFT) & 0xff, 0);
> +
> +	return 0;
> +}
> +
> +static int ioc3kbd_probe(struct platform_device *pdev)
> +{
> +	struct ioc3_serioregs __iomem *regs;
> +	struct device *dev = &pdev->dev;
> +	struct ioc3kbd_data *d;
> +	struct serio *sk, *sa;
> +	struct resource *mem;
> +	int irq, ret;
> +
> +	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	regs = devm_ioremap_resource(&pdev->dev, mem);

We have a brand new helper: devm_platform_ioremap_resource()

> +	if (IS_ERR(regs))
> +		return PTR_ERR(regs);
> +
> +	irq = platform_get_irq(pdev, 0);
> +	if (irq < 0)
> +		return -ENXIO;
> +
> +	d = devm_kzalloc(&pdev->dev, sizeof(struct ioc3kbd_data), GFP_KERNEL);

I think we nor prefer deriving type from the pointer:

	d = evm_kzalloc(&pdev->dev, sizeof(*d), GFP_KERNEL);

> +	if (!d)
> +		return -ENOMEM;
> +
> +	ret = devm_request_irq(&pdev->dev, irq, ioc3kbd_intr, IRQF_SHARED,
> +			       "ioc3-kbd", d);
> +	if (ret) {
> +		dev_err(&pdev->dev, "could not request IRQ %d\n", irq);
> +		return ret;
> +	}

You need to make sure that interrupt will not fire while serio ports are
not yet allocated/registered. Is there a way to inhibit interrupt
generation on controller side?

> +
> +	sk = kzalloc(sizeof(struct serio), GFP_KERNEL);

	sk = kzalloc(sizeof(*sk), GFP_KERNEL);

> +	if (!sk)
> +		return -ENOMEM;
> +
> +	sa = kzalloc(sizeof(struct serio), GFP_KERNEL);

	sa = kzalloc(sizeof(*sa), GFP_KERNEL);

> +	if (!sa) {
> +		kfree(sk);
> +		return -ENOMEM;
> +	}
> +
> +	sk->id.type = SERIO_8042;
> +	sk->write = ioc3kbd_write;
> +	snprintf(sk->name, sizeof(sk->name), "IOC3 keyboard %d", pdev->id);
> +	snprintf(sk->phys, sizeof(sk->phys), "ioc3/serio%dkbd", pdev->id);
> +	sk->port_data = d;
> +	sk->dev.parent = &pdev->dev;
> +
> +	sa->id.type = SERIO_8042;
> +	sa->write = ioc3kbd_write;
> +	snprintf(sa->name, sizeof(sa->name), "IOC3 auxiliary %d", pdev->id);
> +	snprintf(sa->phys, sizeof(sa->phys), "ioc3/serio%daux", pdev->id);
> +	sa->port_data = d;
> +	sa->dev.parent = dev;
> +
> +	d->regs = regs;
> +	d->kbd = sk;
> +	d->aux = sa;
> +
> +	platform_set_drvdata(pdev, d);
> +	serio_register_port(d->kbd);
> +	serio_register_port(d->aux);
> +	return 0;
> +}
> +
> +static int ioc3kbd_remove(struct platform_device *pdev)
> +{
> +	struct ioc3kbd_data *d = platform_get_drvdata(pdev);
> +
> +	serio_unregister_port(d->kbd);
> +	serio_unregister_port(d->aux);

If you unregister ports while interrupt is registered/enabled you may
get a crash.

> +	return 0;
> +}
> +
> +static struct platform_driver ioc3kbd_driver = {
> +	.probe          = ioc3kbd_probe,
> +	.remove         = ioc3kbd_remove,
> +	.driver = {
> +		.name = "ioc3-kbd",
> +	},
> +};
> +module_platform_driver(ioc3kbd_driver);
> +
> +MODULE_AUTHOR("Thomas Bogendoerfer <tbogendoerfer@suse.de>");
> +MODULE_DESCRIPTION("SGI IOC3 serio driver");
> +MODULE_LICENSE("GPL");
> -- 
> 2.13.7
> 

-- 
Dmitry

^ permalink raw reply

* Re: WARNING in is_bpf_text_address
From: Dmitry Vyukov @ 2019-07-01  8:12 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: syzbot, Andrew Morton, Alexei Starovoitov, bpf, Daniel Borkmann,
	David Miller, hawk, Jakub Kicinski, Johannes Berg, Johannes Berg,
	John Fastabend, Martin KaFai Lau, LKML, Waiman Long, Ingo Molnar,
	netdev, Paul McKenney, Peter Zijlstra, Song Liu, syzkaller-bugs,
	Thomas Gleixner, Tejun Heo, Linus Torvalds, Will Deacon,
	xdp-newbies, Yonghong Song
In-Reply-To: <c0e440a1-30aa-a636-fe5c-44f71705857b@acm.org>

On Fri, Jun 28, 2019 at 5:17 PM Bart Van Assche <bvanassche@acm.org> wrote:
>
> On 6/28/19 6:05 AM, syzbot wrote:
> > syzbot has bisected this bug to:
> >
> > commit a0b0fd53e1e67639b303b15939b9c653dbe7a8c4
> > Author: Bart Van Assche <bvanassche@acm.org>
> > Date:   Thu Feb 14 23:00:46 2019 +0000
> >
> >      locking/lockdep: Free lock classes that are no longer in use
> >
> > bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=152f6a9da00000
> > start commit:   abf02e29 Merge tag 'pm-5.2-rc6' of
> > git://git.kernel.org/pu..
> > git tree:       upstream
> > final crash:    https://syzkaller.appspot.com/x/report.txt?x=172f6a9da00000
> > console output: https://syzkaller.appspot.com/x/log.txt?x=132f6a9da00000
> > kernel config:  https://syzkaller.appspot.com/x/.config?x=28ec3437a5394ee0
> > dashboard link:
> > https://syzkaller.appspot.com/bug?extid=bd3bba6ff3fcea7a6ec6
> > syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=14ae828aa00000
> >
> > Reported-by: syzbot+bd3bba6ff3fcea7a6ec6@syzkaller.appspotmail.com
> > Fixes: a0b0fd53e1e6 ("locking/lockdep: Free lock classes that are no
> > longer in use")
> >
> > For information about bisection process see:
> > https://goo.gl/tpsmEJ#bisection
>
> Dmitry, this bisection result does not make any sense to me. Can I mark
> this bisection result myself as invalid?

Hi Bart,

syzbot does not use such bit of info for anything at the moment. So
just saying that it is invalid in this thread is enough to "mark it is
invalid" for all practical purposes. Let's consider it marked.

^ permalink raw reply

* Re: [PATCH bpf] selftests: bpf: fix inlines in test_lwt_seg6local
From: Jiri Benc @ 2019-07-01  8:12 UTC (permalink / raw)
  To: Song Liu
  Cc: Networking, bpf, Alexei Starovoitov, Daniel Borkmann,
	Mathieu Xhonneux
In-Reply-To: <CAPhsuW4Ric_nMGxpKf3mEJw3JDBZYpbeAQwTW_Nrsz79T2zisw@mail.gmail.com>

On Sat, 29 Jun 2019 11:04:54 -0700, Song Liu wrote:
> > Maybe use "__always_inline" as most other tests do?  
> 
> I meant "static __always_inline".

Sure, I can do that. It doesn't seem to be as consistent as you
suggest, though.

There are three different forms used in selftests/bpf/progs:

static __always_inline
static inline __attribute__((__always_inline__))
static inline __attribute__((always_inline))

As this is a bug causing selftests to fail (at least for some clang/llvm
versions), how about applying this to bpf.git as a minimal fix and
unifying the progs in bpf-next?

Thanks,

 Jiri

^ permalink raw reply

* Re: [PATCH next] sysctl: add proc_dointvec_jiffies_minmax to limit the min/max write value
From: Zhiqiang Liu @ 2019-07-01  8:06 UTC (permalink / raw)
  To: Kees Cook, akpm
  Cc: mcgrof, linux-kernel, linux-fsdevel, ebiederm, pbonzini, viro,
	adobriyan, mingfangsen, wangxiaogang3, Zhoukang (A), netdev
In-Reply-To: <dd40ae3b-8e0a-2d55-d402-6f261a6c0e09@huawei.com>

friendly ping ...

On 2019/6/4 23:27, Zhiqiang Liu wrote:
>> On Wed, May 15, 2019 at 10:53:55PM +0800, Zhiqiang Liu wrote:
>>
>> (Please include akpm on CC for next versions of this, as he's likely
>> the person to take this patch.)
> Thanks for your advice. And sorry to reply you so late.
> 
>>>>> In proc_dointvec_jiffies func, the write value is only checked
>>>>> whether it is larger than INT_MAX. If the write value is less
>>>>> than zero, it can also be successfully writen in the data.
>>
>> This appears to be "be design", but I see many "unsigned int" users
>> that might be tricked into giant values... (for example, see
>> net/netfilter/nf_conntrack_standalone.c)
>>
>> Should proc_dointvec_jiffies() just be fixed to disallow negative values
>> entirely? Looking at the implementation, it seems to be very intentional
>> about accepting negative values.
>>
>> However, when I looked through a handful of proc_dointvec_jiffies()
>> users, it looks like they're all expecting a positive value. Many in the
>> networking subsystem are, in fact, writing to unsigned long variables,
>> as I mentioned.
>>
> I totally agree with you. And I also cannot find an scenario that expects
> negative values. Consideing the "negative" scenario may be exist, I add the
> proc_dointvec_jiffies_minmax like proc_dointvec_minmax.
> 
>> Are there real-world cases of wanting to set a negative jiffie value
>> via proc_dointvec_jiffies()?
> Until now, I do not find such cases.
> 
>>>>>
>>>>> Here, we add a new func, proc_dointvec_jiffies_minmax, to limit the
>>>>> min/max write value, which is similar to the proc_dointvec_minmax func.
>>>>>
>>
>> If proc_dointvec_jiffies() can't just be fixed, where will the new
>> function get used? It seems all the "unsigned int" users could benefit.
>>
> I tend to add the proc_dointvec_jiffies_minmax func to provide more choices and
> not change the previous use of proc_dointvec_jiffies func.
> 
> Thanks for your reply again.
> 
> 
> .
> 


^ permalink raw reply

* Re: [RFC PATCH v5] rtl8xxxu: Improve TX performance of RTL8723BU on rtl8xxxu driver
From: Daniel Drake @ 2019-07-01  7:57 UTC (permalink / raw)
  To: Chris Chiu
  Cc: Jes Sorensen, Kalle Valo, David Miller, linux-wireless, netdev,
	Linux Kernel, Linux Upstreaming Team
In-Reply-To: <20190617065600.40405-1-chiu@endlessm.com>

On Mon, Jun 17, 2019 at 2:56 PM Chris Chiu <chiu@endlessm.com> wrote:
> With this commit, the tx rate of each data and qos data packet will
> be 39Mbps (MCS4) with the 0xF00000 as the tx rate mask. The 20th bit
> to 23th bit means MCS4 to MCS7. It means that the firmware still picks
> the lowest rate from the rate mask and explains why the tx rate of
> data and qos data is always lowest 1Mbps because the default rate mask
> passed is always 0xFFFFFFF ranges from the basic CCK rate, OFDM rate,
> and MCS rate. However, with Realtek's driver, the tx rate observed from
> wireshark under the same condition is almost 65Mbps or 72Mbps
suggestion: add:
, indicating that rtl8xxxu could still be further improved.

Then remove this paragraph, I think we're in agreement of the approach here:
> I believe the firmware of RTL8723BU may need fix. And I think we
> can still bring in the dm_watchdog as rtlwifi to improve from the
> driver side. Please leave precious comments for my commits and
> suggest what I can do better. Or suggest if there's any better idea
> to fix this. Thanks.

> Signed-off-by: Chris Chiu <chiu@endlessm.com>
Reviewed-by: Daniel Drake <drake@endlessm.com>

> +        * is supported and no iface_combinations are providec.

typo: provided

^ permalink raw reply

* [PATCH net] r8152: fix the setting of detecting the linking change for runtime suspend
From: Hayes Wang @ 2019-07-01  7:53 UTC (permalink / raw)
  To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang

1. Rename r8153b_queue_wake() to r8153_queue_wake().

2. Correct the setting. The enable bit should be 0xd38c bit 0. Besides,
   the 0xd38a bit 0 and 0xd398 bit 8 have to be cleared for both enabled
   and disabled.

Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/usb/r8152.c | 38 +++++++++++++++++++++++++++-----------
 1 file changed, 27 insertions(+), 11 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index e0dcb681cfe5..101d1325f3f1 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -53,6 +53,9 @@
 #define PAL_BDC_CR		0xd1a0
 #define PLA_TEREDO_TIMER	0xd2cc
 #define PLA_REALWOW_TIMER	0xd2e8
+#define PLA_SUSPEND_FLAG	0xd38a
+#define PLA_INDICATE_FALG	0xd38c
+#define PLA_EXTRA_STATUS	0xd398
 #define PLA_EFUSE_DATA		0xdd00
 #define PLA_EFUSE_CMD		0xdd02
 #define PLA_LEDSEL		0xdd90
@@ -336,6 +339,15 @@
 /* PLA_BOOT_CTRL */
 #define AUTOLOAD_DONE		0x0002
 
+/* PLA_SUSPEND_FLAG */
+#define LINK_CHG_EVENT		BIT(0)
+
+/* PLA_INDICATE_FALG */
+#define UPCOMING_RUNTIME_D3	BIT(0)
+
+/* PLA_EXTRA_STATUS */
+#define LINK_CHANGE_FLAG	BIT(8)
+
 /* USB_USB2PHY */
 #define USB2PHY_SUSPEND		0x0001
 #define USB2PHY_L1		0x0002
@@ -2806,20 +2818,24 @@ static void r8153b_power_cut_en(struct r8152 *tp, bool enable)
 	ocp_write_word(tp, MCU_TYPE_USB, USB_MISC_0, ocp_data);
 }
 
-static void r8153b_queue_wake(struct r8152 *tp, bool enable)
+static void r8153_queue_wake(struct r8152 *tp, bool enable)
 {
 	u32 ocp_data;
 
-	ocp_data = ocp_read_byte(tp, MCU_TYPE_PLA, 0xd38a);
+	ocp_data = ocp_read_byte(tp, MCU_TYPE_PLA, PLA_INDICATE_FALG);
 	if (enable)
-		ocp_data |= BIT(0);
+		ocp_data |= UPCOMING_RUNTIME_D3;
 	else
-		ocp_data &= ~BIT(0);
-	ocp_write_byte(tp, MCU_TYPE_PLA, 0xd38a, ocp_data);
+		ocp_data &= ~UPCOMING_RUNTIME_D3;
+	ocp_write_byte(tp, MCU_TYPE_PLA, PLA_INDICATE_FALG, ocp_data);
+
+	ocp_data = ocp_read_byte(tp, MCU_TYPE_PLA, PLA_SUSPEND_FLAG);
+	ocp_data &= ~LINK_CHG_EVENT;
+	ocp_write_byte(tp, MCU_TYPE_PLA, PLA_SUSPEND_FLAG, ocp_data);
 
-	ocp_data = ocp_read_byte(tp, MCU_TYPE_PLA, 0xd38c);
-	ocp_data &= ~BIT(0);
-	ocp_write_byte(tp, MCU_TYPE_PLA, 0xd38c, ocp_data);
+	ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_EXTRA_STATUS);
+	ocp_data &= ~LINK_CHANGE_FLAG;
+	ocp_write_word(tp, MCU_TYPE_PLA, PLA_EXTRA_STATUS, ocp_data);
 }
 
 static bool rtl_can_wakeup(struct r8152 *tp)
@@ -2887,14 +2903,14 @@ static void rtl8153_runtime_enable(struct r8152 *tp, bool enable)
 static void rtl8153b_runtime_enable(struct r8152 *tp, bool enable)
 {
 	if (enable) {
-		r8153b_queue_wake(tp, true);
+		r8153_queue_wake(tp, true);
 		r8153b_u1u2en(tp, false);
 		r8153_u2p3en(tp, false);
 		rtl_runtime_suspend_enable(tp, true);
 		r8153b_ups_en(tp, true);
 	} else {
 		r8153b_ups_en(tp, false);
-		r8153b_queue_wake(tp, false);
+		r8153_queue_wake(tp, false);
 		rtl_runtime_suspend_enable(tp, false);
 		r8153_u2p3en(tp, true);
 		r8153b_u1u2en(tp, true);
@@ -4221,7 +4237,7 @@ static void r8153b_init(struct r8152 *tp)
 
 	r8153b_power_cut_en(tp, false);
 	r8153b_ups_en(tp, false);
-	r8153b_queue_wake(tp, false);
+	r8153_queue_wake(tp, false);
 	rtl_runtime_suspend_enable(tp, false);
 	r8153b_u1u2en(tp, true);
 	usb_enable_lpm(tp->udev);
-- 
2.21.0


^ permalink raw reply related

* Re: [PATCH net-next 0/2] net: ipv4: fix circular-list infinite loop
From: Tariq Toukan @ 2019-07-01  7:21 UTC (permalink / raw)
  To: Ran Rozenstein, David Miller, fw@strlen.de
  Cc: netdev@vger.kernel.org, Maor Gottlieb
In-Reply-To: <AM4PR0501MB27693D777034422FC18A98B9C5F90@AM4PR0501MB2769.eurprd05.prod.outlook.com>



On 7/1/2019 9:46 AM, Ran Rozenstein wrote:
> 
> 
>> -----Original Message-----
>> From: Tariq Toukan
>> Sent: Sunday, June 30, 2019 10:57
>> To: David Miller <davem@davemloft.net>; fw@strlen.de
>> Cc: netdev@vger.kernel.org; Ran Rozenstein <ranro@mellanox.com>; Tariq
>> Toukan <tariqt@mellanox.com>
>> Subject: Re: [PATCH net-next 0/2] net: ipv4: fix circular-list infinite loop
>>
>>
>>
>> On 6/27/2019 7:54 PM, David Miller wrote:
>>> From: Florian Westphal <fw@strlen.de>
>>> Date: Thu, 27 Jun 2019 14:03:31 +0200
>>>
>>>> Tariq and Ran reported a regression caused by net-next commit
>>>> 2638eb8b50cf ("net: ipv4: provide __rcu annotation for ifa_list").
>>>>
>>>> This happens when net.ipv4.conf.$dev.promote_secondaries sysctl is
>>>> enabled -- we can arrange for ifa->next to point at ifa, so next
>>>> process that tries to walk the list loops forever.
>>>>
>>>> Fix this and extend rtnetlink.sh with a small test case for this.
>>>
>>> Series applied, thanks Florian.
>>>
>>
>> Thanks Florian!
>>
>> Ran, please test and update.
>>
>> Tariq
> 
> Thanks Florian.
> Didn't reproduce tonight with the fixes.
> 
> Ran.
> 

Sounds good!

Thanks,
Tariq

^ permalink raw reply

* RE: [PATCH net-next 0/2] net: ipv4: fix circular-list infinite loop
From: Ran Rozenstein @ 2019-07-01  6:46 UTC (permalink / raw)
  To: Tariq Toukan, David Miller, fw@strlen.de
  Cc: netdev@vger.kernel.org, Maor Gottlieb
In-Reply-To: <d419cd16-e693-2214-fa41-4c9c81f1649d@mellanox.com>



> -----Original Message-----
> From: Tariq Toukan
> Sent: Sunday, June 30, 2019 10:57
> To: David Miller <davem@davemloft.net>; fw@strlen.de
> Cc: netdev@vger.kernel.org; Ran Rozenstein <ranro@mellanox.com>; Tariq
> Toukan <tariqt@mellanox.com>
> Subject: Re: [PATCH net-next 0/2] net: ipv4: fix circular-list infinite loop
> 
> 
> 
> On 6/27/2019 7:54 PM, David Miller wrote:
> > From: Florian Westphal <fw@strlen.de>
> > Date: Thu, 27 Jun 2019 14:03:31 +0200
> >
> >> Tariq and Ran reported a regression caused by net-next commit
> >> 2638eb8b50cf ("net: ipv4: provide __rcu annotation for ifa_list").
> >>
> >> This happens when net.ipv4.conf.$dev.promote_secondaries sysctl is
> >> enabled -- we can arrange for ifa->next to point at ifa, so next
> >> process that tries to walk the list loops forever.
> >>
> >> Fix this and extend rtnetlink.sh with a small test case for this.
> >
> > Series applied, thanks Florian.
> >
> 
> Thanks Florian!
> 
> Ran, please test and update.
> 
> Tariq

Thanks Florian.
Didn't reproduce tonight with the fixes.

Ran.


^ permalink raw reply

* Re: r8169 not working on 5.2.0rc6 with GPD MicroPC
From: Heiner Kallweit @ 2019-07-01  5:50 UTC (permalink / raw)
  To: Karsten Wiborg, Andrew Lunn; +Cc: nic_swsd, romieu, netdev
In-Reply-To: <116e4be6-e710-eb2d-0992-a132f62a8727@web.de>

On 01.07.2019 00:21, Karsten Wiborg wrote:
> Hi Heiner,
> 
> On 30/06/2019 23:55, Heiner Kallweit wrote:
>> This one shows that the vendor driver (r8168) uses a random MAC address.
>> Means the driver can't read a valid MAC address from the chip, maybe due
>> to a broken BIOS.
>> Alternatively you could use r8169 and set a MAC address manually with
>> ifconfig <if> hw ether <MAC address>
> Hmm, did some more testing:
> did a rmmod r8168 and (after "un"blacklisting the r8169) modprobed the
> r8169. This time r8169 came up nicely but with a complete different MAC
> (forgot to not than one though).
> So I guess the vendor compilation did other stuff besides just compiling
> the r8168 kernel module.
> 
> Did another test:
> blacklisted the r8168, renamed r8168.ko to r8168.bak, depmod -a and
> powercycled the system. Funny it came up with both r8168 and r8169
> loaded and I got my intended IP address from. DHCP, so r8168 somewhat
> got loaded and used his MAC.
> Did another rmmod r8168, rmmod r8169 and then modprobe r8169.
> Even though I did NOT configure a MAC address myself manually it came up
> with a new MAC address and of course got a dynamich IP address.
> So I don't know where the vendor somewhat changed something (with his
> compiling/installing) to the effect that r8169 now works?!?
> 
When the vendor driver assigns a random MAC address, it writes it to the
chip. The related registers may be persistent (can't say exactly due to
missing documentation).

> Regards,
> Karsten
> 
Heiner

^ permalink raw reply

* Re: memory leak in create_ctx
From: syzbot @ 2019-07-01  5:40 UTC (permalink / raw)
  To: aviadye, borisp, daniel, davejwatson, davem, john.fastabend,
	linux-kernel, netdev, syzkaller-bugs
In-Reply-To: <5d1999ff323f_18a42abda71925b4cf@john-XPS-13-9370.notmuch>

Hello,

syzbot has tested the proposed patch but the reproducer still triggered  
crash:
memory leak in create_ctx

2019/07/01 05:38:26 executed programs: 23
BUG: memory leak
unreferenced object 0xffff888102914e00 (size 512):
   comm "syz-executor.4", pid 7333, jiffies 4294944085 (age 13.950s)
   hex dump (first 32 bytes):
     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
   backtrace:
     [<000000002f2bb8be>] kmemleak_alloc_recursive  
include/linux/kmemleak.h:55 [inline]
     [<000000002f2bb8be>] slab_post_alloc_hook mm/slab.h:439 [inline]
     [<000000002f2bb8be>] slab_alloc mm/slab.c:3326 [inline]
     [<000000002f2bb8be>] kmem_cache_alloc_trace+0x13d/0x280 mm/slab.c:3553
     [<00000000b76aef16>] kmalloc include/linux/slab.h:547 [inline]
     [<00000000b76aef16>] kzalloc include/linux/slab.h:742 [inline]
     [<00000000b76aef16>] create_ctx+0x25/0x70 net/tls/tls_main.c:648
     [<00000000dc9c9d2e>] tls_init net/tls/tls_main.c:837 [inline]
     [<00000000dc9c9d2e>] tls_init+0x97/0x1f0 net/tls/tls_main.c:819
     [<000000009d663c39>] __tcp_set_ulp net/ipv4/tcp_ulp.c:126 [inline]
     [<000000009d663c39>] tcp_set_ulp+0xe2/0x190 net/ipv4/tcp_ulp.c:147
     [<00000000551f7621>] do_tcp_setsockopt.isra.0+0x19a/0xd60  
net/ipv4/tcp.c:2789
     [<00000000d02c41d7>] tcp_setsockopt+0x71/0x80 net/ipv4/tcp.c:3103
     [<0000000085d221c1>] sock_common_setsockopt+0x38/0x50  
net/core/sock.c:3129
     [<00000000d294eeda>] __sys_setsockopt+0x98/0x120 net/socket.c:2072
     [<00000000f1f1d607>] __do_sys_setsockopt net/socket.c:2083 [inline]
     [<00000000f1f1d607>] __se_sys_setsockopt net/socket.c:2080 [inline]
     [<00000000f1f1d607>] __x64_sys_setsockopt+0x26/0x30 net/socket.c:2080
     [<00000000fbd4f794>] do_syscall_64+0x76/0x1a0  
arch/x86/entry/common.c:301
     [<000000007383b736>] entry_SYSCALL_64_after_hwframe+0x44/0xa9

BUG: memory leak
unreferenced object 0xffff888103860c00 (size 512):
   comm "syz-executor.0", pid 7342, jiffies 4294944115 (age 13.650s)
   hex dump (first 32 bytes):
     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
   backtrace:
     [<000000002f2bb8be>] kmemleak_alloc_recursive  
include/linux/kmemleak.h:55 [inline]
     [<000000002f2bb8be>] slab_post_alloc_hook mm/slab.h:439 [inline]
     [<000000002f2bb8be>] slab_alloc mm/slab.c:3326 [inline]
     [<000000002f2bb8be>] kmem_cache_alloc_trace+0x13d/0x280 mm/slab.c:3553
     [<00000000b76aef16>] kmalloc include/linux/slab.h:547 [inline]
     [<00000000b76aef16>] kzalloc include/linux/slab.h:742 [inline]
     [<00000000b76aef16>] create_ctx+0x25/0x70 net/tls/tls_main.c:648
     [<00000000dc9c9d2e>] tls_init net/tls/tls_main.c:837 [inline]
     [<00000000dc9c9d2e>] tls_init+0x97/0x1f0 net/tls/tls_main.c:819
     [<000000009d663c39>] __tcp_set_ulp net/ipv4/tcp_ulp.c:126 [inline]
     [<000000009d663c39>] tcp_set_ulp+0xe2/0x190 net/ipv4/tcp_ulp.c:147
     [<00000000551f7621>] do_tcp_setsockopt.isra.0+0x19a/0xd60  
net/ipv4/tcp.c:2789
     [<00000000d02c41d7>] tcp_setsockopt+0x71/0x80 net/ipv4/tcp.c:3103
     [<0000000085d221c1>] sock_common_setsockopt+0x38/0x50  
net/core/sock.c:3129
     [<00000000d294eeda>] __sys_setsockopt+0x98/0x120 net/socket.c:2072
     [<00000000f1f1d607>] __do_sys_setsockopt net/socket.c:2083 [inline]
     [<00000000f1f1d607>] __se_sys_setsockopt net/socket.c:2080 [inline]
     [<00000000f1f1d607>] __x64_sys_setsockopt+0x26/0x30 net/socket.c:2080
     [<00000000fbd4f794>] do_syscall_64+0x76/0x1a0  
arch/x86/entry/common.c:301
     [<000000007383b736>] entry_SYSCALL_64_after_hwframe+0x44/0xa9

BUG: memory leak
unreferenced object 0xffff88810e3e1c00 (size 512):
   comm "syz-executor.5", pid 7384, jiffies 4294944151 (age 13.290s)
   hex dump (first 32 bytes):
     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
   backtrace:
     [<000000002f2bb8be>] kmemleak_alloc_recursive  
include/linux/kmemleak.h:55 [inline]
     [<000000002f2bb8be>] slab_post_alloc_hook mm/slab.h:439 [inline]
     [<000000002f2bb8be>] slab_alloc mm/slab.c:3326 [inline]
     [<000000002f2bb8be>] kmem_cache_alloc_trace+0x13d/0x280 mm/slab.c:3553
     [<00000000b76aef16>] kmalloc include/linux/slab.h:547 [inline]
     [<00000000b76aef16>] kzalloc include/linux/slab.h:742 [inline]
     [<00000000b76aef16>] create_ctx+0x25/0x70 net/tls/tls_main.c:648
     [<00000000dc9c9d2e>] tls_init net/tls/tls_main.c:837 [inline]
     [<00000000dc9c9d2e>] tls_init+0x97/0x1f0 net/tls/tls_main.c:819
     [<000000009d663c39>] __tcp_set_ulp net/ipv4/tcp_ulp.c:126 [inline]
     [<000000009d663c39>] tcp_set_ulp+0xe2/0x190 net/ipv4/tcp_ulp.c:147
     [<00000000551f7621>] do_tcp_setsockopt.isra.0+0x19a/0xd60  
net/ipv4/tcp.c:2789
     [<00000000d02c41d7>] tcp_setsockopt+0x71/0x80 net/ipv4/tcp.c:3103
     [<0000000085d221c1>] sock_common_setsockopt+0x38/0x50  
net/core/sock.c:3129
     [<00000000d294eeda>] __sys_setsockopt+0x98/0x120 net/socket.c:2072
     [<00000000f1f1d607>] __do_sys_setsockopt net/socket.c:2083 [inline]
     [<00000000f1f1d607>] __se_sys_setsockopt net/socket.c:2080 [inline]
     [<00000000f1f1d607>] __x64_sys_setsockopt+0x26/0x30 net/socket.c:2080
     [<00000000fbd4f794>] do_syscall_64+0x76/0x1a0  
arch/x86/entry/common.c:301
     [<000000007383b736>] entry_SYSCALL_64_after_hwframe+0x44/0xa9

BUG: memory leak
unreferenced object 0xffff88811a0a7200 (size 512):
   comm "syz-executor.0", pid 7423, jiffies 4294944702 (age 7.780s)
   hex dump (first 32 bytes):
     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
   backtrace:
     [<000000002f2bb8be>] kmemleak_alloc_recursive  
include/linux/kmemleak.h:55 [inline]
     [<000000002f2bb8be>] slab_post_alloc_hook mm/slab.h:439 [inline]
     [<000000002f2bb8be>] slab_alloc mm/slab.c:3326 [inline]
     [<000000002f2bb8be>] kmem_cache_alloc_trace+0x13d/0x280 mm/slab.c:3553
     [<00000000b76aef16>] kmalloc include/linux/slab.h:547 [inline]
     [<00000000b76aef16>] kzalloc include/linux/slab.h:742 [inline]
     [<00000000b76aef16>] create_ctx+0x25/0x70 net/tls/tls_main.c:648
     [<00000000dc9c9d2e>] tls_init net/tls/tls_main.c:837 [inline]
     [<00000000dc9c9d2e>] tls_init+0x97/0x1f0 net/tls/tls_main.c:819
     [<000000009d663c39>] __tcp_set_ulp net/ipv4/tcp_ulp.c:126 [inline]
     [<000000009d663c39>] tcp_set_ulp+0xe2/0x190 net/ipv4/tcp_ulp.c:147
     [<00000000551f7621>] do_tcp_setsockopt.isra.0+0x19a/0xd60  
net/ipv4/tcp.c:2789
     [<00000000d02c41d7>] tcp_setsockopt+0x71/0x80 net/ipv4/tcp.c:3103
     [<0000000085d221c1>] sock_common_setsockopt+0x38/0x50  
net/core/sock.c:3129
     [<00000000d294eeda>] __sys_setsockopt+0x98/0x120 net/socket.c:2072
     [<00000000f1f1d607>] __do_sys_setsockopt net/socket.c:2083 [inline]
     [<00000000f1f1d607>] __se_sys_setsockopt net/socket.c:2080 [inline]
     [<00000000f1f1d607>] __x64_sys_setsockopt+0x26/0x30 net/socket.c:2080
     [<00000000fbd4f794>] do_syscall_64+0x76/0x1a0  
arch/x86/entry/common.c:301
     [<000000007383b736>] entry_SYSCALL_64_after_hwframe+0x44/0xa9



Tested on:

commit:         0b58d013 bpf: tls, implement unhash to avoid transition ou..
git tree:       git://github.com/cilium/linux ktls-unhash
console output: https://syzkaller.appspot.com/x/log.txt?x=134f2e0ba00000
kernel config:  https://syzkaller.appspot.com/x/.config?x=3bd5897d1df43b97
compiler:       gcc (GCC) 9.0.0 20181231 (experimental)


^ permalink raw reply

* RE: memory leak in create_ctx
From: John Fastabend @ 2019-07-01  5:28 UTC (permalink / raw)
  To: syzbot, aviadye, borisp, daniel, davejwatson, davem,
	john.fastabend, linux-kernel, netdev, syzkaller-bugs
In-Reply-To: <000000000000a420af058ad4bca2@google.com>

syzbot wrote:
> Hello,
> 
> syzbot found the following crash on:
> 
> HEAD commit:    79c3ba32 Merge tag 'drm-fixes-2019-06-07-1' of git://anong..
> git tree:       upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=170e0bfea00000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=d5c73825cbdc7326
> dashboard link: https://syzkaller.appspot.com/bug?extid=06537213db7ba2745c4a
> compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=10aa806aa00000
> 
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+06537213db7ba2745c4a@syzkaller.appspotmail.com
> 
> IPv6: ADDRCONF(NETDEV_CHANGE): team0: link becomes ready
> 2019/06/08 14:55:51 executed programs: 15
> 2019/06/08 14:55:56 executed programs: 31
> 2019/06/08 14:56:02 executed programs: 51
> BUG: memory leak
> unreferenced object 0xffff888117ceae00 (size 512):
>    comm "syz-executor.3", pid 7233, jiffies 4294949016 (age 13.640s)
>    hex dump (first 32 bytes):
>      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>    backtrace:
>      [<00000000e6550967>] kmemleak_alloc_recursive  
> include/linux/kmemleak.h:55 [inline]
>      [<00000000e6550967>] slab_post_alloc_hook mm/slab.h:439 [inline]
>      [<00000000e6550967>] slab_alloc mm/slab.c:3326 [inline]
>      [<00000000e6550967>] kmem_cache_alloc_trace+0x13d/0x280 mm/slab.c:3553
>      [<0000000014132182>] kmalloc include/linux/slab.h:547 [inline]
>      [<0000000014132182>] kzalloc include/linux/slab.h:742 [inline]
>      [<0000000014132182>] create_ctx+0x25/0x70 net/tls/tls_main.c:601
>      [<00000000e08e1a44>] tls_init net/tls/tls_main.c:787 [inline]
>      [<00000000e08e1a44>] tls_init+0x97/0x1e0 net/tls/tls_main.c:769
>      [<0000000037b0c43c>] __tcp_set_ulp net/ipv4/tcp_ulp.c:126 [inline]
>      [<0000000037b0c43c>] tcp_set_ulp+0xe2/0x190 net/ipv4/tcp_ulp.c:147
>      [<000000007a284277>] do_tcp_setsockopt.isra.0+0x19a/0xd60  
> net/ipv4/tcp.c:2784
>      [<00000000f35f3415>] tcp_setsockopt+0x71/0x80 net/ipv4/tcp.c:3098
>      [<00000000c840962c>] sock_common_setsockopt+0x38/0x50  
> net/core/sock.c:3124
>      [<0000000006b0801f>] __sys_setsockopt+0x98/0x120 net/socket.c:2072
>      [<00000000a6309f52>] __do_sys_setsockopt net/socket.c:2083 [inline]
>      [<00000000a6309f52>] __se_sys_setsockopt net/socket.c:2080 [inline]
>      [<00000000a6309f52>] __x64_sys_setsockopt+0x26/0x30 net/socket.c:2080
>      [<00000000fa555bbc>] do_syscall_64+0x76/0x1a0  
> arch/x86/entry/common.c:301
>      [<00000000a06d7d1a>] entry_SYSCALL_64_after_hwframe+0x44/0xa9
> 
> BUG: memory leak
> unreferenced object 0xffff88810965dc00 (size 512):
>    comm "syz-executor.2", pid 7235, jiffies 4294949016 (age 13.640s)
>    hex dump (first 32 bytes):
>      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>    backtrace:
>      [<00000000e6550967>] kmemleak_alloc_recursive  
> include/linux/kmemleak.h:55 [inline]
>      [<00000000e6550967>] slab_post_alloc_hook mm/slab.h:439 [inline]
>      [<00000000e6550967>] slab_alloc mm/slab.c:3326 [inline]
>      [<00000000e6550967>] kmem_cache_alloc_trace+0x13d/0x280 mm/slab.c:3553
>      [<0000000014132182>] kmalloc include/linux/slab.h:547 [inline]
>      [<0000000014132182>] kzalloc include/linux/slab.h:742 [inline]
>      [<0000000014132182>] create_ctx+0x25/0x70 net/tls/tls_main.c:601
>      [<00000000e08e1a44>] tls_init net/tls/tls_main.c:787 [inline]
>      [<00000000e08e1a44>] tls_init+0x97/0x1e0 net/tls/tls_main.c:769
>      [<0000000037b0c43c>] __tcp_set_ulp net/ipv4/tcp_ulp.c:126 [inline]
>      [<0000000037b0c43c>] tcp_set_ulp+0xe2/0x190 net/ipv4/tcp_ulp.c:147
>      [<000000007a284277>] do_tcp_setsockopt.isra.0+0x19a/0xd60  
> net/ipv4/tcp.c:2784
>      [<00000000f35f3415>] tcp_setsockopt+0x71/0x80 net/ipv4/tcp.c:3098
>      [<00000000c840962c>] sock_common_setsockopt+0x38/0x50  
> net/core/sock.c:3124
>      [<0000000006b0801f>] __sys_setsockopt+0x98/0x120 net/socket.c:2072
>      [<00000000a6309f52>] __do_sys_setsockopt net/socket.c:2083 [inline]
>      [<00000000a6309f52>] __se_sys_setsockopt net/socket.c:2080 [inline]
>      [<00000000a6309f52>] __x64_sys_setsockopt+0x26/0x30 net/socket.c:2080
>      [<00000000fa555bbc>] do_syscall_64+0x76/0x1a0  
> arch/x86/entry/common.c:301
>      [<00000000a06d7d1a>] entry_SYSCALL_64_after_hwframe+0x44/0xa9
> 
> BUG: memory leak
> unreferenced object 0xffff8881207d7600 (size 512):
>    comm "syz-executor.5", pid 7244, jiffies 4294949019 (age 13.610s)
>    hex dump (first 32 bytes):
>      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>      00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>    backtrace:
>      [<00000000e6550967>] kmemleak_alloc_recursive  
> include/linux/kmemleak.h:55 [inline]
>      [<00000000e6550967>] slab_post_alloc_hook mm/slab.h:439 [inline]
>      [<00000000e6550967>] slab_alloc mm/slab.c:3326 [inline]
>      [<00000000e6550967>] kmem_cache_alloc_trace+0x13d/0x280 mm/slab.c:3553
>      [<0000000014132182>] kmalloc include/linux/slab.h:547 [inline]
>      [<0000000014132182>] kzalloc include/linux/slab.h:742 [inline]
>      [<0000000014132182>] create_ctx+0x25/0x70 net/tls/tls_main.c:601
>      [<00000000e08e1a44>] tls_init net/tls/tls_main.c:787 [inline]
>      [<00000000e08e1a44>] tls_init+0x97/0x1e0 net/tls/tls_main.c:769
>      [<0000000037b0c43c>] __tcp_set_ulp net/ipv4/tcp_ulp.c:126 [inline]
>      [<0000000037b0c43c>] tcp_set_ulp+0xe2/0x190 net/ipv4/tcp_ulp.c:147
>      [<000000007a284277>] do_tcp_setsockopt.isra.0+0x19a/0xd60  
> net/ipv4/tcp.c:2784
>      [<00000000f35f3415>] tcp_setsockopt+0x71/0x80 net/ipv4/tcp.c:3098
>      [<00000000c840962c>] sock_common_setsockopt+0x38/0x50  
> net/core/sock.c:3124
>      [<0000000006b0801f>] __sys_setsockopt+0x98/0x120 net/socket.c:2072
>      [<00000000a6309f52>] __do_sys_setsockopt net/socket.c:2083 [inline]
>      [<00000000a6309f52>] __se_sys_setsockopt net/socket.c:2080 [inline]
>      [<00000000a6309f52>] __x64_sys_setsockopt+0x26/0x30 net/socket.c:2080
>      [<00000000fa555bbc>] do_syscall_64+0x76/0x1a0  
> arch/x86/entry/common.c:301
>      [<00000000a06d7d1a>] entry_SYSCALL_64_after_hwframe+0x44/0xa9
> 
> 
> 
> ---
> 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

#syz test: git://github.com/cilium/linux ktls-unhash

^ permalink raw reply

* Re: [RFC PATCH v5] rtl8xxxu: Improve TX performance of RTL8723BU on rtl8xxxu driver
From: Chris Chiu @ 2019-07-01  5:09 UTC (permalink / raw)
  To: Jes Sorensen, Kalle Valo, David Miller
  Cc: linux-wireless, netdev, Linux Kernel, Linux Upstreaming Team
In-Reply-To: <20190617065600.40405-1-chiu@endlessm.com>

On Mon, Jun 17, 2019 at 2:56 PM Chris Chiu <chiu@endlessm.com> wrote:
>
> We have 3 laptops which connect the wifi by the same RTL8723BU.
> The PCI VID/PID of the wifi chip is 10EC:B720 which is supported.
> They have the same problem with the in-kernel rtl8xxxu driver, the
> iperf (as a client to an ethernet-connected server) gets ~1Mbps.
> Nevertheless, the signal strength is reported as around -40dBm,
> which is quite good. From the wireshark capture, the tx rate for each
> data and qos data packet is only 1Mbps. Compare to the Realtek driver
> at https://github.com/lwfinger/rtl8723bu, the same iperf test gets
> ~12Mbps or better. The signal strength is reported similarly around
> -40dBm. That's why we want to improve.
>
> After reading the source code of the rtl8xxxu driver and Realtek's, the
> major difference is that Realtek's driver has a watchdog which will keep
> monitoring the signal quality and updating the rate mask just like the
> rtl8xxxu_gen2_update_rate_mask() does if signal quality changes.
> And this kind of watchdog also exists in rtlwifi driver of some specific
> chips, ex rtl8192ee, rtl8188ee, rtl8723ae, rtl8821ae...etc. They have
> the same member function named dm_watchdog and will invoke the
> corresponding dm_refresh_rate_adaptive_mask to adjust the tx rate
> mask.
>
> With this commit, the tx rate of each data and qos data packet will
> be 39Mbps (MCS4) with the 0xF00000 as the tx rate mask. The 20th bit
> to 23th bit means MCS4 to MCS7. It means that the firmware still picks
> the lowest rate from the rate mask and explains why the tx rate of
> data and qos data is always lowest 1Mbps because the default rate mask
> passed is always 0xFFFFFFF ranges from the basic CCK rate, OFDM rate,
> and MCS rate. However, with Realtek's driver, the tx rate observed from
> wireshark under the same condition is almost 65Mbps or 72Mbps.
>
> I believe the firmware of RTL8723BU may need fix. And I think we
> can still bring in the dm_watchdog as rtlwifi to improve from the
> driver side. Please leave precious comments for my commits and
> suggest what I can do better. Or suggest if there's any better idea
> to fix this. Thanks.
>
> Signed-off-by: Chris Chiu <chiu@endlessm.com>
> ---
>
>
> Notes:
>   v2:
>    - Fix errors and warnings complained by checkpatch.pl
>    - Replace data structure rate_adaptive by 2 member variables
>    - Make rtl8xxxu_wireless_mode non-static
>    - Runs refresh_rate_mask() only in station mode
>   v3:
>    - Remove ugly rtl8xxxu_watchdog data structure
>    - Make sure only one vif exists
>   v4:
>    - Move cancel_delayed_work from rtl8xxxu_disconnect to rtl8xxxu_stop
>    - Clear priv->vif in rtl8xxxu_remove_interface
>    - Add rateid as the function argument of update_rate_mask
>    - Rephrase the comment for priv->vif more explicit.
>   v5:
>    - Make refresh_rate_mask() generic for all sub-drivers.
>    - Add definitions for SNR related to help determine rssi_level
>
>
>  .../net/wireless/realtek/rtl8xxxu/rtl8xxxu.h  |  55 ++++-
>  .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 226 +++++++++++++++++-
>  2 files changed, 274 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> index 8828baf26e7b..1498a8c94d5f 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> @@ -1195,6 +1195,48 @@ struct rtl8723bu_c2h {
>
>  struct rtl8xxxu_fileops;
>
> +/*mlme related.*/
> +enum wireless_mode {
> +       WIRELESS_MODE_UNKNOWN = 0,
> +       /* Sub-Element */
> +       WIRELESS_MODE_B = BIT(0),
> +       WIRELESS_MODE_G = BIT(1),
> +       WIRELESS_MODE_A = BIT(2),
> +       WIRELESS_MODE_N_24G = BIT(3),
> +       WIRELESS_MODE_N_5G = BIT(4),
> +       WIRELESS_AUTO = BIT(5),
> +       WIRELESS_MODE_AC = BIT(6),
> +       WIRELESS_MODE_MAX = 0x7F,
> +};
> +
> +/* from rtlwifi/wifi.h */
> +enum ratr_table_mode_new {
> +       RATEID_IDX_BGN_40M_2SS = 0,
> +       RATEID_IDX_BGN_40M_1SS = 1,
> +       RATEID_IDX_BGN_20M_2SS_BN = 2,
> +       RATEID_IDX_BGN_20M_1SS_BN = 3,
> +       RATEID_IDX_GN_N2SS = 4,
> +       RATEID_IDX_GN_N1SS = 5,
> +       RATEID_IDX_BG = 6,
> +       RATEID_IDX_G = 7,
> +       RATEID_IDX_B = 8,
> +       RATEID_IDX_VHT_2SS = 9,
> +       RATEID_IDX_VHT_1SS = 10,
> +       RATEID_IDX_MIX1 = 11,
> +       RATEID_IDX_MIX2 = 12,
> +       RATEID_IDX_VHT_3SS = 13,
> +       RATEID_IDX_BGN_3SS = 14,
> +};
> +
> +#define RTL8XXXU_RATR_STA_INIT 0
> +#define RTL8XXXU_RATR_STA_HIGH 1
> +#define RTL8XXXU_RATR_STA_MID  2
> +#define RTL8XXXU_RATR_STA_LOW  3
> +
> +#define RTL8XXXU_NOISE_FLOOR_MIN       -95
> +#define RTL8XXXU_SNR_THRESH_HIGH       50
> +#define RTL8XXXU_SNR_THRESH_LOW        20
> +
>  struct rtl8xxxu_priv {
>         struct ieee80211_hw *hw;
>         struct usb_device *udev;
> @@ -1299,6 +1341,13 @@ struct rtl8xxxu_priv {
>         u8 pi_enabled:1;
>         u8 no_pape:1;
>         u8 int_buf[USB_INTR_CONTENT_LENGTH];
> +       u8 rssi_level;
> +       /*
> +        * Only one virtual interface permitted because only STA mode
> +        * is supported and no iface_combinations are providec.
> +        */
> +       struct ieee80211_vif *vif;
> +       struct delayed_work ra_watchdog;
>  };
>
>  struct rtl8xxxu_rx_urb {
> @@ -1334,7 +1383,7 @@ struct rtl8xxxu_fileops {
>         void (*set_tx_power) (struct rtl8xxxu_priv *priv, int channel,
>                               bool ht40);
>         void (*update_rate_mask) (struct rtl8xxxu_priv *priv,
> -                                 u32 ramask, int sgi);
> +                                 u32 ramask, u8 rateid, int sgi);
>         void (*report_connect) (struct rtl8xxxu_priv *priv,
>                                 u8 macid, bool connect);
>         void (*fill_txdesc) (struct ieee80211_hw *hw, struct ieee80211_hdr *hdr,
> @@ -1419,9 +1468,9 @@ void rtl8xxxu_gen2_config_channel(struct ieee80211_hw *hw);
>  void rtl8xxxu_gen1_usb_quirks(struct rtl8xxxu_priv *priv);
>  void rtl8xxxu_gen2_usb_quirks(struct rtl8xxxu_priv *priv);
>  void rtl8xxxu_update_rate_mask(struct rtl8xxxu_priv *priv,
> -                              u32 ramask, int sgi);
> +                              u32 ramask, u8 rateid, int sgi);
>  void rtl8xxxu_gen2_update_rate_mask(struct rtl8xxxu_priv *priv,
> -                                   u32 ramask, int sgi);
> +                                   u32 ramask, u8 rateid, int sgi);
>  void rtl8xxxu_gen1_report_connect(struct rtl8xxxu_priv *priv,
>                                   u8 macid, bool connect);
>  void rtl8xxxu_gen2_report_connect(struct rtl8xxxu_priv *priv,
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index 039e5ca9d2e4..474dea2291a9 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -4311,7 +4311,8 @@ static void rtl8xxxu_sw_scan_complete(struct ieee80211_hw *hw,
>         rtl8xxxu_write8(priv, REG_BEACON_CTRL, val8);
>  }
>
> -void rtl8xxxu_update_rate_mask(struct rtl8xxxu_priv *priv, u32 ramask, int sgi)
> +void rtl8xxxu_update_rate_mask(struct rtl8xxxu_priv *priv,
> +                              u32 ramask, u8 rateid, int sgi)
>  {
>         struct h2c_cmd h2c;
>
> @@ -4331,7 +4332,7 @@ void rtl8xxxu_update_rate_mask(struct rtl8xxxu_priv *priv, u32 ramask, int sgi)
>  }
>
>  void rtl8xxxu_gen2_update_rate_mask(struct rtl8xxxu_priv *priv,
> -                                   u32 ramask, int sgi)
> +                                   u32 ramask, u8 rateid, int sgi)
>  {
>         struct h2c_cmd h2c;
>         u8 bw = 0;
> @@ -4345,7 +4346,7 @@ void rtl8xxxu_gen2_update_rate_mask(struct rtl8xxxu_priv *priv,
>         h2c.b_macid_cfg.ramask3 = (ramask >> 24) & 0xff;
>
>         h2c.ramask.arg = 0x80;
> -       h2c.b_macid_cfg.data1 = 0;
> +       h2c.b_macid_cfg.data1 = rateid;
>         if (sgi)
>                 h2c.b_macid_cfg.data1 |= BIT(7);
>
> @@ -4485,6 +4486,40 @@ static void rtl8xxxu_set_basic_rates(struct rtl8xxxu_priv *priv, u32 rate_cfg)
>         rtl8xxxu_write8(priv, REG_INIRTS_RATE_SEL, rate_idx);
>  }
>
> +static u16
> +rtl8xxxu_wireless_mode(struct ieee80211_hw *hw, struct ieee80211_sta *sta)
> +{
> +       u16 network_type = WIRELESS_MODE_UNKNOWN;
> +       u32 rate_mask;
> +
> +       rate_mask = (sta->supp_rates[0] & 0xfff) |
> +                   (sta->ht_cap.mcs.rx_mask[0] << 12) |
> +                   (sta->ht_cap.mcs.rx_mask[0] << 20);
> +
> +       if (hw->conf.chandef.chan->band == NL80211_BAND_5GHZ) {
> +               if (sta->vht_cap.vht_supported)
> +                       network_type = WIRELESS_MODE_AC;
> +               else if (sta->ht_cap.ht_supported)
> +                       network_type = WIRELESS_MODE_N_5G;
> +
> +               network_type |= WIRELESS_MODE_A;
> +       } else {
> +               if (sta->vht_cap.vht_supported)
> +                       network_type = WIRELESS_MODE_AC;
> +               else if (sta->ht_cap.ht_supported)
> +                       network_type = WIRELESS_MODE_N_24G;
> +
> +               if (sta->supp_rates[0] <= 0xf)
> +                       network_type |= WIRELESS_MODE_B;
> +               else if (sta->supp_rates[0] & 0xf)
> +                       network_type |= (WIRELESS_MODE_B | WIRELESS_MODE_G);
> +               else
> +                       network_type |= WIRELESS_MODE_G;
> +       }
> +
> +       return network_type;
> +}
> +
>  static void
>  rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
>                           struct ieee80211_bss_conf *bss_conf, u32 changed)
> @@ -4527,7 +4562,10 @@ rtl8xxxu_bss_info_changed(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
>                                 sgi = 1;
>                         rcu_read_unlock();
>
> -                       priv->fops->update_rate_mask(priv, ramask, sgi);
> +                       priv->vif = vif;
> +                       priv->rssi_level = RTL8XXXU_RATR_STA_INIT;
> +
> +                       priv->fops->update_rate_mask(priv, ramask, 0, sgi);
>
>                         rtl8xxxu_write8(priv, REG_BCN_MAX_ERR, 0xff);
>
> @@ -5471,6 +5509,10 @@ static int rtl8xxxu_add_interface(struct ieee80211_hw *hw,
>
>         switch (vif->type) {
>         case NL80211_IFTYPE_STATION:
> +               if (!priv->vif)
> +                       priv->vif = vif;
> +               else
> +                       return -EOPNOTSUPP;
>                 rtl8xxxu_stop_tx_beacon(priv);
>
>                 val8 = rtl8xxxu_read8(priv, REG_BEACON_CTRL);
> @@ -5494,6 +5536,9 @@ static void rtl8xxxu_remove_interface(struct ieee80211_hw *hw,
>         struct rtl8xxxu_priv *priv = hw->priv;
>
>         dev_dbg(&priv->udev->dev, "%s\n", __func__);
> +
> +       if (priv->vif)
> +               priv->vif = NULL;
>  }
>
>  static int rtl8xxxu_config(struct ieee80211_hw *hw, u32 changed)
> @@ -5779,6 +5824,174 @@ rtl8xxxu_ampdu_action(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
>         return 0;
>  }
>
> +static u8 rtl8xxxu_signal_to_snr(int signal)
> +{
> +       if (signal < RTL8XXXU_NOISE_FLOOR_MIN)
> +               signal = RTL8XXXU_NOISE_FLOOR_MIN;
> +       return (u8)(signal - RTL8XXXU_NOISE_FLOOR_MIN);
> +}
> +
> +static void rtl8xxxu_refresh_rate_mask(struct rtl8xxxu_priv *priv,
> +                                      int signal, struct ieee80211_sta *sta)
> +{
> +       struct ieee80211_hw *hw = priv->hw;
> +       u16 wireless_mode;
> +       u8 rssi_level, ratr_idx;
> +       u8 txbw_40mhz;
> +       u8 snr, snr_thresh_high, snr_thresh_low;
> +       u8 go_up_gap = 5;
> +
> +       rssi_level = priv->rssi_level;
> +       snr = rtl8xxxu_signal_to_snr(signal);
> +       snr_thresh_high = RTL8XXXU_SNR_THRESH_HIGH;
> +       snr_thresh_low = RTL8XXXU_SNR_THRESH_LOW;
> +       txbw_40mhz = (hw->conf.chandef.width == NL80211_CHAN_WIDTH_40) ? 1 : 0;
> +
> +       switch (rssi_level) {
> +       case RTL8XXXU_RATR_STA_MID:
> +               snr_thresh_high += go_up_gap;
> +               break;
> +       case RTL8XXXU_RATR_STA_LOW:
> +               snr_thresh_high += go_up_gap;
> +               snr_thresh_low += go_up_gap;
> +               break;
> +       default:
> +               break;
> +       }
> +
> +       if (snr > snr_thresh_high)
> +               rssi_level = RTL8XXXU_RATR_STA_HIGH;
> +       else if (snr > snr_thresh_low)
> +               rssi_level = RTL8XXXU_RATR_STA_MID;
> +       else
> +               rssi_level = RTL8XXXU_RATR_STA_LOW;
> +
> +       if (rssi_level != priv->rssi_level) {
> +               int sgi = 0;
> +               u32 rate_bitmap = 0;
> +
> +               rcu_read_lock();
> +               rate_bitmap = (sta->supp_rates[0] & 0xfff) |
> +                               (sta->ht_cap.mcs.rx_mask[0] << 12) |
> +                               (sta->ht_cap.mcs.rx_mask[1] << 20);
> +               if (sta->ht_cap.cap &
> +                   (IEEE80211_HT_CAP_SGI_40 | IEEE80211_HT_CAP_SGI_20))
> +                       sgi = 1;
> +               rcu_read_unlock();
> +
> +               wireless_mode = rtl8xxxu_wireless_mode(hw, sta);
> +               switch (wireless_mode) {
> +               case WIRELESS_MODE_B:
> +                       ratr_idx = RATEID_IDX_B;
> +                       if (rate_bitmap & 0x0000000c)
> +                               rate_bitmap &= 0x0000000d;
> +                       else
> +                               rate_bitmap &= 0x0000000f;
> +                       break;
> +               case WIRELESS_MODE_A:
> +               case WIRELESS_MODE_G:
> +                       ratr_idx = RATEID_IDX_G;
> +                       if (rssi_level == RTL8XXXU_RATR_STA_HIGH)
> +                               rate_bitmap &= 0x00000f00;
> +                       else
> +                               rate_bitmap &= 0x00000ff0;
> +                       break;
> +               case (WIRELESS_MODE_B | WIRELESS_MODE_G):
> +                       ratr_idx = RATEID_IDX_BG;
> +                       if (rssi_level == RTL8XXXU_RATR_STA_HIGH)
> +                               rate_bitmap &= 0x00000f00;
> +                       else if (rssi_level == RTL8XXXU_RATR_STA_MID)
> +                               rate_bitmap &= 0x00000ff0;
> +                       else
> +                               rate_bitmap &= 0x00000ff5;
> +                       break;
> +               case WIRELESS_MODE_N_24G:
> +               case WIRELESS_MODE_N_5G:
> +               case (WIRELESS_MODE_G | WIRELESS_MODE_N_24G):
> +               case (WIRELESS_MODE_A | WIRELESS_MODE_N_5G):
> +                       if (priv->tx_paths == 2 && priv->rx_paths == 2)
> +                               ratr_idx = RATEID_IDX_GN_N2SS;
> +                       else
> +                               ratr_idx = RATEID_IDX_GN_N1SS;
> +               case (WIRELESS_MODE_B | WIRELESS_MODE_G | WIRELESS_MODE_N_24G):
> +               case (WIRELESS_MODE_B | WIRELESS_MODE_N_24G):
> +                       if (txbw_40mhz) {
> +                               if (priv->tx_paths == 2 && priv->rx_paths == 2)
> +                                       ratr_idx = RATEID_IDX_BGN_40M_2SS;
> +                               else
> +                                       ratr_idx = RATEID_IDX_BGN_40M_1SS;
> +                       } else {
> +                               if (priv->tx_paths == 2 && priv->rx_paths == 2)
> +                                       ratr_idx = RATEID_IDX_BGN_20M_2SS_BN;
> +                               else
> +                                       ratr_idx = RATEID_IDX_BGN_20M_1SS_BN;
> +                       }
> +
> +                       if (priv->tx_paths == 2 && priv->rx_paths == 2) {
> +                               if (rssi_level == RTL8XXXU_RATR_STA_HIGH) {
> +                                       rate_bitmap &= 0x0f8f0000;
> +                               } else if (rssi_level == RTL8XXXU_RATR_STA_MID) {
> +                                       rate_bitmap &= 0x0f8ff000;
> +                               } else {
> +                                       if (txbw_40mhz)
> +                                               rate_bitmap &= 0x0f8ff015;
> +                                       else
> +                                               rate_bitmap &= 0x0f8ff005;
> +                               }
> +                       } else {
> +                               if (rssi_level == RTL8XXXU_RATR_STA_HIGH) {
> +                                       rate_bitmap &= 0x000f0000;
> +                               } else if (rssi_level == RTL8XXXU_RATR_STA_MID) {
> +                                       rate_bitmap &= 0x000ff000;
> +                               } else {
> +                                       if (txbw_40mhz)
> +                                               rate_bitmap &= 0x000ff015;
> +                                       else
> +                                               rate_bitmap &= 0x000ff005;
> +                               }
> +                       }
> +                       break;
> +               default:
> +                       ratr_idx = RATEID_IDX_BGN_40M_2SS;
> +                       rate_bitmap &= 0x0fffffff;
> +                       break;
> +               }
> +
> +               priv->rssi_level = rssi_level;
> +               priv->fops->update_rate_mask(priv, rate_bitmap, ratr_idx, sgi);
> +       }
> +}
> +
> +static void rtl8xxxu_watchdog_callback(struct work_struct *work)
> +{
> +       struct ieee80211_vif *vif;
> +       struct rtl8xxxu_priv *priv;
> +
> +       priv = container_of(work, struct rtl8xxxu_priv, ra_watchdog.work);
> +       vif = priv->vif;
> +
> +       if (vif && vif->type == NL80211_IFTYPE_STATION) {
> +               int signal;
> +               struct ieee80211_sta *sta;
> +
> +               rcu_read_lock();
> +               sta = ieee80211_find_sta(vif, vif->bss_conf.bssid);
> +               if (!sta) {
> +                       struct device *dev = &priv->udev->dev;
> +
> +                       dev_info(dev, "%s: no sta found\n", __func__);
> +                       rcu_read_unlock();
> +                       return;
> +               }
> +               rcu_read_unlock();
> +
> +               signal = ieee80211_ave_rssi(vif);
> +               rtl8xxxu_refresh_rate_mask(priv, signal, sta);
> +       }
> +
> +       schedule_delayed_work(&priv->ra_watchdog, 2 * HZ);
> +}
> +
>  static int rtl8xxxu_start(struct ieee80211_hw *hw)
>  {
>         struct rtl8xxxu_priv *priv = hw->priv;
> @@ -5835,6 +6048,8 @@ static int rtl8xxxu_start(struct ieee80211_hw *hw)
>
>                 ret = rtl8xxxu_submit_rx_urb(priv, rx_urb);
>         }
> +
> +       schedule_delayed_work(&priv->ra_watchdog, 2 * HZ);
>  exit:
>         /*
>          * Accept all data and mgmt frames
> @@ -5886,6 +6101,8 @@ static void rtl8xxxu_stop(struct ieee80211_hw *hw)
>         if (priv->usb_interrupts)
>                 rtl8xxxu_write32(priv, REG_USB_HIMR, 0);
>
> +       cancel_delayed_work_sync(&priv->ra_watchdog);
> +
>         rtl8xxxu_free_rx_resources(priv);
>         rtl8xxxu_free_tx_resources(priv);
>  }
> @@ -6058,6 +6275,7 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
>         INIT_LIST_HEAD(&priv->rx_urb_pending_list);
>         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);
>
>         usb_set_intfdata(interface, hw);
>
> --
> 2.21.0
>

Gentle ping. Cheers.

Chris

^ permalink raw reply

* Re: [PATCH] rtl8xxxu: Fix wifi low signal strength issue of RTL8723BU
From: Chris Chiu @ 2019-07-01  5:09 UTC (permalink / raw)
  To: Jes Sorensen, Kalle Valo, David Miller
  Cc: linux-wireless, netdev, Linux Kernel, Linux Upstreaming Team
In-Reply-To: <20190627095247.8792-1-chiu@endlessm.com>

On Thu, Jun 27, 2019 at 5:52 PM Chris Chiu <chiu@endlessm.com> wrote:
>
> The WiFi tx power of RTL8723BU is extremely low after booting. So
> the WiFi scan gives very limited AP list and it always fails to
> connect to the selected AP. This module only supports 1x1 antenna
> and the antenna is switched to bluetooth due to some incorrect
> register settings.
>
> This commit hand over the antenna control to PTA, the wifi signal
> will be back to normal and the bluetooth scan can also work at the
> same time. However, the btcoexist still needs to be handled under
> different circumstances. If there's a BT connection established,
> the wifi still fails to connect until disconneting the BT.
>
> Signed-off-by: Chris Chiu <chiu@endlessm.com>
> ---
>  drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c | 9 ++++++---
>  drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c  | 3 ++-
>  2 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
> index 3adb1d3d47ac..6c3c70d93ac1 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
> @@ -1525,7 +1525,7 @@ static void rtl8723b_enable_rf(struct rtl8xxxu_priv *priv)
>         /*
>          * WLAN action by PTA
>          */
> -       rtl8xxxu_write8(priv, REG_WLAN_ACT_CONTROL_8723B, 0x04);
> +       rtl8xxxu_write8(priv, REG_WLAN_ACT_CONTROL_8723B, 0x0c);
>
>         /*
>          * BT select S0/S1 controlled by WiFi
> @@ -1568,9 +1568,12 @@ static void rtl8723b_enable_rf(struct rtl8xxxu_priv *priv)
>         rtl8xxxu_gen2_h2c_cmd(priv, &h2c, sizeof(h2c.ant_sel_rsv));
>
>         /*
> -        * 0x280, 0x00, 0x200, 0x80 - not clear
> +        * Different settings per different antenna position.
> +        * Antenna switch to BT: 0x280, 0x00 (inverse)
> +        * Antenna switch to WiFi: 0x0, 0x280 (inverse)
> +        * Antenna controlled by PTA: 0x200, 0x80 (inverse)
>          */
> -       rtl8xxxu_write32(priv, REG_S0S1_PATH_SWITCH, 0x00);
> +       rtl8xxxu_write32(priv, REG_S0S1_PATH_SWITCH, 0x80);
>
>         /*
>          * Software control, antenna at WiFi side
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index 8136e268b4e6..87b2179a769e 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -3891,12 +3891,13 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
>
>         /* Check if MAC is already powered on */
>         val8 = rtl8xxxu_read8(priv, REG_CR);
> +       val16 = rtl8xxxu_read16(priv, REG_SYS_CLKR);
>
>         /*
>          * Fix 92DU-VC S3 hang with the reason is that secondary mac is not
>          * initialized. First MAC returns 0xea, second MAC returns 0x00
>          */
> -       if (val8 == 0xea)
> +       if (val8 == 0xea || !(val16 & BIT(11)))
>                 macpower = false;
>         else
>                 macpower = true;
> --
> 2.11.0
>

Gentle ping. Cheers.

Chris

^ 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