* [PATCH 5/8] net: ixp4xx: Spelling s/XSacle/XScale/
From: Geert Uytterhoeven @ 2019-07-31 13:22 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev, linux-kernel, Geert Uytterhoeven
In-Reply-To: <20190731132216.17194-1-geert+renesas@glider.be>
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
drivers/net/ethernet/xscale/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/xscale/Kconfig b/drivers/net/ethernet/xscale/Kconfig
index 2f354ba029a61491..cd0a8f46e7c6c143 100644
--- a/drivers/net/ethernet/xscale/Kconfig
+++ b/drivers/net/ethernet/xscale/Kconfig
@@ -13,7 +13,7 @@ config NET_VENDOR_XSCALE
Note that the answer to this question does not directly affect the
kernel: saying N will just cause the configurator to skip all
- the questions about XSacle IXP devices. If you say Y, you will be
+ the questions about XScale IXP devices. If you say Y, you will be
asked for your specific card in the following questions.
if NET_VENDOR_XSCALE
--
2.17.1
^ permalink raw reply related
* [PATCH 3/8] net: apple: Fix manufacturer name in Kconfig help text
From: Geert Uytterhoeven @ 2019-07-31 13:22 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev, linux-kernel, Geert Uytterhoeven
In-Reply-To: <20190731132216.17194-1-geert+renesas@glider.be>
The help text refers to IBM instead of Apple, presumably because it was
copied from the former.
Fixes: 8fb6b0908176704a ("bmac/mace/macmace/mac89x0/cs89x0: Move the Macintosh (Apple) drivers")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
drivers/net/ethernet/apple/Kconfig | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/apple/Kconfig b/drivers/net/ethernet/apple/Kconfig
index fde7ae33e302b6bc..f78b9c841296c94b 100644
--- a/drivers/net/ethernet/apple/Kconfig
+++ b/drivers/net/ethernet/apple/Kconfig
@@ -11,8 +11,8 @@ config NET_VENDOR_APPLE
If you have a network (Ethernet) card belonging to this class, say Y.
Note that the answer to this question doesn't directly affect the
- kernel: saying N will just cause the configurator to skip all
- the questions about IBM devices. If you say Y, you will be asked for
+ kernel: saying N will just cause the configurator to skip all the
+ questions about Apple devices. If you say Y, you will be asked for
your specific card in the following questions.
if NET_VENDOR_APPLE
--
2.17.1
^ permalink raw reply related
* [PATCH 1/8] net: 8390: Fix manufacturer name in Kconfig help text
From: Geert Uytterhoeven @ 2019-07-31 13:22 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev, linux-kernel, Geert Uytterhoeven
In-Reply-To: <20190731132216.17194-1-geert+renesas@glider.be>
The help text refers to Western Digital instead of National
Semiconductor 8390, presumably because it was copied from the former.
Fixes: 644570b830266ff3 ("8390: Move the 8390 related drivers")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
drivers/net/ethernet/8390/Kconfig | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/8390/Kconfig b/drivers/net/ethernet/8390/Kconfig
index 2a3e2450968eeb06..a9478577b49560f2 100644
--- a/drivers/net/ethernet/8390/Kconfig
+++ b/drivers/net/ethernet/8390/Kconfig
@@ -12,8 +12,8 @@ config NET_VENDOR_8390
Note that the answer to this question doesn't directly affect the
kernel: saying N will just cause the configurator to skip all
- the questions about Western Digital cards. If you say Y, you will be
- asked for your specific card in the following questions.
+ the questions about National Semiconductor 8390 cards. If you say Y,
+ you will be asked for your specific card in the following questions.
if NET_VENDOR_8390
--
2.17.1
^ permalink raw reply related
* Re: [PATCH V2 7/9] vhost: do not use RCU to synchronize MMU notifier with worker
From: Jason Wang @ 2019-07-31 13:28 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: mst, kvm, virtualization, netdev, linux-kernel, linux-mm
In-Reply-To: <20190731123935.GC3946@ziepe.ca>
On 2019/7/31 下午8:39, Jason Gunthorpe wrote:
> On Wed, Jul 31, 2019 at 04:46:53AM -0400, Jason Wang wrote:
>> We used to use RCU to synchronize MMU notifier with worker. This leads
>> calling synchronize_rcu() in invalidate_range_start(). But on a busy
>> system, there would be many factors that may slow down the
>> synchronize_rcu() which makes it unsuitable to be called in MMU
>> notifier.
>>
>> A solution is SRCU but its overhead is obvious with the expensive full
>> memory barrier. Another choice is to use seqlock, but it doesn't
>> provide a synchronization method between readers and writers. The last
>> choice is to use vq mutex, but it need to deal with the worst case
>> that MMU notifier must be blocked and wait for the finish of swap in.
>>
>> So this patch switches use a counter to track whether or not the map
>> was used. The counter was increased when vq try to start or finish
>> uses the map. This means, when it was even, we're sure there's no
>> readers and MMU notifier is synchronized. When it was odd, it means
>> there's a reader we need to wait it to be even again then we are
>> synchronized.
> You just described a seqlock.
Kind of, see my explanation below.
>
> We've been talking about providing this as some core service from mmu
> notifiers because nearly every use of this API needs it.
That would be very helpful.
>
> IMHO this gets the whole thing backwards, the common pattern is to
> protect the 'shadow pte' data with a seqlock (usually open coded),
> such that the mmu notififer side has the write side of that lock and
> the read side is consumed by the thread accessing or updating the SPTE.
Yes, I've considered something like that. But the problem is, mmu
notifier (writer) need to wait for the vhost worker to finish the read
before it can do things like setting dirty pages and unmapping page. It
looks to me seqlock doesn't provide things like this. Or are you
suggesting that taking writer seq lock in vhost worker and busy wait for
seqcount to be even in MMU notifier (something similar to what this
patch did)? I don't do this because e.g:
write_seqcount_begin()
map = vq->map[X]
write or read through map->addr directly
write_seqcount_end()
There's no rmb() in write_seqcount_begin(), so map could be read before
write_seqcount_begin(), but it looks to me now that this doesn't harm at
all, maybe we can try this way.
>
>
>> Reported-by: Michael S. Tsirkin <mst@redhat.com>
>> Fixes: 7f466032dc9e ("vhost: access vq metadata through kernel virtual address")
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> drivers/vhost/vhost.c | 145 ++++++++++++++++++++++++++----------------
>> drivers/vhost/vhost.h | 7 +-
>> 2 files changed, 94 insertions(+), 58 deletions(-)
>>
>> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>> index cfc11f9ed9c9..db2c81cb1e90 100644
>> +++ b/drivers/vhost/vhost.c
>> @@ -324,17 +324,16 @@ static void vhost_uninit_vq_maps(struct vhost_virtqueue *vq)
>>
>> spin_lock(&vq->mmu_lock);
>> for (i = 0; i < VHOST_NUM_ADDRS; i++) {
>> - map[i] = rcu_dereference_protected(vq->maps[i],
>> - lockdep_is_held(&vq->mmu_lock));
>> + map[i] = vq->maps[i];
>> if (map[i]) {
>> vhost_set_map_dirty(vq, map[i], i);
>> - rcu_assign_pointer(vq->maps[i], NULL);
>> + vq->maps[i] = NULL;
>> }
>> }
>> spin_unlock(&vq->mmu_lock);
>>
>> - /* No need for synchronize_rcu() or kfree_rcu() since we are
>> - * serialized with memory accessors (e.g vq mutex held).
>> + /* No need for synchronization since we are serialized with
>> + * memory accessors (e.g vq mutex held).
>> */
>>
>> for (i = 0; i < VHOST_NUM_ADDRS; i++)
>> @@ -362,6 +361,44 @@ static bool vhost_map_range_overlap(struct vhost_uaddr *uaddr,
>> return !(end < uaddr->uaddr || start > uaddr->uaddr - 1 + uaddr->size);
>> }
>>
>> +static void inline vhost_vq_access_map_begin(struct vhost_virtqueue *vq)
>> +{
>> + int ref = READ_ONCE(vq->ref);
> Is a lock/single threaded supposed to be held for this?
Yes, only vhost worker kthread can accept this.
>
>> +
>> + smp_store_release(&vq->ref, ref + 1);
>> + /* Make sure ref counter is visible before accessing the map */
>> + smp_load_acquire(&vq->ref);
> release/acquire semantics are intended to protect blocks of related
> data, so reading something with acquire and throwing away the result
> is nonsense.
Actually I want to use smp_mb() here, so I admit it's a trick that even
won't work. But now I think I can just use write_seqcount_begin() here.
>
>> +}
>> +
>> +static void inline vhost_vq_access_map_end(struct vhost_virtqueue *vq)
>> +{
>> + int ref = READ_ONCE(vq->ref);
> If the write to vq->ref is not locked this algorithm won't work, if it
> is locked the READ_ONCE is not needed.
Yes.
>
>> + /* Make sure vq access is done before increasing ref counter */
>> + smp_store_release(&vq->ref, ref + 1);
>> +}
>> +
>> +static void inline vhost_vq_sync_access(struct vhost_virtqueue *vq)
>> +{
>> + int ref;
>> +
>> + /* Make sure map change was done before checking ref counter */
>> + smp_mb();
> This is probably smp_rmb after reading ref, and if you are setting ref
> with smp_store_release then this should be smp_load_acquire() without
> an explicit mb.
We had something like:
spin_lock();
vq->maps[index] = NULL;
spin_unlock();
vhost_vq_sync_access(vq);
we need to make sure the read of ref is done after setting
vq->maps[index] to NULL. It looks to me neither smp_load_acquire() nor
smp_store_release() can help in this case.
>
>> + ref = READ_ONCE(vq->ref);
>> + if (ref & 0x1) {
>> + /* When ref change, we are sure no reader can see
>> + * previous map */
>> + while (READ_ONCE(vq->ref) == ref) {
>> + set_current_state(TASK_RUNNING);
>> + schedule();
>> + }
>> + }
> This is basically read_seqcount_begin()' with a schedule instead of
> cpu_relax
Yes it is.
>
>
>> + /* Make sure ref counter was checked before any other
>> + * operations that was dene on map. */
>> + smp_mb();
> should be in a smp_load_acquire()
Right, if we use smp_load_acquire() to load the counter.
>
>> +}
>> +
>> static void vhost_invalidate_vq_start(struct vhost_virtqueue *vq,
>> int index,
>> unsigned long start,
>> @@ -376,16 +413,15 @@ static void vhost_invalidate_vq_start(struct vhost_virtqueue *vq,
>> spin_lock(&vq->mmu_lock);
>> ++vq->invalidate_count;
>>
>> - map = rcu_dereference_protected(vq->maps[index],
>> - lockdep_is_held(&vq->mmu_lock));
>> + map = vq->maps[index];
>> if (map) {
>> vhost_set_map_dirty(vq, map, index);
>> - rcu_assign_pointer(vq->maps[index], NULL);
>> + vq->maps[index] = NULL;
>> }
>> spin_unlock(&vq->mmu_lock);
>>
>> if (map) {
>> - synchronize_rcu();
>> + vhost_vq_sync_access(vq);
> What prevents racing with vhost_vq_access_map_end here?
vhost_vq_access_map_end() uses smp_store_release() for the counter. Is
this not sufficient?
>
>> vhost_map_unprefetch(map);
>> }
>> }
> Overall I don't like it.
>
> We are trying to get rid of these botique mmu notifier patterns in
> drivers.
I agree, so do you think we can take write lock in vhost worker then
wait for the counter to be even in MMU notifier? It looks much cleaner
than this patch.
Thanks
>
> Jason
^ permalink raw reply
* Re: [PATCH V2 4/9] vhost: reset invalidate_count in vhost_set_vring_num_addr()
From: Jason Wang @ 2019-07-31 13:29 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: mst, kvm, virtualization, netdev, linux-kernel, linux-mm
In-Reply-To: <20190731124124.GD3946@ziepe.ca>
On 2019/7/31 下午8:41, Jason Gunthorpe wrote:
> On Wed, Jul 31, 2019 at 04:46:50AM -0400, Jason Wang wrote:
>> The vhost_set_vring_num_addr() could be called in the middle of
>> invalidate_range_start() and invalidate_range_end(). If we don't reset
>> invalidate_count after the un-registering of MMU notifier, the
>> invalidate_cont will run out of sync (e.g never reach zero). This will
>> in fact disable the fast accessor path. Fixing by reset the count to
>> zero.
>>
>> Reported-by: Michael S. Tsirkin <mst@redhat.com>
> Did Michael report this as well?
Correct me if I was wrong. I think it's point 4 described in
https://lkml.org/lkml/2019/7/21/25.
Thanks
>
>> Fixes: 7f466032dc9e ("vhost: access vq metadata through kernel virtual address")
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> drivers/vhost/vhost.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>> index 2a3154976277..2a7217c33668 100644
>> +++ b/drivers/vhost/vhost.c
>> @@ -2073,6 +2073,10 @@ static long vhost_vring_set_num_addr(struct vhost_dev *d,
>> d->has_notifier = false;
>> }
>>
>> + /* reset invalidate_count in case we are in the middle of
>> + * invalidate_start() and invalidate_end().
>> + */
>> + vq->invalidate_count = 0;
>> vhost_uninit_vq_maps(vq);
>> #endif
>>
^ permalink raw reply
* Re: [PATCH] net: bridge: Allow bridge to joing multicast groups
From: Andrew Lunn @ 2019-07-31 13:45 UTC (permalink / raw)
To: Allan W. Nielsen
Cc: Ido Schimmel, Nikolay Aleksandrov, Horatiu Vultur, roopa, davem,
bridge, netdev, linux-kernel
In-Reply-To: <20190731080149.oyqcrw42utxjizsx@lx-anielsen.microsemi.net>
> > 2) The interface is part of a bridge. There are a few sub-cases
> >
> > a) IGMP snooping is being performed. We can block multicast where
> > there is no interest in the group. But this is limited to IP
> > multicast.
> Agree. And this is done today by installing an explicit offload rule to limit
> the flooding of a specific group.
>
> > b) IGMP snooping is not being used and all interfaces in the bridge
> > are ports of the switch. IP Multicast can be blocked to the CPU.
> Does it matter if IGMP snooping is used or not? A more general statement could
> be:
>
> - "All interfaces in the bridge are ports of the switch. IP Multicast can be
> blocked to the CPU."
We have seen IPv6 neighbour discovery break in conditions like this. I
don't know the exact details.
You also need to watch out for 224.0.0.0/24. This is the link local
multicast range. There is no need to join MC addresses in that
range. It is assumed they will always be received. So even if IGMP is
enabled, you still need to pass some multicast traffic to the CPU.
> > So one possibility here is to teach the SW bridge about non-IP
> > multicast addresses. Initially the switch should forward all MAC
> > multicast frames to the CPU. If the frame is not an IPv4 or IPv6
> > frame, and there has not been a call to set_rx_mode() for the MAC
> > address on the br0 interface, and the bridge only contains switch
> > ports, switchdev could be used to block the multicast to the CPU
> > frame, but forward it out all other ports of the bridge.
> Close, but not exactly (due to the arguments above).
>
> Here is how I see it:
>
> Teach the SW bridge about non-IP multicast addresses. Initially the switch
> should forward all MAC multicast frames to the CPU. Today MDB rules can be
> installed (either static or dynamic by IGMP), which limit the flooding of IPv4/6
> multicast streams. In the same way, we should have a way to install a rule
> (FDM/ or MDB) to limit the flooding of L2 multicast frames.
>
> If foreign interfaces (or br0 it self) is part of the destination list, then
> traffic also needs to go to the CPU.
>
> By doing this, we can for explicitly configured dst mac address:
> - limit the flooding on the on the SW bridge interfaces
> - limit the flooding on the on the HW bridge interfaces
> - prevent them to go to the CPU if they are not needed
This is all very complex because of all the different corner cases. So
i don't think we want a user API to do the CPU part, we want the
network stack to do it. Otherwise the user is going to get is wrong,
break their network, and then come running to the list for help.
This also fits with how we do things in DSA. There is deliberately no
user space concept for configuring the DSA CPU port. To user space,
the switch is just a bunch of Linux interfaces. Everything to do with
the CPU port is hidden away in the DSA core layer, the DSA drivers,
and a little bit in the bridge.
Andrew
^ permalink raw reply
* [PATCH 1/2] cxgb4: sched: Use refcount_t for refcount
From: Chuhong Yuan @ 2019-07-31 13:55 UTC (permalink / raw)
Cc: Vishal Kulkarni, David S . Miller, netdev, linux-kernel,
Chuhong Yuan
refcount_t is better for reference counters since its
implementation can prevent overflows.
So convert atomic_t ref counters to refcount_t.
Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
---
drivers/net/ethernet/chelsio/cxgb4/sched.c | 8 ++++----
drivers/net/ethernet/chelsio/cxgb4/sched.h | 2 +-
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/sched.c b/drivers/net/ethernet/chelsio/cxgb4/sched.c
index 60218dc676a8..2d04ffb31528 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/sched.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/sched.c
@@ -173,7 +173,7 @@ static int t4_sched_queue_unbind(struct port_info *pi, struct ch_sched_queue *p)
list_del(&qe->list);
kvfree(qe);
- if (atomic_dec_and_test(&e->refcnt)) {
+ if (refcount_dec_and_test(&e->refcnt)) {
e->state = SCHED_STATE_UNUSED;
memset(&e->info, 0, sizeof(e->info));
}
@@ -216,7 +216,7 @@ static int t4_sched_queue_bind(struct port_info *pi, struct ch_sched_queue *p)
goto out_err;
list_add_tail(&qe->list, &e->queue_list);
- atomic_inc(&e->refcnt);
+ refcount_inc(&e->refcnt);
return err;
out_err:
@@ -434,7 +434,7 @@ static struct sched_class *t4_sched_class_alloc(struct port_info *pi,
if (err)
return NULL;
memcpy(&e->info, &np, sizeof(e->info));
- atomic_set(&e->refcnt, 0);
+ refcount_set(&e->refcnt, 0);
e->state = SCHED_STATE_ACTIVE;
}
@@ -488,7 +488,7 @@ struct sched_table *t4_init_sched(unsigned int sched_size)
s->tab[i].idx = i;
s->tab[i].state = SCHED_STATE_UNUSED;
INIT_LIST_HEAD(&s->tab[i].queue_list);
- atomic_set(&s->tab[i].refcnt, 0);
+ refcount_set(&s->tab[i].refcnt, 0);
}
return s;
}
diff --git a/drivers/net/ethernet/chelsio/cxgb4/sched.h b/drivers/net/ethernet/chelsio/cxgb4/sched.h
index 168fb4ce3759..23a6ca1e6d3e 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/sched.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/sched.h
@@ -69,7 +69,7 @@ struct sched_class {
u8 idx;
struct ch_sched_params info;
struct list_head queue_list;
- atomic_t refcnt;
+ refcount_t refcnt;
};
struct sched_table { /* per port scheduling table */
--
2.20.1
^ permalink raw reply related
* [PATCH 2/2] cxgb4: smt: Use refcount_t for refcount
From: Chuhong Yuan @ 2019-07-31 13:55 UTC (permalink / raw)
Cc: Vishal Kulkarni, David S . Miller, netdev, linux-kernel,
Chuhong Yuan
refcount_t is better for reference counters since its
implementation can prevent overflows.
So convert atomic_t ref counters to refcount_t.
Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
---
drivers/net/ethernet/chelsio/cxgb4/smt.c | 14 +++++++-------
drivers/net/ethernet/chelsio/cxgb4/smt.h | 2 +-
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/smt.c b/drivers/net/ethernet/chelsio/cxgb4/smt.c
index eaf1fb74689c..b008d522bef6 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/smt.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/smt.c
@@ -57,7 +57,7 @@ struct smt_data *t4_init_smt(void)
s->smtab[i].state = SMT_STATE_UNUSED;
memset(&s->smtab[i].src_mac, 0, ETH_ALEN);
spin_lock_init(&s->smtab[i].lock);
- atomic_set(&s->smtab[i].refcnt, 0);
+ refcount_set(&s->smtab[i].refcnt, 0);
}
return s;
}
@@ -68,7 +68,7 @@ static struct smt_entry *find_or_alloc_smte(struct smt_data *s, u8 *smac)
struct smt_entry *e, *end;
for (e = &s->smtab[0], end = &s->smtab[s->smt_size]; e != end; ++e) {
- if (atomic_read(&e->refcnt) == 0) {
+ if (refcount_read(&e->refcnt) == 0) {
if (!first_free)
first_free = e;
} else {
@@ -98,7 +98,7 @@ static struct smt_entry *find_or_alloc_smte(struct smt_data *s, u8 *smac)
static void t4_smte_free(struct smt_entry *e)
{
spin_lock_bh(&e->lock);
- if (atomic_read(&e->refcnt) == 0) { /* hasn't been recycled */
+ if (refcount_read(&e->refcnt) == 0) { /* hasn't been recycled */
e->state = SMT_STATE_UNUSED;
}
spin_unlock_bh(&e->lock);
@@ -111,7 +111,7 @@ static void t4_smte_free(struct smt_entry *e)
*/
void cxgb4_smt_release(struct smt_entry *e)
{
- if (atomic_dec_and_test(&e->refcnt))
+ if (refcount_dec_and_test(&e->refcnt))
t4_smte_free(e);
}
EXPORT_SYMBOL(cxgb4_smt_release);
@@ -215,14 +215,14 @@ static struct smt_entry *t4_smt_alloc_switching(struct adapter *adap, u16 pfvf,
e = find_or_alloc_smte(s, smac);
if (e) {
spin_lock(&e->lock);
- if (!atomic_read(&e->refcnt)) {
- atomic_set(&e->refcnt, 1);
+ if (!refcount_read(&e->refcnt)) {
+ refcount_set(&e->refcnt, 1);
e->state = SMT_STATE_SWITCHING;
e->pfvf = pfvf;
memcpy(e->src_mac, smac, ETH_ALEN);
write_smt_entry(adap, e);
} else {
- atomic_inc(&e->refcnt);
+ refcount_inc(&e->refcnt);
}
spin_unlock(&e->lock);
}
diff --git a/drivers/net/ethernet/chelsio/cxgb4/smt.h b/drivers/net/ethernet/chelsio/cxgb4/smt.h
index d6c2cc271398..4774606a0101 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/smt.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/smt.h
@@ -59,7 +59,7 @@ struct smt_entry {
u16 idx;
u16 pfvf;
u8 src_mac[ETH_ALEN];
- atomic_t refcnt;
+ refcount_t refcnt;
spinlock_t lock; /* protect smt entry add,removal */
};
--
2.20.1
^ permalink raw reply related
* Re: [PATCH net-next] net/tls: prevent skb_orphan() from leaking TLS plain text with offload
From: Boris Pismenny @ 2019-07-31 13:57 UTC (permalink / raw)
To: Jakub Kicinski, davem@davemloft.net
Cc: netdev@vger.kernel.org, oss-drivers@netronome.com,
edumazet@google.com, Aviad Yehezkel, davejwatson@fb.com,
john.fastabend@gmail.com, daniel@iogearbox.net,
willemb@google.com, xiyou.wangcong@gmail.com, fw@strlen.de,
alexei.starovoitov@gmail.com
In-Reply-To: <20190730211258.13748-1-jakub.kicinski@netronome.com>
Hi Jacub,
On 7/31/2019 12:12 AM, Jakub Kicinski wrote:
> sk_validate_xmit_skb() and drivers depend on the sk member of
> struct sk_buff to identify segments requiring encryption.
> Any operation which removes or does not preserve the original TLS
> socket such as skb_orphan() or skb_clone() will cause clear text
> leaks.
>
> Make the TCP socket underlying an offloaded TLS connection
> mark all skbs as decrypted, if TLS TX is in offload mode.
> Then in sk_validate_xmit_skb() catch skbs which have no socket
> (or a socket with no validation) and decrypted flag set.
>
> Note that CONFIG_SOCK_VALIDATE_XMIT, CONFIG_TLS_DEVICE and
> sk->sk_validate_xmit_skb are slightly interchangeable right now,
> they all imply TLS offload. The new checks are guarded by
> CONFIG_TLS_DEVICE because that's the option guarding the
> sk_buff->decrypted member.
>
> Second, smaller issue with orphaning is that it breaks
> the guarantee that packets will be delivered to device
> queues in-order. All TLS offload drivers depend on that
> scheduling property. This means skb_orphan_partial()'s
> trick of preserving partial socket references will cause
> issues in the drivers. We need a full orphan, and as a
> result netem delay/throttling will cause all TLS offload
> skbs to be dropped.
>
> Reusing the sk_buff->decrypted flag also protects from
> leaking clear text when incoming, decrypted skb is redirected
> (e.g. by TC).
Nice!
>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> ---
> I'm sending this for net-next because of lack of confidence
> in my own abilities. It should apply cleanly to net... :)
>
> Documentation/networking/tls-offload.rst | 9 --------
> include/net/sock.h | 28 +++++++++++++++++++++++-
> net/core/skbuff.c | 3 +++
> net/core/sock.c | 22 ++++++++++++++-----
> net/ipv4/tcp.c | 4 +++-
> net/ipv4/tcp_output.c | 3 +++
> net/tls/tls_device.c | 2 ++
> 7 files changed, 55 insertions(+), 16 deletions(-)
>
> diff --git a/Documentation/networking/tls-offload.rst b/Documentation/networking/tls-offload.rst
> index 048e5ca44824..2bc3ab5515d8 100644
> --- a/Documentation/networking/tls-offload.rst
> +++ b/Documentation/networking/tls-offload.rst
> @@ -499,15 +499,6 @@ offloads, old connections will remain active after flags are cleared.
> Known bugs
> ==========
>
> -skb_orphan() leaks clear text
> ------------------------------
> -
> -Currently drivers depend on the :c:member:`sk` member of
> -:c:type:`struct sk_buff <sk_buff>` to identify segments requiring
> -encryption. Any operation which removes or does not preserve the socket
> -association such as :c:func:`skb_orphan` or :c:func:`skb_clone`
> -will cause the driver to miss the packets and lead to clear text leaks.
> -
> Redirects leak clear text
> -------------------------
Doesn't this patch cover the redirect case as well?
>
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 228db3998e46..90f3f552c789 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -814,6 +814,7 @@ enum sock_flags {
> SOCK_TXTIME,
> SOCK_XDP, /* XDP is attached */
> SOCK_TSTAMP_NEW, /* Indicates 64 bit timestamps always */
> + SOCK_CRYPTO_TX_PLAIN_TEXT, /* Generate skbs with decrypted flag set */
> };
>
> #define SK_FLAGS_TIMESTAMP ((1UL << SOCK_TIMESTAMP) | (1UL << SOCK_TIMESTAMPING_RX_SOFTWARE))
> @@ -2480,8 +2481,26 @@ static inline bool sk_fullsock(const struct sock *sk)
> return (1 << sk->sk_state) & ~(TCPF_TIME_WAIT | TCPF_NEW_SYN_RECV);
> }
>
> +static inline void sk_set_tx_crypto(const struct sock *sk, struct sk_buff *skb)
> +{
> +#ifdef CONFIG_TLS_DEVICE
> + skb->decrypted = sock_flag(sk, SOCK_CRYPTO_TX_PLAIN_TEXT);
> +#endif
> +}
Have you considered the following alternative to calling
sk_set_tx_crypto() - Add a new MSG_DECRYPTE flag that will set the
skb->decrypted bit in the do_tcp_sendpages function.
Then, the rest of the TCP code can propagate this bit from the existing skb.
> +
> +static inline bool sk_tx_crypto_match(const struct sock *sk,
> + const struct sk_buff *skb)
> +{
> +#ifdef CONFIG_TLS_DEVICE
> + return sock_flag(sk, SOCK_CRYPTO_TX_PLAIN_TEXT) == !!skb->decrypted;
> +#else
> + return true;
> +#endif
> +}
> +
> /* Checks if this SKB belongs to an HW offloaded socket
> * and whether any SW fallbacks are required based on dev.
> + * Check decrypted mark in case skb_orphan() cleared socket.
> */
> static inline struct sk_buff *sk_validate_xmit_skb(struct sk_buff *skb,
> struct net_device *dev)
> @@ -2489,8 +2508,15 @@ static inline struct sk_buff *sk_validate_xmit_skb(struct sk_buff *skb,
> #ifdef CONFIG_SOCK_VALIDATE_XMIT
> struct sock *sk = skb->sk;
>
> - if (sk && sk_fullsock(sk) && sk->sk_validate_xmit_skb)
> + if (sk && sk_fullsock(sk) && sk->sk_validate_xmit_skb) {
> skb = sk->sk_validate_xmit_skb(sk, dev, skb);
> +#ifdef CONFIG_TLS_DEVICE
> + } else if (unlikely(skb->decrypted)) {
> + pr_warn_ratelimited("unencrypted skb with no associated socket - dropping\n");
> + kfree_skb(skb);
> + skb = NULL;
> +#endif
> + }
> #endif
>
> return skb;
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 0b788df5a75b..9e92684479b9 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -3794,6 +3794,9 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
>
> skb_reserve(nskb, headroom);
> __skb_put(nskb, doffset);
> +#ifdef CONFIG_TLS_DEVICE
> + nskb->decrypted = head_skb->decrypted;
> +#endif
> }
>
> if (segs)
> diff --git a/net/core/sock.c b/net/core/sock.c
> index d57b0cc995a0..b0c10b518e65 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -1992,6 +1992,22 @@ void skb_set_owner_w(struct sk_buff *skb, struct sock *sk)
> }
> EXPORT_SYMBOL(skb_set_owner_w);
>
> +static bool can_skb_orphan_partial(const struct sk_buff *skb)
> +{
> +#ifdef CONFIG_TLS_DEVICE
> + /* Drivers depend on in-order delivery for crypto offload,
> + * partial orphan breaks out-of-order-OK logic.
> + */
> + if (skb->decrypted)
> + return false;
> +#endif
> +#ifdef CONFIG_INET
> + if (skb->destructor == tcp_wfree)
> + return true;
> +#endif
> + return skb->destructor == sock_wfree;
> +}
> +
> /* This helper is used by netem, as it can hold packets in its
> * delay queue. We want to allow the owner socket to send more
> * packets, as if they were already TX completed by a typical driver.
> @@ -2003,11 +2019,7 @@ void skb_orphan_partial(struct sk_buff *skb)
> if (skb_is_tcp_pure_ack(skb))
> return;
>
> - if (skb->destructor == sock_wfree
> -#ifdef CONFIG_INET
> - || skb->destructor == tcp_wfree
> -#endif
> - ) {
> + if (can_skb_orphan_partial(skb)) {
> struct sock *sk = skb->sk;
>
> if (refcount_inc_not_zero(&sk->sk_refcnt)) {
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index f62f0e7e3cdd..dee93eab02fe 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -984,6 +984,7 @@ ssize_t do_tcp_sendpages(struct sock *sk, struct page *page, int offset,
> if (!skb)
> goto wait_for_memory;
>
> + sk_set_tx_crypto(sk, skb);
> skb_entail(sk, skb);
> copy = size_goal;
> }
> @@ -993,7 +994,8 @@ ssize_t do_tcp_sendpages(struct sock *sk, struct page *page, int offset,
>
> i = skb_shinfo(skb)->nr_frags;
> can_coalesce = skb_can_coalesce(skb, i, page, offset);
> - if (!can_coalesce && i >= sysctl_max_skb_frags) {
> + if ((!can_coalesce && i >= sysctl_max_skb_frags) ||
> + !sk_tx_crypto_match(sk, skb)) {
I see that sk_tx_crypto_match is called only here to handle cases where
the socket expected crypto offload, while the skb is not marked
decrypted. AFAIU, this should not be possible, because we set the
skb->eor bit for the last plaintext skb before sending any traffic that
expects crypto offload.
^ permalink raw reply
* Re: KASAN: use-after-free Read in release_sock
From: syzbot @ 2019-07-31 13:58 UTC (permalink / raw)
To: davem, linux-hams, linux-kernel, netdev, ralf, syzkaller-bugs,
xiyou.wangcong
In-Reply-To: <000000000000de000a058e9025db@google.com>
syzbot has bisected this bug to:
commit c8c8218ec5af5d2598381883acbefbf604e56b5e
Author: Cong Wang <xiyou.wangcong@gmail.com>
Date: Thu Jun 27 21:30:58 2019 +0000
netrom: fix a memory leak in nr_rx_frame()
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=13b52242600000
start commit: 629f8205 Merge tag 'for-linus-20190730' of git://git.kerne..
git tree: upstream
final crash: https://syzkaller.appspot.com/x/report.txt?x=10752242600000
console output: https://syzkaller.appspot.com/x/log.txt?x=17b52242600000
kernel config: https://syzkaller.appspot.com/x/.config?x=4c7b914a2680c9c6
dashboard link: https://syzkaller.appspot.com/bug?extid=107429d62fb1d293602f
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=16a7c8dc600000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1498cfa2600000
Reported-by: syzbot+107429d62fb1d293602f@syzkaller.appspotmail.com
Fixes: c8c8218ec5af ("netrom: fix a memory leak in nr_rx_frame()")
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
^ permalink raw reply
* Re: [PATCH net-next v2 6/6] net: dsa: mv88e6xxx: add PTP support for MV88E6250 family
From: Richard Cochran @ 2019-07-31 14:06 UTC (permalink / raw)
To: Hubert Feurstein
Cc: netdev, linux-kernel, Andrew Lunn, Vivien Didelot,
Florian Fainelli, David S. Miller, Rasmus Villemoes
In-Reply-To: <20190731082351.3157-7-h.feurstein@gmail.com>
On Wed, Jul 31, 2019 at 10:23:51AM +0200, Hubert Feurstein wrote:
> This adds PTP support for the MV88E6250 family.
>
> Signed-off-by: Hubert Feurstein <h.feurstein@gmail.com>
> ---
> drivers/net/dsa/mv88e6xxx/chip.c | 4 ++
> drivers/net/dsa/mv88e6xxx/chip.h | 4 ++
> drivers/net/dsa/mv88e6xxx/ptp.c | 79 +++++++++++++++++++++++++++-----
> drivers/net/dsa/mv88e6xxx/ptp.h | 2 +
> 4 files changed, 78 insertions(+), 11 deletions(-)
Acked-by: Richard Cochran <richardcochran@gmail.com>
^ permalink raw reply
* Re: [Intel-wired-lan] [PATCH bpf-next v4 03/11] libbpf: add flags to umem config
From: Björn Töpel @ 2019-07-31 14:25 UTC (permalink / raw)
To: Kevin Laatz
Cc: Netdev, Alexei Starovoitov, Daniel Borkmann,
Björn Töpel, Karlsson, Magnus, Jakub Kicinski,
Jonathan Lemon, Saeed Mahameed, Maxim Mikityanskiy,
Stephen Hemminger, Bruce Richardson, ciara.loftus,
intel-wired-lan, bpf
In-Reply-To: <20190730085400.10376-4-kevin.laatz@intel.com>
On Tue, 30 Jul 2019 at 19:43, Kevin Laatz <kevin.laatz@intel.com> wrote:
>
> This patch adds a 'flags' field to the umem_config and umem_reg structs.
> This will allow for more options to be added for configuring umems.
>
> The first use for the flags field is to add a flag for unaligned chunks
> mode. These flags can either be user-provided or filled with a default.
>
> Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
> Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
>
> ---
> v2:
> - Removed the headroom check from this patch. It has moved to the
> previous patch.
>
> v4:
> - modified chunk flag define
> ---
> tools/include/uapi/linux/if_xdp.h | 9 +++++++--
> tools/lib/bpf/xsk.c | 3 +++
> tools/lib/bpf/xsk.h | 2 ++
> 3 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/tools/include/uapi/linux/if_xdp.h b/tools/include/uapi/linux/if_xdp.h
> index faaa5ca2a117..a691802d7915 100644
> --- a/tools/include/uapi/linux/if_xdp.h
> +++ b/tools/include/uapi/linux/if_xdp.h
> @@ -17,6 +17,10 @@
> #define XDP_COPY (1 << 1) /* Force copy-mode */
> #define XDP_ZEROCOPY (1 << 2) /* Force zero-copy mode */
>
> +/* Flags for xsk_umem_config flags */
> +#define XDP_UMEM_UNALIGNED_CHUNK_FLAG_SHIFT 15
> +#define XDP_UMEM_UNALIGNED_CHUNK_FLAG (1 << XDP_UMEM_UNALIGNED_CHUNK_FLAG_SHIFT)
> +
> struct sockaddr_xdp {
> __u16 sxdp_family;
> __u16 sxdp_flags;
> @@ -49,8 +53,9 @@ struct xdp_mmap_offsets {
> #define XDP_OPTIONS 8
>
> struct xdp_umem_reg {
> - __u64 addr; /* Start of packet data area */
> - __u64 len; /* Length of packet data area */
> + __u64 addr; /* Start of packet data area */
> + __u64 len:48; /* Length of packet data area */
> + __u64 flags:16; /* Flags for umem */
> __u32 chunk_size;
> __u32 headroom;
> };
> diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
> index 5007b5d4fd2c..5e7e4d420ee0 100644
> --- a/tools/lib/bpf/xsk.c
> +++ b/tools/lib/bpf/xsk.c
> @@ -116,6 +116,7 @@ static void xsk_set_umem_config(struct xsk_umem_config *cfg,
> cfg->comp_size = XSK_RING_CONS__DEFAULT_NUM_DESCS;
> cfg->frame_size = XSK_UMEM__DEFAULT_FRAME_SIZE;
> cfg->frame_headroom = XSK_UMEM__DEFAULT_FRAME_HEADROOM;
> + cfg->flags = XSK_UMEM__DEFAULT_FLAGS;
> return;
> }
>
> @@ -123,6 +124,7 @@ static void xsk_set_umem_config(struct xsk_umem_config *cfg,
> cfg->comp_size = usr_cfg->comp_size;
> cfg->frame_size = usr_cfg->frame_size;
> cfg->frame_headroom = usr_cfg->frame_headroom;
> + cfg->flags = usr_cfg->flags;
> }
>
> static int xsk_set_xdp_socket_config(struct xsk_socket_config *cfg,
> @@ -182,6 +184,7 @@ int xsk_umem__create(struct xsk_umem **umem_ptr, void *umem_area, __u64 size,
> mr.len = size;
> mr.chunk_size = umem->config.frame_size;
> mr.headroom = umem->config.frame_headroom;
> + mr.flags = umem->config.flags;
>
> err = setsockopt(umem->fd, SOL_XDP, XDP_UMEM_REG, &mr, sizeof(mr));
> if (err) {
> diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
> index 833a6e60d065..44a03d8c34b9 100644
> --- a/tools/lib/bpf/xsk.h
> +++ b/tools/lib/bpf/xsk.h
> @@ -170,12 +170,14 @@ LIBBPF_API int xsk_socket__fd(const struct xsk_socket *xsk);
> #define XSK_UMEM__DEFAULT_FRAME_SHIFT 12 /* 4096 bytes */
> #define XSK_UMEM__DEFAULT_FRAME_SIZE (1 << XSK_UMEM__DEFAULT_FRAME_SHIFT)
> #define XSK_UMEM__DEFAULT_FRAME_HEADROOM 0
> +#define XSK_UMEM__DEFAULT_FLAGS 0
>
> struct xsk_umem_config {
> __u32 fill_size;
> __u32 comp_size;
> __u32 frame_size;
> __u32 frame_headroom;
> + __u32 flags;
And the flags addition here, unfortunately, requires symbol versioning
of xsk_umem__create(). That'll be the first in libbpf! :-)
Björn
> };
>
> /* Flags for the libbpf_flags field. */
> --
> 2.17.1
>
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply
* Re: kernel BUG at net/rxrpc/local_object.c:LINE!
From: David Howells @ 2019-07-31 14:30 UTC (permalink / raw)
To: Dmitry Vyukov
Cc: dhowells, syzbot, Eric Biggers, David Miller, linux-afs, LKML,
netdev, syzkaller-bugs
In-Reply-To: <CACT4Y+YjdV8CqX5=PzKsHnLsJOzsydqiq3igYDm_=nSdmFo2YQ@mail.gmail.com>
Dmitry Vyukov <dvyukov@google.com> wrote:
> Re bisection, I don't know if there are some more subtle things as
> play (you are in the better position to judge that), but bisection log
> looks good, it tracked the target crash throughout and wasn't
> distracted by any unrelated bugs, etc. So I don't see any obvious
> reasons to not trust it.
Can you turn on:
echo 1 > /sys/kernel/debug/tracing/events/rxrpc/rxrpc_local/enable
and capture the trace log at the point it crashes?
David
^ permalink raw reply
* Re: kernel BUG at net/rxrpc/local_object.c:LINE!
From: Dmitry Vyukov @ 2019-07-31 14:46 UTC (permalink / raw)
To: David Howells
Cc: syzbot, Eric Biggers, David Miller, linux-afs, LKML, netdev,
syzkaller-bugs
In-Reply-To: <20330.1564583454@warthog.procyon.org.uk>
On Wed, Jul 31, 2019 at 4:30 PM David Howells <dhowells@redhat.com> wrote:
>
> Dmitry Vyukov <dvyukov@google.com> wrote:
>
> > Re bisection, I don't know if there are some more subtle things as
> > play (you are in the better position to judge that), but bisection log
> > looks good, it tracked the target crash throughout and wasn't
> > distracted by any unrelated bugs, etc. So I don't see any obvious
> > reasons to not trust it.
>
> Can you turn on:
>
> echo 1 > /sys/kernel/debug/tracing/events/rxrpc/rxrpc_local/enable
>
> and capture the trace log at the point it crashes?
Please send a patch for testing that enables this tracing
unconditionally. This should have the same effect. There is no way to
hook into a middle of the automated process and arbitrary tune things.
^ permalink raw reply
* [PATCH] peak_usb: Fix info-leaks to USB devices
From: Tomas Bortoli @ 2019-07-31 14:54 UTC (permalink / raw)
To: wg, mkl, linux-can
Cc: davem, gregkh, alexios.zavras, tglx, allison, netdev,
linux-kernel, syzkaller, Tomas Bortoli
Uninitialized Kernel memory can leak to USB devices.
Fix by using kzalloc() instead of kmalloc() on the affected buffers.
Signed-off-by: Tomas Bortoli <tomasbortoli@gmail.com>
Reported-by: syzbot+d6a5a1a3657b596ef132@syzkaller.appspotmail.com
Reported-by: syzbot+513e4d0985298538bf9b@syzkaller.appspotmail.com
---
Crash logs:
1.
BUG: KMSAN: kernel-usb-infoleak in usb_submit_urb+0x7ef/0x1f50 drivers/usb/core/urb.c:405
CPU: 0 PID: 3359 Comm: kworker/0:2 Not tainted 5.2.0-rc4+ #7
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+0x191/0x1f0 lib/dump_stack.c:113
kmsan_report+0x162/0x2d0 mm/kmsan/kmsan.c:611
kmsan_internal_check_memory+0x974/0xa80 mm/kmsan/kmsan.c:705
kmsan_handle_urb+0x28/0x40 mm/kmsan/kmsan_hooks.c:617
usb_submit_urb+0x7ef/0x1f50 drivers/usb/core/urb.c:405
usb_start_wait_urb+0x143/0x410 drivers/usb/core/message.c:58
usb_internal_control_msg drivers/usb/core/message.c:102 [inline]
usb_control_msg+0x49f/0x7f0 drivers/usb/core/message.c:156
pcan_usb_pro_send_req+0x26b/0x3e0 drivers/net/can/usb/peak_usb/pcan_usb_pro.c:336
pcan_usb_fd_drv_loaded drivers/net/can/usb/peak_usb/pcan_usb_fd.c:460 [inline]
pcan_usb_fd_init+0x16ee/0x1900 drivers/net/can/usb/peak_usb/pcan_usb_fd.c:885
peak_usb_create_dev drivers/net/can/usb/peak_usb/pcan_usb_core.c:809 [inline]
peak_usb_probe+0x1416/0x1b20 drivers/net/can/usb/peak_usb/pcan_usb_core.c:907
usb_probe_interface+0xd19/0x1310 drivers/usb/core/driver.c:361
really_probe+0x1344/0x1d90 drivers/base/dd.c:513
driver_probe_device+0x1ba/0x510 drivers/base/dd.c:670
__device_attach_driver+0x5b8/0x790 drivers/base/dd.c:777
bus_for_each_drv+0x28e/0x3b0 drivers/base/bus.c:454
__device_attach+0x489/0x750 drivers/base/dd.c:843
device_initial_probe+0x4a/0x60 drivers/base/dd.c:890
2.
BUG: KMSAN: kernel-usb-infoleak in usb_submit_urb+0x7ef/0x1f50 /drivers/usb/core/urb.c:405
CPU: 1 PID: 3814 Comm: kworker/1:2 Not tainted 5.2.0+ #15
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+0x191/0x1f0 /lib/dump_stack.c:113
kmsan_report+0x162/0x2d0 /mm/kmsan/kmsan_report.c:109
kmsan_internal_check_memory+0x974/0xa80 /mm/kmsan/kmsan.c:551
kmsan_handle_urb+0x28/0x40 /mm/kmsan/kmsan_hooks.c:621
usb_submit_urb+0x7ef/0x1f50 /drivers/usb/core/urb.c:405
usb_start_wait_urb+0x143/0x410 /drivers/usb/core/message.c:58
usb_internal_control_msg /drivers/usb/core/message.c:102 [inline]
usb_control_msg+0x49f/0x7f0 /drivers/usb/core/message.c:156
pcan_usb_pro_send_req /drivers/net/can/usb/peak_usb/pcan_usb_pro.c:336 [inline]
pcan_usb_pro_drv_loaded /drivers/net/can/usb/peak_usb/pcan_usb_pro.c:504 [inline]
pcan_usb_pro_init+0x1319/0x1720 /drivers/net/can/usb/peak_usb/pcan_usb_pro.c:894
peak_usb_create_dev /drivers/net/can/usb/peak_usb/pcan_usb_core.c:809 [inline]
peak_usb_probe+0x1416/0x1b20 /drivers/net/can/usb/peak_usb/pcan_usb_core.c:907
usb_probe_interface+0xd19/0x1310 /drivers/usb/core/driver.c:361
really_probe+0x1344/0x1d90 /drivers/base/dd.c:513
driver_probe_device+0x1ba/0x510 /drivers/base/dd.c:670
__device_attach_driver+0x5b8/0x790 /drivers/base/dd.c:777
bus_for_each_drv+0x28e/0x3b0 /drivers/base/bus.c:454
__device_attach+0x489/0x750 /drivers/base/dd.c:843
device_initial_probe+0x4a/0x60 /drivers/base/dd.c:890
bus_probe_device+0x131/0x390 /drivers/base/bus.c:514
device_add+0x25b5/0x2df0 /drivers/base/core.c:2111
usb_set_configuration+0x309f/0x3710 /drivers/usb/core/message.c:2027
generic_probe+0xe7/0x280 /drivers/usb/core/generic.c:210
usb_probe_device+0x146/0x200 /drivers/usb/core/driver.c:266
really_probe+0x1344/0x1d90 /drivers/base/dd.c:513
driver_probe_device+0x1ba/0x510 /drivers/base/dd.c:670
__device_attach_driver+0x5b8/0x790 /drivers/base/dd.c:777
bus_for_each_drv+0x28e/0x3b0 /drivers/base/bus.c:454
__device_attach+0x489/0x750 /drivers/base/dd.c:843
device_initial_probe+0x4a/0x60 /drivers/base/dd.c:890
bus_probe_device+0x131/0x390 /drivers/base/bus.c:514
device_add+0x25b5/0x2df0 /drivers/base/core.c:2111
usb_new_device+0x23e5/0x2fb0 /drivers/usb/core/hub.c:2534
hub_port_connect /drivers/usb/core/hub.c:5089 [inline]
hub_port_connect_change /drivers/usb/core/hub.c:5204 [inline]
port_event /drivers/usb/core/hub.c:5350 [inline]
hub_event+0x5853/0x7320 /drivers/usb/core/hub.c:5432
process_one_work+0x1572/0x1f00 /kernel/workqueue.c:2269
worker_thread+0x111b/0x2460 /kernel/workqueue.c:2415
kthread+0x4b5/0x4f0 /kernel/kthread.c:256
ret_from_fork+0x35/0x40 /arch/x86/entry/entry_64.S:355
Uninit was created at:
kmsan_save_stack_with_flags /mm/kmsan/kmsan.c:187 [inline]
kmsan_internal_poison_shadow+0x53/0xa0 /mm/kmsan/kmsan.c:146
kmsan_slab_alloc+0xaa/0x120 /mm/kmsan/kmsan_hooks.c:175
slab_alloc_node /mm/slub.c:2771 [inline]
slab_alloc /mm/slub.c:2780 [inline]
kmem_cache_alloc_trace+0x873/0xa50 /mm/slub.c:2797
kmalloc /./include/linux/slab.h:547 [inline]
pcan_usb_pro_drv_loaded /drivers/net/can/usb/peak_usb/pcan_usb_pro.c:497 [inline]
pcan_usb_pro_init+0xe96/0x1720 /drivers/net/can/usb/peak_usb/pcan_usb_pro.c:894
peak_usb_create_dev /drivers/net/can/usb/peak_usb/pcan_usb_core.c:809 [inline]
peak_usb_probe+0x1416/0x1b20 /drivers/net/can/usb/peak_usb/pcan_usb_core.c:907
usb_probe_interface+0xd19/0x1310 /drivers/usb/core/driver.c:361
really_probe+0x1344/0x1d90 /drivers/base/dd.c:513
driver_probe_device+0x1ba/0x510 /drivers/base/dd.c:670
__device_attach_driver+0x5b8/0x790 /drivers/base/dd.c:777
bus_for_each_drv+0x28e/0x3b0 /drivers/base/bus.c:454
__device_attach+0x489/0x750 /drivers/base/dd.c:843
device_initial_probe+0x4a/0x60 /drivers/base/dd.c:890
bus_probe_device+0x131/0x390 /drivers/base/bus.c:514
device_add+0x25b5/0x2df0 /drivers/base/core.c:2111
usb_set_configuration+0x309f/0x3710 /drivers/usb/core/message.c:2027
generic_probe+0xe7/0x280 /drivers/usb/core/generic.c:210
usb_probe_device+0x146/0x200 /drivers/usb/core/driver.c:266
really_probe+0x1344/0x1d90 /drivers/base/dd.c:513
driver_probe_device+0x1ba/0x510 /drivers/base/dd.c:670
__device_attach_driver+0x5b8/0x790 /drivers/base/dd.c:777
bus_for_each_drv+0x28e/0x3b0 /drivers/base/bus.c:454
__device_attach+0x489/0x750 /drivers/base/dd.c:843
device_initial_probe+0x4a/0x60 /drivers/base/dd.c:890
bus_probe_device+0x131/0x390 /drivers/base/bus.c:514
device_add+0x25b5/0x2df0 /drivers/base/core.c:2111
usb_new_device+0x23e5/0x2fb0 /drivers/usb/core/hub.c:2534
hub_port_connect /drivers/usb/core/hub.c:5089 [inline]
hub_port_connect_change /drivers/usb/core/hub.c:5204 [inline]
port_event /drivers/usb/core/hub.c:5350 [inline]
hub_event+0x5853/0x7320 /drivers/usb/core/hub.c:5432
process_one_work+0x1572/0x1f00 /kernel/workqueue.c:2269
worker_thread+0x111b/0x2460 /kernel/workqueue.c:2415
kthread+0x4b5/0x4f0 /kernel/kthread.c:256
ret_from_fork+0x35/0x40 /arch/x86/entry/entry_64.S:355
Bytes 2-15 of 16 are uninitialized
Memory access of size 16 starts at ffff8881030286e0
==================================================================
drivers/net/can/usb/peak_usb/pcan_usb_fd.c | 2 +-
drivers/net/can/usb/peak_usb/pcan_usb_pro.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
index 34761c3a6286..47cc1ff5b88e 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
@@ -841,7 +841,7 @@ static int pcan_usb_fd_init(struct peak_usb_device *dev)
goto err_out;
/* allocate command buffer once for all for the interface */
- pdev->cmd_buffer_addr = kmalloc(PCAN_UFD_CMD_BUFFER_SIZE,
+ pdev->cmd_buffer_addr = kzalloc(PCAN_UFD_CMD_BUFFER_SIZE,
GFP_KERNEL);
if (!pdev->cmd_buffer_addr)
goto err_out_1;
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_pro.c b/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
index 178bb7cff0c1..53cb2f72bdd0 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_pro.c
@@ -494,7 +494,7 @@ static int pcan_usb_pro_drv_loaded(struct peak_usb_device *dev, int loaded)
u8 *buffer;
int err;
- buffer = kmalloc(PCAN_USBPRO_FCT_DRVLD_REQ_LEN, GFP_KERNEL);
+ buffer = kzalloc(PCAN_USBPRO_FCT_DRVLD_REQ_LEN, GFP_KERNEL);
if (!buffer)
return -ENOMEM;
--
2.11.0
^ permalink raw reply related
* Re: Reminder: 99 open syzbot bugs in net subsystem
From: David Ahern @ 2019-07-31 15:13 UTC (permalink / raw)
To: Eric Dumazet, David Miller, dvyukov, netdev, fw, i.maximets,
edumazet, linux-kernel, syzkaller-bugs
In-Reply-To: <20190731025722.GE687@sol.localdomain>
On 7/30/19 8:57 PM, Eric Biggers wrote:
> syzbot finds a lot of security bugs, and security bugs are important. And the
> bugs are still there regardless of whether they're reported by human or bot.
>
> Also, there *are* bugs being fixed because of these reminders; some subsystem
> maintainers have even fixed all the bugs in their subsystem. But I can
> understand that for subsystems with a lot of open bug reports it's overwhelming.
>
> What I'll try doing next time (if there *is* a next time; it isn't actually my
> job to do any of this, I just care about the security and reliability of
> Linux...) is for subsystems with lots of open bug reports, only listing the ones
> actually seen in the last week or so, and perhaps also spending some more time
> manually checking those bugs. That should cut down the noise a lot.
I don't think anyone questions the overall value of syzbot. It's the
maintenance of bug reports that needs refining.
As an example, this one:
https://syzkaller.appspot.com/bug?id=079bd8408abd95b492f127edf0df44ddc09d9405
was in reality a very short-lived bug in net-next but because bpf-next
managed to merge net-next in the small time window, the bug life seems
more extended that it apparently was (fuzzy words since we do not know
which commit fixed it).
Also, there is inconsistency with the report. It shows a bisected commit of:
commit f40b6ae2b612446dc970d7b51eeec47bd1619f82
Author: David Ahern <dsahern@gmail.com>
Date: Thu May 23 03:27:55 2019 +0000
ipv6: Move pcpu cached routes to fib6_nh
yet the report shows an entry in net tree on April 27. Even the net
instance on June 14 is questionable given that the above commit is only
in net-next on June 14.
Taking all of those references out and there are 2 'real', unique
reports - the linux-next on May 31 and the net-next on June 5.
Given that nothing has appeared in the last 8 weeks it seems clear to me
that this bug has been fixed we just don't know by which commit.
If there is a way to reduce to some of that information or even to have
a button on that console that says 'apparently fixed' and close it would
be a help.
^ permalink raw reply
* Re: kernel BUG at net/rxrpc/local_object.c:LINE!
From: David Howells @ 2019-07-31 15:19 UTC (permalink / raw)
To: Dmitry Vyukov
Cc: dhowells, syzbot, Eric Biggers, David Miller, linux-afs, LKML,
netdev, syzkaller-bugs
In-Reply-To: <CACT4Y+Y4cRgaRPJ_gz_53k85inDKq+X+bWmOTv1gPLo=Yod1=A@mail.gmail.com>
Dmitry Vyukov <dvyukov@google.com> wrote:
> Please send a patch for testing that enables this tracing
> unconditionally. This should have the same effect. There is no way to
> hook into a middle of the automated process and arbitrary tune things.
I don't know how to do that off hand. Do you have an example?
Anyway, I think rxrpc_local_processor() is broken with respect to refcounting
as it gets scheduled when usage==0, but that doesn't stop it being rescheduled
again by a network packet before it manages to close the UDP socket.
David
^ permalink raw reply
* Re: [PATCH net 0/2] mlxsw: Two small fixes
From: David Miller @ 2019-07-31 15:24 UTC (permalink / raw)
To: idosch; +Cc: netdev, jiri, petrm, mlxsw, idosch
In-Reply-To: <20190731063315.9381-1-idosch@idosch.org>
From: Ido Schimmel <idosch@idosch.org>
Date: Wed, 31 Jul 2019 09:33:13 +0300
> From: Ido Schimmel <idosch@mellanox.com>
>
> Patch #1 from Jiri fixes the error path of the module initialization
> function. Found during manual code inspection.
>
> Patch #2 from Petr further reduces the default shared buffer pool sizes
> in order to work around a problem that was originally described in
> commit e891ce1dd2a5 ("mlxsw: spectrum_buffers: Reduce pool size on
> Spectrum-2").
Series applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH net] drop_monitor: Add missing uAPI file to MAINTAINERS file
From: David Miller @ 2019-07-31 15:27 UTC (permalink / raw)
To: idosch; +Cc: netdev, nhorman, idosch
In-Reply-To: <20190731063819.10001-1-idosch@idosch.org>
From: Ido Schimmel <idosch@idosch.org>
Date: Wed, 31 Jul 2019 09:38:19 +0300
> From: Ido Schimmel <idosch@mellanox.com>
>
> Fixes: 6e43650cee64 ("add maintainer for network drop monitor kernel service")
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Applied.
^ permalink raw reply
* RE: [PATCH net-next v4 0/4] enetc: Add mdio bus driver for the PCIe MDIO endpoint
From: Claudiu Manoil @ 2019-07-31 15:30 UTC (permalink / raw)
To: David Miller
Cc: andrew@lunn.ch, robh+dt@kernel.org, Leo Li, Alexandru Marginean,
netdev@vger.kernel.org, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190730.095344.401137621326119500.davem@davemloft.net>
>-----Original Message-----
>From: David Miller <davem@davemloft.net>
>Sent: Tuesday, July 30, 2019 7:54 PM
>To: Claudiu Manoil <claudiu.manoil@nxp.com>
>Cc: andrew@lunn.ch; robh+dt@kernel.org; Leo Li <leoyang.li@nxp.com>;
>Alexandru Marginean <alexandru.marginean@nxp.com>;
>netdev@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
>kernel@lists.infradead.org; linux-kernel@vger.kernel.org
>Subject: Re: [PATCH net-next v4 0/4] enetc: Add mdio bus driver for the PCIe
>MDIO endpoint
>
>From: David Miller <davem@davemloft.net>
>Date: Tue, 30 Jul 2019 09:44:36 -0700 (PDT)
>
>> From: Claudiu Manoil <claudiu.manoil@nxp.com>
>> Date: Tue, 30 Jul 2019 12:45:15 +0300
>>
>>> First patch fixes a sparse issue and cleans up accessors to avoid
>>> casting to __iomem.
>>> Second patch just registers the PCIe endpoint device containing
>>> the MDIO registers as a standalone MDIO bus driver, to allow
>>> an alternative way to control the MDIO bus. The same code used
>>> by the ENETC ports (eth controllers) to manage MDIO via local
>>> registers applies and is reused.
>>>
>>> Bindings are provided for the new MDIO node, similarly to ENETC
>>> port nodes bindings.
>>>
>>> Last patch enables the ENETC port 1 and its RGMII PHY on the
>>> LS1028A QDS board, where the MDIO muxing configuration relies
>>> on the MDIO support provided in the first patch.
>> ...
>>
>> Series applied, thank you.
>
>Actually this doesn't compile, I had to revert:
>
Sorry, I overlooked the module part. Turns out I have to define a separate
module for this driver (mdio), refactor common code between the mdio module
and the enetc-pf module, clean up the Makefile... and do more checks.
^ permalink raw reply
* Re: kernel BUG at net/rxrpc/local_object.c:LINE!
From: Dmitry Vyukov @ 2019-07-31 15:31 UTC (permalink / raw)
To: David Howells
Cc: syzbot, Eric Biggers, David Miller, linux-afs, LKML, netdev,
syzkaller-bugs
In-Reply-To: <22318.1564586386@warthog.procyon.org.uk>
On Wed, Jul 31, 2019 at 5:19 PM David Howells <dhowells@redhat.com> wrote:
>
> Dmitry Vyukov <dvyukov@google.com> wrote:
>
> > Please send a patch for testing that enables this tracing
> > unconditionally. This should have the same effect. There is no way to
> > hook into a middle of the automated process and arbitrary tune things.
>
> I don't know how to do that off hand. Do you have an example?
Few messages above I asked it to test:
https://groups.google.com/d/msg/syzkaller-bugs/gEnZkmEWf1s/r2_X_KVQAQAJ
Basically, git repo + branch + patch. Here are the docs:
https://github.com/google/syzkaller/blob/master/docs/syzbot.md#testing-patches
> Anyway, I think rxrpc_local_processor() is broken with respect to refcounting
> as it gets scheduled when usage==0, but that doesn't stop it being rescheduled
> again by a network packet before it manages to close the UDP socket.
>
> David
^ permalink raw reply
* Re: [PATCH] net: ethernet: et131x: Use GFP_KERNEL instead of GFP_ATOMIC when allocating tx_ring->tcb_ring
From: David Miller @ 2019-07-31 15:36 UTC (permalink / raw)
To: christophe.jaillet
Cc: mark.einon, willy, f.fainelli, andrew, netdev, linux-kernel,
kernel-janitors
In-Reply-To: <20190731073842.16948-1-christophe.jaillet@wanadoo.fr>
From: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Date: Wed, 31 Jul 2019 09:38:42 +0200
> There is no good reason to use GFP_ATOMIC here. Other memory allocations
> are performed with GFP_KERNEL (see other 'dma_alloc_coherent()' below and
> 'kzalloc()' in 'et131x_rx_dma_memory_alloc()')
>
> Use GFP_KERNEL which should be enough.
>
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH 1/2] net: ag71xx: Slighly simplify code in 'ag71xx_rings_init()'
From: David Miller @ 2019-07-31 15:38 UTC (permalink / raw)
To: christophe.jaillet
Cc: jcliburn, chris.snook, netdev, linux-kernel, kernel-janitors
In-Reply-To: <08fbcfe0f913644fe538656221a15790a1a83f1d.1564560130.git.christophe.jaillet@wanadoo.fr>
From: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Date: Wed, 31 Jul 2019 10:06:38 +0200
> A few lines above, we have:
> tx_size = BIT(tx->order);
>
> So use 'tx_size' directly to be consistent with the way 'rx->descs_cpu' and
> 'rx->descs_dma' are computed below.
>
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Applied to net-next.
^ permalink raw reply
* Re: [PATCH 2/2] net: ag71xx: Use GFP_KERNEL instead of GFP_ATOMIC in 'ag71xx_rings_init()'
From: David Miller @ 2019-07-31 15:39 UTC (permalink / raw)
To: christophe.jaillet
Cc: jcliburn, chris.snook, netdev, linux-kernel, kernel-janitors
In-Reply-To: <75ee52ae065ce9cb1f32d88939b166773316dc45.1564560130.git.christophe.jaillet@wanadoo.fr>
From: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Date: Wed, 31 Jul 2019 10:06:48 +0200
> There is no need to use GFP_ATOMIC here, GFP_KERNEL should be enough.
> The 'kcalloc()' just a few lines above, already uses GFP_KERNEL.
>
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Applied to 'net'
^ permalink raw reply
* [PATCH net] net: dsa: mv88e6xxx: drop adjust_link to enabled phylink
From: Hubert Feurstein @ 2019-07-31 15:42 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: Hubert Feurstein, Andrew Lunn, Vivien Didelot, Florian Fainelli,
David S. Miller
We have to drop the adjust_link callback in order to finally migrate to
phylink.
Otherwise we get the following warning during startup:
"mv88e6xxx 2188000.ethernet-1:10: Using legacy PHYLIB callbacks. Please
migrate to PHYLINK!"
The warning is generated in the function dsa_port_link_register_of in
dsa/port.c:
int dsa_port_link_register_of(struct dsa_port *dp)
{
struct dsa_switch *ds = dp->ds;
if (!ds->ops->adjust_link)
return dsa_port_phylink_register(dp);
dev_warn(ds->dev,
"Using legacy PHYLIB callbacks. Please migrate to PHYLINK!\n");
[...]
}
Signed-off-by: Hubert Feurstein <h.feurstein@gmail.com>
---
drivers/net/dsa/mv88e6xxx/chip.c | 26 --------------------------
1 file changed, 26 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 366f70bfe055..37e8babd035f 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -27,7 +27,6 @@
#include <linux/platform_data/mv88e6xxx.h>
#include <linux/netdevice.h>
#include <linux/gpio/consumer.h>
-#include <linux/phy.h>
#include <linux/phylink.h>
#include <net/dsa.h>
@@ -482,30 +481,6 @@ static int mv88e6xxx_phy_is_internal(struct dsa_switch *ds, int port)
return port < chip->info->num_internal_phys;
}
-/* We expect the switch to perform auto negotiation if there is a real
- * phy. However, in the case of a fixed link phy, we force the port
- * settings from the fixed link settings.
- */
-static void mv88e6xxx_adjust_link(struct dsa_switch *ds, int port,
- struct phy_device *phydev)
-{
- struct mv88e6xxx_chip *chip = ds->priv;
- int err;
-
- if (!phy_is_pseudo_fixed_link(phydev) &&
- mv88e6xxx_phy_is_internal(ds, port))
- return;
-
- mv88e6xxx_reg_lock(chip);
- err = mv88e6xxx_port_setup_mac(chip, port, phydev->link, phydev->speed,
- phydev->duplex, phydev->pause,
- phydev->interface);
- mv88e6xxx_reg_unlock(chip);
-
- if (err && err != -EOPNOTSUPP)
- dev_err(ds->dev, "p%d: failed to configure MAC\n", port);
-}
-
static void mv88e6065_phylink_validate(struct mv88e6xxx_chip *chip, int port,
unsigned long *mask,
struct phylink_link_state *state)
@@ -4755,7 +4730,6 @@ static int mv88e6xxx_port_egress_floods(struct dsa_switch *ds, int port,
static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
.get_tag_protocol = mv88e6xxx_get_tag_protocol,
.setup = mv88e6xxx_setup,
- .adjust_link = mv88e6xxx_adjust_link,
.phylink_validate = mv88e6xxx_validate,
.phylink_mac_link_state = mv88e6xxx_link_state,
.phylink_mac_config = mv88e6xxx_mac_config,
--
2.22.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox