* Instability of i40e driver on 4.9 kernel
From: Pavlos Parissis @ 2017-10-19 23:02 UTC (permalink / raw)
To: netdev@vger.kernel.org, intel-wired-lan@lists.osuosl.org; +Cc: Alexander Duyck
[-- Attachment #1.1: Type: text/plain, Size: 1573 bytes --]
Hi all,
We have been running 4.9 kernels for several months on CentOS 7.3 and for few weeks on CentOS 7.4,
and, after we replaced 10GbE cobber cards(X540-AT2 with ixgbe driver) with X710 10GbE SFP cards
using i40e driver, we noticed sever instabilities on our servers.
On several servers the links were marked down and up again, without any obvious reasons expect a lot
of errors on kernel.log. We run Bird Internet daemon on our servers in order to establish BGP
peerings with routers and we have observed flapping on BGP peerings. At the same time we had BGP
peering stabilities issues we had kernel errors. We decided to go back to 3.10 kernel from CentOS,
but that process wasn't smooth as latest firmware gave us problems with speed detection. We rolled
back to two version old and speed detection issue was resolved. We have been running 3.10 several
weeks without any problems. Even we want certain functionality from kernel 4.9, we decided to switch
back to 3.10 as stability of our systems has higher priority.
I need to mention that in all occurrences of the issue we didn't see any anomalies, such DDOS
attacks and etc.
I have opened https://communities.intel.com/message/501682#501682 and there you can find all the
error messages and other information.
Since we noticed the issues, I have been following netdev ML and I know that there are a lot of
improvements/patched queued up for 4.14 and I am hoping those patches fix our issue and most
importantly are sent to linux-stable for inclusion in 4.9 kernel.
Cheers,
Pavlos
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: RFC(v2): Audit Kernel Container IDs
From: Aleksa Sarai @ 2017-10-19 23:11 UTC (permalink / raw)
To: Richard Guy Briggs, Steve Grubb
Cc: mszeredi-H+wXaHxf7aLQT0dZR+AlfA, trondmy-7I+n7zu2hftEKMMhf/gKZA,
Andy Lutomirski, jlayton-H+wXaHxf7aLQT0dZR+AlfA,
Carlos O'Donell, Linux API, Linux Containers, Paul Moore,
Linux Kernel, Eric Paris, Al Viro, David Howells, Linux Audit,
Simo Sorce, Linux Network Development, Linux FS Devel,
cgroups-u79uwXL29TY76Z2rM5mHXA, Eric W. Biederman
In-Reply-To: <20171019195747.4ssujtaj3f5ipsoh-bcJWsdo4jJjeVoXN4CMphl7TgLCtbB0G@public.gmane.org>
>>> The registration is a pseudo filesystem (proc, since PID tree already
>>> exists) write of a u8[16] UUID representing the container ID to a file
>>> representing a process that will become the first process in a new
>>> container. This write might place restrictions on mount namespaces
>>> required to define a container, or at least careful checking of
>>> namespaces in the kernel to verify permissions of the orchestrator so it
>>> can't change its own container ID. A bind mount of nsfs may be
>>> necessary in the container orchestrator's mntNS.
>>> Note: Use a 128-bit scalar rather than a string to make compares faster
>>> and simpler.
>>>
>>> Require a new CAP_CONTAINER_ADMIN to be able to carry out the
>>> registration.
>>
>> Wouldn't CAP_AUDIT_WRITE be sufficient? After all, this is for auditing.
>
> No, because then any process with that capability (vsftpd) could change
> its own container ID. This is discussed more in other parts of the
> thread...
Not if we make the container ID append-only (to support nesting), or
write-once (the other idea thrown around). In that case, you can't move
"out" from a particular container ID, you can only go "deeper". These
semantics don't make sense for generic containers, but since the point
of this facility is *specifically* for audit I imagine that not being
able to move a process from a sub-container's ID is a benefit.
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
https://www.cyphar.com/
^ permalink raw reply
* Re: RFC(v2): Audit Kernel Container IDs
From: Aleksa Sarai @ 2017-10-19 23:15 UTC (permalink / raw)
To: Richard Guy Briggs, Steve Grubb
Cc: cgroups, mszeredi, David Howells, Simo Sorce, jlayton,
Carlos O'Donell, Linux API, Linux Containers, Linux Kernel,
Paul Moore, Linux Audit, Al Viro, Andy Lutomirski, Eric Paris,
Linux FS Devel, trondmy, Linux Network Development,
Eric W. Biederman
In-Reply-To: <8f495870-dd6c-23b9-b82b-4228a441c729@suse.de>
>>>> The registration is a pseudo filesystem (proc, since PID tree already
>>>> exists) write of a u8[16] UUID representing the container ID to a file
>>>> representing a process that will become the first process in a new
>>>> container. This write might place restrictions on mount namespaces
>>>> required to define a container, or at least careful checking of
>>>> namespaces in the kernel to verify permissions of the orchestrator
>>>> so it
>>>> can't change its own container ID. A bind mount of nsfs may be
>>>> necessary in the container orchestrator's mntNS.
>>>> Note: Use a 128-bit scalar rather than a string to make compares faster
>>>> and simpler.
>>>>
>>>> Require a new CAP_CONTAINER_ADMIN to be able to carry out the
>>>> registration.
>>>
>>> Wouldn't CAP_AUDIT_WRITE be sufficient? After all, this is for auditing.
>>
>> No, because then any process with that capability (vsftpd) could change
>> its own container ID. This is discussed more in other parts of the
>> thread...
>
> Not if we make the container ID append-only (to support nesting), or
> write-once (the other idea thrown around). In that case, you can't move
> "out" from a particular container ID, you can only go "deeper". These
> semantics don't make sense for generic containers, but since the point
> of this facility is *specifically* for audit I imagine that not being
> able to move a process from a sub-container's ID is a benefit.
[This assumes it's CAP_AUDIT_CONTROL which is what we are discussing in
a sister thread.]
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
https://www.cyphar.com/
^ permalink raw reply
* Re: [PATCH 18/27] bpf: Restrict kernel image access functions when the kernel is locked down
From: Alexei Starovoitov @ 2017-10-19 23:31 UTC (permalink / raw)
To: David Howells
Cc: jlee-IBi9RG/b67k, linux-security-module-u79uwXL29TY76Z2rM5mHXA,
gnomes-qBU/x9rampVanCEyBjwyrvXRex20P6io,
linux-efi-u79uwXL29TY76Z2rM5mHXA,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
jforbes-H+wXaHxf7aLQT0dZR+AlfA, Daniel Borkmann, David S. Miller,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <482.1508453314-S6HVgzuS8uM4Awkfq6JHfwNdhmdF6hFW@public.gmane.org>
On Thu, Oct 19, 2017 at 11:48:34PM +0100, David Howells wrote:
> Alexei Starovoitov <alexei.starovoitov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
> > > @@ -65,6 +65,11 @@ BPF_CALL_3(bpf_probe_read, void *, dst, u32, size, const void *, unsafe_ptr)
> > > {
> > > int ret;
> > >
> > > + if (kernel_is_locked_down("BPF")) {
> > > + memset(dst, 0, size);
> > > + return -EPERM;
> > > + }
> >
> > That doesn't help the lockdown purpose.
> > If you don't trust the root the only way to prevent bpf read
> > memory is to disable the whole thing.
> > Have a single check in sys_bpf() to disallow everything if kernel_is_locked_down()
> > and don't add overhead to critical path like bpf_probe_read().
>
> TBH, I've no idea how bpf does anything, so I can't say whether this is
> better, overkill or insufficient.
ok. To make it clear:
Nacked-by: Alexei Starovoitov <ast-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
For the current patch.
Unnecessary checks for no good reason in performance critical
functions are not acceptable.
^ permalink raw reply
* Re: [PATCH 32/58] isdn/gigaset: Convert timers to use timer_setup()
From: Paul Bolle @ 2017-10-19 23:31 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Linus Torvalds, Kees Cook, David S. Miller, Karsten Keil,
Johan Hovold, gigaset307x-common, netdev, linux-kernel
In-Reply-To: <alpine.DEB.2.20.1710200025470.2054@nanos>
[CC-ing Linus because I quote him.]
On Fri, 2017-10-20 at 00:28 +0200, Thomas Gleixner wrote:
> Well, that does not explain why
>
> drivers->cs + i
>
> would be corrupted. That would require that this cs -> urb link points at
> driver magically and then wreckages that driver data structure. Might be
> the case, but if so then there are dragons burried somehwere
Let's assume dragons are buried somewhere.
We need users to show us that they met a dragon, right? (I care little about
dragons no-one ever stumbles upon.)
In the explanation of commit 9f5af546e6ac ("isdn/i4l: fix buffer overflow")
Linus added:
[ ISDN seems to be effectively unmaintained, and the I4L driver in
particular is long deprecated, but in case somebody uses this..
- Linus ]
ISDN is pretty niche. So it's no surprise that in mainline it's divided into
three parts: I4L, CAPI, and mISDN.
Arnd Bergmann has suggested more than once to move I4L to staging. (As far as
I know, moving drivers to staging effectively means removing those drivers,
but anyhow.) I'd say we'd just should do that. The stuff has been deemed
deprecated since basically forever.
I never cared about mISDN, but as far as I can see mISDN has quietly left
mainline.
The only actively maintained CAPI drivers are gigaset's drivers. But I'm
afraid maintaining gigaset basically means seeing treewide cleanups fly by and
keeping the various fuzzers happy. I don't mind, and I could keep on doing
that for years. But still, I'd love to hear someone say: yes, I still care
about mainline ISDN.
Does that person still exists?
Thanks,
Paul Bolle
^ permalink raw reply
* [PATCH] net: ethtool: remove error check for legacy setting transceiver type
From: Niklas Söderlund @ 2017-10-19 23:32 UTC (permalink / raw)
To: David S. Miller, Florian Fainelli, netdev
Cc: linux-renesas-soc, Geert Uytterhoeven, Renjith R V,
Niklas Söderlund
Commit 9cab88726929605 ("net: ethtool: Add back transceiver type")
restores the transceiver type to struct ethtool_link_settings and
convert_link_ksettings_to_legacy_settings() but forgets to remove the
error check for the same in convert_legacy_settings_to_link_ksettings().
This prevents older versions of ethtool to change link settings.
# ethtool --version
ethtool version 3.16
# ethtool -s eth0 autoneg on speed 100 duplex full
Cannot set new settings: Invalid argument
not setting speed
not setting duplex
not setting autoneg
While newer versions of ethtool works.
# ethtool --version
ethtool version 4.10
# ethtool -s eth0 autoneg on speed 100 duplex full
[ 57.703268] sh-eth ee700000.ethernet eth0: Link is Down
[ 59.618227] sh-eth ee700000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx
Fixes: 19cab88726929605 ("net: ethtool: Add back transceiver type")
Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
net/core/ethtool.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
Hi,
The commit 9cab88726929605 ("net: ethtool: Add back transceiver type")
itself is not enough to trigger the error, the commit after it is also
needed ceb628134a75564d ("net: phy: Keep reporting transceiver type").
This is because the later sets cmd->base.transceiver so that the type
can be reported and the fault triggered. I hope however I did the
correct thing with the Fixes tag.
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 3228411ada0fa77e..9a9a3d77e3274fc3 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -436,7 +436,7 @@ bool ethtool_convert_link_mode_to_legacy_u32(u32 *legacy_u32,
EXPORT_SYMBOL(ethtool_convert_link_mode_to_legacy_u32);
/* return false if legacy contained non-0 deprecated fields
- * transceiver/maxtxpkt/maxrxpkt. rest of ksettings always updated
+ * maxtxpkt/maxrxpkt. rest of ksettings always updated
*/
static bool
convert_legacy_settings_to_link_ksettings(
@@ -451,8 +451,7 @@ convert_legacy_settings_to_link_ksettings(
* deprecated legacy fields, and they should not use
* %ETHTOOL_GLINKSETTINGS/%ETHTOOL_SLINKSETTINGS
*/
- if (legacy_settings->transceiver ||
- legacy_settings->maxtxpkt ||
+ if (legacy_settings->maxtxpkt ||
legacy_settings->maxrxpkt)
retval = false;
--
2.14.2
^ permalink raw reply related
* Re: [PATCH] net: ethtool: remove error check for legacy setting transceiver type
From: Florian Fainelli @ 2017-10-19 23:38 UTC (permalink / raw)
To: Niklas Söderlund, David S. Miller, netdev
Cc: linux-renesas-soc, Geert Uytterhoeven, Renjith R V
In-Reply-To: <20171019233208.15444-1-niklas.soderlund+renesas@ragnatech.se>
On 10/19/2017 04:32 PM, Niklas Söderlund wrote:
> Commit 9cab88726929605 ("net: ethtool: Add back transceiver type")
> restores the transceiver type to struct ethtool_link_settings and
> convert_link_ksettings_to_legacy_settings() but forgets to remove the
> error check for the same in convert_legacy_settings_to_link_ksettings().
> This prevents older versions of ethtool to change link settings.
>
> # ethtool --version
> ethtool version 3.16
>
> # ethtool -s eth0 autoneg on speed 100 duplex full
> Cannot set new settings: Invalid argument
> not setting speed
> not setting duplex
> not setting autoneg
>
> While newer versions of ethtool works.
>
> # ethtool --version
> ethtool version 4.10
>
> # ethtool -s eth0 autoneg on speed 100 duplex full
> [ 57.703268] sh-eth ee700000.ethernet eth0: Link is Down
> [ 59.618227] sh-eth ee700000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx
>
> Fixes: 19cab88726929605 ("net: ethtool: Add back transceiver type")
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Good catch, thanks Niklas! Surprisingly I was also testing with an older
version of ethtool but I realize I was mostly testing the "get" part and
not the "set" part, so clearly an oversight here...
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> net/core/ethtool.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> Hi,
>
> The commit 9cab88726929605 ("net: ethtool: Add back transceiver type")
> itself is not enough to trigger the error, the commit after it is also
> needed ceb628134a75564d ("net: phy: Keep reporting transceiver type").
> This is because the later sets cmd->base.transceiver so that the type
> can be reported and the fault triggered. I hope however I did the
> correct thing with the Fixes tag.
Both having made it in the same release, that should be fine, thanks!
>
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index 3228411ada0fa77e..9a9a3d77e3274fc3 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -436,7 +436,7 @@ bool ethtool_convert_link_mode_to_legacy_u32(u32 *legacy_u32,
> EXPORT_SYMBOL(ethtool_convert_link_mode_to_legacy_u32);
>
> /* return false if legacy contained non-0 deprecated fields
> - * transceiver/maxtxpkt/maxrxpkt. rest of ksettings always updated
> + * maxtxpkt/maxrxpkt. rest of ksettings always updated
> */
> static bool
> convert_legacy_settings_to_link_ksettings(
> @@ -451,8 +451,7 @@ convert_legacy_settings_to_link_ksettings(
> * deprecated legacy fields, and they should not use
> * %ETHTOOL_GLINKSETTINGS/%ETHTOOL_SLINKSETTINGS
> */
> - if (legacy_settings->transceiver ||
> - legacy_settings->maxtxpkt ||
> + if (legacy_settings->maxtxpkt ||
> legacy_settings->maxrxpkt)
> retval = false;
>
>
--
Florian
^ permalink raw reply
* Re: [PATCH net-next 0/8] tools: bpftool: add a "version" command, and fix several items
From: Alexei Starovoitov @ 2017-10-19 23:42 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, oss-drivers
In-Reply-To: <20171019224626.31608-1-jakub.kicinski@netronome.com>
On Thu, Oct 19, 2017 at 03:46:18PM -0700, Jakub Kicinski wrote:
> Quentin says:
>
> The first seven patches of this series bring several minor fixes to
> bpftool. Please see individual commit logs for details.
>
> Last patch adds a "version" commands to bpftool, which is in fact the
> version of the kernel from which it was compiled.
lgtm
Acked-by: Alexei Starovoitov <ast@kernel.org>
^ permalink raw reply
* Re: [PATCH net-next v7 3/5] security: bpf: Add LSM hooks for bpf object related syscall
From: James Morris @ 2017-10-20 0:54 UTC (permalink / raw)
To: Chenbo Feng
Cc: netdev, SELinux, linux-security-module, Jeffrey Vander Stoep,
Alexei Starovoitov, lorenzo, Daniel Borkmann, Stephen Smalley,
Paul Moore, Chenbo Feng
In-Reply-To: <20171018200026.146093-4-chenbofeng.kernel@gmail.com>
On Wed, 18 Oct 2017, Chenbo Feng wrote:
> From: Chenbo Feng <fengc@google.com>
>
> Introduce several LSM hooks for the syscalls that will allow the
> userspace to access to eBPF object such as eBPF programs and eBPF maps.
> The security check is aimed to enforce a per object security protection
> for eBPF object so only processes with the right priviliges can
> read/write to a specific map or use a specific eBPF program. Besides
> that, a general security hook is added before the multiplexer of bpf
> syscall to check the cmd and the attribute used for the command. The
> actual security module can decide which command need to be checked and
> how the cmd should be checked.
>
> Signed-off-by: Chenbo Feng <fengc@google.com>
Acked-by: James Morris <james.l.morris@oracle.com>
--
James Morris
<james.l.morris@oracle.com>
^ permalink raw reply
* Re: [PATCH net-next v7 4/5] selinux: bpf: Add selinux check for eBPF syscall operations
From: James Morris @ 2017-10-20 0:57 UTC (permalink / raw)
To: Chenbo Feng
Cc: netdev, SELinux, linux-security-module, Jeffrey Vander Stoep,
Alexei Starovoitov, lorenzo, Daniel Borkmann, Stephen Smalley,
Paul Moore, Chenbo Feng
In-Reply-To: <20171018200026.146093-5-chenbofeng.kernel@gmail.com>
On Wed, 18 Oct 2017, Chenbo Feng wrote:
> From: Chenbo Feng <fengc@google.com>
>
> Implement the actual checks introduced to eBPF related syscalls. This
> implementation use the security field inside bpf object to store a sid that
> identify the bpf object. And when processes try to access the object,
> selinux will check if processes have the right privileges. The creation
> of eBPF object are also checked at the general bpf check hook and new
> cmd introduced to eBPF domain can also be checked there.
>
> Signed-off-by: Chenbo Feng <fengc@google.com>
> Acked-by: Alexei Starovoitov <ast@kernel.org>
Reviewed-by: James Morris <james.l.morris@oracle.com>
--
James Morris
<james.l.morris@oracle.com>
^ permalink raw reply
* Re: [PATCH net-next v2 5/6] devlink: Adding num MSI-X vectors per VF NVRAM config param
From: Florian Fainelli @ 2017-10-20 1:01 UTC (permalink / raw)
To: Jiri Pirko, Yuval Mintz
Cc: Steve Lin, netdev@vger.kernel.org, Jiri Pirko,
davem@davemloft.net, michael.chan@broadcom.com,
linville@tuxdriver.com, gospo@broadcom.com, andrew,
vivien.didelot
In-Reply-To: <20171019214339.GH1978@nanopsycho.orion>
On 10/19/2017 02:43 PM, Jiri Pirko wrote:
> Thu, Oct 19, 2017 at 11:39:55PM CEST, jiri@resnulli.us wrote:
>> Thu, Oct 19, 2017 at 10:32:21PM CEST, yuvalm@mellanox.com wrote:
>>>> Adding DEVLINK_PERM_CONFIG_MSIX_VECTORS_PER_VF permanent
>>>> config
>>>> parameter. Defines number of MSI-X vectors allocated per VF.
>>>> Value is permanent (stored in NVRAM), so becomes the new default
>>>> value for this device.
>>>
>>> Sounds like you're having this enforce the same configuration for all child VFs.
>>
>> Yeah, this sounds like per-port config.
>
> This opens old but lately silent discussion about introducing new port
> types for different things. Like VF, dsa CPU port or dsa inter-chip
> ports.
FWIW, the "issue" with representing VF, DSA CPU port or DSA inter-chip
port is that you would be representing a pipe, so there is obviously a
question of whether your represent one end or both ends of that pipe,
and how do you make sure both stay in sync if you represent those.
For instance, for an inter-switch connection, I could decide to
configure VLANs 1-3 tagged on one end of the connection, and forget to
that on the other end of the connection, and that's just one example
where things can go seriously wrong.
>
>>
>>
>>>
>>>>
>>>> Signed-off-by: Steve Lin <steven.lin1@broadcom.com>
>>>> Acked-by: Andy Gospodarek <gospo@broadcom.com>
>>>> ---
>>>> include/uapi/linux/devlink.h | 1 +
>>>> 1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
>>>> index 8ad6c63..ef163b6 100644
>>>> --- a/include/uapi/linux/devlink.h
>>>> +++ b/include/uapi/linux/devlink.h
>>>> @@ -260,6 +260,7 @@ enum devlink_perm_config_param {
>>>> DEVLINK_PERM_CONFIG_SRIOV_ENABLED,
>>>> DEVLINK_PERM_CONFIG_NUM_VF_PER_PF,
>>>> DEVLINK_PERM_CONFIG_MAX_NUM_PF_MSIX_VECT,
>>>> + DEVLINK_PERM_CONFIG_MSIX_VECTORS_PER_VF,
>>>> };
>>>>
>>>> #endif /* _UAPI_LINUX_DEVLINK_H_ */
>>>> --
>>>> 2.7.4
>>>
--
Florian
^ permalink raw reply
* Re: [PATCH net-next v7 5/5] selinux: bpf: Add addtional check for bpf object file receive
From: James Morris @ 2017-10-20 1:28 UTC (permalink / raw)
To: Chenbo Feng
Cc: netdev, SELinux, linux-security-module, Jeffrey Vander Stoep,
Alexei Starovoitov, lorenzo, Daniel Borkmann, Stephen Smalley,
Paul Moore, Chenbo Feng
In-Reply-To: <20171018200026.146093-6-chenbofeng.kernel@gmail.com>
On Wed, 18 Oct 2017, Chenbo Feng wrote:
> From: Chenbo Feng <fengc@google.com>
>
> Introduce a bpf object related check when sending and receiving files
> through unix domain socket as well as binder. It checks if the receiving
> process have privilege to read/write the bpf map or use the bpf program.
> This check is necessary because the bpf maps and programs are using a
> anonymous inode as their shared inode so the normal way of checking the
> files and sockets when passing between processes cannot work properly on
> eBPF object. This check only works when the BPF_SYSCALL is configured.
>
> Signed-off-by: Chenbo Feng <fengc@google.com>
> Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
Reviewed-by: James Morris <james.l.morris@oracle.com>
--
James Morris
<james.l.morris@oracle.com>
^ permalink raw reply
* [PATCH net-next 2/2] net: hns3: Add mac loopback selftest support in hns3 driver
From: Yunsheng Lin @ 2017-10-20 2:19 UTC (permalink / raw)
To: davem
Cc: huangdaode, xuwei5, liguozhu, Yisen.Zhuang, gabriele.paoloni,
john.garry, linuxarm, yisen.zhuang, salil.mehta, lipeng321,
netdev, linux-kernel
In-Reply-To: <1508465962-60176-1-git-send-email-linyunsheng@huawei.com>
This patch adds mac loopback selftest support for ethtool cmd
by checking if a transmitted packet can be received correctly
when mac loopback is enabled.
Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
---
.../ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 54 ++++
.../ethernet/hisilicon/hns3/hns3pf/hns3_ethtool.c | 273 +++++++++++++++++++++
2 files changed, 327 insertions(+)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index c322b45..e5454e0 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -3149,6 +3149,59 @@ static void hclge_cfg_mac_mode(struct hclge_dev *hdev, bool enable)
"mac enable fail, ret =%d.\n", ret);
}
+static int hclge_set_loopback(struct hnae3_handle *handle,
+ enum hnae3_loop loop_mode, bool en)
+{
+ struct hclge_vport *vport = hclge_get_vport(handle);
+ struct hclge_config_mac_mode_cmd *req;
+ struct hclge_dev *hdev = vport->back;
+ struct hclge_desc desc;
+ u32 loop_en;
+ int ret;
+
+ switch (loop_mode) {
+ case HNAE3_MAC_INTER_LOOP_MAC:
+ req = (struct hclge_config_mac_mode_cmd *)&desc.data[0];
+ /* 1 Read out the MAC mode config at first */
+ hclge_cmd_setup_basic_desc(&desc,
+ HCLGE_OPC_CONFIG_MAC_MODE,
+ true);
+ ret = hclge_cmd_send(&hdev->hw, &desc, 1);
+ if (ret) {
+ dev_err(&hdev->pdev->dev,
+ "mac loopback get fail, ret =%d.\n",
+ ret);
+ return ret;
+ }
+
+ /* 2 Then setup the loopback flag */
+ loop_en = le32_to_cpu(req->txrx_pad_fcs_loop_en);
+ if (en)
+ hnae_set_bit(loop_en, HCLGE_MAC_APP_LP_B, 1);
+ else
+ hnae_set_bit(loop_en, HCLGE_MAC_APP_LP_B, 0);
+
+ req->txrx_pad_fcs_loop_en = cpu_to_le32(loop_en);
+
+ /* 3 Config mac work mode with loopback flag
+ * and its original configure parameters
+ */
+ hclge_cmd_reuse_desc(&desc, false);
+ ret = hclge_cmd_send(&hdev->hw, &desc, 1);
+ if (ret)
+ dev_err(&hdev->pdev->dev,
+ "mac loopback set fail, ret =%d.\n", ret);
+ break;
+ default:
+ ret = -ENOTSUPP;
+ dev_err(&hdev->pdev->dev,
+ "loop_mode %d is not supported\n", loop_mode);
+ break;
+ }
+
+ return ret;
+}
+
static int hclge_tqp_enable(struct hclge_dev *hdev, int tqp_id,
int stream_id, bool enable)
{
@@ -4486,6 +4539,7 @@ static void hclge_uninit_ae_dev(struct hnae3_ae_dev *ae_dev)
.unmap_ring_from_vector = hclge_unmap_ring_from_vector,
.get_vector = hclge_get_vector,
.set_promisc_mode = hclge_set_promisc_mode,
+ .set_loopback = hclge_set_loopback,
.start = hclge_ae_start,
.stop = hclge_ae_stop,
.get_status = hclge_get_status,
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_ethtool.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_ethtool.c
index ddbd7f3..6c469e4 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_ethtool.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_ethtool.c
@@ -59,6 +59,16 @@ struct hns3_stats {
#define HNS3_TQP_STATS_COUNT (HNS3_TXQ_STATS_COUNT + HNS3_RXQ_STATS_COUNT)
+#define HNS3_SELF_TEST_TPYE_NUM 1
+#define HNS3_NIC_LB_TEST_PKT_NUM 1
+#define HNS3_NIC_LB_TEST_RING_ID 0
+#define HNS3_NIC_LB_TEST_PACKET_SIZE 128
+
+/* Nic loopback test err */
+#define HNS3_NIC_LB_TEST_NO_MEM_ERR 1
+#define HNS3_NIC_LB_TEST_TX_CNT_ERR 2
+#define HNS3_NIC_LB_TEST_RX_CNT_ERR 3
+
struct hns3_link_mode_mapping {
u32 hns3_link_mode;
u32 ethtool_link_mode;
@@ -77,6 +87,268 @@ struct hns3_link_mode_mapping {
{HNS3_LM_1000BASET_FULL_BIT, ETHTOOL_LINK_MODE_1000baseT_Full_BIT},
};
+static int hns3_lp_setup(struct net_device *ndev, enum hnae3_loop loop)
+{
+ struct hnae3_handle *h = hns3_get_handle(ndev);
+ int ret;
+
+ if (!h->ae_algo->ops->set_loopback ||
+ !h->ae_algo->ops->set_promisc_mode)
+ return -EOPNOTSUPP;
+
+ switch (loop) {
+ case HNAE3_MAC_INTER_LOOP_MAC:
+ ret = h->ae_algo->ops->set_loopback(h, loop, true);
+ break;
+ case HNAE3_MAC_LOOP_NONE:
+ ret = h->ae_algo->ops->set_loopback(h,
+ HNAE3_MAC_INTER_LOOP_MAC, false);
+ break;
+ default:
+ ret = -ENOTSUPP;
+ break;
+ }
+
+ if (ret)
+ return ret;
+
+ if (loop == HNAE3_MAC_LOOP_NONE)
+ h->ae_algo->ops->set_promisc_mode(h, ndev->flags & IFF_PROMISC);
+ else
+ h->ae_algo->ops->set_promisc_mode(h, 1);
+
+ return ret;
+}
+
+static int hns3_lp_up(struct net_device *ndev, enum hnae3_loop loop_mode)
+{
+ struct hnae3_handle *h = hns3_get_handle(ndev);
+ int ret;
+
+ if (!h->ae_algo->ops->start)
+ return -EOPNOTSUPP;
+
+ ret = h->ae_algo->ops->start(h);
+ if (ret) {
+ netdev_err(ndev,
+ "hns3_lb_up ae start return error: %d\n", ret);
+ return ret;
+ }
+
+ ret = hns3_lp_setup(ndev, loop_mode);
+ usleep_range(10000, 20000);
+
+ return ret;
+}
+
+static int hns3_lp_down(struct net_device *ndev)
+{
+ struct hnae3_handle *h = hns3_get_handle(ndev);
+ int ret;
+
+ if (!h->ae_algo->ops->stop)
+ return -EOPNOTSUPP;
+
+ ret = hns3_lp_setup(ndev, HNAE3_MAC_LOOP_NONE);
+ if (ret) {
+ netdev_err(ndev, "lb_setup return error: %d\n", ret);
+ return ret;
+ }
+
+ h->ae_algo->ops->stop(h);
+ usleep_range(10000, 20000);
+
+ return 0;
+}
+
+static void hns3_lp_setup_skb(struct sk_buff *skb)
+{
+ struct net_device *ndev = skb->dev;
+ unsigned char *packet;
+ struct ethhdr *ethh;
+ unsigned int i;
+
+ skb_reserve(skb, NET_IP_ALIGN);
+ ethh = skb_put(skb, sizeof(struct ethhdr));
+ packet = skb_put(skb, HNS3_NIC_LB_TEST_PACKET_SIZE);
+
+ memcpy(ethh->h_dest, ndev->dev_addr, ETH_ALEN);
+ eth_zero_addr(ethh->h_source);
+ ethh->h_proto = htons(ETH_P_ARP);
+ skb_reset_mac_header(skb);
+
+ for (i = 0; i < HNS3_NIC_LB_TEST_PACKET_SIZE; i++)
+ packet[i] = (unsigned char)(i & 0xff);
+}
+
+static void hns3_lb_check_skb_data(struct hns3_enet_ring *ring,
+ struct sk_buff *skb)
+{
+ struct hns3_enet_tqp_vector *tqp_vector = ring->tqp_vector;
+ unsigned char *packet = skb->data;
+ u32 i;
+
+ for (i = 0; i < skb->len; i++)
+ if (packet[i] != (unsigned char)(i & 0xff))
+ break;
+
+ /* The packet is correctly received */
+ if (i == skb->len)
+ tqp_vector->rx_group.total_packets++;
+ else
+ print_hex_dump(KERN_ERR, "selftest:", DUMP_PREFIX_OFFSET, 16, 1,
+ skb->data, skb->len, true);
+
+ dev_kfree_skb_any(skb);
+}
+
+static u32 hns3_lb_check_rx_ring(struct hns3_nic_priv *priv, u32 budget)
+{
+ struct hnae3_handle *h = priv->ae_handle;
+ struct hnae3_knic_private_info *kinfo;
+ u32 i, rcv_good_pkt_total = 0;
+
+ kinfo = &h->kinfo;
+ for (i = kinfo->num_tqps; i < kinfo->num_tqps * 2; i++) {
+ struct hns3_enet_ring *ring = priv->ring_data[i].ring;
+ struct hns3_enet_ring_group *rx_group;
+ u64 pre_rx_pkt;
+
+ rx_group = &ring->tqp_vector->rx_group;
+ pre_rx_pkt = rx_group->total_packets;
+
+ hns3_clean_rx_ring(ring, budget, hns3_lb_check_skb_data);
+
+ rcv_good_pkt_total += (rx_group->total_packets - pre_rx_pkt);
+ rx_group->total_packets = pre_rx_pkt;
+ }
+ return rcv_good_pkt_total;
+}
+
+static void hns3_lb_clear_tx_ring(struct hns3_nic_priv *priv, u32 start_ringid,
+ u32 end_ringid, u32 budget)
+{
+ u32 i;
+
+ for (i = start_ringid; i <= end_ringid; i++) {
+ struct hns3_enet_ring *ring = priv->ring_data[i].ring;
+
+ hns3_clean_tx_ring(ring, budget);
+ }
+}
+
+/**
+ * hns3_lp_run_test - run loopback test
+ * @ndev: net device
+ * @mode: loopback type
+ */
+static int hns3_lp_run_test(struct net_device *ndev, enum hnae3_loop mode)
+{
+ struct hns3_nic_priv *priv = netdev_priv(ndev);
+ struct sk_buff *skb;
+ u32 i, good_cnt;
+ int ret_val = 0;
+
+ skb = alloc_skb(HNS3_NIC_LB_TEST_PACKET_SIZE + ETH_HLEN + NET_IP_ALIGN,
+ GFP_KERNEL);
+ if (!skb)
+ return HNS3_NIC_LB_TEST_NO_MEM_ERR;
+
+ skb->dev = ndev;
+ hns3_lp_setup_skb(skb);
+ skb->queue_mapping = HNS3_NIC_LB_TEST_RING_ID;
+
+ good_cnt = 0;
+ for (i = 0; i < HNS3_NIC_LB_TEST_PKT_NUM; i++) {
+ netdev_tx_t tx_ret;
+
+ skb_get(skb);
+ tx_ret = hns3_nic_net_xmit(skb, ndev);
+ if (tx_ret == NETDEV_TX_OK)
+ good_cnt++;
+ else
+ netdev_err(ndev, "hns3_lb_run_test xmit failed: %d\n",
+ tx_ret);
+ }
+ if (good_cnt != HNS3_NIC_LB_TEST_PKT_NUM) {
+ ret_val = HNS3_NIC_LB_TEST_TX_CNT_ERR;
+ netdev_err(ndev, "mode %d sent fail, cnt=0x%x, budget=0x%x\n",
+ mode, good_cnt, HNS3_NIC_LB_TEST_PKT_NUM);
+ goto out;
+ }
+
+ /* Allow 200 milliseconds for packets to go from Tx to Rx */
+ msleep(200);
+
+ good_cnt = hns3_lb_check_rx_ring(priv, HNS3_NIC_LB_TEST_PKT_NUM);
+ if (good_cnt != HNS3_NIC_LB_TEST_PKT_NUM) {
+ ret_val = HNS3_NIC_LB_TEST_RX_CNT_ERR;
+ netdev_err(ndev, "mode %d recv fail, cnt=0x%x, budget=0x%x\n",
+ mode, good_cnt, HNS3_NIC_LB_TEST_PKT_NUM);
+ }
+
+out:
+ hns3_lb_clear_tx_ring(priv, HNS3_NIC_LB_TEST_RING_ID,
+ HNS3_NIC_LB_TEST_RING_ID,
+ HNS3_NIC_LB_TEST_PKT_NUM);
+
+ kfree_skb(skb);
+ return ret_val;
+}
+
+/**
+ * hns3_nic_self_test - self test
+ * @ndev: net device
+ * @eth_test: test cmd
+ * @data: test result
+ */
+static void hns3_self_test(struct net_device *ndev,
+ struct ethtool_test *eth_test, u64 *data)
+{
+ struct hns3_nic_priv *priv = netdev_priv(ndev);
+ struct hnae3_handle *h = priv->ae_handle;
+ int st_param[HNS3_SELF_TEST_TPYE_NUM][2];
+ bool if_running = netif_running(ndev);
+ int test_index = 0;
+ u32 i;
+
+ /* Only do offline selftest, or pass by default */
+ if (eth_test->flags != ETH_TEST_FL_OFFLINE)
+ return;
+
+ st_param[HNAE3_MAC_INTER_LOOP_MAC][0] = HNAE3_MAC_INTER_LOOP_MAC;
+ st_param[HNAE3_MAC_INTER_LOOP_MAC][1] =
+ h->flags & HNAE3_SUPPORT_MAC_LOOPBACK;
+
+ if (if_running)
+ dev_close(ndev);
+
+ set_bit(HNS3_NIC_STATE_TESTING, &priv->state);
+
+ for (i = 0; i < HNS3_SELF_TEST_TPYE_NUM; i++) {
+ enum hnae3_loop loop_type = (enum hnae3_loop)st_param[i][0];
+
+ if (!st_param[i][1])
+ continue;
+
+ data[test_index] = hns3_lp_up(ndev, loop_type);
+ if (!data[test_index]) {
+ data[test_index] = hns3_lp_run_test(ndev, loop_type);
+ hns3_lp_down(ndev);
+ }
+
+ if (data[test_index])
+ eth_test->flags |= ETH_TEST_FL_FAILED;
+
+ test_index++;
+ }
+
+ clear_bit(HNS3_NIC_STATE_TESTING, &priv->state);
+
+ if (if_running)
+ dev_open(ndev);
+}
+
static void hns3_driv_to_eth_caps(u32 caps, struct ethtool_link_ksettings *cmd,
bool is_advertised)
{
@@ -553,6 +825,7 @@ static int hns3_set_rxnfc(struct net_device *netdev, struct ethtool_rxnfc *cmd)
}
static const struct ethtool_ops hns3_ethtool_ops = {
+ .self_test = hns3_self_test,
.get_drvinfo = hns3_get_drvinfo,
.get_link = hns3_get_link,
.get_ringparam = hns3_get_ringparam,
--
1.9.1
^ permalink raw reply related
* [PATCH net-next 0/2] Add mac loopback selftest support in hns3 driver
From: Yunsheng Lin @ 2017-10-20 2:19 UTC (permalink / raw)
To: davem
Cc: huangdaode, xuwei5, liguozhu, Yisen.Zhuang, gabriele.paoloni,
john.garry, linuxarm, yisen.zhuang, salil.mehta, lipeng321,
netdev, linux-kernel
This patchset refactors the skb receiving and transmitting function
before adding mac loopback selftest support in hns3 driver.
Yunsheng Lin (2):
net: hns3: Refactor the skb receiving and transmitting function
net: hns3: Add mac loopback selftest support in hns3 driver
.../ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 54 ++++
.../net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c | 17 +-
.../net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.h | 4 +
.../ethernet/hisilicon/hns3/hns3pf/hns3_ethtool.c | 273 +++++++++++++++++++++
4 files changed, 343 insertions(+), 5 deletions(-)
--
1.9.1
^ permalink raw reply
* RE: [Intel-wired-lan] [next-queue PATCH v9 6/6] igb: Add support for CBS offload
From: Brown, Aaron F @ 2017-10-20 1:47 UTC (permalink / raw)
To: Gomes, Vinicius, netdev@vger.kernel.org,
intel-wired-lan@lists.osuosl.org
Cc: rodney.cummings@ni.com, Guedes, Andre, jiri@resnulli.us,
Briano, Ivan, richardcochran@gmail.com, henrik@austad.us,
jhs@mojatatu.com, levipearson@gmail.com, Ong, Boon Leong,
xiyou.wangcong@gmail.com, Sanchez-Palencia, Jesus
In-Reply-To: <20171017010128.22141-7-vinicius.gomes@intel.com>
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On Behalf
> Of Vinicius Costa Gomes
> Sent: Monday, October 16, 2017 6:01 PM
> To: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org
> Cc: rodney.cummings@ni.com; Guedes, Andre <andre.guedes@intel.com>;
> jiri@resnulli.us; Briano, Ivan <ivan.briano@intel.com>;
> richardcochran@gmail.com; henrik@austad.us; jhs@mojatatu.com;
> levipearson@gmail.com; Ong, Boon Leong <boon.leong.ong@intel.com>;
> xiyou.wangcong@gmail.com; Sanchez-Palencia, Jesus <jesus.sanchez-
> palencia@intel.com>
> Subject: [Intel-wired-lan] [next-queue PATCH v9 6/6] igb: Add support for
> CBS offload
>
> From: Andre Guedes <andre.guedes@intel.com>
>
> This patch adds support for Credit-Based Shaper (CBS) qdisc offload
> from Traffic Control system. This support enable us to leverage the
> Forwarding and Queuing for Time-Sensitive Streams (FQTSS) features
> from Intel i210 Ethernet Controller. FQTSS is the former 802.1Qav
> standard which was merged into 802.1Q in 2014. It enables traffic
> prioritization and bandwidth reservation via the Credit-Based Shaper
> which is implemented in hardware by i210 controller.
>
> The patch introduces the igb_setup_tc() function which implements the
> support for CBS qdisc hardware offload in the IGB driver. CBS offload
> is the only traffic control offload supported by the driver at the
> moment.
>
> FQTSS transmission mode from i210 controller is automatically enabled
> by the IGB driver when the CBS is enabled for the first hardware
> queue. Likewise, FQTSS mode is automatically disabled when CBS is
> disabled for the last hardware queue. Changing FQTSS mode requires NIC
> reset.
>
> FQTSS feature is supported by i210 controller only.
>
> Signed-off-by: Andre Guedes <andre.guedes@intel.com>
> ---
> drivers/net/ethernet/intel/igb/e1000_defines.h | 23 ++
> drivers/net/ethernet/intel/igb/e1000_regs.h | 8 +
> drivers/net/ethernet/intel/igb/igb.h | 6 +
> drivers/net/ethernet/intel/igb/igb_main.c | 347
> +++++++++++++++++++++++++
> 4 files changed, 384 insertions(+)
>From a regression standpoint:
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
^ permalink raw reply
* Re: [PATCH] net/ethernet/sgi: Code cleanup
From: Joshua Kinard @ 2017-10-20 2:00 UTC (permalink / raw)
To: David Miller; +Cc: ralf, linux-mips, netdev, tglx, linux-kernel
In-Reply-To: <20171019.132704.299365871401082792.davem@davemloft.net>
On 10/19/2017 08:27, David Miller wrote:
> From: Joshua Kinard <kumba@gentoo.org>
> Date: Tue, 17 Oct 2017 13:54:30 -0400
>
>> From: Joshua Kinard <kumba@gentoo.org>
>>
>> The below patch attempts to clean up the code for the in-tree driver
>> for IOC3 ethernet and serial console support, primarily used by SGI
>> MIPS platforms. Notable changes include:
>>
>> - Lots of whitespace cleanup
>> - Using shorthand integer types (u16, u32, etc) where appropriate
>
> These seem to be arbitrary, "unsigned int" is a fine value for a
> hash computation.
I can back those changes out if you'd like. I was aiming more for consistency.
There is one specific change that is required, where the IP protocol number is
being parsed into a signed char field when it should be unsigned char. I can
send that in as its own patch if you'd like.
> You're also making many different kinds of changes in one patch
> which makes it very difficult to review.
This patch is more of a preparation patch, because I have additional pending
changes to move towards using the IOC3 "metadriver" (drivers/sn/ioc3.c) on the
supported MIPS/SGI systems. That metadriver is only used by IA64 right now.
The changes in this patch were primarily to reduce the diff size of the
metadriver patch when I get time to get it cleaned up.
I can either try for a v2 of this patch to split it up, or hold off and make it
part of the metadriver patch. I figured since that patch will need to go
through several subsystem reviewers, better to try sending this one in first.
> This driver is also for such ancient hardware, that the risk
> of potentially breaking the driver far outweighs the value of
> "cleaning up" the code.
I am familiar with this hardware, actually. I have several SGI machines
powered on periodically that use this particular chip, and am familiar with its
quirks and frustrations. Although it's been a while since I tested this
specific batch of changes by itself, the last time I did, the machine booted
fine. With the metadriver patch applied on top, I know this driver works
well-enough.
--
Joshua Kinard
Gentoo/MIPS
kumba@gentoo.org
6144R/F5C6C943 2015-04-27
177C 1972 1FB8 F254 BAD0 3E72 5C63 F4E3 F5C6 C943
"The past tempts us, the present confuses us, the future frightens us. And our
lives slip away, moment by moment, lost in that vast, terrible in-between."
--Emperor Turhan, Centauri Republic
^ permalink raw reply
* v4.14-rc3/arm64 DABT exception in atomic_inc() / __skb_clone()
From: Wei Wei @ 2017-10-20 2:16 UTC (permalink / raw)
To: linux-arm-kernel
Cc: linux-kernel, netdev, edumazet, davem, willemb, syzkaller
Hi all,
I have fuzzed v4.14-rc3 using syzkaller and found a bug similar to that one [1].
But the call trace isn’t the same. The atomic_inc() might handle a corrupted
skb_buff.
The logs and config have been uploaded to my github repo [2].
[1] https://lkml.org/lkml/2017/10/2/216
[2] https://github.com/dotweiba/skb_clone_atomic_inc_bug
Thanks,
Wei
Unable to handle kernel paging request at virtual address ffff80005bfb81ed
Mem abort info:
Exception class = DABT (current EL), IL = 32 bits
SET = 0, FnV = 0
EA = 0, S1PTW = 0
Data abort info:
ISV = 0, ISS = 0x00000033
CM = 0, WnR = 0
swapper pgtable: 4k pages, 48-bit VAs, pgd = ffff20000b366000
[ffff80005bfb81ed] *pgd=00000000beff7003, *pud=00e8000080000711
Internal error: Oops: 96000021 [#1] PREEMPT SMP
Modules linked in:
CPU: 3 PID: 4725 Comm: syz-executor0 Not tainted 4.14.0-rc3 #3
Hardware name: linux,dummy-virt (DT)
task: ffff800074409e00 task.stack: ffff800033db0000
PC is at __skb_clone+0x430/0x5b0
LR is at __skb_clone+0x1dc/0x5b0
pc : [<ffff200009705f50>] lr : [<ffff200009705cfc>] pstate: 10000145
sp : ffff800033db33d0
x29: ffff800033db33d0 x28: ffff2000098ac378
x27: ffff100006a860e1 x26: 1ffff000067b66b6
x25: ffff8000743340a0 x24: ffff800035430708
x23: ffff80005bfb80c9 x22: ffff800035430710
x21: 0000000000000380 x20: ffff800035430640
x19: ffff8000354312c0 x18: 0000000000000000
x17: 00000000004af000 x16: ffff20000845e8c8
x15: 000000001e518060 x14: 0000ffffd8316070
x13: 0000ffffd8316090 x12: ffffffffffffffff
x11: 1ffff00006a8626f x10: ffff100006a8626f
x9 : dfff200000000000 x8 : 0082009000900608
x7 : 0000000000000000 x6 : ffff800035431380
x5 : ffff100006a86270 x4 : 0000000000000000
x3 : 1ffff00006a86273 x2 : 0000000000000000
x1 : 0000000000000100 x0 : ffff80005bfb81ed
Process syz-executor0 (pid: 4725, stack limit = 0xffff800033db0000)
Call trace:
Exception stack(0xffff800033db3290 to 0xffff800033db33d0)
3280: ffff80005bfb81ed 0000000000000100
32a0: 0000000000000000 1ffff00006a86273 0000000000000000 ffff100006a86270
32c0: ffff800035431380 0000000000000000 0082009000900608 dfff200000000000
32e0: ffff100006a8626f 1ffff00006a8626f ffffffffffffffff 0000ffffd8316090
3300: 0000ffffd8316070 000000001e518060 ffff20000845e8c8 00000000004af000
3320: 0000000000000000 ffff8000354312c0 ffff800035430640 0000000000000380
3340: ffff800035430710 ffff80005bfb80c9 ffff800035430708 ffff8000743340a0
3360: 1ffff000067b66b6 ffff100006a860e1 ffff2000098ac378 ffff800033db33d0
3380: ffff200009705cfc ffff800033db33d0 ffff200009705f50 0000000010000145
33a0: ffff8000354312c0 ffff800035430640 0001000000000000 ffff800074334000
33c0: ffff800033db33d0 ffff200009705f50
[<ffff200009705f50>] __skb_clone+0x430/0x5b0
[<ffff20000971520c>] skb_clone+0x164/0x2c8
[<ffff2000098ac498>] arp_rcv+0x120/0x488
[<ffff200009741878>] __netif_receive_skb_core+0x11e8/0x18c8
[<ffff2000097479b0>] __netif_receive_skb+0x30/0x198
[<ffff200009751fd8>] netif_receive_skb_internal+0x98/0x370
[<ffff2000097522cc>] netif_receive_skb+0x1c/0x28
[<ffff2000090730e0>] tun_get_user+0x12f0/0x2e40
[<ffff200009074ddc>] tun_chr_write_iter+0xbc/0x140
[<ffff200008457284>] do_iter_readv_writev+0x2d4/0x468
[<ffff20000845a5a0>] do_iter_write+0x148/0x498
[<ffff20000845aac0>] vfs_writev+0x118/0x250
[<ffff20000845acbc>] do_writev+0xc4/0x1e8
[<ffff20000845e8fc>] SyS_writev+0x34/0x48
Exception stack(0xffff800033db3ec0 to 0xffff800033db4000)
3ec0: 0000000000000015 0000ffff829985e0 0000000000000001 0000ffff8299851c
3ee0: 0000ffff82999068 0000ffff82998f60 0000ffff82999650 0000000000000000
3f00: 0000000000000042 0000000000000036 0000000000406608 0000ffff82998400
3f20: 0000ffff82998f60 0000ffffd8316090 0000ffffd8316070 000000001e518060
3f40: 0000000000000000 00000000004af000 0000000000000000 0000000000000036
3f60: 0000000020004fca 0000000020000000 000000000046ccf0 0000000000000530
3f80: 000000000046cce8 00000000004ade98 0000000000000000 00000000395fa6f0
3fa0: 0000ffff82998f60 0000ffff82998560 0000000000431448 0000ffff82998520
3fc0: 000000000043145c 0000000080000000 0000000000000015 0000000000000042
3fe0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[<ffff200008083ef0>] el0_svc_naked+0x24/0x28
Code: f9406680 8b010000 91009000 f9800011 (885f7c01)
---[ end trace 261e7ac1458ccc0a ]---
^ permalink raw reply
* [PATCH net-next 1/2] net: hns3: Refactor the skb receiving and transmitting function
From: Yunsheng Lin @ 2017-10-20 2:19 UTC (permalink / raw)
To: davem
Cc: huangdaode, xuwei5, liguozhu, Yisen.Zhuang, gabriele.paoloni,
john.garry, linuxarm, yisen.zhuang, salil.mehta, lipeng321,
netdev, linux-kernel
In-Reply-To: <1508465962-60176-1-git-send-email-linyunsheng@huawei.com>
This patch refactors the skb receiving and transmitting functions
and export them in order to support the ethtool's mac loopback
selftest.
Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
---
drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c | 17 ++++++++++++-----
drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.h | 4 ++++
2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c
index 8fa4e65..8383d67 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.c
@@ -900,8 +900,7 @@ static void hns_nic_dma_unmap(struct hns3_enet_ring *ring, int next_to_use_orig)
}
}
-static netdev_tx_t hns3_nic_net_xmit(struct sk_buff *skb,
- struct net_device *netdev)
+netdev_tx_t hns3_nic_net_xmit(struct sk_buff *skb, struct net_device *netdev)
{
struct hns3_nic_priv *priv = netdev_priv(netdev);
struct hns3_nic_ring_data *ring_data =
@@ -1943,6 +1942,11 @@ static void hns3_rx_checksum(struct hns3_enet_ring *ring, struct sk_buff *skb,
}
}
+static void hns3_rx_skb(struct hns3_enet_ring *ring, struct sk_buff *skb)
+{
+ napi_gro_receive(&ring->tqp_vector->napi, skb);
+}
+
static int hns3_handle_rx_bd(struct hns3_enet_ring *ring,
struct sk_buff **out_skb, int *out_bnum)
{
@@ -2077,7 +2081,9 @@ static int hns3_handle_rx_bd(struct hns3_enet_ring *ring,
return 0;
}
-static int hns3_clean_rx_ring(struct hns3_enet_ring *ring, int budget)
+int hns3_clean_rx_ring(
+ struct hns3_enet_ring *ring, int budget,
+ void (*rx_fn)(struct hns3_enet_ring *, struct sk_buff *))
{
#define RCB_NOF_ALLOC_RX_BUFF_ONCE 16
struct net_device *netdev = ring->tqp->handle->kinfo.netdev;
@@ -2115,7 +2121,7 @@ static int hns3_clean_rx_ring(struct hns3_enet_ring *ring, int budget)
/* Do update ip stack process */
skb->protocol = eth_type_trans(skb, netdev);
- (void)napi_gro_receive(&ring->tqp_vector->napi, skb);
+ rx_fn(ring, skb);
recv_pkts++;
}
@@ -2258,7 +2264,8 @@ static int hns3_nic_common_poll(struct napi_struct *napi, int budget)
rx_budget = max(budget / tqp_vector->num_tqps, 1);
hns3_for_each_ring(ring, tqp_vector->rx_group) {
- int rx_cleaned = hns3_clean_rx_ring(ring, rx_budget);
+ int rx_cleaned = hns3_clean_rx_ring(ring, rx_budget,
+ hns3_rx_skb);
if (rx_cleaned >= rx_budget)
clean_complete = false;
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.h
index 6659989..6228b26 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hns3_enet.h
@@ -597,6 +597,10 @@ static inline void hns3_write_reg(void __iomem *base, u32 reg, u32 value)
int hns3_clean_tx_ring(struct hns3_enet_ring *ring, int budget);
int hns3_init_all_ring(struct hns3_nic_priv *priv);
int hns3_uninit_all_ring(struct hns3_nic_priv *priv);
+netdev_tx_t hns3_nic_net_xmit(struct sk_buff *skb, struct net_device *netdev);
+int hns3_clean_rx_ring(
+ struct hns3_enet_ring *ring, int budget,
+ void (*rx_fn)(struct hns3_enet_ring *, struct sk_buff *));
#ifdef CONFIG_HNS3_DCB
void hns3_dcbnl_setup(struct hnae3_handle *handle);
--
1.9.1
^ permalink raw reply related
* Re: RFC(v2): Audit Kernel Container IDs
From: Steve Grubb @ 2017-10-20 2:25 UTC (permalink / raw)
To: Aleksa Sarai
Cc: Richard Guy Briggs, mszeredi-H+wXaHxf7aLQT0dZR+AlfA,
trondmy-7I+n7zu2hftEKMMhf/gKZA, Andy Lutomirski,
jlayton-H+wXaHxf7aLQT0dZR+AlfA, Carlos O'Donell, Linux API,
Linux Containers, Paul Moore, Linux Kernel, Eric Paris, Al Viro,
David Howells, Linux Audit, Simo Sorce, Linux Network Development,
Linux FS Devel, cgroups-u79uwXL29TY76Z2rM5mHXA, Eric W. Biederman
In-Reply-To: <8f495870-dd6c-23b9-b82b-4228a441c729-l3A5Bk7waGM@public.gmane.org>
On Thursday, October 19, 2017 7:11:33 PM EDT Aleksa Sarai wrote:
> >>> The registration is a pseudo filesystem (proc, since PID tree already
> >>> exists) write of a u8[16] UUID representing the container ID to a file
> >>> representing a process that will become the first process in a new
> >>> container. This write might place restrictions on mount namespaces
> >>> required to define a container, or at least careful checking of
> >>> namespaces in the kernel to verify permissions of the orchestrator so it
> >>> can't change its own container ID. A bind mount of nsfs may be
> >>> necessary in the container orchestrator's mntNS.
> >>> Note: Use a 128-bit scalar rather than a string to make compares faster
> >>> and simpler.
> >>>
> >>> Require a new CAP_CONTAINER_ADMIN to be able to carry out the
> >>> registration.
> >>
> >> Wouldn't CAP_AUDIT_WRITE be sufficient? After all, this is for auditing.
> >
> > No, because then any process with that capability (vsftpd) could change
> > its own container ID. This is discussed more in other parts of the
> > thread...
For the record, I changed my mind. CAP_AUDIT_CONTROL is the correct
capability.
> Not if we make the container ID append-only (to support nesting), or
> write-once (the other idea thrown around).
Well...I like to use lessons learned if they can be applied. In the normal
world without containers we have uid, auid, and session_id. uid is who you are
now, auid is how you got into the system, session_id distinguishes individual
auids. We have a default auid of -1 for system objects and a real number for
people.
I think there should be the equivalent of auid and session_id but tailored for
containers. Loginuid == container id. It can be set, overridden, or appended
to (we'll figure this out later) in very limited circumstances.
Container_session == session which is tamper-proof. This way things can enter
a container with the same ID but under a different session. And everything
else gets to inherit the original ID. This way we can trace actions to
something that entered the container rather than normal system activity in the
container.
What a security officer wants to know is what did people do inside the
system / container. The system objects we typically don't care about. Sure
they might get hacked and then work on behalf of someone, but they would
almost always pop a shell so that they can have freedom. That should set off
an AVC or create other activity that gets picked up.
-Steve
> In that case, you can't move "out" from a particular container ID, you can
> only go "deeper". These semantics don't make sense for generic containers,
> but since the point of this facility is *specifically* for audit I imagine
> that not being able to move a process from a sub-container's ID is a
> benefit.
^ permalink raw reply
* Re: [PATCH 18/27] bpf: Restrict kernel image access functions when the kernel is locked down
From: joeyli @ 2017-10-20 2:47 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David Howells, linux-security-module, gnomes, linux-efi,
matthew.garrett, gregkh, linux-kernel, jforbes, Daniel Borkmann,
David S. Miller, netdev, Gary Lin
In-Reply-To: <20171019221829.7m5nczg3ltqmhzom@ast-mbp>
Hi Alexei,
Thanks for your review!
On Thu, Oct 19, 2017 at 03:18:30PM -0700, Alexei Starovoitov wrote:
> On Thu, Oct 19, 2017 at 03:52:49PM +0100, David Howells wrote:
> > From: Chun-Yi Lee <jlee@suse.com>
> >
> > There are some bpf functions can be used to read kernel memory:
> > bpf_probe_read, bpf_probe_write_user and bpf_trace_printk. These allow
> > private keys in kernel memory (e.g. the hibernation image signing key) to
> > be read by an eBPF program. Prohibit those functions when the kernel is
> > locked down.
> >
> > Signed-off-by: Chun-Yi Lee <jlee@suse.com>
> > Signed-off-by: David Howells <dhowells@redhat.com>
> > cc: netdev@vger.kernel.org
> > ---
> >
> > kernel/trace/bpf_trace.c | 11 +++++++++++
> > 1 file changed, 11 insertions(+)
> >
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index dc498b605d5d..35e85a3fdb37 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -65,6 +65,11 @@ BPF_CALL_3(bpf_probe_read, void *, dst, u32, size, const void *, unsafe_ptr)
> > {
> > int ret;
> >
> > + if (kernel_is_locked_down("BPF")) {
> > + memset(dst, 0, size);
> > + return -EPERM;
> > + }
>
> That doesn't help the lockdown purpose.
> If you don't trust the root the only way to prevent bpf read
> memory is to disable the whole thing.
Not totally untrust root, I don't want that root reads
arbitrary memory address through bpf.
Is it not enough to lock down bpf_probe_read, bpf_probe_write_user
and bpf_trace_printk?
> Have a single check in sys_bpf() to disallow everything if kernel_is_locked_down()
> and don't add overhead to critical path like bpf_probe_read().
>
Yes, it give overhead to bpf_probe_read but it prevents arbitrary
memory read.
Another idea is signing bpf bytecode then verifying signture when
loading to kernel.
Thanks a lot!
Joey Lee
^ permalink raw reply
* Re: v4.14-rc3/arm64 DABT exception in atomic_inc() / __skb_clone()
From: Eric Dumazet @ 2017-10-20 2:53 UTC (permalink / raw)
To: Wei Wei
Cc: linux-arm-kernel, LKML, netdev, David Miller, Willem de Bruijn,
syzkaller
In-Reply-To: <EAA60182-6F08-412E-8F8B-FD4B0309A858@gmail.com>
On Thu, Oct 19, 2017 at 7:16 PM, Wei Wei <dotweiba@gmail.com> wrote:
> Hi all,
>
> I have fuzzed v4.14-rc3 using syzkaller and found a bug similar to that one [1].
> But the call trace isn’t the same. The atomic_inc() might handle a corrupted
> skb_buff.
>
> The logs and config have been uploaded to my github repo [2].
>
> [1] https://lkml.org/lkml/2017/10/2/216
> [2] https://github.com/dotweiba/skb_clone_atomic_inc_bug
>
> Thanks,
> Wei
>
> Unable to handle kernel paging request at virtual address ffff80005bfb81ed
> Mem abort info:
> Exception class = DABT (current EL), IL = 32 bits
> SET = 0, FnV = 0
> EA = 0, S1PTW = 0
> Data abort info:
> ISV = 0, ISS = 0x00000033
> CM = 0, WnR = 0
> swapper pgtable: 4k pages, 48-bit VAs, pgd = ffff20000b366000
> [ffff80005bfb81ed] *pgd=00000000beff7003, *pud=00e8000080000711
> Internal error: Oops: 96000021 [#1] PREEMPT SMP
> Modules linked in:
> CPU: 3 PID: 4725 Comm: syz-executor0 Not tainted 4.14.0-rc3 #3
> Hardware name: linux,dummy-virt (DT)
> task: ffff800074409e00 task.stack: ffff800033db0000
> PC is at __skb_clone+0x430/0x5b0
> LR is at __skb_clone+0x1dc/0x5b0
> pc : [<ffff200009705f50>] lr : [<ffff200009705cfc>] pstate: 10000145
> sp : ffff800033db33d0
> x29: ffff800033db33d0 x28: ffff2000098ac378
> x27: ffff100006a860e1 x26: 1ffff000067b66b6
> x25: ffff8000743340a0 x24: ffff800035430708
> x23: ffff80005bfb80c9 x22: ffff800035430710
> x21: 0000000000000380 x20: ffff800035430640
> x19: ffff8000354312c0 x18: 0000000000000000
> x17: 00000000004af000 x16: ffff20000845e8c8
> x15: 000000001e518060 x14: 0000ffffd8316070
> x13: 0000ffffd8316090 x12: ffffffffffffffff
> x11: 1ffff00006a8626f x10: ffff100006a8626f
> x9 : dfff200000000000 x8 : 0082009000900608
> x7 : 0000000000000000 x6 : ffff800035431380
> x5 : ffff100006a86270 x4 : 0000000000000000
> x3 : 1ffff00006a86273 x2 : 0000000000000000
> x1 : 0000000000000100 x0 : ffff80005bfb81ed
> Process syz-executor0 (pid: 4725, stack limit = 0xffff800033db0000)
> Call trace:
> Exception stack(0xffff800033db3290 to 0xffff800033db33d0)
> 3280: ffff80005bfb81ed 0000000000000100
> 32a0: 0000000000000000 1ffff00006a86273 0000000000000000 ffff100006a86270
> 32c0: ffff800035431380 0000000000000000 0082009000900608 dfff200000000000
> 32e0: ffff100006a8626f 1ffff00006a8626f ffffffffffffffff 0000ffffd8316090
> 3300: 0000ffffd8316070 000000001e518060 ffff20000845e8c8 00000000004af000
> 3320: 0000000000000000 ffff8000354312c0 ffff800035430640 0000000000000380
> 3340: ffff800035430710 ffff80005bfb80c9 ffff800035430708 ffff8000743340a0
> 3360: 1ffff000067b66b6 ffff100006a860e1 ffff2000098ac378 ffff800033db33d0
> 3380: ffff200009705cfc ffff800033db33d0 ffff200009705f50 0000000010000145
> 33a0: ffff8000354312c0 ffff800035430640 0001000000000000 ffff800074334000
> 33c0: ffff800033db33d0 ffff200009705f50
> [<ffff200009705f50>] __skb_clone+0x430/0x5b0
> [<ffff20000971520c>] skb_clone+0x164/0x2c8
> [<ffff2000098ac498>] arp_rcv+0x120/0x488
> [<ffff200009741878>] __netif_receive_skb_core+0x11e8/0x18c8
> [<ffff2000097479b0>] __netif_receive_skb+0x30/0x198
> [<ffff200009751fd8>] netif_receive_skb_internal+0x98/0x370
> [<ffff2000097522cc>] netif_receive_skb+0x1c/0x28
> [<ffff2000090730e0>] tun_get_user+0x12f0/0x2e40
> [<ffff200009074ddc>] tun_chr_write_iter+0xbc/0x140
> [<ffff200008457284>] do_iter_readv_writev+0x2d4/0x468
> [<ffff20000845a5a0>] do_iter_write+0x148/0x498
> [<ffff20000845aac0>] vfs_writev+0x118/0x250
> [<ffff20000845acbc>] do_writev+0xc4/0x1e8
> [<ffff20000845e8fc>] SyS_writev+0x34/0x48
> Exception stack(0xffff800033db3ec0 to 0xffff800033db4000)
> 3ec0: 0000000000000015 0000ffff829985e0 0000000000000001 0000ffff8299851c
> 3ee0: 0000ffff82999068 0000ffff82998f60 0000ffff82999650 0000000000000000
> 3f00: 0000000000000042 0000000000000036 0000000000406608 0000ffff82998400
> 3f20: 0000ffff82998f60 0000ffffd8316090 0000ffffd8316070 000000001e518060
> 3f40: 0000000000000000 00000000004af000 0000000000000000 0000000000000036
> 3f60: 0000000020004fca 0000000020000000 000000000046ccf0 0000000000000530
> 3f80: 000000000046cce8 00000000004ade98 0000000000000000 00000000395fa6f0
> 3fa0: 0000ffff82998f60 0000ffff82998560 0000000000431448 0000ffff82998520
> 3fc0: 000000000043145c 0000000080000000 0000000000000015 0000000000000042
> 3fe0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> [<ffff200008083ef0>] el0_svc_naked+0x24/0x28
> Code: f9406680 8b010000 91009000 f9800011 (885f7c01)
> ---[ end trace 261e7ac1458ccc0a ]---
Please provide proper file:line information in this trace.
You can use scripts/decode_stacktrace.sh
Thanks.
^ permalink raw reply
* Re: [patch net v2 1/4] net/sched: Change tc_action refcnt and bindcnt to atomic
From: Cong Wang @ 2017-10-20 3:00 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: Chris Mi, Linux Kernel Network Developers, Lucas Bates,
Jiri Pirko, David Miller
In-Reply-To: <b9444485-8e34-48ac-2b4f-abbd9c6d455f@mojatatu.com>
On Thu, Oct 19, 2017 at 7:21 AM, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> On 17-10-18 12:43 PM, Cong Wang wrote:
>>
>> On Tue, Oct 17, 2017 at 6:03 PM, Chris Mi <chrism@mellanox.com> wrote:
>>>>
>>>> -----Original Message-----
>
>
>>
>> You listed 3 problems, and you think they are 3 different ones, here
>> I argue problem 3 (using RCU callbacks) is the cause of problem 1
>> (refcnt not atomic). This is why I mentioned I have been thinking about
>> removing RCU callbacks, because it probably could fix all of them.
>>
>
> Cong,
> Given this is a known bug (the test case Chris presented crashes the
> kernel) - would it make sense to have a patch that goes to -net
> to fix this while your approach and discussion outcome goes into
> net-next?
I am not sure. Because Chris' patchset is large too and I don't think
it could fix all crashes, so it has little value to just apply them for -net.
^ permalink raw reply
* Re: v4.14-rc3/arm64 DABT exception in atomic_inc() / __skb_clone()
From: Wei Wei @ 2017-10-20 3:13 UTC (permalink / raw)
To: Eric Dumazet
Cc: linux-arm-kernel, LKML, netdev, David Miller, Willem de Bruijn,
syzkaller
In-Reply-To: <CANn89iJgoiY_ky4F_1iE_o6moZXp5e6OcNZZe=B8F-OhWJx8nQ@mail.gmail.com>
Sry. Here it is.
Unable to handle kernel paging request at virtual address ffff80005bfb81ed
Mem abort info:
Exception class = DABT (current EL), IL = 32 bits
SET = 0, FnV = 0
EA = 0, S1PTW = 0
Data abort info:
ISV = 0, ISS = 0x00000033
CM = 0, WnR = 0
swapper pgtable: 4k pages, 48-bit VAs, pgd = ffff20000b366000
[ffff80005bfb81ed] *pgd=00000000beff7003, *pud=00e8000080000711
Internal error: Oops: 96000021 [#1] PREEMPT SMP
Modules linked in:
CPU: 3 PID: 4725 Comm: syz-executor0 Not tainted 4.14.0-rc3 #3
Hardware name: linux,dummy-virt (DT)
task: ffff800074409e00 task.stack: ffff800033db0000
PC is at __skb_clone (/./arch/arm64/include/asm/atomic_ll_sc.h:113 (discriminator 4) /net/core/skbuff.c:873 (discriminator 4))
LR is at __skb_clone (/net/core/skbuff.c:861 (discriminator 4))
pc : lr : pstate: 10000145
sp : ffff800033db33d0
x29: ffff800033db33d0 x28: ffff2000098ac378
x27: ffff100006a860e1 x26: 1ffff000067b66b6
x25: ffff8000743340a0 x24: ffff800035430708
x23: ffff80005bfb80c9 x22: ffff800035430710
x21: 0000000000000380 x20: ffff800035430640
x19: ffff8000354312c0 x18: 0000000000000000
x17: 00000000004af000 x16: ffff20000845e8c8
x15: 000000001e518060 x14: 0000ffffd8316070
x13: 0000ffffd8316090 x12: ffffffffffffffff
x11: 1ffff00006a8626f x10: ffff100006a8626f
x9 : dfff200000000000 x8 : 0082009000900608
x7 : 0000000000000000 x6 : ffff800035431380
x5 : ffff100006a86270 x4 : 0000000000000000
x3 : 1ffff00006a86273 x2 : 0000000000000000
x1 : 0000000000000100 x0 : ffff80005bfb81ed
Process syz-executor0 (pid: 4725, stack limit = 0xffff800033db0000)
Call trace:
Exception stack(0xffff800033db3290 to 0xffff800033db33d0)
3280: ffff80005bfb81ed 0000000000000100
32a0: 0000000000000000 1ffff00006a86273 0000000000000000 ffff100006a86270
32c0: ffff800035431380 0000000000000000 0082009000900608 dfff200000000000
32e0: ffff100006a8626f 1ffff00006a8626f ffffffffffffffff 0000ffffd8316090
3300: 0000ffffd8316070 000000001e518060 ffff20000845e8c8 00000000004af000
3320: 0000000000000000 ffff8000354312c0 ffff800035430640 0000000000000380
3340: ffff800035430710 ffff80005bfb80c9 ffff800035430708 ffff8000743340a0
3360: 1ffff000067b66b6 ffff100006a860e1 ffff2000098ac378 ffff800033db33d0
3380: ffff200009705cfc ffff800033db33d0 ffff200009705f50 0000000010000145
33a0: ffff8000354312c0 ffff800035430640 0001000000000000 ffff800074334000
33c0: ffff800033db33d0 ffff200009705f50
__skb_clone (/./arch/arm64/include/asm/atomic_ll_sc.h:113 (discriminator 4) /net/core/skbuff.c:873 (discriminator 4))
skb_clone (/net/core/skbuff.c:1286)
arp_rcv (/./include/linux/skbuff.h:1518 /net/ipv4/arp.c:946)
__netif_receive_skb_core (/net/core/dev.c:1859 /net/core/dev.c:1874 /net/core/dev.c:4416)
__netif_receive_skb (/net/core/dev.c:4466)
netif_receive_skb_internal (/net/core/dev.c:4539)
netif_receive_skb (/net/core/dev.c:4564)
tun_get_user (/./include/linux/bottom_half.h:31 /drivers/net/tun.c:1219 /drivers/net/tun.c:1553)
tun_chr_write_iter (/drivers/net/tun.c:1579)
do_iter_readv_writev (/./include/linux/fs.h:1770 /fs/read_write.c:673)
do_iter_write (/fs/read_write.c:952)
vfs_writev (/fs/read_write.c:997)
do_writev (/fs/read_write.c:1032)
SyS_writev (/fs/read_write.c:1102)
Exception stack(0xffff800033db3ec0 to 0xffff800033db4000)
3ec0: 0000000000000015 0000ffff829985e0 0000000000000001 0000ffff8299851c
3ee0: 0000ffff82999068 0000ffff82998f60 0000ffff82999650 0000000000000000
3f00: 0000000000000042 0000000000000036 0000000000406608 0000ffff82998400
3f20: 0000ffff82998f60 0000ffffd8316090 0000ffffd8316070 000000001e518060
3f40: 0000000000000000 00000000004af000 0000000000000000 0000000000000036
3f60: 0000000020004fca 0000000020000000 000000000046ccf0 0000000000000530
3f80: 000000000046cce8 00000000004ade98 0000000000000000 00000000395fa6f0
3fa0: 0000ffff82998f60 0000ffff82998560 0000000000431448 0000ffff82998520
3fc0: 000000000043145c 0000000080000000 0000000000000015 0000000000000042
3fe0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
el0_svc_naked (/arch/arm64/kernel/entry.S:853)
Code: f9406680 8b010000 91009000 f9800011 (885f7c01)
All code
========
0: 80 66 40 f9 andb $0xf9,0x40(%rsi)
4: 00 00 add %al,(%rax)
6: 01 8b 00 90 00 91 add %ecx,-0x6eff7000(%rbx)
c: 11 00 adc %eax,(%rax)
e: 80 f9 01 cmp $0x1,%cl
11: 7c 5f jl 0x72
13:* 88 00 mov %al,(%rax) <-- trapping instruction
15: 00 00 add %al,(%rax)
...
Code starting with the faulting instruction
===========================================
0: 01 7c 5f 88 add %edi,-0x78(%rdi,%rbx,2)
4: 00 00 add %al,(%rax)
...
—[ end trace 261e7ac1458ccc0a ]---
Thanks,
Wei
> On 19 Oct 2017, at 10:53 PM, Eric Dumazet <edumazet@google.com> wrote:
>
> On Thu, Oct 19, 2017 at 7:16 PM, Wei Wei <dotweiba@gmail.com> wrote:
>> Hi all,
>>
>> I have fuzzed v4.14-rc3 using syzkaller and found a bug similar to that one [1].
>> But the call trace isn’t the same. The atomic_inc() might handle a corrupted
>> skb_buff.
>>
>> The logs and config have been uploaded to my github repo [2].
>>
>> [1] https://lkml.org/lkml/2017/10/2/216
>> [2] https://github.com/dotweiba/skb_clone_atomic_inc_bug
>>
>> Thanks,
>> Wei
>>
>> Unable to handle kernel paging request at virtual address ffff80005bfb81ed
>> Mem abort info:
>> Exception class = DABT (current EL), IL = 32 bits
>> SET = 0, FnV = 0
>> EA = 0, S1PTW = 0
>> Data abort info:
>> ISV = 0, ISS = 0x00000033
>> CM = 0, WnR = 0
>> swapper pgtable: 4k pages, 48-bit VAs, pgd = ffff20000b366000
>> [ffff80005bfb81ed] *pgd=00000000beff7003, *pud=00e8000080000711
>> Internal error: Oops: 96000021 [#1] PREEMPT SMP
>> Modules linked in:
>> CPU: 3 PID: 4725 Comm: syz-executor0 Not tainted 4.14.0-rc3 #3
>> Hardware name: linux,dummy-virt (DT)
>> task: ffff800074409e00 task.stack: ffff800033db0000
>> PC is at __skb_clone+0x430/0x5b0
>> LR is at __skb_clone+0x1dc/0x5b0
>> pc : [<ffff200009705f50>] lr : [<ffff200009705cfc>] pstate: 10000145
>> sp : ffff800033db33d0
>> x29: ffff800033db33d0 x28: ffff2000098ac378
>> x27: ffff100006a860e1 x26: 1ffff000067b66b6
>> x25: ffff8000743340a0 x24: ffff800035430708
>> x23: ffff80005bfb80c9 x22: ffff800035430710
>> x21: 0000000000000380 x20: ffff800035430640
>> x19: ffff8000354312c0 x18: 0000000000000000
>> x17: 00000000004af000 x16: ffff20000845e8c8
>> x15: 000000001e518060 x14: 0000ffffd8316070
>> x13: 0000ffffd8316090 x12: ffffffffffffffff
>> x11: 1ffff00006a8626f x10: ffff100006a8626f
>> x9 : dfff200000000000 x8 : 0082009000900608
>> x7 : 0000000000000000 x6 : ffff800035431380
>> x5 : ffff100006a86270 x4 : 0000000000000000
>> x3 : 1ffff00006a86273 x2 : 0000000000000000
>> x1 : 0000000000000100 x0 : ffff80005bfb81ed
>> Process syz-executor0 (pid: 4725, stack limit = 0xffff800033db0000)
>> Call trace:
>> Exception stack(0xffff800033db3290 to 0xffff800033db33d0)
>> 3280: ffff80005bfb81ed 0000000000000100
>> 32a0: 0000000000000000 1ffff00006a86273 0000000000000000 ffff100006a86270
>> 32c0: ffff800035431380 0000000000000000 0082009000900608 dfff200000000000
>> 32e0: ffff100006a8626f 1ffff00006a8626f ffffffffffffffff 0000ffffd8316090
>> 3300: 0000ffffd8316070 000000001e518060 ffff20000845e8c8 00000000004af000
>> 3320: 0000000000000000 ffff8000354312c0 ffff800035430640 0000000000000380
>> 3340: ffff800035430710 ffff80005bfb80c9 ffff800035430708 ffff8000743340a0
>> 3360: 1ffff000067b66b6 ffff100006a860e1 ffff2000098ac378 ffff800033db33d0
>> 3380: ffff200009705cfc ffff800033db33d0 ffff200009705f50 0000000010000145
>> 33a0: ffff8000354312c0 ffff800035430640 0001000000000000 ffff800074334000
>> 33c0: ffff800033db33d0 ffff200009705f50
>> [<ffff200009705f50>] __skb_clone+0x430/0x5b0
>> [<ffff20000971520c>] skb_clone+0x164/0x2c8
>> [<ffff2000098ac498>] arp_rcv+0x120/0x488
>> [<ffff200009741878>] __netif_receive_skb_core+0x11e8/0x18c8
>> [<ffff2000097479b0>] __netif_receive_skb+0x30/0x198
>> [<ffff200009751fd8>] netif_receive_skb_internal+0x98/0x370
>> [<ffff2000097522cc>] netif_receive_skb+0x1c/0x28
>> [<ffff2000090730e0>] tun_get_user+0x12f0/0x2e40
>> [<ffff200009074ddc>] tun_chr_write_iter+0xbc/0x140
>> [<ffff200008457284>] do_iter_readv_writev+0x2d4/0x468
>> [<ffff20000845a5a0>] do_iter_write+0x148/0x498
>> [<ffff20000845aac0>] vfs_writev+0x118/0x250
>> [<ffff20000845acbc>] do_writev+0xc4/0x1e8
>> [<ffff20000845e8fc>] SyS_writev+0x34/0x48
>> Exception stack(0xffff800033db3ec0 to 0xffff800033db4000)
>> 3ec0: 0000000000000015 0000ffff829985e0 0000000000000001 0000ffff8299851c
>> 3ee0: 0000ffff82999068 0000ffff82998f60 0000ffff82999650 0000000000000000
>> 3f00: 0000000000000042 0000000000000036 0000000000406608 0000ffff82998400
>> 3f20: 0000ffff82998f60 0000ffffd8316090 0000ffffd8316070 000000001e518060
>> 3f40: 0000000000000000 00000000004af000 0000000000000000 0000000000000036
>> 3f60: 0000000020004fca 0000000020000000 000000000046ccf0 0000000000000530
>> 3f80: 000000000046cce8 00000000004ade98 0000000000000000 00000000395fa6f0
>> 3fa0: 0000ffff82998f60 0000ffff82998560 0000000000431448 0000ffff82998520
>> 3fc0: 000000000043145c 0000000080000000 0000000000000015 0000000000000042
>> 3fe0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>> [<ffff200008083ef0>] el0_svc_naked+0x24/0x28
>> Code: f9406680 8b010000 91009000 f9800011 (885f7c01)
>> ---[ end trace 261e7ac1458ccc0a ]---
>
> Please provide proper file:line information in this trace.
>
> You can use scripts/decode_stacktrace.sh
>
> Thanks.
^ permalink raw reply
* Re: Get rid of RCU callbacks in TC filters?
From: Cong Wang @ 2017-10-20 3:15 UTC (permalink / raw)
To: John Fastabend
Cc: Paul E. McKenney, Jamal Hadi Salim, Chris Mi,
Linux Kernel Network Developers, Daniel Borkmann, Eric Dumazet,
David Miller, Jiri Pirko
In-Reply-To: <52808e54-2f59-49c8-37be-d226e79fe286@gmail.com>
On Thu, Oct 19, 2017 at 8:34 AM, John Fastabend
<john.fastabend@gmail.com> wrote:
>
> My take on this would be to stay with the current RCU callbacks and try
> to simplify the implementation. Falling back to sync operations seems
> like a step backwards to me. I know update/delete of filters is currently
> a pain point for some use cases so getting the RTNL out of the way may
> become a requirement to support those (alternatively maybe batching is
> good enough).
For me it looks like very hard to make tc action destroy code completely
race-free in RCU callbacks, at least looks harder than getting rid of
RCU callbacks.
>
> I guess at a high level with Cris' patches actions are now doing reference
> counting correctly. If shared filters also do reference counting similarly
> we should be OK right? (yes I know simplifying maybe too much to be
> meaningful)
I don't know what you mean by "doing reference counting correctly",
if you mean making them atomic, as I already explained to Chris, it
is not necessary at all if we remove RCU callbacks. Refcnt doesn't
have to be atomic if it is always serialized with a lock.
>
> Are we aware of any outstanding problem areas?
>
Potentially many problems, since tc action destroy code could be
called either with a RTNL lock (fine) or in a RCU callback without
RTNL lock (buggy), these two paths race with each other and RCU
callbacks race among themselves too.
^ permalink raw reply
* Re: Get rid of RCU callbacks in TC filters?
From: Cong Wang @ 2017-10-20 3:26 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Jamal Hadi Salim, Chris Mi, Linux Kernel Network Developers,
Daniel Borkmann, Eric Dumazet, David Miller, Jiri Pirko
In-Reply-To: <20171018193548.GM3521@linux.vnet.ibm.com>
On Wed, Oct 18, 2017 at 12:35 PM, Paul E. McKenney
<paulmck@linux.vnet.ibm.com> wrote:
> On Wed, Oct 18, 2017 at 10:36:28AM -0700, Cong Wang wrote:
>> Hi, all
>>
>> Recently, the RCU callbacks used in TC filters and TC actions keep
>> drawing my attention, they introduce at least 4 race condition bugs:
>>
>> 1. A simple one fixed by Daniel:
>>
>> commit c78e1746d3ad7d548bdf3fe491898cc453911a49
>> Author: Daniel Borkmann <daniel@iogearbox.net>
>> Date: Wed May 20 17:13:33 2015 +0200
>>
>> net: sched: fix call_rcu() race on classifier module unloads
>>
>> 2. A very nasty one fixed by me:
>>
>> commit 1697c4bb5245649a23f06a144cc38c06715e1b65
>> Author: Cong Wang <xiyou.wangcong@gmail.com>
>> Date: Mon Sep 11 16:33:32 2017 -0700
>>
>> net_sched: carefully handle tcf_block_put()
>>
>> 3. Two more bugs found by Chris:
>> https://patchwork.ozlabs.org/patch/826696/
>> https://patchwork.ozlabs.org/patch/826695/
>>
>>
>> Usually RCU callbacks are simple, however for TC filters and actions,
>> they are complex because at least TC actions could be destroyed
>> together with the TC filter in one callback. And RCU callbacks are
>> invoked in BH context, without locking they are parallel too. All of
>> these contribute to the cause of these nasty bugs. It looks like they
>> bring us more problems than benefits.
>>
>> Therefore, I have been thinking about getting rid of these callbacks,
>> because they are not strictly necessary, callers of these call_rcu()
>> are all on slow path and have RTNL lock, so blocking is permitted in
>> their contexts, and _I think_ it does not harm to use
>> synchronize_rcu() on slow paths, at least I can argue RTNL lock is
>> already there and is a bottleneck if we really care. :)
>>
>> There are 3 solutions here:
>>
>> 1) Get rid of these RCU callbacks and use synchronize_rcu(). The
>> downside is this could hurt the performance of deleting TC filters,
>> but again it is slow path comparing to skb classification path. Note,
>> it is _not_ merely replacing call_rcu() with synchronize_rcu(),
>> because many call_rcu()'s are actually in list iterations, we have to
>> use a local list and call list_del_rcu()+list_add() before
>> synchronize_rcu() (Or is there any other API I am not aware of?). If
>> people really hate synchronize_rcu() because of performance, we could
>> also defer the work to a workqueue and callers could keep their
>> performance as they are.
>>
>> 2) Introduce a spinlock to serialize these RCU callbacks. But as I
>> said in commit 1697c4bb5245 ("net_sched: carefully handle
>> tcf_block_put()"), it is very hard to do because of tcf_chain_dump().
>> Potentially we need to do a lot of work to make it possible, if not
>> impossible.
>>
>> 3) Keep these RCU callbacks and fix all race conditions. Like what
>> Chris tries to do in his patchset, but my argument is that we can not
>> prove we are really race-free even with Chris' patches and his patches
>> are already large enough.
>>
>>
>> What do you think? Any other ideas?
>
> 4) Move from call_rcu() to synchronize_rcu(), but if feasible use one
> synchronize_rcu() for multiple deletions/iterations.
This is what I meant by using a local list, perhaps I didn't make it clear.
>
> 5) Keep call_rcu(), but have the RCU callback schedule a workqueue.
> The workqueue could then use blocking primitives, for example, acquiring
> RTNL.
Yeah, this could work too but we would get one more async...
filter delete -> call_rcu() -> schedule_work() -> action destroy
>
> 6) As with #5, have the RCU callback schedule a workqueue, but aggregate
> workqueue scheduling using a timer. This would reduce the number of
> RTNL acquisitions.
Ouch, sounds like even one more async:
filter delete -> call_rcu() -> schedule_work() -> timer -> flush_work()
-> action destroy
:-(
>
> 7) As with #5, have the RCU callback schedule a workqueue, but have each
> iterator accumulate a list of things removed and do call_rcu() on the
> list. This is an alternative way of aggregating to reduce the number
> of RTNL acquisitions.
Yeah, this seems working too.
>
> There are many other ways to skin this cat.
>
We still have to pick one. :) Any preference? I want to keep it as simple
as possible, otherwise some day I would not understand it either.
Thanks for all the ideas!
^ 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