* [PATCH v2] vhost: block speculation of translated descriptors
From: Michael S. Tsirkin @ 2019-09-11 12:10 UTC (permalink / raw)
To: linux-kernel; +Cc: Jason Wang, kvm, virtualization, netdev
iovec addresses coming from vhost are assumed to be
pre-validated, but in fact can be speculated to a value
out of range.
Userspace address are later validated with array_index_nospec so we can
be sure kernel info does not leak through these addresses, but vhost
must also not leak userspace info outside the allowed memory table to
guests.
Following the defence in depth principle, make sure
the address is not validated out of node range.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Jason Wang <jasowang@redhat.com>
Tested-by: Jason Wang <jasowang@redhat.com>
---
changes from v1: fix build on 32 bit
drivers/vhost/vhost.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 5dc174ac8cac..34ea219936e3 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -2071,8 +2071,10 @@ static int translate_desc(struct vhost_virtqueue *vq, u64 addr, u32 len,
_iov = iov + ret;
size = node->size - addr + node->start;
_iov->iov_len = min((u64)len - s, size);
- _iov->iov_base = (void __user *)(unsigned long)
- (node->userspace_addr + addr - node->start);
+ _iov->iov_base = (void __user *)
+ ((unsigned long)node->userspace_addr +
+ array_index_nospec((unsigned long)(addr - node->start),
+ node->size));
s += size;
addr += size;
++ret;
--
MST
^ permalink raw reply related
* Re: [PATCH v2] vhost: block speculation of translated descriptors
From: Michal Hocko @ 2019-09-11 12:16 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: linux-kernel, Jason Wang, kvm, virtualization, netdev
In-Reply-To: <20190911120908.28410-1-mst@redhat.com>
On Wed 11-09-19 08:10:00, Michael S. Tsirkin wrote:
> iovec addresses coming from vhost are assumed to be
> pre-validated, but in fact can be speculated to a value
> out of range.
>
> Userspace address are later validated with array_index_nospec so we can
> be sure kernel info does not leak through these addresses, but vhost
> must also not leak userspace info outside the allowed memory table to
> guests.
>
> Following the defence in depth principle, make sure
> the address is not validated out of node range.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Acked-by: Jason Wang <jasowang@redhat.com>
> Tested-by: Jason Wang <jasowang@redhat.com>
no need to mark fo stable? Other spectre fixes tend to be backported
even when the security implications are not really clear. The risk
should be low and better to be covered in case.
> ---
>
> changes from v1: fix build on 32 bit
>
> drivers/vhost/vhost.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 5dc174ac8cac..34ea219936e3 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -2071,8 +2071,10 @@ static int translate_desc(struct vhost_virtqueue *vq, u64 addr, u32 len,
> _iov = iov + ret;
> size = node->size - addr + node->start;
> _iov->iov_len = min((u64)len - s, size);
> - _iov->iov_base = (void __user *)(unsigned long)
> - (node->userspace_addr + addr - node->start);
> + _iov->iov_base = (void __user *)
> + ((unsigned long)node->userspace_addr +
> + array_index_nospec((unsigned long)(addr - node->start),
> + node->size));
> s += size;
> addr += size;
> ++ret;
> --
> MST
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH v2] vhost: block speculation of translated descriptors
From: Michael S. Tsirkin @ 2019-09-11 12:25 UTC (permalink / raw)
To: Michal Hocko; +Cc: linux-kernel, Jason Wang, kvm, virtualization, netdev
In-Reply-To: <20190911121628.GT4023@dhcp22.suse.cz>
On Wed, Sep 11, 2019 at 02:16:28PM +0200, Michal Hocko wrote:
> On Wed 11-09-19 08:10:00, Michael S. Tsirkin wrote:
> > iovec addresses coming from vhost are assumed to be
> > pre-validated, but in fact can be speculated to a value
> > out of range.
> >
> > Userspace address are later validated with array_index_nospec so we can
> > be sure kernel info does not leak through these addresses, but vhost
> > must also not leak userspace info outside the allowed memory table to
> > guests.
> >
> > Following the defence in depth principle, make sure
> > the address is not validated out of node range.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > Acked-by: Jason Wang <jasowang@redhat.com>
> > Tested-by: Jason Wang <jasowang@redhat.com>
>
> no need to mark fo stable? Other spectre fixes tend to be backported
> even when the security implications are not really clear. The risk
> should be low and better to be covered in case.
This is not really a fix - more a defence in depth thing,
quite similar to e.g. commit b3bbfb3fb5d25776b8e3f361d2eedaabb0b496cd
x86: Introduce __uaccess_begin_nospec() and uaccess_try_nospec
in scope.
That one doesn't seem to be tagged for stable. Was it queued
there in practice?
> > ---
> >
> > changes from v1: fix build on 32 bit
> >
> > drivers/vhost/vhost.c | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > index 5dc174ac8cac..34ea219936e3 100644
> > --- a/drivers/vhost/vhost.c
> > +++ b/drivers/vhost/vhost.c
> > @@ -2071,8 +2071,10 @@ static int translate_desc(struct vhost_virtqueue *vq, u64 addr, u32 len,
> > _iov = iov + ret;
> > size = node->size - addr + node->start;
> > _iov->iov_len = min((u64)len - s, size);
> > - _iov->iov_base = (void __user *)(unsigned long)
> > - (node->userspace_addr + addr - node->start);
> > + _iov->iov_base = (void __user *)
> > + ((unsigned long)node->userspace_addr +
> > + array_index_nospec((unsigned long)(addr - node->start),
> > + node->size));
> > s += size;
> > addr += size;
> > ++ret;
> > --
> > MST
>
> --
> Michal Hocko
> SUSE Labs
^ permalink raw reply
* Re: [PATCH 6/7] net: dsa: mv88e6xxx: add egress rate limiting
From: kbuild test robot @ 2019-09-11 12:26 UTC (permalink / raw)
To: Robert Beckett
Cc: kbuild-all, netdev, Robert Beckett, Andrew Lunn, Vivien Didelot,
Florian Fainelli, David S. Miller
In-Reply-To: <20190910154238.9155-7-bob.beckett@collabora.com>
[-- Attachment #1: Type: text/plain, Size: 5217 bytes --]
Hi Robert,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[cannot apply to v5.3-rc8 next-20190904]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Robert-Beckett/net-dsa-mv88e6xxx-features-to-handle-network-storms/20190911-142233
config: x86_64-fedora-25 (attached as .config)
compiler: gcc-7 (Debian 7.4.0-11) 7.4.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> drivers/net//dsa/mv88e6xxx/chip.c:3529:31: error: 'mv88e6097_port_egress_rate_limiting' undeclared here (not in a function); did you mean 'mv88e6xxx_port_egress_rate_limiting'?
.port_egress_rate_limiting = mv88e6097_port_egress_rate_limiting,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
mv88e6xxx_port_egress_rate_limiting
vim +3529 drivers/net//dsa/mv88e6xxx/chip.c
b3469dd8adade1 Vivien Didelot 2016-09-29 3510
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 3511 static const struct mv88e6xxx_ops mv88e6250_ops = {
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 3512 /* MV88E6XXX_FAMILY_6250 */
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 3513 .ieee_pri_map = mv88e6250_g1_ieee_pri_map,
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 3514 .ip_pri_map = mv88e6085_g1_ip_pri_map,
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 3515 .irl_init_all = mv88e6352_g2_irl_init_all,
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 3516 .get_eeprom = mv88e6xxx_g2_get_eeprom16,
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 3517 .set_eeprom = mv88e6xxx_g2_set_eeprom16,
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 3518 .set_switch_mac = mv88e6xxx_g2_set_switch_mac,
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 3519 .phy_read = mv88e6xxx_g2_smi_phy_read,
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 3520 .phy_write = mv88e6xxx_g2_smi_phy_write,
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 3521 .port_set_link = mv88e6xxx_port_set_link,
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 3522 .port_set_duplex = mv88e6xxx_port_set_duplex,
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 3523 .port_set_rgmii_delay = mv88e6352_port_set_rgmii_delay,
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 3524 .port_set_speed = mv88e6250_port_set_speed,
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 3525 .port_tag_remap = mv88e6095_port_tag_remap,
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 3526 .port_set_frame_mode = mv88e6351_port_set_frame_mode,
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 3527 .port_set_egress_floods = mv88e6352_port_set_egress_floods,
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 3528 .port_set_ether_type = mv88e6351_port_set_ether_type,
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 @3529 .port_egress_rate_limiting = mv88e6097_port_egress_rate_limiting,
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 3530 .port_pause_limit = mv88e6097_port_pause_limit,
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 3531 .port_disable_pri_override = mv88e6xxx_port_disable_pri_override,
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 3532 .port_link_state = mv88e6250_port_link_state,
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 3533 .stats_snapshot = mv88e6320_g1_stats_snapshot,
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 3534 .stats_set_histogram = mv88e6095_g1_stats_set_histogram,
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 3535 .stats_get_sset_count = mv88e6250_stats_get_sset_count,
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 3536 .stats_get_strings = mv88e6250_stats_get_strings,
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 3537 .stats_get_stats = mv88e6250_stats_get_stats,
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 3538 .set_cpu_port = mv88e6095_g1_set_cpu_port,
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 3539 .set_egress_port = mv88e6095_g1_set_egress_port,
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 3540 .watchdog_ops = &mv88e6250_watchdog_ops,
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 3541 .mgmt_rsvd2cpu = mv88e6352_g2_mgmt_rsvd2cpu,
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 3542 .pot_clear = mv88e6xxx_g2_pot_clear,
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 3543 .reset = mv88e6250_g1_reset,
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 3544 .vtu_getnext = mv88e6250_g1_vtu_getnext,
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 3545 .vtu_loadpurge = mv88e6250_g1_vtu_loadpurge,
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 3546 .phylink_validate = mv88e6065_phylink_validate,
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 3547 };
1f71836f5d96e4 Rasmus Villemoes 2019-06-04 3548
:::::: The code at line 3529 was first introduced by commit
:::::: 1f71836f5d96e4c87fad16db86d324bee47e1d30 net: dsa: mv88e6xxx: add support for mv88e6250
:::::: TO: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
:::::: CC: David S. Miller <davem@davemloft.net>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 50818 bytes --]
^ permalink raw reply
* Re: [PATCH v2] vhost: block speculation of translated descriptors
From: Michal Hocko @ 2019-09-11 12:33 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: linux-kernel, Jason Wang, kvm, virtualization, netdev
In-Reply-To: <20190911082236-mutt-send-email-mst@kernel.org>
On Wed 11-09-19 08:25:03, Michael S. Tsirkin wrote:
> On Wed, Sep 11, 2019 at 02:16:28PM +0200, Michal Hocko wrote:
> > On Wed 11-09-19 08:10:00, Michael S. Tsirkin wrote:
> > > iovec addresses coming from vhost are assumed to be
> > > pre-validated, but in fact can be speculated to a value
> > > out of range.
> > >
> > > Userspace address are later validated with array_index_nospec so we can
> > > be sure kernel info does not leak through these addresses, but vhost
> > > must also not leak userspace info outside the allowed memory table to
> > > guests.
> > >
> > > Following the defence in depth principle, make sure
> > > the address is not validated out of node range.
> > >
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > Acked-by: Jason Wang <jasowang@redhat.com>
> > > Tested-by: Jason Wang <jasowang@redhat.com>
> >
> > no need to mark fo stable? Other spectre fixes tend to be backported
> > even when the security implications are not really clear. The risk
> > should be low and better to be covered in case.
>
> This is not really a fix - more a defence in depth thing,
> quite similar to e.g. commit b3bbfb3fb5d25776b8e3f361d2eedaabb0b496cd
> x86: Introduce __uaccess_begin_nospec() and uaccess_try_nospec
> in scope.
>
> That one doesn't seem to be tagged for stable. Was it queued
> there in practice?
not marked for stable but it went in. At least to 4.4.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH net-next 5/5] sctp: add spt_pathcpthld in struct sctp_paddrthlds
From: Marcelo Ricardo Leitner @ 2019-09-11 12:56 UTC (permalink / raw)
To: Xin Long
Cc: David Laight, network dev, linux-sctp@vger.kernel.org,
Neil Horman, davem@davemloft.net
In-Reply-To: <CADvbK_dqNas+vwP2t3LqWyabNnzRDO=PZPe4p+zE-vQJTnfKpA@mail.gmail.com>
On Wed, Sep 11, 2019 at 05:38:33PM +0800, Xin Long wrote:
> On Wed, Sep 11, 2019 at 5:21 PM Xin Long <lucien.xin@gmail.com> wrote:
> >
> > On Wed, Sep 11, 2019 at 5:03 PM David Laight <David.Laight@aculab.com> wrote:
> > >
> > > From: Xin Long [mailto:lucien.xin@gmail.com]
> > > > Sent: 11 September 2019 09:52
> > > > On Tue, Sep 10, 2019 at 9:19 PM David Laight <David.Laight@aculab.com> wrote:
> > > > >
> > > > > From: Xin Long
> > > > > > Sent: 09 September 2019 08:57
> > > > > > Section 7.2 of rfc7829: "Peer Address Thresholds (SCTP_PEER_ADDR_THLDS)
> > > > > > Socket Option" extends 'struct sctp_paddrthlds' with 'spt_pathcpthld'
> > > > > > added to allow a user to change ps_retrans per sock/asoc/transport, as
> > > > > > other 2 paddrthlds: pf_retrans, pathmaxrxt.
> > > > > >
> > > > > > Note that ps_retrans is not allowed to be greater than pf_retrans.
> > > > > >
> > > > > > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > > > > > ---
> > > > > > include/uapi/linux/sctp.h | 1 +
> > > > > > net/sctp/socket.c | 10 ++++++++++
> > > > > > 2 files changed, 11 insertions(+)
> > > > > >
> > > > > > diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
> > > > > > index a15cc28..dfd81e1 100644
> > > > > > --- a/include/uapi/linux/sctp.h
> > > > > > +++ b/include/uapi/linux/sctp.h
> > > > > > @@ -1069,6 +1069,7 @@ struct sctp_paddrthlds {
> > > > > > struct sockaddr_storage spt_address;
> > > > > > __u16 spt_pathmaxrxt;
> > > > > > __u16 spt_pathpfthld;
> > > > > > + __u16 spt_pathcpthld;
> > > > > > };
> > > > > >
> > > > > > /*
> > > > > > diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> > > > > > index 5e2098b..5b9774d 100644
> > > > > > --- a/net/sctp/socket.c
> > > > > > +++ b/net/sctp/socket.c
> > > > > > @@ -3954,6 +3954,9 @@ static int sctp_setsockopt_paddr_thresholds(struct sock *sk,
> > > > >
> > > > > This code does:
> > > > > if (optlen < sizeof(struct sctp_paddrthlds))
> > > > > return -EINVAL;
> > > > here will become:
> > > >
> > > > if (optlen >= sizeof(struct sctp_paddrthlds)) {
> > > > optlen = sizeof(struct sctp_paddrthlds);
> > > > } else if (optlen >= ALIGN(offsetof(struct sctp_paddrthlds,
> > > > spt_pathcpthld), 4))
> > > > optlen = ALIGN(offsetof(struct sctp_paddrthlds,
> > > > spt_pathcpthld), 4);
> > > > val.spt_pathcpthld = 0xffff;
> > > > else {
> > > > return -EINVAL;
> > > > }
> > >
> > > Hmmm...
> > > If the kernel has to default 'val.spt_pathcpthld = 0xffff'
> > > then recompiling an existing application with the new uapi
> > > header is going to lead to very unexpected behaviour.
> > >
> > > The best you can hope for is that the application memset the
> > > structure to zero.
> > > But more likely it is 'random' on-stack data.
> > 0xffff is a value to disable the feature 'Primary Path Switchover'.
> > you're right that user might set it to zero unexpectly with their
> > old application rebuilt.
> >
> > A safer way is to introduce "sysctl net.sctp.ps_retrans", it won't
> > matter if users set spt_pathcpthld properly when they're not aware
> > of this feature, like "sysctl net.sctp.pF_retrans". Looks better?
> Sorry for confusing, "sysctl net.sctp.ps_retrans" is already there
> (its value is 0xffff by default),
> we just need to do this in sctp_setsockopt_paddr_thresholds():
>
> if (copy_from_user(&val, (struct sctp_paddrthlds __user *)optval,
> optlen))
> return -EFAULT;
>
> if (sock_net(sk)->sctp.ps_retrans == 0xffff)
> val.spt_pathcpthld = 0xffff;
I'm confused with the snippets, but if I got them right, this is after
dealing with proper len and could leave val.spt_pathcpthld
uninitialized if the application used the old format and sysctl is !=
0xffff.
>
> if (val.spt_pathpfthld > val.spt_pathcpthld)
> return -EINVAL;
>
> >
> > >
> > > David
> > >
> > > -
> > > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> > > Registration No: 1397386 (Wales)
>
^ permalink raw reply
* Re: [PATCH v2] vhost: block speculation of translated descriptors
From: Michael S. Tsirkin @ 2019-09-11 13:03 UTC (permalink / raw)
To: Michal Hocko; +Cc: linux-kernel, Jason Wang, kvm, virtualization, netdev
In-Reply-To: <20190911123316.GX4023@dhcp22.suse.cz>
On Wed, Sep 11, 2019 at 02:33:16PM +0200, Michal Hocko wrote:
> On Wed 11-09-19 08:25:03, Michael S. Tsirkin wrote:
> > On Wed, Sep 11, 2019 at 02:16:28PM +0200, Michal Hocko wrote:
> > > On Wed 11-09-19 08:10:00, Michael S. Tsirkin wrote:
> > > > iovec addresses coming from vhost are assumed to be
> > > > pre-validated, but in fact can be speculated to a value
> > > > out of range.
> > > >
> > > > Userspace address are later validated with array_index_nospec so we can
> > > > be sure kernel info does not leak through these addresses, but vhost
> > > > must also not leak userspace info outside the allowed memory table to
> > > > guests.
> > > >
> > > > Following the defence in depth principle, make sure
> > > > the address is not validated out of node range.
> > > >
> > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > > Acked-by: Jason Wang <jasowang@redhat.com>
> > > > Tested-by: Jason Wang <jasowang@redhat.com>
> > >
> > > no need to mark fo stable? Other spectre fixes tend to be backported
> > > even when the security implications are not really clear. The risk
> > > should be low and better to be covered in case.
> >
> > This is not really a fix - more a defence in depth thing,
> > quite similar to e.g. commit b3bbfb3fb5d25776b8e3f361d2eedaabb0b496cd
> > x86: Introduce __uaccess_begin_nospec() and uaccess_try_nospec
> > in scope.
> >
> > That one doesn't seem to be tagged for stable. Was it queued
> > there in practice?
>
> not marked for stable but it went in. At least to 4.4.
So I guess the answer is I don't know. If you feel it's
justified, then sure, feel free to forward.
--
MST
^ permalink raw reply
* [PATCH] mac80211: Do not send Layer 2 Update frame before authorization
From: Jouni Malinen @ 2019-09-11 13:03 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, David Miller, netdev, Jouni Malinen
The Layer 2 Update frame is used to update bridges when a station roams
to another AP even if that STA does not transmit any frames after the
reassociation. This behavior was described in IEEE Std 802.11F-2003 as
something that would happen based on MLME-ASSOCIATE.indication, i.e.,
before completing 4-way handshake. However, this IEEE trial-use
recommended practice document was published before RSN (IEEE Std
802.11i-2004) and as such, did not consider RSN use cases. Furthermore,
IEEE Std 802.11F-2003 was withdrawn in 2006 and as such, has not been
maintained amd should not be used anymore.
Sending out the Layer 2 Update frame immediately after association is
fine for open networks (and also when using SAE, FT protocol, or FILS
authentication when the station is actually authenticated by the time
association completes). However, it is not appropriate for cases where
RSN is used with PSK or EAP authentication since the station is actually
fully authenticated only once the 4-way handshake completes after
authentication and attackers might be able to use the unauthenticated
triggering of Layer 2 Update frame transmission to disrupt bridge
behavior.
Fix this by postponing transmission of the Layer 2 Update frame from
station entry addition to the point when the station entry is marked
authorized. Similarly, send out the VLAN binding update only if the STA
entry has already been authorized.
Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
---
net/mac80211/cfg.c | 14 ++++----------
net/mac80211/sta_info.c | 4 ++++
2 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index ed56b0c6fe19..817f37b64eb5 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1532,7 +1532,6 @@ static int ieee80211_add_station(struct wiphy *wiphy, struct net_device *dev,
struct sta_info *sta;
struct ieee80211_sub_if_data *sdata;
int err;
- int layer2_update;
if (params->vlan) {
sdata = IEEE80211_DEV_TO_SUB_IF(params->vlan);
@@ -1575,18 +1574,12 @@ static int ieee80211_add_station(struct wiphy *wiphy, struct net_device *dev,
test_sta_flag(sta, WLAN_STA_ASSOC))
rate_control_rate_init(sta);
- layer2_update = sdata->vif.type == NL80211_IFTYPE_AP_VLAN ||
- sdata->vif.type == NL80211_IFTYPE_AP;
-
err = sta_info_insert_rcu(sta);
if (err) {
rcu_read_unlock();
return err;
}
- if (layer2_update)
- cfg80211_send_layer2_update(sta->sdata->dev, sta->sta.addr);
-
rcu_read_unlock();
return 0;
@@ -1684,10 +1677,11 @@ static int ieee80211_change_station(struct wiphy *wiphy,
sta->sdata = vlansdata;
ieee80211_check_fast_xmit(sta);
- if (test_sta_flag(sta, WLAN_STA_AUTHORIZED))
+ if (test_sta_flag(sta, WLAN_STA_AUTHORIZED)) {
ieee80211_vif_inc_num_mcast(sta->sdata);
-
- cfg80211_send_layer2_update(sta->sdata->dev, sta->sta.addr);
+ cfg80211_send_layer2_update(sta->sdata->dev,
+ sta->sta.addr);
+ }
}
err = sta_apply_parameters(local, sta, params);
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index df553070206c..bd11fef2139f 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -1979,6 +1979,10 @@ int sta_info_move_state(struct sta_info *sta,
ieee80211_check_fast_xmit(sta);
ieee80211_check_fast_rx(sta);
}
+ if (sta->sdata->vif.type == NL80211_IFTYPE_AP_VLAN ||
+ sta->sdata->vif.type == NL80211_IFTYPE_AP)
+ cfg80211_send_layer2_update(sta->sdata->dev,
+ sta->sta.addr);
break;
default:
break;
--
2.20.1
^ permalink raw reply related
* Re: WARNING at net/mac80211/sta_info.c:1057 (__sta_info_destroy_part2())
From: Ben Greear @ 2019-09-11 13:03 UTC (permalink / raw)
To: Johannes Berg, Linus Torvalds
Cc: David S. Miller, Kalle Valo, linux-wireless, Netdev,
Linux List Kernel Mailing
In-Reply-To: <30679d3f86731475943856196478677e70a349a9.camel@sipsolutions.net>
On 09/11/2019 05:04 AM, Johannes Berg wrote:
> On Wed, 2019-09-11 at 12:58 +0100, Linus Torvalds wrote:
>>
>> And I didn't think about it or double-check, because the errors that
>> then followed later _looked_ like that TX power failing that I thought
>> hadn't happened.
>
> Yeah, it could be something already got stuck there, hard to say.
>
>>> Since we see that something actually did an rfkill operation. Did you
>>> push a button there?
>>
>> No, I tried to turn off and turn on Wifi manually (no button, just the
>> settings panel).
>
> That does usually also cause rfkill, so that explains how we got down
> this particular code path.
>
>> I didn't notice the WARN_ON(), I just noticed that there was no
>> networking, and "turn it off and on again" is obviously the first
>> thing to try ;)
>
> :-)
>
>> Sep 11 10:27:13 xps13 kernel: WARNING: CPU: 4 PID: 1246 at
>> net/mac80211/sta_info.c:1057 __sta_info_destroy_part2+0x147/0x150
>> [mac80211]
>>
>> but if you want full logs I can send them in private to you.
>
> No, it's fine, though maybe Kalle does - he was stepping out for a while
> but said he'd look later.
>
> This is the interesting time - 10:27:13 we get one of the first
> failures. Really the first one was this:
>
>> Sep 11 10:27:07 xps13 kernel: ath10k_pci 0000:02:00.0: wmi command 16387 timeout, restarting hardware
>
>
>> I do suspect it's atheros and suspend/resume or something. The
>> wireless clearly worked for a while after the resume, but then at some
>> point it stopped.
>
> I'm not really sure it's related to suspend/resume at all, the firmware
> seems to just have gotten stuck, and the device and firmware most likely
> got reset over the suspend/resume anyway.
>
>>> The only explanation I therefore have is that something is just taking
>>> *forever* in that code path, hence my question about timing information
>>> on the logs.
>>
>> Yeah, maybe it would time out everything eventually. But not for a
>> long time. It hadn't cleared up by
>>
>> Sep 11 10:36:21 xps13 gnome-session-f[6837]: gnome-session-failed:
>> Fatal IO error 0 (Success) on X server :0.
>
> Ok, that's way longer than I would have guessed even! That's over 9
> minutes, that'd be close to 200 commands having to be issued and timing
> out ...
>
> I don't know. What I wrote before is basically all I can say, I think
> the driver gets stuck somewhere waiting for the device "forever", and
> the stack just doesn't get to release the lock, causing all the follow-
> up problems.
It looks to me like the ath10k firmware is not responding to commands and/or
is out of its WMI tx credits. The code often takes a lock and then blocks for up to 3
or so seconds waiting for a response from the firmware, and the mac80211 calling
code is often already holding rtnl. Pretty much every mac80211 call will cause a
WMI message and thus potentially hit this timeout.
This can easily cause rtnl to be held for 3 seconds, but after that, I believe
upstream ath10k will now time out and kill the firmware and restart. (I run
a significantly modified ath10k driver, and that is how mine works, at least.)
In this case, it looks like restarting the firmware/NIC failed, and I guess
that must get it in a state where it is still blocking and trying to talk
to the firmware? Or maybe deadlock down inside ath10k driver.
For what it's worth, we see that WARN_ON often when ath10k firmware crashes, but it
seems to not be a big deal and the system normally recovers fine.
Out of curiosity, I'm interested to know what ath10k NIC chipset this is from.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* [PATCH iproute2-next] devlink: add 'reset_dev_on_drv_probe' devlink param
From: Simon Horman @ 2019-09-11 13:05 UTC (permalink / raw)
To: David Ahern
Cc: Jiri Pirko, netdev, oss-drivers, Jakub Kicinski,
Dirk van der Merwe, Simon Horman
From: Dirk van der Merwe <dirk.vandermerwe@netronome.com>
Add support for the new devlink parameter along with string to uint
conversion.
Signed-off-by: Dirk van der Merwe <dirk.vandermerwe@netronome.com>
Signed-off-by: Simon Horman <simon.horman@netronome.com>
---
Depends on devlink.h changes present in net-next commit
5bbd21df5a07 ("devlink: add 'reset_dev_on_drv_probe' param")
---
devlink/devlink.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index 2f084c020765..15877a04f5d6 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -2253,6 +2253,26 @@ static const struct param_val_conv param_val_conv[] = {
.vstr = "flash",
.vuint = DEVLINK_PARAM_FW_LOAD_POLICY_VALUE_FLASH,
},
+ {
+ .name = "reset_dev_on_drv_probe",
+ .vstr = "unknown",
+ .vuint = DEVLINK_PARAM_RESET_DEV_ON_DRV_PROBE_VALUE_UNKNOWN,
+ },
+ {
+ .name = "reset_dev_on_drv_probe",
+ .vstr = "always",
+ .vuint = DEVLINK_PARAM_RESET_DEV_ON_DRV_PROBE_VALUE_ALWAYS,
+ },
+ {
+ .name = "reset_dev_on_drv_probe",
+ .vstr = "never",
+ .vuint = DEVLINK_PARAM_RESET_DEV_ON_DRV_PROBE_VALUE_NEVER,
+ },
+ {
+ .name = "reset_dev_on_drv_probe",
+ .vstr = "disk",
+ .vuint = DEVLINK_PARAM_RESET_DEV_ON_DRV_PROBE_VALUE_DISK,
+ },
};
#define PARAM_VAL_CONV_LEN ARRAY_SIZE(param_val_conv)
--
2.11.0
^ permalink raw reply related
* Re: [PATCH] mac80211: Do not send Layer 2 Update frame before authorization
From: Johannes Berg @ 2019-09-11 13:06 UTC (permalink / raw)
To: Jouni Malinen; +Cc: linux-wireless, David Miller, netdev
In-Reply-To: <20190911130305.23704-1-jouni@codeaurora.org>
On Wed, 2019-09-11 at 16:03 +0300, Jouni Malinen wrote:
> The Layer 2 Update frame is used to update bridges when a station roams
> to another AP even if that STA does not transmit any frames after the
> reassociation. This behavior was described in IEEE Std 802.11F-2003 as
> something that would happen based on MLME-ASSOCIATE.indication, i.e.,
> before completing 4-way handshake. However, this IEEE trial-use
> recommended practice document was published before RSN (IEEE Std
> 802.11i-2004) and as such, did not consider RSN use cases. Furthermore,
> IEEE Std 802.11F-2003 was withdrawn in 2006 and as such, has not been
> maintained amd should not be used anymore.
>
> Sending out the Layer 2 Update frame immediately after association is
> fine for open networks (and also when using SAE, FT protocol, or FILS
> authentication when the station is actually authenticated by the time
> association completes). However, it is not appropriate for cases where
> RSN is used with PSK or EAP authentication since the station is actually
> fully authenticated only once the 4-way handshake completes after
> authentication and attackers might be able to use the unauthenticated
> triggering of Layer 2 Update frame transmission to disrupt bridge
> behavior.
>
> Fix this by postponing transmission of the Layer 2 Update frame from
> station entry addition to the point when the station entry is marked
> authorized. Similarly, send out the VLAN binding update only if the STA
> entry has already been authorized.
Reviewed-by: Johannes Berg <johannes@sipsolutions.net>
Dave, if you were still planning to send a pull request to Linus before
he closes the tree on Sunday this would be good to include (and we
should also backport it to stable later).
If not, I can pick it up afterwards, let me know.
Thanks,
johannes
^ permalink raw reply
* Re: [PATCH v2] vhost: block speculation of translated descriptors
From: Michal Hocko @ 2019-09-11 13:12 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: linux-kernel, Jason Wang, kvm, virtualization, netdev
In-Reply-To: <20190911085807-mutt-send-email-mst@kernel.org>
On Wed 11-09-19 09:03:10, Michael S. Tsirkin wrote:
> On Wed, Sep 11, 2019 at 02:33:16PM +0200, Michal Hocko wrote:
> > On Wed 11-09-19 08:25:03, Michael S. Tsirkin wrote:
> > > On Wed, Sep 11, 2019 at 02:16:28PM +0200, Michal Hocko wrote:
> > > > On Wed 11-09-19 08:10:00, Michael S. Tsirkin wrote:
> > > > > iovec addresses coming from vhost are assumed to be
> > > > > pre-validated, but in fact can be speculated to a value
> > > > > out of range.
> > > > >
> > > > > Userspace address are later validated with array_index_nospec so we can
> > > > > be sure kernel info does not leak through these addresses, but vhost
> > > > > must also not leak userspace info outside the allowed memory table to
> > > > > guests.
> > > > >
> > > > > Following the defence in depth principle, make sure
> > > > > the address is not validated out of node range.
> > > > >
> > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > > > Acked-by: Jason Wang <jasowang@redhat.com>
> > > > > Tested-by: Jason Wang <jasowang@redhat.com>
> > > >
> > > > no need to mark fo stable? Other spectre fixes tend to be backported
> > > > even when the security implications are not really clear. The risk
> > > > should be low and better to be covered in case.
> > >
> > > This is not really a fix - more a defence in depth thing,
> > > quite similar to e.g. commit b3bbfb3fb5d25776b8e3f361d2eedaabb0b496cd
> > > x86: Introduce __uaccess_begin_nospec() and uaccess_try_nospec
> > > in scope.
> > >
> > > That one doesn't seem to be tagged for stable. Was it queued
> > > there in practice?
> >
> > not marked for stable but it went in. At least to 4.4.
>
> So I guess the answer is I don't know. If you feel it's
> justified, then sure, feel free to forward.
Well, that obviously depends on you as a maintainer but the point is
that spectre gatgets are quite hard to find. There is a smack check
AFAIK but that generates quite some false possitives and it is PITA to
crawl through those. If you want an interesting (I am not saying
vulnerable on purpose) gatget then it would be great to mark it for
stable so all stable consumers (disclaimer: I am not one of those) and
add that really great feeling of safety ;)
So take this as my 2c
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* pull-request: mac80211-next 2019-09-11
From: Johannes Berg @ 2019-09-11 13:13 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-wireless
Hi Dave,
As detailed below, here are some more changes for -next, almost
certainly the final round since the merge window is around the
corner now.
Please pull and let me know if there's any problem.
Thanks,
johannes
The following changes since commit c76c992525245ec1c7b6738bf887c42099abab02:
nexthops: remove redundant assignment to variable err (2019-08-22 12:14:05 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git tags/mac80211-next-for-davem-2019-09-11
for you to fetch changes up to c1d3ad84eae35414b6b334790048406bd6301b12:
cfg80211: Purge frame registrations on iftype change (2019-09-11 10:45:10 +0200)
----------------------------------------------------------------
We have a number of changes, but things are settling down:
* a fix in the new 6 GHz channel support
* a fix for recent minstrel (rate control) updates
for an infinite loop
* handle interface type changes better wrt. management frame
registrations (for management frames sent to userspace)
* add in-BSS RX time to survey information
* handle HW rfkill properly if !CONFIG_RFKILL
* send deauth on IBSS station expiry, to avoid state mismatches
* handle deferred crypto tailroom updates in mac80211 better
when device restart happens
* fix a spectre-v1 - really a continuation of a previous patch
* advertise NL80211_CMD_UPDATE_FT_IES as supported if so
* add some missing parsing in VHT extended NSS support
* support HE in mac80211_hwsim
* let mac80211 drivers determine the max MTU themselves
along with the usual cleanups etc.
----------------------------------------------------------------
Arend van Spriel (1):
cfg80211: fix boundary value in ieee80211_frequency_to_channel()
Colin Ian King (1):
mac80211: minstrel_ht: fix infinite loop because supported is not being shifted
Denis Kenzior (1):
cfg80211: Purge frame registrations on iftype change
Felix Fietkau (1):
cfg80211: add local BSS receive time to survey information
Johannes Berg (4):
cfg80211: always shut down on HW rfkill
mac80211: list features in WEP/TKIP disable in better order
mac80211: remove unnecessary key condition
mac80211: IBSS: send deauth when expiring inactive STAs
Lior Cohen (1):
mac80211: clear crypto tx tailroom counter upon keys enable
Luca Coelho (1):
mac80211: don't check if key is NULL in ieee80211_key_link()
Masashi Honma (1):
nl80211: Fix possible Spectre-v1 for CQM RSSI thresholds
Matthew Wang (1):
nl80211: add NL80211_CMD_UPDATE_FT_IES to supported commands
Mordechay Goodstein (1):
mac80211: vht: add support VHT EXT NSS BW in parsing VHT
Sven Eckelmann (1):
mac80211_hwsim: Register support for HE meshpoint
Wen Gong (1):
mac80211: allow drivers to set max MTU
zhong jiang (1):
cfg80211: Do not compare with boolean in nl80211_common_reg_change_event
drivers/net/wireless/mac80211_hwsim.c | 283 +++++++++++++++++++++++-----------
include/net/cfg80211.h | 4 +
include/net/mac80211.h | 3 +
include/uapi/linux/nl80211.h | 3 +
net/mac80211/ibss.c | 8 +
net/mac80211/ieee80211_i.h | 3 +-
net/mac80211/iface.c | 2 +-
net/mac80211/key.c | 48 ++----
net/mac80211/key.h | 4 +-
net/mac80211/main.c | 1 +
net/mac80211/mlme.c | 13 +-
net/mac80211/rc80211_minstrel_ht.c | 2 +-
net/mac80211/util.c | 11 +-
net/mac80211/vht.c | 10 +-
net/wireless/core.c | 13 +-
net/wireless/core.h | 2 +-
net/wireless/nl80211.c | 17 +-
net/wireless/util.c | 3 +-
net/wireless/wext-compat.c | 5 +-
19 files changed, 274 insertions(+), 161 deletions(-)
^ permalink raw reply
* Re: WARNING at net/mac80211/sta_info.c:1057 (__sta_info_destroy_part2())
From: Linus Torvalds @ 2019-09-11 13:21 UTC (permalink / raw)
To: Ben Greear
Cc: Johannes Berg, David S. Miller, Kalle Valo, linux-wireless,
Netdev, Linux List Kernel Mailing
In-Reply-To: <2d673d55-eb27-8573-b8ae-a493335723cf@candelatech.com>
On Wed, Sep 11, 2019 at 2:03 PM Ben Greear <greearb@candelatech.com> wrote:
>
> Out of curiosity, I'm interested to know what ath10k NIC chipset this is from.
It's a Dell XPS 13 9380, with
02:00.0 Network controller: Qualcomm Atheros QCA6174 802.11ac
Wireless Network Adapter (rev 32)
Subsystem: Bigfoot Networks, Inc. Killer 1435 Wireless-AC
(numeric PCI ID 168c:003e, subsystem 1a56:143a).
The ath10k driver says
qca6174 hw3.2 target 0x05030000 chip_id 0x00340aff sub 1a56:143a
firmware ver WLAN.RM.4.4.1-00140-QCARMSWPZ-1 api 6 features
wowlan,ignore-otp,mfp crc32 29eb8ca1
board_file api 2 bmi_id N/A crc32 4ed3569e
if that tells you anything more.
Linus
^ permalink raw reply
* Re: WARNING at net/mac80211/sta_info.c:1057 (__sta_info_destroy_part2())
From: Ben Greear @ 2019-09-11 13:31 UTC (permalink / raw)
To: Linus Torvalds
Cc: Johannes Berg, David S. Miller, Kalle Valo, linux-wireless,
Netdev, Linux List Kernel Mailing
In-Reply-To: <CAHk-=wgAXAw=U_kthB9mG+MBocpawxCzo=6WDrbGgOUr+ac3CA@mail.gmail.com>
On 09/11/2019 06:21 AM, Linus Torvalds wrote:
> On Wed, Sep 11, 2019 at 2:03 PM Ben Greear <greearb@candelatech.com> wrote:
>>
>> Out of curiosity, I'm interested to know what ath10k NIC chipset this is from.
>
> It's a Dell XPS 13 9380, with
>
> 02:00.0 Network controller: Qualcomm Atheros QCA6174 802.11ac
> Wireless Network Adapter (rev 32)
> Subsystem: Bigfoot Networks, Inc. Killer 1435 Wireless-AC
>
> (numeric PCI ID 168c:003e, subsystem 1a56:143a).
>
> The ath10k driver says
>
> qca6174 hw3.2 target 0x05030000 chip_id 0x00340aff sub 1a56:143a
> firmware ver WLAN.RM.4.4.1-00140-QCARMSWPZ-1 api 6 features
> wowlan,ignore-otp,mfp crc32 29eb8ca1
> board_file api 2 bmi_id N/A crc32 4ed3569e
>
> if that tells you anything more.
That means it is something I have never used nor have firmware for, but
the WMI logic should be similar to what I described and have experienced
with other chips.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: WARNING at net/mac80211/sta_info.c:1057 (__sta_info_destroy_part2())
From: Kalle Valo @ 2019-09-11 13:32 UTC (permalink / raw)
To: Johannes Berg
Cc: Linus Torvalds, David S. Miller, linux-wireless, Netdev,
Linux List Kernel Mailing
In-Reply-To: <30679d3f86731475943856196478677e70a349a9.camel@sipsolutions.net>
Johannes Berg <johannes@sipsolutions.net> writes:
>> Sep 11 10:27:13 xps13 kernel: WARNING: CPU: 4 PID: 1246 at
>> net/mac80211/sta_info.c:1057 __sta_info_destroy_part2+0x147/0x150
>> [mac80211]
>>
>> but if you want full logs I can send them in private to you.
>
> No, it's fine, though maybe Kalle does - he was stepping out for a while
> but said he'd look later.
Linus, it would help if you could send me full logs with timestamps.
Also if you can, please grep your logs to see if these wmi timeouts have
happened before.
--
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH v2] vhost: block speculation of translated descriptors
From: Michael S. Tsirkin @ 2019-09-11 13:51 UTC (permalink / raw)
To: Michal Hocko; +Cc: linux-kernel, Jason Wang, kvm, virtualization, netdev
In-Reply-To: <20190911131235.GZ4023@dhcp22.suse.cz>
On Wed, Sep 11, 2019 at 03:12:35PM +0200, Michal Hocko wrote:
> On Wed 11-09-19 09:03:10, Michael S. Tsirkin wrote:
> > On Wed, Sep 11, 2019 at 02:33:16PM +0200, Michal Hocko wrote:
> > > On Wed 11-09-19 08:25:03, Michael S. Tsirkin wrote:
> > > > On Wed, Sep 11, 2019 at 02:16:28PM +0200, Michal Hocko wrote:
> > > > > On Wed 11-09-19 08:10:00, Michael S. Tsirkin wrote:
> > > > > > iovec addresses coming from vhost are assumed to be
> > > > > > pre-validated, but in fact can be speculated to a value
> > > > > > out of range.
> > > > > >
> > > > > > Userspace address are later validated with array_index_nospec so we can
> > > > > > be sure kernel info does not leak through these addresses, but vhost
> > > > > > must also not leak userspace info outside the allowed memory table to
> > > > > > guests.
> > > > > >
> > > > > > Following the defence in depth principle, make sure
> > > > > > the address is not validated out of node range.
> > > > > >
> > > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > > > > Acked-by: Jason Wang <jasowang@redhat.com>
> > > > > > Tested-by: Jason Wang <jasowang@redhat.com>
> > > > >
> > > > > no need to mark fo stable? Other spectre fixes tend to be backported
> > > > > even when the security implications are not really clear. The risk
> > > > > should be low and better to be covered in case.
> > > >
> > > > This is not really a fix - more a defence in depth thing,
> > > > quite similar to e.g. commit b3bbfb3fb5d25776b8e3f361d2eedaabb0b496cd
> > > > x86: Introduce __uaccess_begin_nospec() and uaccess_try_nospec
> > > > in scope.
> > > >
> > > > That one doesn't seem to be tagged for stable. Was it queued
> > > > there in practice?
> > >
> > > not marked for stable but it went in. At least to 4.4.
> >
> > So I guess the answer is I don't know. If you feel it's
> > justified, then sure, feel free to forward.
>
> Well, that obviously depends on you as a maintainer but the point is
> that spectre gatgets are quite hard to find. There is a smack check
> AFAIK but that generates quite some false possitives and it is PITA to
> crawl through those. If you want an interesting (I am not saying
> vulnerable on purpose) gatget then it would be great to mark it for
> stable so all stable consumers (disclaimer: I am not one of those) and
> add that really great feeling of safety ;)
>
> So take this as my 2c
OK it seems security@kernel.org is the way to handle these things.
I'll try that.
> --
> Michal Hocko
> SUSE Labs
^ permalink raw reply
* Re: [PATCH v2] vhost: block speculation of translated descriptors
From: Michael S. Tsirkin @ 2019-09-11 13:52 UTC (permalink / raw)
To: linux-kernel; +Cc: Jason Wang, kvm, virtualization, netdev, security
In-Reply-To: <20190911120908.28410-1-mst@redhat.com>
On Wed, Sep 11, 2019 at 08:10:00AM -0400, Michael S. Tsirkin wrote:
> iovec addresses coming from vhost are assumed to be
> pre-validated, but in fact can be speculated to a value
> out of range.
>
> Userspace address are later validated with array_index_nospec so we can
> be sure kernel info does not leak through these addresses, but vhost
> must also not leak userspace info outside the allowed memory table to
> guests.
>
> Following the defence in depth principle, make sure
> the address is not validated out of node range.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Acked-by: Jason Wang <jasowang@redhat.com>
> Tested-by: Jason Wang <jasowang@redhat.com>
> ---
Cc: security@kernel.org
Pls advise on whether you'd like me to merge this directly,
Cc stable, or handle it in some other way.
> changes from v1: fix build on 32 bit
>
> drivers/vhost/vhost.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 5dc174ac8cac..34ea219936e3 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -2071,8 +2071,10 @@ static int translate_desc(struct vhost_virtqueue *vq, u64 addr, u32 len,
> _iov = iov + ret;
> size = node->size - addr + node->start;
> _iov->iov_len = min((u64)len - s, size);
> - _iov->iov_base = (void __user *)(unsigned long)
> - (node->userspace_addr + addr - node->start);
> + _iov->iov_base = (void __user *)
> + ((unsigned long)node->userspace_addr +
> + array_index_nospec((unsigned long)(addr - node->start),
> + node->size));
> s += size;
> addr += size;
> ++ret;
> --
> MST
^ permalink raw reply
* Re: [PATCH v3 0/5] Introduce variable length mdev alias
From: Alex Williamson @ 2019-09-11 13:56 UTC (permalink / raw)
To: Parav Pandit
Cc: Jiri Pirko, kwankhede@nvidia.com, cohuck@redhat.com,
davem@davemloft.net, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <AM0PR05MB4866F76F807409ED887537D7D1B70@AM0PR05MB4866.eurprd05.prod.outlook.com>
On Mon, 9 Sep 2019 20:42:32 +0000
Parav Pandit <parav@mellanox.com> wrote:
> Hi Alex,
>
> > -----Original Message-----
> > From: Parav Pandit <parav@mellanox.com>
> > Sent: Sunday, September 1, 2019 11:25 PM
> > To: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> > kwankhede@nvidia.com; cohuck@redhat.com; davem@davemloft.net
> > Cc: kvm@vger.kernel.org; linux-kernel@vger.kernel.org;
> > netdev@vger.kernel.org; Parav Pandit <parav@mellanox.com>
> > Subject: [PATCH v3 0/5] Introduce variable length mdev alias
> >
> > To have consistent naming for the netdevice of a mdev and to have consistent
> > naming of the devlink port [1] of a mdev, which is formed using
> > phys_port_name of the devlink port, current UUID is not usable because UUID
> > is too long.
> >
> > UUID in string format is 36-characters long and in binary 128-bit.
> > Both formats are not able to fit within 15 characters limit of netdev name.
> >
> > It is desired to have mdev device naming consistent using UUID.
> > So that widely used user space framework such as ovs [2] can make use of
> > mdev representor in similar way as PCIe SR-IOV VF and PF representors.
> >
> > Hence,
> > (a) mdev alias is created which is derived using sha1 from the mdev name.
> > (b) Vendor driver describes how long an alias should be for the child mdev
> > created for a given parent.
> > (c) Mdev aliases are unique at system level.
> > (d) alias is created optionally whenever parent requested.
> > This ensures that non networking mdev parents can function without alias
> > creation overhead.
> >
> > This design is discussed at [3].
> >
> > An example systemd/udev extension will have,
> >
> > 1. netdev name created using mdev alias available in sysfs.
> >
> > mdev UUID=83b8f4f2-509f-382f-3c1e-e6bfe0fa1001
> > mdev 12 character alias=cd5b146a80a5
> >
> > netdev name of this mdev = enmcd5b146a80a5 Here en = Ethernet link m =
> > mediated device
> >
> > 2. devlink port phys_port_name created using mdev alias.
> > devlink phys_port_name=pcd5b146a80a5
> >
> > This patchset enables mdev core to maintain unique alias for a mdev.
> >
> > Patch-1 Introduces mdev alias using sha1.
> > Patch-2 Ensures that mdev alias is unique in a system.
> > Patch-3 Exposes mdev alias in a sysfs hirerchy, update Documentation
> > Patch-4 Introduces mdev_alias() API.
> > Patch-5 Extends mtty driver to optionally provide alias generation.
> > This also enables to test UUID based sha1 collision and trigger error handling
> > for duplicate sha1 results.
> >
> > [1] http://man7.org/linux/man-pages/man8/devlink-port.8.html
> > [2] https://docs.openstack.org/os-vif/latest/user/plugins/ovs.html
> > [3] https://patchwork.kernel.org/cover/11084231/
> >
> > ---
> > Changelog:
> > v2->v3:
> > - Addressed comment from Yunsheng Lin
> > - Changed strcmp() ==0 to !strcmp()
> > - Addressed comment from Cornelia Hunk
> > - Merged sysfs Documentation patch with syfs patch
> > - Added more description for alias return value
>
> Did you get a chance review this updated series?
> I addressed Cornelia's and yours comment.
> I do not think allocating alias memory twice, once for comparison and
> once for storing is good idea or moving alias generation logic inside
> the mdev_list_lock(). So I didn't address that suggestion of
> Cornelia.
Sorry, I'm at LPC this week. I agree, I don't think the double
allocation is necessary, I thought the comment was sufficient to
clarify null'ing the variable. It's awkward, but seems correct.
I'm not sure what we do with this patch series though, has the real
consumer of this even been proposed? It feels optimistic to include at
this point. We've used the sample driver as a placeholder in the past
for mdev_uuid(), but we arrived at that via a conversion rather than
explicitly adding the API. Please let me know where the consumer
patches stand, perhaps it would make more sense for them to go in
together rather than risk adding an unused API. Thanks,
Alex
> > v1->v2:
> > - Corrected a typo from 'and' to 'an'
> > - Addressed comments from Alex Williamson
> > - Kept mdev_device naturally aligned
> > - Added error checking for crypt_*() calls
> > - Moved alias NULL check at beginning
> > - Added mdev_alias() API
> > - Updated mtty driver to show example mdev_alias() usage
> > - Changed return type of generate_alias() from int to char*
> > v0->v1:
> > - Addressed comments from Alex Williamson, Cornelia Hunk and Mark
> > Bloch
> > - Moved alias length check outside of the parent lock
> > - Moved alias and digest allocation from kvzalloc to kzalloc
> > - &alias[0] changed to alias
> > - alias_length check is nested under get_alias_length callback
> > check
> > - Changed comments to start with an empty line
> > - Added comment where alias memory ownership is handed over to mdev
> > device
> > - Fixed cleaunup of hash if mdev_bus_register() fails
> > - Updated documentation for new sysfs alias file
> > - Improved commit logs to make description more clear
> > - Fixed inclusiong of alias for NULL check
> > - Added ratelimited debug print for sha1 hash collision error
> >
> > Parav Pandit (5):
> > mdev: Introduce sha1 based mdev alias
> > mdev: Make mdev alias unique among all mdevs
> > mdev: Expose mdev alias in sysfs tree
> > mdev: Introduce an API mdev_alias
> > mtty: Optionally support mtty alias
> >
> > .../driver-api/vfio-mediated-device.rst | 9 ++
> > drivers/vfio/mdev/mdev_core.c | 142
> > +++++++++++++++++- drivers/vfio/mdev/mdev_private.h
> > | 5 +- drivers/vfio/mdev/mdev_sysfs.c | 26 +++-
> > include/linux/mdev.h | 5 +
> > samples/vfio-mdev/mtty.c | 13 ++
> > 6 files changed, 190 insertions(+), 10 deletions(-)
> >
> > --
> > 2.19.2
>
^ permalink raw reply
* Re: pull-request: mac80211-next 2019-09-11
From: David Miller @ 2019-09-11 13:57 UTC (permalink / raw)
To: johannes; +Cc: netdev, linux-wireless
In-Reply-To: <20190911131326.24032-1-johannes@sipsolutions.net>
From: Johannes Berg <johannes@sipsolutions.net>
Date: Wed, 11 Sep 2019 15:13:25 +0200
> As detailed below, here are some more changes for -next, almost
> certainly the final round since the merge window is around the
> corner now.
>
> Please pull and let me know if there's any problem.
Pulled, thanks Johannes.
^ permalink raw reply
* Re: [PATCH] mac80211: Do not send Layer 2 Update frame before authorization
From: David Miller @ 2019-09-11 13:59 UTC (permalink / raw)
To: johannes; +Cc: jouni, linux-wireless, netdev
In-Reply-To: <d0de07f0918863c8bc9bebccd8c6a7402a2ad173.camel@sipsolutions.net>
From: Johannes Berg <johannes@sipsolutions.net>
Date: Wed, 11 Sep 2019 15:06:03 +0200
> On Wed, 2019-09-11 at 16:03 +0300, Jouni Malinen wrote:
>> The Layer 2 Update frame is used to update bridges when a station roams
>> to another AP even if that STA does not transmit any frames after the
>> reassociation. This behavior was described in IEEE Std 802.11F-2003 as
>> something that would happen based on MLME-ASSOCIATE.indication, i.e.,
>> before completing 4-way handshake. However, this IEEE trial-use
>> recommended practice document was published before RSN (IEEE Std
>> 802.11i-2004) and as such, did not consider RSN use cases. Furthermore,
>> IEEE Std 802.11F-2003 was withdrawn in 2006 and as such, has not been
>> maintained amd should not be used anymore.
>>
>> Sending out the Layer 2 Update frame immediately after association is
>> fine for open networks (and also when using SAE, FT protocol, or FILS
>> authentication when the station is actually authenticated by the time
>> association completes). However, it is not appropriate for cases where
>> RSN is used with PSK or EAP authentication since the station is actually
>> fully authenticated only once the 4-way handshake completes after
>> authentication and attackers might be able to use the unauthenticated
>> triggering of Layer 2 Update frame transmission to disrupt bridge
>> behavior.
>>
>> Fix this by postponing transmission of the Layer 2 Update frame from
>> station entry addition to the point when the station entry is marked
>> authorized. Similarly, send out the VLAN binding update only if the STA
>> entry has already been authorized.
>
> Reviewed-by: Johannes Berg <johannes@sipsolutions.net>
>
> Dave, if you were still planning to send a pull request to Linus before
> he closes the tree on Sunday this would be good to include (and we
> should also backport it to stable later).
>
> If not, I can pick it up afterwards, let me know.
Ok I applied this directly, thanks.
^ permalink raw reply
* Re: [PATCH net] net/rds: An rds_sock is added too early to the hash table
From: David Miller @ 2019-09-11 14:06 UTC (permalink / raw)
To: ka-cheong.poon; +Cc: netdev, santosh.shilimkar, rds-devel
In-Reply-To: <1568195885-6285-1-git-send-email-ka-cheong.poon@oracle.com>
From: Ka-Cheong Poon <ka-cheong.poon@oracle.com>
Date: Wed, 11 Sep 2019 02:58:05 -0700
> In rds_bind(), an rds_sock is added to the RDS bind hash table before
> rs_transport is set. This means that the socket can be found by the
> receive code path when rs_transport is NULL. And the receive code
> path de-references rs_transport for congestion update check. This can
> cause a panic. An rds_sock should not be added to the bind hash table
> before all the needed fields are set.
>
> Reported-by: syzbot+4b4f8163c2e246df3c4c@syzkaller.appspotmail.com
> Signed-off-by: Ka-Cheong Poon <ka-cheong.poon@oracle.com>
Applied.
^ permalink raw reply
* Re: [PATCH] NFC: st95hf: fix spelling mistake "receieve" -> "receive"
From: David Miller @ 2019-09-11 14:07 UTC (permalink / raw)
To: colin.king; +Cc: gregkh, netdev, kernel-janitors, linux-kernel
In-Reply-To: <20190911103848.17966-1-colin.king@canonical.com>
From: Colin King <colin.king@canonical.com>
Date: Wed, 11 Sep 2019 11:38:48 +0100
> From: Colin Ian King <colin.king@canonical.com>
>
> There is a spelling mistake in a dev_err message. Fix it.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Applied.
^ permalink raw reply
* [PATCH] net: hns3: fix spelling mistake "undeflow" -> "underflow"
From: Colin King @ 2019-09-11 14:08 UTC (permalink / raw)
To: Yisen Zhuang, Salil Mehta, David S . Miller, Huazhong Tan, netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
There is a spelling mistake in a .msg literal string. Fix it.
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_err.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_err.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_err.c
index 58c6231aaa00..87dece0e745d 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_err.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_err.c
@@ -98,7 +98,7 @@ static const struct hclge_hw_error hclge_igu_egu_tnl_int[] = {
.reset_level = HNAE3_GLOBAL_RESET },
{ .int_msk = BIT(1), .msg = "rx_stp_fifo_overflow",
.reset_level = HNAE3_GLOBAL_RESET },
- { .int_msk = BIT(2), .msg = "rx_stp_fifo_undeflow",
+ { .int_msk = BIT(2), .msg = "rx_stp_fifo_underflow",
.reset_level = HNAE3_GLOBAL_RESET },
{ .int_msk = BIT(3), .msg = "tx_buf_overflow",
.reset_level = HNAE3_GLOBAL_RESET },
--
2.20.1
^ permalink raw reply related
* Re: [PATCH net-next 0/2] devlink: add unknown 'fw_load_policy' value
From: David Miller @ 2019-09-11 14:10 UTC (permalink / raw)
To: simon.horman; +Cc: jakub.kicinski, netdev, oss-drivers, dirk.vandermerwe
In-Reply-To: <20190911110833.9005-1-simon.horman@netronome.com>
From: Simon Horman <simon.horman@netronome.com>
Date: Wed, 11 Sep 2019 12:08:31 +0100
> Dirk says:
>
> Recently we added an unknown value for the 'reset_dev_on_drv_probe' devlink
> parameter. Extend the 'fw_load_policy' parameter in the same way.
>
> The only driver that uses this right now is the nfp driver.
Series applied to net-next, thanks.
^ 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