* [PATCH net] tipc: add NULL pointer check before calling kfree_rcu
From: Xin Long @ 2019-09-03 9:53 UTC (permalink / raw)
To: network dev; +Cc: davem, Jon Maloy, Ying Xue, tipc-discussion
Unlike kfree(p), kfree_rcu(p, rcu) won't do NULL pointer check. When
tipc_nametbl_remove_publ returns NULL, the panic below happens:
BUG: unable to handle kernel NULL pointer dereference at 0000000000000068
RIP: 0010:__call_rcu+0x1d/0x290
Call Trace:
<IRQ>
tipc_publ_notify+0xa9/0x170 [tipc]
tipc_node_write_unlock+0x8d/0x100 [tipc]
tipc_node_link_down+0xae/0x1d0 [tipc]
tipc_node_check_dest+0x3ea/0x8f0 [tipc]
? tipc_disc_rcv+0x2c7/0x430 [tipc]
tipc_disc_rcv+0x2c7/0x430 [tipc]
? tipc_rcv+0x6bb/0xf20 [tipc]
tipc_rcv+0x6bb/0xf20 [tipc]
? ip_route_input_slow+0x9cf/0xb10
tipc_udp_recv+0x195/0x1e0 [tipc]
? tipc_udp_is_known_peer+0x80/0x80 [tipc]
udp_queue_rcv_skb+0x180/0x460
udp_unicast_rcv_skb.isra.56+0x75/0x90
__udp4_lib_rcv+0x4ce/0xb90
ip_local_deliver_finish+0x11c/0x210
ip_local_deliver+0x6b/0xe0
? ip_rcv_finish+0xa9/0x410
ip_rcv+0x273/0x362
Fixes: 97ede29e80ee ("tipc: convert name table read-write lock to RCU")
Reported-by: Li Shuang <shuali@redhat.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/tipc/name_distr.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c
index 44abc8e..241ed22 100644
--- a/net/tipc/name_distr.c
+++ b/net/tipc/name_distr.c
@@ -223,7 +223,8 @@ static void tipc_publ_purge(struct net *net, struct publication *publ, u32 addr)
publ->key);
}
- kfree_rcu(p, rcu);
+ if (p)
+ kfree_rcu(p, rcu);
}
/**
--
2.1.0
^ permalink raw reply related
* Re: [PATCH] net: sched: taprio: Fix potential integer overflow in taprio_set_picos_per_byte
From: Vladimir Oltean @ 2019-09-03 10:12 UTC (permalink / raw)
To: Eric Dumazet
Cc: Gustavo A. R. Silva, Jamal Hadi Salim, Cong Wang, Jiri Pirko,
David S. Miller, netdev, lkml
In-Reply-To: <cb7d53cd-3f1e-146b-c1ab-f11a584a7224@gmail.com>
On Tue, 3 Sep 2019 at 10:19, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
>
> On 9/3/19 3:08 AM, Gustavo A. R. Silva wrote:
> > Add suffix LL to constant 1000 in order to avoid a potential integer
> > overflow and give the compiler complete information about the proper
> > arithmetic to use. Notice that this constant is being used in a context
> > that expects an expression of type s64, but it's currently evaluated
> > using 32-bit arithmetic.
> >
> > Addresses-Coverity-ID: 1453459 ("Unintentional integer overflow")
> > Fixes: f04b514c0ce2 ("taprio: Set default link speed to 10 Mbps in taprio_set_picos_per_byte")
> > Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> > ---
> > net/sched/sch_taprio.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
> > index 8d8bc2ec5cd6..956f837436ea 100644
> > --- a/net/sched/sch_taprio.c
> > +++ b/net/sched/sch_taprio.c
> > @@ -966,7 +966,7 @@ static void taprio_set_picos_per_byte(struct net_device *dev,
> >
> > skip:
> > picos_per_byte = div64_s64(NSEC_PER_SEC * 1000LL * 8,
> > - speed * 1000 * 1000);
> > + speed * 1000LL * 1000);
> >
> > atomic64_set(&q->picos_per_byte, picos_per_byte);
> > netdev_dbg(dev, "taprio: set %s's picos_per_byte to: %lld, linkspeed: %d\n",
> >
>
> But, why even multiplying by 1,000,000 in the first place, this seems silly,
> a standard 32 bit divide could be used instead.
>
> ->
>
> diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
> index 8d8bc2ec5cd6281d811fd5d8a5c5211ebb0edd73..944b1af3215668e927d486b6c6c65c4599fb9539 100644
> --- a/net/sched/sch_taprio.c
> +++ b/net/sched/sch_taprio.c
> @@ -965,8 +965,7 @@ static void taprio_set_picos_per_byte(struct net_device *dev,
> speed = ecmd.base.speed;
>
> skip:
> - picos_per_byte = div64_s64(NSEC_PER_SEC * 1000LL * 8,
> - speed * 1000 * 1000);
> + picos_per_byte = (USEC_PER_SEC * 8) / speed;
>
> atomic64_set(&q->picos_per_byte, picos_per_byte);
> netdev_dbg(dev, "taprio: set %s's picos_per_byte to: %lld, linkspeed: %d\n",
>
>
>
Right. And while we're at it, there's still the potential
division-by-zero problem which I still don't know how to solve without
implementing a full-blown __ethtool_get_link_ksettings parser that
checks against all the possible outputs it can have under the "no
carrier" condition - see "[RFC PATCH 1/1] phylink: Set speed to
SPEED_UNKNOWN when there is no PHY connected" for details.
And there's also a third fix to be made: the netdev_dbg should be made
to print "speed" instead of "ecmd.base.speed".
Thanks,
-Vladimir
^ permalink raw reply
* Re: [PATCH v3] tun: fix use-after-free when register netdev failed
From: Jason Wang @ 2019-09-03 10:50 UTC (permalink / raw)
To: Yang Yingliang
Cc: David Miller, netdev, eric dumazet, xiyou wangcong, weiyongjun1
In-Reply-To: <5D6E17A7.1020102@huawei.com>
----- Original Message -----
>
>
> On 2019/9/3 14:06, Jason Wang wrote:
> >
> > On 2019/9/3 下午1:42, Yang Yingliang wrote:
> >>
> >>
> >> On 2019/9/3 11:03, Jason Wang wrote:
> >>>
> >>> On 2019/9/3 上午9:45, Yang Yingliang wrote:
> >>>>
> >>>>
> >>>> On 2019/9/2 13:32, Jason Wang wrote:
> >>>>>
> >>>>> On 2019/8/23 下午5:36, Yang Yingliang wrote:
> >>>>>>
> >>>>>>
> >>>>>> On 2019/8/23 11:05, Jason Wang wrote:
> >>>>>>> ----- Original Message -----
> >>>>>>>>
> >>>>>>>> On 2019/8/22 14:07, Yang Yingliang wrote:
> >>>>>>>>>
> >>>>>>>>> On 2019/8/22 10:13, Jason Wang wrote:
> >>>>>>>>>> On 2019/8/20 上午10:28, Jason Wang wrote:
> >>>>>>>>>>> On 2019/8/20 上午9:25, David Miller wrote:
> >>>>>>>>>>>> From: Yang Yingliang <yangyingliang@huawei.com>
> >>>>>>>>>>>> Date: Mon, 19 Aug 2019 21:31:19 +0800
> >>>>>>>>>>>>
> >>>>>>>>>>>>> Call tun_attach() after register_netdevice() to make sure
> >>>>>>>>>>>>> tfile->tun
> >>>>>>>>>>>>> is not published until the netdevice is registered. So the
> >>>>>>>>>>>>> read/write
> >>>>>>>>>>>>> thread can not use the tun pointer that may freed by
> >>>>>>>>>>>>> free_netdev().
> >>>>>>>>>>>>> (The tun and dev pointer are allocated by
> >>>>>>>>>>>>> alloc_netdev_mqs(), they
> >>>>>>>>>>>>> can
> >>>>>>>>>>>>> be freed by netdev_freemem().)
> >>>>>>>>>>>> register_netdevice() must always be the last operation in
> >>>>>>>>>>>> the order of
> >>>>>>>>>>>> network device setup.
> >>>>>>>>>>>>
> >>>>>>>>>>>> At the point register_netdevice() is called, the device is
> >>>>>>>>>>>> visible
> >>>>>>>>>>>> globally
> >>>>>>>>>>>> and therefore all of it's software state must be fully
> >>>>>>>>>>>> initialized and
> >>>>>>>>>>>> ready for us.
> >>>>>>>>>>>>
> >>>>>>>>>>>> You're going to have to find another solution to these
> >>>>>>>>>>>> problems.
> >>>>>>>>>>>
> >>>>>>>>>>> The device is loosely coupled with sockets/queues. Each side is
> >>>>>>>>>>> allowed to be go away without caring the other side. So in this
> >>>>>>>>>>> case, there's a small window that network stack think the
> >>>>>>>>>>> device has
> >>>>>>>>>>> one queue but actually not, the code can then safely drop them.
> >>>>>>>>>>> Maybe it's ok here with some comments?
> >>>>>>>>>>>
> >>>>>>>>>>> Or if not, we can try to hold the device before tun_attach
> >>>>>>>>>>> and drop
> >>>>>>>>>>> it after register_netdevice().
> >>>>>>>>>>
> >>>>>>>>>> Hi Yang:
> >>>>>>>>>>
> >>>>>>>>>> I think maybe we can try to hold refcnt instead of playing
> >>>>>>>>>> real num
> >>>>>>>>>> queues here. Do you want to post a V4?
> >>>>>>>>> I think the refcnt can prevent freeing the memory in this case.
> >>>>>>>>> When register_netdevice() failed, free_netdev() will be called
> >>>>>>>>> directly,
> >>>>>>>>> dev->pcpu_refcnt and dev are freed without checking refcnt of
> >>>>>>>>> dev.
> >>>>>>>> How about using patch-v1 that using a flag to check whether the
> >>>>>>>> device
> >>>>>>>> registered successfully.
> >>>>>>>>
> >>>>>>> As I said, it lacks sufficient locks or barriers. To be clear, I
> >>>>>>> meant
> >>>>>>> something like (compile-test only):
> >>>>>>>
> >>>>>>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> >>>>>>> index db16d7a13e00..e52678f9f049 100644
> >>>>>>> --- a/drivers/net/tun.c
> >>>>>>> +++ b/drivers/net/tun.c
> >>>>>>> @@ -2828,6 +2828,7 @@ static int tun_set_iff(struct net *net,
> >>>>>>> struct file *file, struct ifreq *ifr)
> >>>>>>> (ifr->ifr_flags & TUN_FEATURES);
> >>>>>>> INIT_LIST_HEAD(&tun->disabled);
> >>>>>>> + dev_hold(dev);
> >>>>>>> err = tun_attach(tun, file, false,
> >>>>>>> ifr->ifr_flags & IFF_NAPI,
> >>>>>>> ifr->ifr_flags & IFF_NAPI_FRAGS);
> >>>>>>> if (err < 0)
> >>>>>>> @@ -2836,6 +2837,7 @@ static int tun_set_iff(struct net *net,
> >>>>>>> struct file *file, struct ifreq *ifr)
> >>>>>>> err = register_netdevice(tun->dev);
> >>>>>>> if (err < 0)
> >>>>>>> goto err_detach;
> >>>>>>> + dev_put(dev);
> >>>>>>> }
> >>>>>>> netif_carrier_on(tun->dev);
> >>>>>>> @@ -2852,11 +2854,13 @@ static int tun_set_iff(struct net *net,
> >>>>>>> struct file *file, struct ifreq *ifr)
> >>>>>>> return 0;
> >>>>>>> err_detach:
> >>>>>>> + dev_put(dev);
> >>>>>>> tun_detach_all(dev);
> >>>>>>> /* register_netdevice() already called
> >>>>>>> tun_free_netdev() */
> >>>>>>> goto err_free_dev;
> >>>>>>> err_free_flow:
> >>>>>>> + dev_put(dev);
> >>>>>>> tun_flow_uninit(tun);
> >>>>>>> security_tun_dev_free_security(tun->security);
> >>>>>>> err_free_stat:
> >>>>>>>
> >>>>>>> What's your thought?
> >>>>>>
> >>>>>> The dev pointer are freed without checking the refcount in
> >>>>>> free_netdev() called by err_free_dev
> >>>>>>
> >>>>>> path, so I don't understand how the refcount protects this pointer.
> >>>>>>
> >>>>>
> >>>>> The refcount are guaranteed to be zero there, isn't it?
> >>>> No, it's not.
> >>>>
> >>>> err_free_dev:
> >>>> free_netdev(dev);
> >>>>
> >>>> void free_netdev(struct net_device *dev)
> >>>> {
> >>>> ...
> >>>> /* pcpu_refcnt can be freed without checking refcount */
> >>>> free_percpu(dev->pcpu_refcnt);
> >>>> dev->pcpu_refcnt = NULL;
> >>>>
> >>>> /* Compatibility with error handling in drivers */
> >>>> if (dev->reg_state == NETREG_UNINITIALIZED) {
> >>>> /* dev can be freed without checking refcount */
> >>>> netdev_freemem(dev);
> >>>> return;
> >>>> }
> >>>> ...
> >>>> }
> >>>
> >>>
> >>> Right, but what I meant is in my patch, when code reaches
> >>> free_netdev() the refcnt is zero. What did I miss?
> >> Yes, but it can't fix the UAF problem.
> >
> >
> > Well, it looks to me that the dev_put() in tun_put() won't release the
> > device in this case.
>
> The device is not released in tun_put().
> This is how the UAF occurs:
>
> CPUA CPUB
> tun_set_iff()
> alloc_netdev_mqs()
> tun_attach()
> tun_chr_read_iter()
> tun_get()
> tun_do_read()
> tun_ring_recv()
> register_netdevice() <-- inject error
> goto err_detach
> tun_detach_all() <-- set RCV_SHUTDOWN
> free_netdev() <-- called from
> err_free_dev path
> netdev_freemem() <-- free the memory
> without check refcount
> (In this path, the refcount cannot prevent
> freeing the memory of dev, and the memory
> will be used by dev_put() called by
> tun_chr_read_iter() on CPUB.)
> (Break from
> tun_ring_recv(),
> because RCV_SHUTDOWN
> is set)
> tun_put()
> dev_put() <-- use the
> memory freed by
> netdev_freemem()
>
>
My bad, thanks for the patience. Since all evil come from the
tfile->tun, how about delay the publishing of tfile->tun until the
success of registration to make sure dev_put() and dev_hold() work.
(Compile test only)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index db16d7a13e00..aab0be40d443 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -787,7 +787,8 @@ static void tun_detach_all(struct net_device *dev)
}
static int tun_attach(struct tun_struct *tun, struct file *file,
- bool skip_filter, bool napi, bool napi_frags)
+ bool skip_filter, bool napi, bool napi_frags,
+ bool publish_tun)
{
struct tun_file *tfile = file->private_data;
struct net_device *dev = tun->dev;
@@ -870,7 +871,8 @@ static int tun_attach(struct tun_struct *tun, struct file *file,
* initialized tfile; otherwise we risk using half-initialized
* object.
*/
- rcu_assign_pointer(tfile->tun, tun);
+ if (publish_tun)
+ rcu_assign_pointer(tfile->tun, tun);
rcu_assign_pointer(tun->tfiles[tun->numqueues], tfile);
tun->numqueues++;
tun_set_real_num_queues(tun);
@@ -2730,7 +2732,7 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
err = tun_attach(tun, file, ifr->ifr_flags & IFF_NOFILTER,
ifr->ifr_flags & IFF_NAPI,
- ifr->ifr_flags & IFF_NAPI_FRAGS);
+ ifr->ifr_flags & IFF_NAPI_FRAGS, true);
if (err < 0)
return err;
@@ -2829,13 +2831,17 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
INIT_LIST_HEAD(&tun->disabled);
err = tun_attach(tun, file, false, ifr->ifr_flags & IFF_NAPI,
- ifr->ifr_flags & IFF_NAPI_FRAGS);
+ ifr->ifr_flags & IFF_NAPI_FRAGS, false);
if (err < 0)
goto err_free_flow;
err = register_netdevice(tun->dev);
if (err < 0)
goto err_detach;
+ /* free_netdev() won't check refcnt, to aovid race
+ * with dev_put() we need publish tun after registration.
+ */
+ rcu_assign_pointer(tfile->tun, tun);
}
netif_carrier_on(tun->dev);
@@ -2978,7 +2984,7 @@ static int tun_set_queue(struct file *file, struct ifreq *ifr)
if (ret < 0)
goto unlock;
ret = tun_attach(tun, file, false, tun->flags & IFF_NAPI,
- tun->flags & IFF_NAPI_FRAGS);
+ tun->flags & IFF_NAPI_FRAGS, true);
} else if (ifr->ifr_flags & IFF_DETACH_QUEUE) {
tun = rtnl_dereference(tfile->tun);
if (!tun || !(tun->flags & IFF_MULTI_QUEUE) || tfile->detached)
--
2.18.1
> >
> > Thanks
> >
>
>
>
^ permalink raw reply related
* Re: [PATCH] net: 9p: Fix possible null-pointer dereferences in p9_cm_event_handler()
From: Dominique Martinet @ 2019-09-03 10:55 UTC (permalink / raw)
To: Jia-Ju Bai; +Cc: ericvh, lucho, davem, v9fs-developer, netdev, linux-kernel
In-Reply-To: <20190724125545.GA12982@nautica>
Jia-Ju,
Dominique Martinet wrote on Wed, Jul 24, 2019:
> Jia-Ju Bai wrote on Wed, Jul 24, 2019:
> > In p9_cm_event_handler(), there is an if statement on 260 to check
> > whether rdma is NULL, which indicates that rdma can be NULL.
> > If so, using rdma->xxx may cause a possible null-pointer dereference.
>
> The final dereference (complete(&rdma->cm_done) line 285) has been here
> from the start, so we would have seen crashes by now if rdma could be
> null at this point.
>
> Let's do it the other way around and remove the useless "if (rdma)" that
> has been here from day 1 instead ; I basically did the same with
> c->status a few months ago (from a coverity report)...
Did you get anywhere with this, or should I submit a new patch myself ?
In the later case I'll tag this as Reported-by you
Thanks,
--
Dominique
^ permalink raw reply
* Re: [RFC v3] vhost: introduce mdev based hardware vhost backend
From: Michael S. Tsirkin @ 2019-09-03 11:26 UTC (permalink / raw)
To: Tiwei Bie
Cc: jasowang, alex.williamson, maxime.coquelin, linux-kernel, kvm,
virtualization, netdev, dan.daly, cunming.liang, zhihong.wang,
lingshan.zhu
In-Reply-To: <20190828053712.26106-1-tiwei.bie@intel.com>
On Wed, Aug 28, 2019 at 01:37:12PM +0800, Tiwei Bie wrote:
> Details about this can be found here:
>
> https://lwn.net/Articles/750770/
>
> What's new in this version
> ==========================
>
> There are three choices based on the discussion [1] in RFC v2:
>
> > #1. We expose a VFIO device, so we can reuse the VFIO container/group
> > based DMA API and potentially reuse a lot of VFIO code in QEMU.
> >
> > But in this case, we have two choices for the VFIO device interface
> > (i.e. the interface on top of VFIO device fd):
> >
> > A) we may invent a new vhost protocol (as demonstrated by the code
> > in this RFC) on VFIO device fd to make it work in VFIO's way,
> > i.e. regions and irqs.
> >
> > B) Or as you proposed, instead of inventing a new vhost protocol,
> > we can reuse most existing vhost ioctls on the VFIO device fd
> > directly. There should be no conflicts between the VFIO ioctls
> > (type is 0x3B) and VHOST ioctls (type is 0xAF) currently.
> >
> > #2. Instead of exposing a VFIO device, we may expose a VHOST device.
> > And we will introduce a new mdev driver vhost-mdev to do this.
> > It would be natural to reuse the existing kernel vhost interface
> > (ioctls) on it as much as possible. But we will need to invent
> > some APIs for DMA programming (reusing VHOST_SET_MEM_TABLE is a
> > choice, but it's too heavy and doesn't support vIOMMU by itself).
>
> This version is more like a quick PoC to try Jason's proposal on
> reusing vhost ioctls. And the second way (#1/B) in above three
> choices was chosen in this version to demonstrate the idea quickly.
>
> Now the userspace API looks like this:
>
> - VFIO's container/group based IOMMU API is used to do the
> DMA programming.
>
> - Vhost's existing ioctls are used to setup the device.
>
> And the device will report device_api as "vfio-vhost".
>
> Note that, there are dirty hacks in this version. If we decide to
> go this way, some refactoring in vhost.c/vhost.h may be needed.
>
> PS. The direct mapping of the notify registers isn't implemented
> in this version.
>
> [1] https://lkml.org/lkml/2019/7/9/101
>
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
....
> +long vhost_mdev_ioctl(struct mdev_device *mdev, unsigned int cmd,
> + unsigned long arg)
> +{
> + void __user *argp = (void __user *)arg;
> + struct vhost_mdev *vdpa;
> + unsigned long minsz;
> + int ret = 0;
> +
> + if (!mdev)
> + return -EINVAL;
> +
> + vdpa = mdev_get_drvdata(mdev);
> + if (!vdpa)
> + return -ENODEV;
> +
> + switch (cmd) {
> + case VFIO_DEVICE_GET_INFO:
> + {
> + struct vfio_device_info info;
> +
> + minsz = offsetofend(struct vfio_device_info, num_irqs);
> +
> + if (copy_from_user(&info, (void __user *)arg, minsz)) {
> + ret = -EFAULT;
> + break;
> + }
> +
> + if (info.argsz < minsz) {
> + ret = -EINVAL;
> + break;
> + }
> +
> + info.flags = VFIO_DEVICE_FLAGS_VHOST;
> + info.num_regions = 0;
> + info.num_irqs = 0;
> +
> + if (copy_to_user((void __user *)arg, &info, minsz)) {
> + ret = -EFAULT;
> + break;
> + }
> +
> + break;
> + }
> + case VFIO_DEVICE_GET_REGION_INFO:
> + case VFIO_DEVICE_GET_IRQ_INFO:
> + case VFIO_DEVICE_SET_IRQS:
> + case VFIO_DEVICE_RESET:
> + ret = -EINVAL;
> + break;
> +
> + case VHOST_MDEV_SET_STATE:
> + ret = vhost_set_state(vdpa, argp);
> + break;
> + case VHOST_GET_FEATURES:
> + ret = vhost_get_features(vdpa, argp);
> + break;
> + case VHOST_SET_FEATURES:
> + ret = vhost_set_features(vdpa, argp);
> + break;
> + case VHOST_GET_VRING_BASE:
> + ret = vhost_get_vring_base(vdpa, argp);
> + break;
> + default:
> + ret = vhost_dev_ioctl(&vdpa->dev, cmd, argp);
> + if (ret == -ENOIOCTLCMD)
> + ret = vhost_vring_ioctl(&vdpa->dev, cmd, argp);
> + }
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(vhost_mdev_ioctl);
I don't have a problem with this approach. A small question:
would it make sense to have two fds: send vhost ioctls
on one and vfio ioctls on another?
We can then pass vfio fd to the vhost fd with a
SET_BACKEND ioctl.
What do you think?
--
MST
^ permalink raw reply
* Re: [Bridge] [PATCH v3 1/2] net: bridge: use mac_len in bridge forwarding
From: Toshiaki Makita @ 2019-09-03 11:37 UTC (permalink / raw)
To: Zahari Doychev, netdev
Cc: makita.toshiaki, jiri, nikolay, simon.horman, roopa, bridge, jhs,
dsahern, xiyou.wangcong, johannes, alexei.starovoitov
In-Reply-To: <20190902181000.25638-1-zahari.doychev@linux.com>
Hi Zahari,
Sorry for reviewing this late.
On 2019/09/03 3:09, Zahari Doychev wrote:
...
> @@ -466,13 +466,14 @@ static bool __allowed_ingress(const struct net_bridge *br,
> /* Tagged frame */
> if (skb->vlan_proto != br->vlan_proto) {
> /* Protocol-mismatch, empty out vlan_tci for new tag */
> - skb_push(skb, ETH_HLEN);
> + skb_push(skb, skb->mac_len);
> skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
> skb_vlan_tag_get(skb));
I think we should insert vlan at skb->data, i.e. mac_header + mac_len, while this
function inserts the tag at mac_header + ETH_HLEN which is not always the correct
offset.
> if (unlikely(!skb))
> return false;
>
> skb_pull(skb, ETH_HLEN);
Now skb->data is mac_header + ETH_HLEN which would be broken when mac_len is not
ETH_HLEN?
> + skb_reset_network_header(skb);
> skb_reset_mac_len(skb);
> *vid = 0;
> tagged = false;
>
Toshiaki Makita
^ permalink raw reply
* KASAN: use-after-free Read in rsi_rx_done_handler
From: syzbot @ 2019-09-03 12:08 UTC (permalink / raw)
To: amitkarwar, andreyknvl, davem, kvalo, linux-kernel, linux-usb,
linux-wireless, netdev, siva8118, syzkaller-bugs
Hello,
syzbot found the following crash on:
HEAD commit: eea39f24 usb-fuzzer: main usb gadget fuzzer driver
git tree: https://github.com/google/kasan.git usb-fuzzer
console output: https://syzkaller.appspot.com/x/log.txt?x=1031c5f2600000
kernel config: https://syzkaller.appspot.com/x/.config?x=d0c62209eedfd54e
dashboard link: https://syzkaller.appspot.com/bug?extid=b563b7f8dbe8223a51e8
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
Unfortunately, I don't have any reproducer for this crash yet.
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+b563b7f8dbe8223a51e8@syzkaller.appspotmail.com
==================================================================
BUG: KASAN: use-after-free in rsi_rx_done_handler+0x3ba/0x490
drivers/net/wireless/rsi/rsi_91x_usb.c:267
Read of size 8 at addr ffff8881cebc0fe8 by task kworker/0:2/102
CPU: 0 PID: 102 Comm: kworker/0:2 Not tainted 5.3.0-rc5+ #28
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Workqueue: usb_hub_wq hub_event
Call Trace:
<IRQ>
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0xca/0x13e lib/dump_stack.c:113
print_address_description+0x6a/0x32c mm/kasan/report.c:351
__kasan_report.cold+0x1a/0x33 mm/kasan/report.c:482
kasan_report+0xe/0x12 mm/kasan/common.c:612
rsi_rx_done_handler+0x3ba/0x490 drivers/net/wireless/rsi/rsi_91x_usb.c:267
__usb_hcd_giveback_urb+0x1f2/0x470 drivers/usb/core/hcd.c:1657
usb_hcd_giveback_urb+0x368/0x420 drivers/usb/core/hcd.c:1722
dummy_timer+0x120f/0x2fa2 drivers/usb/gadget/udc/dummy_hcd.c:1965
call_timer_fn+0x179/0x650 kernel/time/timer.c:1322
expire_timers kernel/time/timer.c:1366 [inline]
__run_timers kernel/time/timer.c:1685 [inline]
__run_timers kernel/time/timer.c:1653 [inline]
run_timer_softirq+0x5cc/0x14b0 kernel/time/timer.c:1698
__do_softirq+0x221/0x912 kernel/softirq.c:292
invoke_softirq kernel/softirq.c:373 [inline]
irq_exit+0x178/0x1a0 kernel/softirq.c:413
exiting_irq arch/x86/include/asm/apic.h:537 [inline]
smp_apic_timer_interrupt+0x12f/0x500 arch/x86/kernel/apic/apic.c:1095
apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:830
</IRQ>
RIP: 0010:arch_local_irq_restore arch/x86/include/asm/irqflags.h:85 [inline]
RIP: 0010:console_unlock+0xa2a/0xc40 kernel/printk/printk.c:2471
Code: 00 89 ee 48 c7 c7 20 89 d3 86 e8 61 ad 03 00 65 ff 0d 72 b5 d9 7e e9
db f9 ff ff e8 80 a0 15 00 e8 2b ca 1a 00 ff 74 24 30 9d <e9> 18 fe ff ff
e8 6c a0 15 00 48 8d 7d 08 48 89 f8 48 c1 e8 03 42
RSP: 0018:ffff8881d5077200 EFLAGS: 00000216 ORIG_RAX: ffffffffffffff13
RAX: 0000000000000007 RBX: 0000000000000200 RCX: 0000000000000006
RDX: 0000000000000000 RSI: 0000000000000008 RDI: ffff8881d5068844
RBP: 0000000000000000 R08: ffff8881d5068000 R09: fffffbfff11ad791
R10: fffffbfff11ad790 R11: ffffffff88d6bc87 R12: 000000000000004d
R13: dffffc0000000000 R14: ffffffff829090d0 R15: ffffffff87077430
vprintk_emit+0x171/0x3e0 kernel/printk/printk.c:1986
vprintk_func+0x75/0x113 kernel/printk/printk_safe.c:386
printk+0xba/0xed kernel/printk/printk.c:2046
really_probe.cold+0x69/0x1f6 drivers/base/dd.c:628
driver_probe_device+0x101/0x1b0 drivers/base/dd.c:721
__device_attach_driver+0x1c2/0x220 drivers/base/dd.c:828
bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:454
__device_attach+0x217/0x360 drivers/base/dd.c:894
bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
device_add+0xae6/0x16f0 drivers/base/core.c:2165
usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023
generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210
usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266
really_probe+0x281/0x6d0 drivers/base/dd.c:548
driver_probe_device+0x101/0x1b0 drivers/base/dd.c:721
__device_attach_driver+0x1c2/0x220 drivers/base/dd.c:828
bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:454
__device_attach+0x217/0x360 drivers/base/dd.c:894
bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
device_add+0xae6/0x16f0 drivers/base/core.c:2165
usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536
hub_port_connect drivers/usb/core/hub.c:5098 [inline]
hub_port_connect_change drivers/usb/core/hub.c:5213 [inline]
port_event drivers/usb/core/hub.c:5359 [inline]
hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441
process_one_work+0x92b/0x1530 kernel/workqueue.c:2269
worker_thread+0x96/0xe20 kernel/workqueue.c:2415
kthread+0x318/0x420 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
Allocated by task 102:
save_stack+0x1b/0x80 mm/kasan/common.c:69
set_track mm/kasan/common.c:77 [inline]
__kasan_kmalloc mm/kasan/common.c:487 [inline]
__kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:460
kmalloc include/linux/slab.h:552 [inline]
kzalloc include/linux/slab.h:748 [inline]
rsi_init_usb_interface drivers/net/wireless/rsi/rsi_91x_usb.c:599 [inline]
rsi_probe+0x11a/0x15a0 drivers/net/wireless/rsi/rsi_91x_usb.c:780
usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361
really_probe+0x281/0x6d0 drivers/base/dd.c:548
driver_probe_device+0x101/0x1b0 drivers/base/dd.c:721
__device_attach_driver+0x1c2/0x220 drivers/base/dd.c:828
bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:454
__device_attach+0x217/0x360 drivers/base/dd.c:894
bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
device_add+0xae6/0x16f0 drivers/base/core.c:2165
usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023
generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210
usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266
really_probe+0x281/0x6d0 drivers/base/dd.c:548
driver_probe_device+0x101/0x1b0 drivers/base/dd.c:721
__device_attach_driver+0x1c2/0x220 drivers/base/dd.c:828
bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:454
__device_attach+0x217/0x360 drivers/base/dd.c:894
bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
device_add+0xae6/0x16f0 drivers/base/core.c:2165
usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536
hub_port_connect drivers/usb/core/hub.c:5098 [inline]
hub_port_connect_change drivers/usb/core/hub.c:5213 [inline]
port_event drivers/usb/core/hub.c:5359 [inline]
hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441
process_one_work+0x92b/0x1530 kernel/workqueue.c:2269
worker_thread+0x96/0xe20 kernel/workqueue.c:2415
kthread+0x318/0x420 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
Freed by task 102:
save_stack+0x1b/0x80 mm/kasan/common.c:69
set_track mm/kasan/common.c:77 [inline]
__kasan_slab_free+0x130/0x180 mm/kasan/common.c:449
slab_free_hook mm/slub.c:1423 [inline]
slab_free_freelist_hook mm/slub.c:1474 [inline]
slab_free mm/slub.c:3016 [inline]
kfree+0xe4/0x2f0 mm/slub.c:3957
rsi_91x_deinit+0x270/0x2f0 drivers/net/wireless/rsi/rsi_91x_main.c:407
rsi_probe+0xcec/0x15a0 drivers/net/wireless/rsi/rsi_91x_usb.c:834
usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361
really_probe+0x281/0x6d0 drivers/base/dd.c:548
driver_probe_device+0x101/0x1b0 drivers/base/dd.c:721
__device_attach_driver+0x1c2/0x220 drivers/base/dd.c:828
bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:454
__device_attach+0x217/0x360 drivers/base/dd.c:894
bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
device_add+0xae6/0x16f0 drivers/base/core.c:2165
usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023
generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210
usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266
really_probe+0x281/0x6d0 drivers/base/dd.c:548
driver_probe_device+0x101/0x1b0 drivers/base/dd.c:721
__device_attach_driver+0x1c2/0x220 drivers/base/dd.c:828
bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:454
__device_attach+0x217/0x360 drivers/base/dd.c:894
bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
device_add+0xae6/0x16f0 drivers/base/core.c:2165
usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536
hub_port_connect drivers/usb/core/hub.c:5098 [inline]
hub_port_connect_change drivers/usb/core/hub.c:5213 [inline]
port_event drivers/usb/core/hub.c:5359 [inline]
hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441
process_one_work+0x92b/0x1530 kernel/workqueue.c:2269
worker_thread+0x96/0xe20 kernel/workqueue.c:2415
kthread+0x318/0x420 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
The buggy address belongs to the object at ffff8881cebc0f00
which belongs to the cache kmalloc-512 of size 512
The buggy address is located 232 bytes inside of
512-byte region [ffff8881cebc0f00, ffff8881cebc1100)
The buggy address belongs to the page:
page:ffffea00073af000 refcount:1 mapcount:0 mapping:ffff8881da002500
index:0x0 compound_mapcount: 0
flags: 0x200000000010200(slab|head)
raw: 0200000000010200 0000000000000000 0000000b00000001 ffff8881da002500
raw: 0000000000000000 00000000800c000c 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff8881cebc0e80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff8881cebc0f00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ffff8881cebc0f80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff8881cebc1000: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8881cebc1080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
^ permalink raw reply
* WARNING in hso_free_net_device
From: syzbot @ 2019-09-03 12:08 UTC (permalink / raw)
To: alexios.zavras, andreyknvl, benquike, davem, gregkh, linux-kernel,
linux-usb, mathias.payer, netdev, rfontana, syzkaller-bugs, tglx
Hello,
syzbot found the following crash on:
HEAD commit: eea39f24 usb-fuzzer: main usb gadget fuzzer driver
git tree: https://github.com/google/kasan.git usb-fuzzer
console output: https://syzkaller.appspot.com/x/log.txt?x=15f17e61600000
kernel config: https://syzkaller.appspot.com/x/.config?x=d0c62209eedfd54e
dashboard link: https://syzkaller.appspot.com/bug?extid=44d53c7255bb1aea22d2
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=10ffdd12600000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=15a738fe600000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+44d53c7255bb1aea22d2@syzkaller.appspotmail.com
usb 1-1: config 0 has no interface number 0
usb 1-1: New USB device found, idVendor=0af0, idProduct=d257,
bcdDevice=4e.87
usb 1-1: New USB device strings: Mfr=0, Product=0, SerialNumber=0
usb 1-1: config 0 descriptor??
hso 1-1:0.15: Can't find BULK IN endpoint
------------[ cut here ]------------
WARNING: CPU: 1 PID: 83 at net/core/dev.c:8167
rollback_registered_many.cold+0x41/0x1bc net/core/dev.c:8167
Kernel panic - not syncing: panic_on_warn set ...
CPU: 1 PID: 83 Comm: kworker/1:2 Not tainted 5.3.0-rc5+ #28
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Workqueue: usb_hub_wq hub_event
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0xca/0x13e lib/dump_stack.c:113
panic+0x2a3/0x6da kernel/panic.c:219
__warn.cold+0x20/0x4a kernel/panic.c:576
report_bug+0x262/0x2a0 lib/bug.c:186
fixup_bug arch/x86/kernel/traps.c:179 [inline]
fixup_bug arch/x86/kernel/traps.c:174 [inline]
do_error_trap+0x12b/0x1e0 arch/x86/kernel/traps.c:272
do_invalid_op+0x32/0x40 arch/x86/kernel/traps.c:291
invalid_op+0x23/0x30 arch/x86/entry/entry_64.S:1028
RIP: 0010:rollback_registered_many.cold+0x41/0x1bc net/core/dev.c:8167
Code: ff e8 c7 26 90 fc 48 c7 c7 40 ec 63 86 e8 24 c8 7a fc 0f 0b e9 93 be
ff ff e8 af 26 90 fc 48 c7 c7 40 ec 63 86 e8 0c c8 7a fc <0f> 0b 4c 89 e7
e8 f9 12 34 fd 31 ff 41 89 c4 89 c6 e8 bd 27 90 fc
RSP: 0018:ffff8881d934f088 EFLAGS: 00010282
RAX: 0000000000000024 RBX: ffff8881d2ad4400 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff81288cfd RDI: ffffed103b269e03
RBP: ffff8881d934f1b8 R08: 0000000000000024 R09: fffffbfff11ad794
R10: fffffbfff11ad793 R11: ffffffff88d6bc9f R12: ffff8881d2ad4470
R13: ffff8881d934f148 R14: dffffc0000000000 R15: 0000000000000000
rollback_registered+0xf2/0x1c0 net/core/dev.c:8243
unregister_netdevice_queue net/core/dev.c:9290 [inline]
unregister_netdevice_queue+0x1d7/0x2b0 net/core/dev.c:9283
unregister_netdevice include/linux/netdevice.h:2631 [inline]
unregister_netdev+0x18/0x20 net/core/dev.c:9331
hso_free_net_device+0xff/0x300 drivers/net/usb/hso.c:2366
hso_create_net_device+0x76d/0x9c0 drivers/net/usb/hso.c:2554
hso_probe+0x28d/0x1a46 drivers/net/usb/hso.c:2931
usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361
really_probe+0x281/0x6d0 drivers/base/dd.c:548
driver_probe_device+0x101/0x1b0 drivers/base/dd.c:721
__device_attach_driver+0x1c2/0x220 drivers/base/dd.c:828
bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:454
__device_attach+0x217/0x360 drivers/base/dd.c:894
bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
device_add+0xae6/0x16f0 drivers/base/core.c:2165
usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023
generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210
usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266
really_probe+0x281/0x6d0 drivers/base/dd.c:548
driver_probe_device+0x101/0x1b0 drivers/base/dd.c:721
__device_attach_driver+0x1c2/0x220 drivers/base/dd.c:828
bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:454
__device_attach+0x217/0x360 drivers/base/dd.c:894
bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
device_add+0xae6/0x16f0 drivers/base/core.c:2165
usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536
hub_port_connect drivers/usb/core/hub.c:5098 [inline]
hub_port_connect_change drivers/usb/core/hub.c:5213 [inline]
port_event drivers/usb/core/hub.c:5359 [inline]
hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441
process_one_work+0x92b/0x1530 kernel/workqueue.c:2269
worker_thread+0x96/0xe20 kernel/workqueue.c:2415
kthread+0x318/0x420 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
Kernel Offset: disabled
Rebooting in 86400 seconds..
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* KASAN: use-after-free Read in atusb_disconnect
From: syzbot @ 2019-09-03 12:08 UTC (permalink / raw)
To: alex.aring, andreyknvl, davem, linux-kernel, linux-usb,
linux-wpan, netdev, stefan, syzkaller-bugs
Hello,
syzbot found the following crash on:
HEAD commit: eea39f24 usb-fuzzer: main usb gadget fuzzer driver
git tree: https://github.com/google/kasan.git usb-fuzzer
console output: https://syzkaller.appspot.com/x/log.txt?x=15c4eba6600000
kernel config: https://syzkaller.appspot.com/x/.config?x=d0c62209eedfd54e
dashboard link: https://syzkaller.appspot.com/bug?extid=f4509a9138a1472e7e80
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=15486ab6600000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=15777f22600000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+f4509a9138a1472e7e80@syzkaller.appspotmail.com
usb 1-1: USB disconnect, device number 2
==================================================================
BUG: KASAN: use-after-free in atusb_disconnect+0x17f/0x1c0
drivers/net/ieee802154/atusb.c:1143
Read of size 8 at addr ffff8881d53eee28 by task kworker/1:2/83
CPU: 1 PID: 83 Comm: kworker/1:2 Not tainted 5.3.0-rc5+ #28
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Workqueue: usb_hub_wq hub_event
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0xca/0x13e lib/dump_stack.c:113
print_address_description+0x6a/0x32c mm/kasan/report.c:351
__kasan_report.cold+0x1a/0x33 mm/kasan/report.c:482
kasan_report+0xe/0x12 mm/kasan/common.c:612
atusb_disconnect+0x17f/0x1c0 drivers/net/ieee802154/atusb.c:1143
usb_unbind_interface+0x1bd/0x8a0 drivers/usb/core/driver.c:423
__device_release_driver drivers/base/dd.c:1134 [inline]
device_release_driver_internal+0x42f/0x500 drivers/base/dd.c:1165
bus_remove_device+0x2dc/0x4a0 drivers/base/bus.c:556
device_del+0x420/0xb10 drivers/base/core.c:2339
usb_disable_device+0x211/0x690 drivers/usb/core/message.c:1237
usb_disconnect+0x284/0x8d0 drivers/usb/core/hub.c:2199
hub_port_connect drivers/usb/core/hub.c:4949 [inline]
hub_port_connect_change drivers/usb/core/hub.c:5213 [inline]
port_event drivers/usb/core/hub.c:5359 [inline]
hub_event+0x1454/0x3640 drivers/usb/core/hub.c:5441
process_one_work+0x92b/0x1530 kernel/workqueue.c:2269
worker_thread+0x96/0xe20 kernel/workqueue.c:2415
kthread+0x318/0x420 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
Allocated by task 12:
save_stack+0x1b/0x80 mm/kasan/common.c:69
set_track mm/kasan/common.c:77 [inline]
__kasan_kmalloc mm/kasan/common.c:487 [inline]
__kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:460
kmalloc include/linux/slab.h:557 [inline]
kzalloc include/linux/slab.h:748 [inline]
wpan_phy_new+0x22/0x290 net/ieee802154/core.c:109
ieee802154_alloc_hw+0x11d/0x750 net/mac802154/main.c:77
atusb_probe+0x9b/0xfa2 drivers/net/ieee802154/atusb.c:1023
usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361
really_probe+0x281/0x6d0 drivers/base/dd.c:548
driver_probe_device+0x101/0x1b0 drivers/base/dd.c:721
__device_attach_driver+0x1c2/0x220 drivers/base/dd.c:828
bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:454
__device_attach+0x217/0x360 drivers/base/dd.c:894
bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
device_add+0xae6/0x16f0 drivers/base/core.c:2165
usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023
generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210
usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266
really_probe+0x281/0x6d0 drivers/base/dd.c:548
driver_probe_device+0x101/0x1b0 drivers/base/dd.c:721
__device_attach_driver+0x1c2/0x220 drivers/base/dd.c:828
bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:454
__device_attach+0x217/0x360 drivers/base/dd.c:894
bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
device_add+0xae6/0x16f0 drivers/base/core.c:2165
usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536
hub_port_connect drivers/usb/core/hub.c:5098 [inline]
hub_port_connect_change drivers/usb/core/hub.c:5213 [inline]
port_event drivers/usb/core/hub.c:5359 [inline]
hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441
process_one_work+0x92b/0x1530 kernel/workqueue.c:2269
worker_thread+0x96/0xe20 kernel/workqueue.c:2415
kthread+0x318/0x420 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
Freed by task 83:
save_stack+0x1b/0x80 mm/kasan/common.c:69
set_track mm/kasan/common.c:77 [inline]
__kasan_slab_free+0x130/0x180 mm/kasan/common.c:449
slab_free_hook mm/slub.c:1423 [inline]
slab_free_freelist_hook mm/slub.c:1474 [inline]
slab_free mm/slub.c:3016 [inline]
kfree+0xe4/0x2f0 mm/slub.c:3957
device_release+0x71/0x200 drivers/base/core.c:1064
kobject_cleanup lib/kobject.c:693 [inline]
kobject_release lib/kobject.c:722 [inline]
kref_put include/linux/kref.h:65 [inline]
kobject_put+0x171/0x280 lib/kobject.c:739
put_device+0x1b/0x30 drivers/base/core.c:2264
atusb_disconnect+0x117/0x1c0 drivers/net/ieee802154/atusb.c:1140
usb_unbind_interface+0x1bd/0x8a0 drivers/usb/core/driver.c:423
__device_release_driver drivers/base/dd.c:1134 [inline]
device_release_driver_internal+0x42f/0x500 drivers/base/dd.c:1165
bus_remove_device+0x2dc/0x4a0 drivers/base/bus.c:556
device_del+0x420/0xb10 drivers/base/core.c:2339
usb_disable_device+0x211/0x690 drivers/usb/core/message.c:1237
usb_disconnect+0x284/0x8d0 drivers/usb/core/hub.c:2199
hub_port_connect drivers/usb/core/hub.c:4949 [inline]
hub_port_connect_change drivers/usb/core/hub.c:5213 [inline]
port_event drivers/usb/core/hub.c:5359 [inline]
hub_event+0x1454/0x3640 drivers/usb/core/hub.c:5441
process_one_work+0x92b/0x1530 kernel/workqueue.c:2269
worker_thread+0x96/0xe20 kernel/workqueue.c:2415
kthread+0x318/0x420 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
The buggy address belongs to the object at ffff8881d53ee600
which belongs to the cache kmalloc-4k of size 4096
The buggy address is located 2088 bytes inside of
4096-byte region [ffff8881d53ee600, ffff8881d53ef600)
The buggy address belongs to the page:
page:ffffea000754fa00 refcount:1 mapcount:0 mapping:ffff8881da00c280
index:0x0 compound_mapcount: 0
flags: 0x200000000010200(slab|head)
raw: 0200000000010200 0000000000000000 0000000600000001 ffff8881da00c280
raw: 0000000000000000 0000000000070007 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff8881d53eed00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8881d53eed80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ffff8881d53eee00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff8881d53eee80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8881d53eef00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* Re: [PATCH] net-ipv6: fix excessive RTF_ADDRCONF flag on ::1/128 local route (and others)
From: Maciej Żenczykowski @ 2019-09-03 12:17 UTC (permalink / raw)
To: Lorenzo Colitti; +Cc: David Ahern, David S . Miller, Linux NetDev
In-Reply-To: <CAKD1Yr2ykCyEiUyY4R+hYoZ+eWGjbE78wtSf2=_ZjLpCyp0n-Q@mail.gmail.com>
Well, if you look at the commit my commit is fixing, ie.
commit c7a1ce397adacaf5d4bb2eab0a738b5f80dc3e43
then you'll see this in the commit description:
"- dst_nocount is handled by the RTF_ADDRCONF flag"
and the patch diff itself is from
"f6i->fib6_flags = RTF_UP | RTF_NONEXTHOP;
f6i->dst_nocount = true;"
to
" .fc_flags = RTF_UP | RTF_ADDRCONF | RTF_NONEXTHOP,"
(and RTF_ANYCAST or RTF_LOCAL is later or'ed in in both versions of the code)
so I'm pretty sure that patch adds ADDRCONF unconditionally to that
function, and my commit unconditionally removes it.
Perhaps since then the call graph has changed???
Unfortunately I'm already in Europe on ancient ipv6 free networks and
since the office move my ipv6 lab is still not up (something about the
newly wired office jacks being blue which means corp, instead of any
other colour for a lab...) so I don't actually have an easy way to
test ipv6 slaac behaviour. :-(
On Tue, Sep 3, 2019 at 6:58 AM Lorenzo Colitti <lorenzo@google.com> wrote:
>
> On Tue, Sep 3, 2019 at 11:18 AM David Ahern <dsahern@gmail.com> wrote:
> > addrconf_f6i_alloc is used for addresses added by userspace
> > (ipv6_add_addr) and anycast. ie., from what I can see it is not used for RAs
>
> Isn't ipv6_add_addr called by addrconf_prefix_rcv_add_addr, which is
> called by addrconf_prefix_rcv, which is called by
> ndisc_router_discovery? That is what happens when we process an RA;
> AFAICS manual configuration is inet6_addr_add, not ipv6_add_addr.
>
> Maciej, with this patch, do SLAAC addresses still have RTF_ADDRCONF?
> Per my previous message, my assumption would be no, but I might be
> misreading the code.
^ permalink raw reply
* [PATCH v2 0/4] can: mcp251x: Make use of device properties
From: Andy Shevchenko @ 2019-09-03 12:42 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde, linux-can,
David S. Miller, netdev
Cc: Andy Shevchenko
The purpose of this series is to simplify driver by switching to use device
properties. In particular it allows to drop legacy platform data.
Patch 1 switches driver to use devm_clk_get_optional() API.
Patch 2 unifies getting the driver data independently of the table which
provides it.
Patch 3 drops extra check for regulator presence by switch to use an already
present wrapper.
And patch 4 gets rid of legacy platform data.
Changelog v2:
- add patch 4 to get rid of legacy platform data
Andy Shevchenko (4):
can: mcp251x: Use devm_clk_get_optional() to get the input clock
can: mcp251x: Make use of device property API
can: mcp251x: Call wrapper instead of regulator_disable()
can: mcp251x: Get rid of legacy platform data
arch/arm/mach-pxa/icontrol.c | 9 ++--
arch/arm/mach-pxa/zeus.c | 9 ++--
drivers/net/can/spi/mcp251x.c | 65 +++++++++++-----------------
include/linux/can/platform/mcp251x.h | 22 ----------
4 files changed, 36 insertions(+), 69 deletions(-)
delete mode 100644 include/linux/can/platform/mcp251x.h
--
2.23.0.rc1
^ permalink raw reply
* [PATCH v2 2/4] can: mcp251x: Make use of device property API
From: Andy Shevchenko @ 2019-09-03 12:42 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde, linux-can,
David S. Miller, netdev
Cc: Andy Shevchenko
In-Reply-To: <20190903124259.60920-1-andriy.shevchenko@linux.intel.com>
Make use of device property API in this driver so that both OF based
system and ACPI based system can use this driver.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/net/can/spi/mcp251x.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/net/can/spi/mcp251x.c b/drivers/net/can/spi/mcp251x.c
index e04b578f2b1f..0b7e743ca0a0 100644
--- a/drivers/net/can/spi/mcp251x.c
+++ b/drivers/net/can/spi/mcp251x.c
@@ -53,8 +53,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/netdevice.h>
-#include <linux/of.h>
-#include <linux/of_device.h>
+#include <linux/property.h>
#include <linux/platform_device.h>
#include <linux/slab.h>
#include <linux/spi/spi.h>
@@ -914,7 +913,7 @@ static int mcp251x_open(struct net_device *net)
priv->tx_skb = NULL;
priv->tx_len = 0;
- if (!spi->dev.of_node)
+ if (!dev_fwnode(&spi->dev))
flags = IRQF_TRIGGER_FALLING;
ret = request_threaded_irq(spi->irq, NULL, mcp251x_can_ist,
@@ -1006,8 +1005,7 @@ MODULE_DEVICE_TABLE(spi, mcp251x_id_table);
static int mcp251x_can_probe(struct spi_device *spi)
{
- const struct of_device_id *of_id = of_match_device(mcp251x_of_match,
- &spi->dev);
+ const void *match = device_get_match_data(&spi->dev);
struct mcp251x_platform_data *pdata = dev_get_platdata(&spi->dev);
struct net_device *net;
struct mcp251x_priv *priv;
@@ -1044,8 +1042,8 @@ static int mcp251x_can_probe(struct spi_device *spi)
priv->can.clock.freq = freq / 2;
priv->can.ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES |
CAN_CTRLMODE_LOOPBACK | CAN_CTRLMODE_LISTENONLY;
- if (of_id)
- priv->model = (enum mcp251x_model)of_id->data;
+ if (match)
+ priv->model = (enum mcp251x_model)match;
else
priv->model = spi_get_device_id(spi)->driver_data;
priv->net = net;
--
2.23.0.rc1
^ permalink raw reply related
* [PATCH v2 4/4] can: mcp251x: Get rid of legacy platform data
From: Andy Shevchenko @ 2019-09-03 12:42 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde, linux-can,
David S. Miller, netdev
Cc: Andy Shevchenko, Daniel Mack, Haojian Zhuang, Robert Jarzmik,
Russell King
In-Reply-To: <20190903124259.60920-1-andriy.shevchenko@linux.intel.com>
Instead of using legacy platform data, switch to use device properties.
For clock frequency we are using well established clock-frequency property.
Users, two for now, are also converted here.
Cc: Daniel Mack <daniel@zonque.org>
Cc: Haojian Zhuang <haojian.zhuang@gmail.com>
Cc: Robert Jarzmik <robert.jarzmik@free.fr>
Cc: Russell King <linux@armlinux.org.uk>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
arch/arm/mach-pxa/icontrol.c | 9 +++++----
arch/arm/mach-pxa/zeus.c | 9 +++++----
drivers/net/can/spi/mcp251x.c | 19 ++++++++-----------
include/linux/can/platform/mcp251x.h | 22 ----------------------
4 files changed, 18 insertions(+), 41 deletions(-)
delete mode 100644 include/linux/can/platform/mcp251x.h
diff --git a/arch/arm/mach-pxa/icontrol.c b/arch/arm/mach-pxa/icontrol.c
index 865b10344ea2..aa4ccb9bb1c1 100644
--- a/arch/arm/mach-pxa/icontrol.c
+++ b/arch/arm/mach-pxa/icontrol.c
@@ -12,6 +12,7 @@
#include <linux/irq.h>
#include <linux/platform_device.h>
+#include <linux/property.h>
#include <linux/gpio.h>
#include <asm/mach-types.h>
@@ -22,7 +23,6 @@
#include <linux/spi/spi.h>
#include <linux/spi/pxa2xx_spi.h>
-#include <linux/can/platform/mcp251x.h>
#include <linux/regulator/machine.h>
#include "generic.h"
@@ -69,8 +69,9 @@ static struct pxa2xx_spi_chip mcp251x_chip_info4 = {
.gpio_cs = ICONTROL_MCP251x_nCS4
};
-static struct mcp251x_platform_data mcp251x_info = {
- .oscillator_frequency = 16E6,
+static const struct property_entry mcp251x_properties = {
+ PROPERTY_ENTRY_U32("clock-frequency", 16000000),
+ {}
};
static struct spi_board_info mcp251x_board_info[] = {
@@ -79,7 +80,7 @@ static struct spi_board_info mcp251x_board_info[] = {
.max_speed_hz = 6500000,
.bus_num = 3,
.chip_select = 0,
- .platform_data = &mcp251x_info,
+ .properties = &mcp251x_properties,
.controller_data = &mcp251x_chip_info1,
.irq = PXA_GPIO_TO_IRQ(ICONTROL_MCP251x_nIRQ1)
},
diff --git a/arch/arm/mach-pxa/zeus.c b/arch/arm/mach-pxa/zeus.c
index da113c8eefbf..645500ef427a 100644
--- a/arch/arm/mach-pxa/zeus.c
+++ b/arch/arm/mach-pxa/zeus.c
@@ -13,6 +13,7 @@
#include <linux/leds.h>
#include <linux/irq.h>
#include <linux/pm.h>
+#include <linux/property.h>
#include <linux/gpio.h>
#include <linux/gpio/machine.h>
#include <linux/serial_8250.h>
@@ -27,7 +28,6 @@
#include <linux/platform_data/i2c-pxa.h>
#include <linux/platform_data/pca953x.h>
#include <linux/apm-emulation.h>
-#include <linux/can/platform/mcp251x.h>
#include <linux/regulator/fixed.h>
#include <linux/regulator/machine.h>
@@ -428,14 +428,15 @@ static struct gpiod_lookup_table can_regulator_gpiod_table = {
},
};
-static struct mcp251x_platform_data zeus_mcp2515_pdata = {
- .oscillator_frequency = 16*1000*1000,
+static const struct property_entry mcp251x_properties = {
+ PROPERTY_ENTRY_U32("clock-frequency", 16000000),
+ {}
};
static struct spi_board_info zeus_spi_board_info[] = {
[0] = {
.modalias = "mcp2515",
- .platform_data = &zeus_mcp2515_pdata,
+ .properties = &mcp251x_properties,
.irq = PXA_GPIO_TO_IRQ(ZEUS_CAN_GPIO),
.max_speed_hz = 1*1000*1000,
.bus_num = 3,
diff --git a/drivers/net/can/spi/mcp251x.c b/drivers/net/can/spi/mcp251x.c
index 6ee0ea51399a..3a4d7089dc7c 100644
--- a/drivers/net/can/spi/mcp251x.c
+++ b/drivers/net/can/spi/mcp251x.c
@@ -20,29 +20,26 @@
*
* Your platform definition file should specify something like:
*
- * static struct mcp251x_platform_data mcp251x_info = {
- * .oscillator_frequency = 8000000,
+ * static const struct property_entry mpc251x_properties[] = {
+ * PROPERTY_ENTRY_U32("clock-frequency", 8000000),
+ * {}
* };
*
* static struct spi_board_info spi_board_info[] = {
* {
* .modalias = "mcp2510",
* // "mcp2515" or "mcp25625" depending on your controller
- * .platform_data = &mcp251x_info,
+ * .properties = &mcp251x_properties,
* .irq = IRQ_EINT13,
* .max_speed_hz = 2*1000*1000,
* .chip_select = 2,
* },
* };
- *
- * Please see mcp251x.h for a description of the fields in
- * struct mcp251x_platform_data.
*/
#include <linux/can/core.h>
#include <linux/can/dev.h>
#include <linux/can/led.h>
-#include <linux/can/platform/mcp251x.h>
#include <linux/clk.h>
#include <linux/completion.h>
#include <linux/delay.h>
@@ -1006,19 +1003,19 @@ MODULE_DEVICE_TABLE(spi, mcp251x_id_table);
static int mcp251x_can_probe(struct spi_device *spi)
{
const void *match = device_get_match_data(&spi->dev);
- struct mcp251x_platform_data *pdata = dev_get_platdata(&spi->dev);
struct net_device *net;
struct mcp251x_priv *priv;
struct clk *clk;
- int freq, ret;
+ u32 freq;
+ int ret;
clk = devm_clk_get_optional(&spi->dev, NULL);
if (IS_ERR(clk))
return PTR_ERR(clk);
freq = clk_get_rate(clk);
- if (freq == 0 && pdata)
- freq = pdata->oscillator_frequency;
+ if (freq == 0)
+ device_property_read_u32(&spi->dev, "clock-frequency", &freq);
/* Sanity check */
if (freq < 1000000 || freq > 25000000)
diff --git a/include/linux/can/platform/mcp251x.h b/include/linux/can/platform/mcp251x.h
deleted file mode 100644
index 9e5ac27fb6c1..000000000000
--- a/include/linux/can/platform/mcp251x.h
+++ /dev/null
@@ -1,22 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef _CAN_PLATFORM_MCP251X_H
-#define _CAN_PLATFORM_MCP251X_H
-
-/*
- *
- * CAN bus driver for Microchip 251x CAN Controller with SPI Interface
- *
- */
-
-#include <linux/spi/spi.h>
-
-/*
- * struct mcp251x_platform_data - MCP251X SPI CAN controller platform data
- * @oscillator_frequency: - oscillator frequency in Hz
- */
-
-struct mcp251x_platform_data {
- unsigned long oscillator_frequency;
-};
-
-#endif /* !_CAN_PLATFORM_MCP251X_H */
--
2.23.0.rc1
^ permalink raw reply related
* [PATCH v2 3/4] can: mcp251x: Call wrapper instead of regulator_disable()
From: Andy Shevchenko @ 2019-09-03 12:42 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde, linux-can,
David S. Miller, netdev
Cc: Andy Shevchenko
In-Reply-To: <20190903124259.60920-1-andriy.shevchenko@linux.intel.com>
There is no need to check for regulator presence in the ->suspend()
since a wrapper does it for us. Due to this we may unconditionally set
AFTER_SUSPEND_POWER flag.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/net/can/spi/mcp251x.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/can/spi/mcp251x.c b/drivers/net/can/spi/mcp251x.c
index 0b7e743ca0a0..6ee0ea51399a 100644
--- a/drivers/net/can/spi/mcp251x.c
+++ b/drivers/net/can/spi/mcp251x.c
@@ -1162,10 +1162,8 @@ static int __maybe_unused mcp251x_can_suspend(struct device *dev)
priv->after_suspend = AFTER_SUSPEND_DOWN;
}
- if (!IS_ERR_OR_NULL(priv->power)) {
- regulator_disable(priv->power);
- priv->after_suspend |= AFTER_SUSPEND_POWER;
- }
+ mcp251x_power_enable(priv->power, 0);
+ priv->after_suspend |= AFTER_SUSPEND_POWER;
return 0;
}
--
2.23.0.rc1
^ permalink raw reply related
* [PATCH v2 1/4] can: mcp251x: Use devm_clk_get_optional() to get the input clock
From: Andy Shevchenko @ 2019-09-03 12:42 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde, linux-can,
David S. Miller, netdev
Cc: Andy Shevchenko
In-Reply-To: <20190903124259.60920-1-andriy.shevchenko@linux.intel.com>
Simplify the code which fetches the input clock by using
devm_clk_get_optional(). This comes with a small functional change: previously
all errors were ignored when platform data is present. Now all errors are
treated as errors. If no input clock is present devm_clk_get_optional() will
return NULL instead of an error which matches the behavior of the old code.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/net/can/spi/mcp251x.c | 30 ++++++++++++------------------
1 file changed, 12 insertions(+), 18 deletions(-)
diff --git a/drivers/net/can/spi/mcp251x.c b/drivers/net/can/spi/mcp251x.c
index 58992fd61cb9..e04b578f2b1f 100644
--- a/drivers/net/can/spi/mcp251x.c
+++ b/drivers/net/can/spi/mcp251x.c
@@ -1014,15 +1014,13 @@ static int mcp251x_can_probe(struct spi_device *spi)
struct clk *clk;
int freq, ret;
- clk = devm_clk_get(&spi->dev, NULL);
- if (IS_ERR(clk)) {
- if (pdata)
- freq = pdata->oscillator_frequency;
- else
- return PTR_ERR(clk);
- } else {
- freq = clk_get_rate(clk);
- }
+ clk = devm_clk_get_optional(&spi->dev, NULL);
+ if (IS_ERR(clk))
+ return PTR_ERR(clk);
+
+ freq = clk_get_rate(clk);
+ if (freq == 0 && pdata)
+ freq = pdata->oscillator_frequency;
/* Sanity check */
if (freq < 1000000 || freq > 25000000)
@@ -1033,11 +1031,9 @@ static int mcp251x_can_probe(struct spi_device *spi)
if (!net)
return -ENOMEM;
- if (!IS_ERR(clk)) {
- ret = clk_prepare_enable(clk);
- if (ret)
- goto out_free;
- }
+ ret = clk_prepare_enable(clk);
+ if (ret)
+ goto out_free;
net->netdev_ops = &mcp251x_netdev_ops;
net->flags |= IFF_ECHO;
@@ -1122,8 +1118,7 @@ static int mcp251x_can_probe(struct spi_device *spi)
mcp251x_power_enable(priv->power, 0);
out_clk:
- if (!IS_ERR(clk))
- clk_disable_unprepare(clk);
+ clk_disable_unprepare(clk);
out_free:
free_candev(net);
@@ -1141,8 +1136,7 @@ static int mcp251x_can_remove(struct spi_device *spi)
mcp251x_power_enable(priv->power, 0);
- if (!IS_ERR(priv->clk))
- clk_disable_unprepare(priv->clk);
+ clk_disable_unprepare(priv->clk);
free_candev(net);
--
2.23.0.rc1
^ permalink raw reply related
* Re: [PATCH v2 0/4] can: mcp251x: Make use of device properties
From: Marc Kleine-Budde @ 2019-09-03 12:50 UTC (permalink / raw)
To: Andy Shevchenko, Wolfgang Grandegger, linux-can, David S. Miller,
netdev
In-Reply-To: <20190903124259.60920-1-andriy.shevchenko@linux.intel.com>
[-- Attachment #1.1: Type: text/plain, Size: 1152 bytes --]
On 9/3/19 2:42 PM, Andy Shevchenko wrote:
> The purpose of this series is to simplify driver by switching to use device
> properties. In particular it allows to drop legacy platform data.
>
> Patch 1 switches driver to use devm_clk_get_optional() API.
>
> Patch 2 unifies getting the driver data independently of the table which
> provides it.
>
> Patch 3 drops extra check for regulator presence by switch to use an already
> present wrapper.
>
> And patch 4 gets rid of legacy platform data.
>
> Changelog v2:
> - add patch 4 to get rid of legacy platform data
Sorry for not telling, your v1 series has already been applied and it's
included in the latest pull request. Can you please rebase your patches
on top of:
https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git/tag/?h=linux-can-next-for-5.4-20190903
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH net] ipmr: remove cache_resolve_queue_len
From: Hangbin Liu @ 2019-09-03 12:55 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: netdev, Phil Karn, Sukumar Gopalakrishnan, David S . Miller,
Alexey Kuznetsov, Hideaki YOSHIFUJI
In-Reply-To: <e0261582-78ce-038f-ed4c-c2694fb70250@cumulusnetworks.com>
Hi Nikolay,
Thanks for the feedback, see comments below.
On Tue, Sep 03, 2019 at 12:15:34PM +0300, Nikolay Aleksandrov wrote:
> On 03/09/2019 11:43, Hangbin Liu wrote:
> > This is a re-post of previous patch wrote by David Miller[1].
> >
> > Phil Karn reported[2] that on busy networks with lots of unresolved
> > multicast routing entries, the creation of new multicast group routes
> > can be extremely slow and unreliable.
> >
> > The reason is we hard-coded multicast route entries with unresolved source
> > addresses(cache_resolve_queue_len) to 10. If some multicast route never
> > resolves and the unresolved source addresses increased, there will
> > be no ability to create new multicast route cache.
> >
> > To resolve this issue, we need either add a sysctl entry to make the
> > cache_resolve_queue_len configurable, or just remove cache_resolve_queue_len
> > directly, as we already have the socket receive queue limits of mrouted
> > socket, pointed by David.
> >
> > From my side, I'd perfer to remove the cache_resolve_queue_len instead
> > of creating two more(IPv4 and IPv6 version) sysctl entry.
> >
> > [1] https://lkml.org/lkml/2018/7/22/11
> > [2] https://lkml.org/lkml/2018/7/21/343
> >
> > Reported-by: Phil Karn <karn@ka9q.net>
> > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> > ---
> > include/linux/mroute_base.h | 2 --
> > net/ipv4/ipmr.c | 27 ++++++++++++++++++---------
> > net/ipv6/ip6mr.c | 10 +++-------
> > 3 files changed, 21 insertions(+), 18 deletions(-)
> >
>
> Hi,
> IMO this is definitely net-next material. A few more comments below.
I thoug this is a bug fix. But it looks more suitable to net-next as you said.
>
> Note that hosts will automatically have this limit lifted to about 270
> entries with current defaults, some might be surprised if they have
> higher limits set and could be left with queues allowing for thousands
> of entries in a linked list.
I think this is just a cache list and should be expired soon. The cache
creation would also failed if there is no buffer.
But if you like, I can write a patch with sysctl parameter.
>
> > +static int queue_count(struct mr_table *mrt)
> > +{
> > + struct list_head *pos;
> > + int count = 0;
> > +
> > + list_for_each(pos, &mrt->mfc_unres_queue)
> > + count++;
> > + return count;
> > +}
>
> I don't think we hold the mfc_unres_lock here while walking
> the unresolved list below in ipmr_fill_table().
ah, yes, I will fix this.
Thanks
Hangbin
^ permalink raw reply
* Re: [PATCH v2 0/4] can: mcp251x: Make use of device properties
From: Andy Shevchenko @ 2019-09-03 13:02 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: Wolfgang Grandegger, linux-can, David S. Miller, netdev
In-Reply-To: <9ba9d56b-c045-74d3-9693-a9a959ffb675@pengutronix.de>
On Tue, Sep 03, 2019 at 02:50:05PM +0200, Marc Kleine-Budde wrote:
> On 9/3/19 2:42 PM, Andy Shevchenko wrote:
> > The purpose of this series is to simplify driver by switching to use device
> > properties. In particular it allows to drop legacy platform data.
> >
> > Patch 1 switches driver to use devm_clk_get_optional() API.
> >
> > Patch 2 unifies getting the driver data independently of the table which
> > provides it.
> >
> > Patch 3 drops extra check for regulator presence by switch to use an already
> > present wrapper.
> >
> > And patch 4 gets rid of legacy platform data.
> >
> > Changelog v2:
> > - add patch 4 to get rid of legacy platform data
>
> Sorry for not telling, your v1 series has already been applied and it's
> included in the latest pull request. Can you please rebase your patches
> on top of:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git/tag/?h=linux-can-next-for-5.4-20190903
Thank you!
Basically first 3 patches didn't change. Consider patch 4 only. Should I resend
it separately as v3?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2] rtlwifi: fix non-kerneldoc comment in usb.c
From: Kalle Valo @ 2019-09-03 13:06 UTC (permalink / raw)
To: Valdis Klētnieks
Cc: Ping-Ke Shih, David S. Miller, linux-wireless, netdev,
linux-kernel
In-Reply-To: <34195.1565229118@turing-police>
"Valdis wrote:
> Fix spurious warning message when building with W=1:
>
> CC [M] drivers/net/wireless/realtek/rtlwifi/usb.o
> drivers/net/wireless/realtek/rtlwifi/usb.c:243: warning: Cannot understand * on line 243 - I thought it was a doc line
> drivers/net/wireless/realtek/rtlwifi/usb.c:760: warning: Cannot understand * on line 760 - I thought it was a doc line
> drivers/net/wireless/realtek/rtlwifi/usb.c:790: warning: Cannot understand * on line 790 - I thought it was a doc line
>
> Clean up the comment format.
>
> Signed-off-by: Valdis Kletnieks <valdis.kletnieks@vt.edu>
Patch applied to wireless-drivers-next.git, thanks.
b6326fc025aa rtlwifi: fix non-kerneldoc comment in usb.c
--
https://patchwork.kernel.org/patch/11083073/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* [PATCH 1/4] ethtool: implement Energy Detect Powerdown support via phy-tunable
From: Alexandru Ardelean @ 2019-09-03 16:06 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: andrew, f.fainelli, hkallweit1, davem, Alexandru Ardelean
In-Reply-To: <20190903160626.7518-1-alexandru.ardelean@analog.com>
The `phy_tunable_id` has been named `ETHTOOL_PHY_EDPD` since it looks like
this feature is common across other PHYs (like EEE), and defining
`ETHTOOL_PHY_ENERGY_DETECT_POWER_DOWN` seems too long.
The way EDPD works, is that the RX block is put to a lower power mode,
except for link-pulse detection circuits. The TX block is also put to low
power mode, but the PHY wakes-up periodically to send link pulses, to avoid
lock-ups in case the other side is also in EDPD mode.
Currently, there are 2 PHY drivers that look like they could use this new
PHY tunable feature: the `adin` && `micrel` PHYs.
The ADIN's datasheet mentions that TX pulses are at intervals of 1 second
default each, and they can be disabled. For the Micrel KSZ9031 PHY, the
datasheet does not mention whether they can be disabled, but mentions that
they can modified.
The way this change is structured, is similar to the PHY tunable downshift
control:
* a `ETHTOOL_PHY_EDPD_DFLT_TX_INTERVAL` value is exposed to cover a default
TX interval; some PHYs could specify a certain value that makes sense
* `ETHTOOL_PHY_EDPD_NO_TX` would disable TX when EDPD is enabled
* `ETHTOOL_PHY_EDPD_DISABLE` will disable EDPD
This should allow PHYs to:
* enable EDPD and not enable TX pulses (interval would be 0)
* enable EDPD and configure TX pulse interval; note that TX interval units
would be PHY specific; we could consider `seconds` as units, but it could
happen that some PHYs would be prefer 500 milliseconds as a unit;
a maximum of 32766 units should be sufficient
* disable EDPD
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
include/uapi/linux/ethtool.h | 5 +++++
net/core/ethtool.c | 6 ++++++
2 files changed, 11 insertions(+)
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index dd06302aa93e..0349e9c4350f 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -259,10 +259,15 @@ struct ethtool_tunable {
#define ETHTOOL_PHY_FAST_LINK_DOWN_ON 0
#define ETHTOOL_PHY_FAST_LINK_DOWN_OFF 0xff
+#define ETHTOOL_PHY_EDPD_DFLT_TX_INTERVAL 0x7fff
+#define ETHTOOL_PHY_EDPD_NO_TX 0x8000
+#define ETHTOOL_PHY_EDPD_DISABLE 0
+
enum phy_tunable_id {
ETHTOOL_PHY_ID_UNSPEC,
ETHTOOL_PHY_DOWNSHIFT,
ETHTOOL_PHY_FAST_LINK_DOWN,
+ ETHTOOL_PHY_EDPD,
/*
* Add your fresh new phy tunable attribute above and remember to update
* phy_tunable_strings[] in net/core/ethtool.c
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 6288e69e94fc..c763106c73fc 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -133,6 +133,7 @@ phy_tunable_strings[__ETHTOOL_PHY_TUNABLE_COUNT][ETH_GSTRING_LEN] = {
[ETHTOOL_ID_UNSPEC] = "Unspec",
[ETHTOOL_PHY_DOWNSHIFT] = "phy-downshift",
[ETHTOOL_PHY_FAST_LINK_DOWN] = "phy-fast-link-down",
+ [ETHTOOL_PHY_EDPD] = "phy-energy-detect-power-down",
};
static int ethtool_get_features(struct net_device *dev, void __user *useraddr)
@@ -2451,6 +2452,11 @@ static int ethtool_phy_tunable_valid(const struct ethtool_tunable *tuna)
tuna->type_id != ETHTOOL_TUNABLE_U8)
return -EINVAL;
break;
+ case ETHTOOL_PHY_EDPD:
+ if (tuna->len != sizeof(u16) ||
+ tuna->type_id != ETHTOOL_TUNABLE_U16)
+ return -EINVAL;
+ break;
default:
return -EINVAL;
}
--
2.20.1
^ permalink raw reply related
* [PATCH 0/4] ethtool: implement Energy Detect Powerdown support via phy-tunable
From: Alexandru Ardelean @ 2019-09-03 16:06 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: andrew, f.fainelli, hkallweit1, davem, Alexandru Ardelean
This patch series is actually 2 series in 1.
First 2 patches implement the kernel support for controlling Energy Detect
Powerdown support via phy-tunable, and the next 2 patches implement the
ethtool user-space control.
Hopefully, this combination of 2 series is an acceptable approach; if not,
I am fine to re-update it based on feedback.
The `phy_tunable_id` has been named `ETHTOOL_PHY_EDPD` since it looks like
this feature is common across other PHYs (like EEE), and defining
`ETHTOOL_PHY_ENERGY_DETECT_POWER_DOWN` seems too long.
The way EDPD works, is that the RX block is put to a lower power mode,
except for link-pulse detection circuits. The TX block is also put to low
power mode, but the PHY wakes-up periodically to send link pulses, to avoid
lock-ups in case the other side is also in EDPD mode.
Currently, there are 2 PHY drivers that look like they could use this new
PHY tunable feature: the `adin` && `micrel` PHYs.
This series updates only the `adin` PHY driver to support this new feature,
as this chip has been tested. A change for `micrel` can be proposed after a
discussion of the PHY-tunable API is resolved.
--
2.20.1
^ permalink raw reply
* [PATCH 3/4] [ethtool] ethtool: sync ethtool-copy.h: adds support for EDPD (3rd Sep 2019)
From: Alexandru Ardelean @ 2019-09-03 16:06 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: andrew, f.fainelli, hkallweit1, davem, Alexandru Ardelean
In-Reply-To: <20190903160626.7518-1-alexandru.ardelean@analog.com>
This change syncs the `ethtool-copy.h` file with Linux net-next to add
support for Energy Detect Powerdown control via phy tunable.
Some formatting also changes with this sync.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
ethtool-copy.h | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/ethtool-copy.h b/ethtool-copy.h
index ad16e8f..8b9a87d 100644
--- a/ethtool-copy.h
+++ b/ethtool-copy.h
@@ -257,10 +257,15 @@ struct ethtool_tunable {
#define ETHTOOL_PHY_FAST_LINK_DOWN_ON 0
#define ETHTOOL_PHY_FAST_LINK_DOWN_OFF 0xff
+#define ETHTOOL_PHY_EDPD_DFLT_TX_INTERVAL 0x7fff
+#define ETHTOOL_PHY_EDPD_NO_TX 0x8000
+#define ETHTOOL_PHY_EDPD_DISABLE 0
+
enum phy_tunable_id {
ETHTOOL_PHY_ID_UNSPEC,
ETHTOOL_PHY_DOWNSHIFT,
ETHTOOL_PHY_FAST_LINK_DOWN,
+ ETHTOOL_PHY_EDPD,
/*
* Add your fresh new phy tunable attribute above and remember to update
* phy_tunable_strings[] in net/core/ethtool.c
@@ -1481,8 +1486,8 @@ enum ethtool_link_mode_bit_indices {
ETHTOOL_LINK_MODE_200000baseLR4_ER4_FR4_Full_BIT = 64,
ETHTOOL_LINK_MODE_200000baseDR4_Full_BIT = 65,
ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT = 66,
- ETHTOOL_LINK_MODE_100baseT1_Full_BIT = 67,
- ETHTOOL_LINK_MODE_1000baseT1_Full_BIT = 68,
+ ETHTOOL_LINK_MODE_100baseT1_Full_BIT = 67,
+ ETHTOOL_LINK_MODE_1000baseT1_Full_BIT = 68,
/* must be last entry */
__ETHTOOL_LINK_MODE_MASK_NBITS
@@ -1712,8 +1717,8 @@ static __inline__ int ethtool_validate_duplex(__u8 duplex)
#define ETH_MODULE_SFF_8436 0x4
#define ETH_MODULE_SFF_8436_LEN 256
-#define ETH_MODULE_SFF_8636_MAX_LEN 640
-#define ETH_MODULE_SFF_8436_MAX_LEN 640
+#define ETH_MODULE_SFF_8636_MAX_LEN 640
+#define ETH_MODULE_SFF_8436_MAX_LEN 640
/* Reset flags */
/* The reset() operation must clear the flags for the components which
--
2.20.1
^ permalink raw reply related
* [PATCH 4/4] [ethtool] ethtool: implement support for Energy Detect Power Down
From: Alexandru Ardelean @ 2019-09-03 16:06 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: andrew, f.fainelli, hkallweit1, davem, Alexandru Ardelean
In-Reply-To: <20190903160626.7518-1-alexandru.ardelean@analog.com>
This change adds control for enabling/disabling Energy Detect Power Down
mode, as well as configuring wake-up intervals for TX pulses, via the new
ETHTOOL_PHY_EDPD control added in PHY tunable, in the kernel.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
ethtool.8.in | 28 +++++++++++++++++
ethtool.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++++---
2 files changed, 111 insertions(+), 4 deletions(-)
diff --git a/ethtool.8.in b/ethtool.8.in
index cd3be91..a32d48b 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -362,11 +362,17 @@ ethtool \- query or control network driver and hardware settings
.A1 on off
.BN msecs
.RB ]
+.RB [
+.B energy\-detect\-power\-down
+.A1 on off
+.BN tx-interval
+.RB ]
.HP
.B ethtool \-\-get\-phy\-tunable
.I devname
.RB [ downshift ]
.RB [ fast-link-down ]
+.RB [ energy-detect-power-down ]
.HP
.B ethtool \-\-reset
.I devname
@@ -1100,6 +1106,24 @@ lB l.
Sets the period after which the link is reported as down. Note that the PHY may choose
the closest supported value. Only on reading back the tunable do you get the actual value.
.TE
+.TP
+.A2 energy-detect-power-down on off
+Specifies whether Energy Detect Power Down (EDPD) should be enabled (if supported).
+This will put the RX and TX circuit blocks into a low power mode, and the PHY will
+wake up periodically to send link pulses to avoid any lock-up situation with a peer
+PHY that may also have EDPD enabled. By default, this setting will also enable the
+periodic transmission of TX pulses.
+.TS
+nokeep;
+lB l.
+.BI tx-interval \ N
+ Some PHYs support configuration of the wake-up interval to send TX pulses.
+ This setting allows the control of this interval, and 0 disables TX pulses
+ if the PHY supports this. Disabling TX pulses can create a lock-up situation
+ where neither of the PHYs wakes the other one. If the PHY supports only
+ a single interval, any non-zero value will enable this.
+.TE
+.TP
.PD
.RE
.TP
@@ -1122,6 +1146,10 @@ Some PHYs support a Fast Link Down Feature and may allow configuration of the de
before a broken link is reported as being down.
Gets the PHY Fast Link Down status / period.
+.TP
+.B energy\-detect\-power\-down
+Gets the current configured setting for Energy Detect Power Down (if supported).
+
.RE
.TP
.B \-\-reset
diff --git a/ethtool.c b/ethtool.c
index c0e2903..c0a18f8 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -4897,6 +4897,30 @@ static int do_get_phy_tunable(struct cmd_context *ctx)
else
fprintf(stdout, "Fast Link Down enabled, %d msecs\n",
cont.msecs);
+ } else if (!strcmp(argp[0], "energy-detect-power-down")) {
+ struct {
+ struct ethtool_tunable ds;
+ u16 tx_interval;
+ } cont;
+
+ cont.ds.cmd = ETHTOOL_PHY_GTUNABLE;
+ cont.ds.id = ETHTOOL_PHY_EDPD;
+ cont.ds.type_id = ETHTOOL_TUNABLE_U16;
+ cont.ds.len = 2;
+ if (send_ioctl(ctx, &cont.ds) < 0) {
+ perror("Cannot Get PHY Energy Detect Power Down value");
+ return 87;
+ }
+
+ if (cont.tx_interval == ETHTOOL_PHY_EDPD_DISABLE)
+ fprintf(stdout, "Energy Detect Power Down: disabled\n");
+ else if (cont.tx_interval == ETHTOOL_PHY_EDPD_NO_TX)
+ fprintf(stdout,
+ "Energy Detect Power Down: enabled, TX disabled\n");
+ else
+ fprintf(stdout,
+ "Energy Detect Power Down: enabled, TX %u intervals\n",
+ cont.tx_interval);
} else {
exit_bad_args();
}
@@ -5018,7 +5042,8 @@ static int parse_named_bool(struct cmd_context *ctx, const char *name, u8 *on)
return 1;
}
-static int parse_named_u8(struct cmd_context *ctx, const char *name, u8 *val)
+static int parse_named_uint(struct cmd_context *ctx, const char *name,
+ void *val, enum tunable_type_id type_id)
{
if (ctx->argc < 2)
return 0;
@@ -5026,7 +5051,16 @@ static int parse_named_u8(struct cmd_context *ctx, const char *name, u8 *val)
if (strcmp(*ctx->argp, name))
return 0;
- *val = get_uint_range(*(ctx->argp + 1), 0, 0xff);
+ switch (type_id) {
+ case ETHTOOL_TUNABLE_U8:
+ *(u8 *)val = get_uint_range(*(ctx->argp + 1), 0, 0xff);
+ break;
+ case ETHTOOL_TUNABLE_U16:
+ *(u16 *)val = get_uint_range(*(ctx->argp + 1), 0, 0xffff);
+ break;
+ default:
+ return 0;
+ }
ctx->argc -= 2;
ctx->argp += 2;
@@ -5034,6 +5068,16 @@ static int parse_named_u8(struct cmd_context *ctx, const char *name, u8 *val)
return 1;
}
+static int parse_named_u8(struct cmd_context *ctx, const char *name, u8 *val)
+{
+ return parse_named_uint(ctx, name, val, ETHTOOL_TUNABLE_U8);
+}
+
+static int parse_named_u16(struct cmd_context *ctx, const char *name, u16 *val)
+{
+ return parse_named_uint(ctx, name, val, ETHTOOL_TUNABLE_U16);
+}
+
static int do_set_phy_tunable(struct cmd_context *ctx)
{
int err = 0;
@@ -5041,6 +5085,8 @@ static int do_set_phy_tunable(struct cmd_context *ctx)
u8 ds_changed = 0, ds_has_cnt = 0, ds_enable = 0;
u8 fld_changed = 0, fld_enable = 0;
u8 fld_msecs = ETHTOOL_PHY_FAST_LINK_DOWN_ON;
+ u8 edpd_changed = 0, edpd_enable = 0;
+ u16 edpd_tx_interval = ETHTOOL_PHY_EDPD_DFLT_TX_INTERVAL;
/* Parse arguments */
if (parse_named_bool(ctx, "downshift", &ds_enable)) {
@@ -5050,6 +5096,11 @@ static int do_set_phy_tunable(struct cmd_context *ctx)
fld_changed = 1;
if (fld_enable)
parse_named_u8(ctx, "msecs", &fld_msecs);
+ } else if (parse_named_bool(ctx, "energy-detect-power-down",
+ &edpd_enable)) {
+ edpd_changed = 1;
+ if (edpd_enable)
+ parse_named_u16(ctx, "tx-interval", &edpd_tx_interval);
} else {
exit_bad_args();
}
@@ -5074,6 +5125,16 @@ static int do_set_phy_tunable(struct cmd_context *ctx)
fld_msecs = ETHTOOL_PHY_FAST_LINK_DOWN_OFF;
else if (fld_msecs == ETHTOOL_PHY_FAST_LINK_DOWN_OFF)
exit_bad_args();
+ } else if (edpd_changed) {
+ if (!edpd_enable)
+ edpd_tx_interval = ETHTOOL_PHY_EDPD_DISABLE;
+ else if (edpd_tx_interval == 0)
+ edpd_tx_interval = ETHTOOL_PHY_EDPD_NO_TX;
+ else if (edpd_tx_interval > ETHTOOL_PHY_EDPD_DFLT_TX_INTERVAL) {
+ fprintf(stderr, "'tx-interval' max value is %d.\n",
+ (ETHTOOL_PHY_EDPD_DFLT_TX_INTERVAL - 1));
+ exit_bad_args();
+ }
}
/* Do it */
@@ -5109,6 +5170,22 @@ static int do_set_phy_tunable(struct cmd_context *ctx)
perror("Cannot Set PHY Fast Link Down value");
err = 87;
}
+ } else if (edpd_changed) {
+ struct {
+ struct ethtool_tunable fld;
+ u16 tx_interval;
+ } cont;
+
+ cont.fld.cmd = ETHTOOL_PHY_STUNABLE;
+ cont.fld.id = ETHTOOL_PHY_EDPD;
+ cont.fld.type_id = ETHTOOL_TUNABLE_U16;
+ cont.fld.len = 2;
+ cont.tx_interval = edpd_tx_interval;
+ err = send_ioctl(ctx, &cont.fld);
+ if (err < 0) {
+ perror("Cannot Set PHY Energy Detect Power Down");
+ err = 87;
+ }
}
return err;
@@ -5361,10 +5438,12 @@ static const struct option {
" [ tx-timer %d ]\n"},
{ "--set-phy-tunable", 1, do_set_phy_tunable, "Set PHY tunable",
" [ downshift on|off [count N] ]\n"
- " [ fast-link-down on|off [msecs N] ]\n"},
+ " [ fast-link-down on|off [msecs N] ]\n"
+ " [ energy-detect-power-down on|off [tx-interval N] ]\n"},
{ "--get-phy-tunable", 1, do_get_phy_tunable, "Get PHY tunable",
" [ downshift ]\n"
- " [ fast-link-down ]\n"},
+ " [ fast-link-down ]\n"
+ " [ energy-detect-power-down ]\n"},
{ "--reset", 1, do_reset, "Reset components",
" [ flags %x ]\n"
" [ mgmt ]\n"
--
2.20.1
^ permalink raw reply related
* [PATCH 2/4] net: phy: adin: implement Energy Detect Powerdown mode via phy-tunable
From: Alexandru Ardelean @ 2019-09-03 16:06 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: andrew, f.fainelli, hkallweit1, davem, Alexandru Ardelean
In-Reply-To: <20190903160626.7518-1-alexandru.ardelean@analog.com>
This driver becomes the first user of the kernel's `ETHTOOL_PHY_EDPD`
phy-tunable feature.
EDPD is also enabled by default on PHY config_init, but can be disabled via
the phy-tunable control.
When enabling EDPD, it's also a good idea (for the ADIN PHYs) to enable TX
periodic pulses, so that in case the other PHY is also on EDPD mode, there
is no lock-up situation where both sides are waiting for the other to
transmit.
Via the phy-tunable control, TX pulses can be disabled if specifying 0
`tx-interval` via ethtool.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
drivers/net/phy/adin.c | 50 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)
diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index 4dec83df048d..742728ab2a5d 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -26,6 +26,11 @@
#define ADIN1300_RX_ERR_CNT 0x0014
+#define ADIN1300_PHY_CTRL_STATUS2 0x0015
+#define ADIN1300_NRG_PD_EN BIT(3)
+#define ADIN1300_NRG_PD_TX_EN BIT(2)
+#define ADIN1300_NRG_PD_STATUS BIT(1)
+
#define ADIN1300_PHY_CTRL2 0x0016
#define ADIN1300_DOWNSPEED_AN_100_EN BIT(11)
#define ADIN1300_DOWNSPEED_AN_10_EN BIT(10)
@@ -328,12 +333,51 @@ static int adin_set_downshift(struct phy_device *phydev, u8 cnt)
ADIN1300_DOWNSPEEDS_EN);
}
+static int adin_get_edpd(struct phy_device *phydev, u16 *tx_interval)
+{
+ int val;
+
+ val = phy_read(phydev, ADIN1300_PHY_CTRL_STATUS2);
+ if (val < 0)
+ return val;
+
+ if (ADIN1300_NRG_PD_EN & val) {
+ if (val & ADIN1300_NRG_PD_TX_EN)
+ *tx_interval = 1;
+ else
+ *tx_interval = ETHTOOL_PHY_EDPD_NO_TX;
+ } else {
+ *tx_interval = ETHTOOL_PHY_EDPD_DISABLE;
+ }
+
+ return 0;
+}
+
+static int adin_set_edpd(struct phy_device *phydev, u16 tx_interval)
+{
+ u16 val;
+
+ if (tx_interval == ETHTOOL_PHY_EDPD_DISABLE)
+ return phy_clear_bits(phydev, ADIN1300_PHY_CTRL_STATUS2,
+ (ADIN1300_NRG_PD_EN | ADIN1300_NRG_PD_TX_EN));
+
+ val = ADIN1300_NRG_PD_EN;
+ if (tx_interval != ETHTOOL_PHY_EDPD_NO_TX)
+ val |= ADIN1300_NRG_PD_TX_EN;
+
+ return phy_modify(phydev, ADIN1300_PHY_CTRL_STATUS2,
+ (ADIN1300_NRG_PD_EN | ADIN1300_NRG_PD_TX_EN),
+ val);
+}
+
static int adin_get_tunable(struct phy_device *phydev,
struct ethtool_tunable *tuna, void *data)
{
switch (tuna->id) {
case ETHTOOL_PHY_DOWNSHIFT:
return adin_get_downshift(phydev, data);
+ case ETHTOOL_PHY_EDPD:
+ return adin_get_edpd(phydev, data);
default:
return -EOPNOTSUPP;
}
@@ -345,6 +389,8 @@ static int adin_set_tunable(struct phy_device *phydev,
switch (tuna->id) {
case ETHTOOL_PHY_DOWNSHIFT:
return adin_set_downshift(phydev, *(const u8 *)data);
+ case ETHTOOL_PHY_EDPD:
+ return adin_set_edpd(phydev, *(const u16 *)data);
default:
return -EOPNOTSUPP;
}
@@ -368,6 +414,10 @@ static int adin_config_init(struct phy_device *phydev)
if (rc < 0)
return rc;
+ rc = adin_set_edpd(phydev, 1);
+ if (rc < 0)
+ return rc;
+
phydev_dbg(phydev, "PHY is using mode '%s'\n",
phy_modes(phydev->interface));
--
2.20.1
^ permalink raw reply related
* Re: [PATCH net] sctp: use transport pf_retrans in sctp_do_8_2_transport_strike
From: Neil Horman @ 2019-09-03 13:15 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, linux-sctp, davem, Marcelo Ricardo Leitner
In-Reply-To: <41769d6033d27d629798e060671a3b21f22e2a21.1567437861.git.lucien.xin@gmail.com>
On Mon, Sep 02, 2019 at 11:24:21PM +0800, Xin Long wrote:
> Transport should use its own pf_retrans to do the error_count
> check, instead of asoc's. Otherwise, it's meaningless to make
> pf_retrans per transport.
>
> Fixes: 5aa93bcf66f4 ("sctp: Implement quick failover draft from tsvwg")
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
> net/sctp/sm_sideeffect.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
> index 1cf5bb5..e52b212 100644
> --- a/net/sctp/sm_sideeffect.c
> +++ b/net/sctp/sm_sideeffect.c
> @@ -547,7 +547,7 @@ static void sctp_do_8_2_transport_strike(struct sctp_cmd_seq *commands,
> if (net->sctp.pf_enable &&
> (transport->state == SCTP_ACTIVE) &&
> (transport->error_count < transport->pathmaxrxt) &&
> - (transport->error_count > asoc->pf_retrans)) {
> + (transport->error_count > transport->pf_retrans)) {
>
> sctp_assoc_control_transport(asoc, transport,
> SCTP_TRANSPORT_PF,
> --
> 2.1.0
>
>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox