* Re: [PATCH 0/3] net: Use kzfree() directly
From: David Miller @ 2019-09-05 10:06 UTC (permalink / raw)
To: zhongjiang; +Cc: anna.schumaker, trond.myklebust, netdev, linux-kernel
In-Reply-To: <1567564752-6430-1-git-send-email-zhongjiang@huawei.com>
From: zhong jiang <zhongjiang@huawei.com>
Date: Wed, 4 Sep 2019 10:39:09 +0800
> With the help of Coccinelle. We find some place to replace.
>
> @@
> expression M, S;
> @@
>
> - memset(M, 0, S);
> - kfree(M);
> + kzfree(M);
Series applied to net-next.
^ permalink raw reply
* Re: [PATCH net-next 4/7] net: hns3: add client node validity judgment
From: Sergei Shtylyov @ 2019-09-05 10:12 UTC (permalink / raw)
To: Huazhong Tan, davem
Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm,
jakub.kicinski, Peng Li
In-Reply-To: <1567606006-39598-5-git-send-email-tanhuazhong@huawei.com>
On 04.09.2019 17:06, Huazhong Tan wrote:
> From: Peng Li <lipeng321@huawei.com>
>
> HNS3 driver can only unregister client which included in hnae3_client_list.
> This patch adds the client node validity judgment.
>
> Signed-off-by: Peng Li <lipeng321@huawei.com>
> Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
> ---
> drivers/net/ethernet/hisilicon/hns3/hnae3.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.c b/drivers/net/ethernet/hisilicon/hns3/hnae3.c
> index 528f624..6aa5257 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hnae3.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.c
> @@ -138,12 +138,28 @@ EXPORT_SYMBOL(hnae3_register_client);
>
> void hnae3_unregister_client(struct hnae3_client *client)
> {
> + struct hnae3_client *client_tmp;
> struct hnae3_ae_dev *ae_dev;
> + bool existed = false;
>
> if (!client)
> return;
>
> mutex_lock(&hnae3_common_lock);
> +
> + list_for_each_entry(client_tmp, &hnae3_client_list, node) {
> + if (client_tmp->type == client->type) {
> + existed = true;
> + break;
> + }
> + }
> +
> + if (!existed) {
> + mutex_unlock(&hnae3_common_lock);
> + pr_err("client %s not existed!\n", client->name);
Did not exist, you mean?
[...]
MBR, Sergei
^ permalink raw reply
* Re: [PATCH net] ipv4: fix ifa_flags reuse problem in using ifconfig tool
From: David Miller @ 2019-09-05 10:13 UTC (permalink / raw)
To: suyj.fnst; +Cc: kuznet, yoshfuji, netdev, linux-kernel
In-Reply-To: <1567582667-56549-1-git-send-email-suyj.fnst@cn.fujitsu.com>
From: Su Yanjun <suyj.fnst@cn.fujitsu.com>
Date: Wed, 4 Sep 2019 15:37:47 +0800
> When NetworkManager has already set ipv4 address then uses
> ifconfig set another ipv4 address. It will use previous ifa_flags
> that will cause device route not be inserted.
>
> As NetworkManager has already support IFA_F_NOPREFIXROUTE flag [1],
> but ifconfig will reuse the ifa_flags. It's weird especially
> some old scripts or program [2] still use ifconfig.
>
> [1] https://gitlab.freedesktop.org/NetworkManager/NetworkManager/
> commit/fec80e7473ad16979af75ed299d68103e7aa3fe9
>
> [2] LTP or TAHI
>
> Signed-off-by: Su Yanjun <suyj.fnst@cn.fujitsu.com>
I don't know about this.
This will lose things like IFA_F_SECONDARY as well.
Sorry, I am not convinced that this change is correct nor safe.
^ permalink raw reply
* Re: [patch net-next] rocker: add missing init_net check in FIB notifier
From: David Miller @ 2019-09-05 10:15 UTC (permalink / raw)
To: jiri; +Cc: netdev, mlxsw
In-Reply-To: <20190904074047.840-1-jiri@resnulli.us>
From: Jiri Pirko <jiri@resnulli.us>
Date: Wed, 4 Sep 2019 09:40:47 +0200
> From: Jiri Pirko <jiri@mellanox.com>
>
> Take only FIB events that are happening in init_net into account. No other
> namespaces are supported.
>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] r8152: adjust the settings of ups flags
From: David Miller @ 2019-09-05 10:16 UTC (permalink / raw)
To: hayeswang; +Cc: netdev, nic_swsd, linux-kernel
In-Reply-To: <1394712342-15778-327-Taiwan-albertk@realtek.com>
From: Hayes Wang <hayeswang@realtek.com>
Date: Wed, 4 Sep 2019 17:34:38 +0800
> The UPS feature only works for runtime suspend, so UPS flags only
> need to be set before enabling runtime suspend. Therefore, I create
> a struct to record relative information, and use it before runtime
> suspend.
>
> All chips could record such information, even though not all of
> them support the feature of UPS. Then, some functions could be
> combined.
>
> Signed-off-by: Hayes Wang <hayeswang@realtek.com>
This does not apply cleanly to net-next, please respin.
^ permalink raw reply
* Re: pull-request: can-next 2019-09-04 j1939,pull-request: can-next 2019-09-04 j1939
From: David Miller @ 2019-09-05 10:18 UTC (permalink / raw)
To: mkl
Cc: netdev, kernel, linux-can, socketcan, bst, ecathinds, dev.kurt,
maxime.jayat, robin, ore, david
In-Reply-To: <d56029d4-2d4c-3cb3-0e5b-e28866db87f1@pengutronix.de>
From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: Wed, 4 Sep 2019 14:29:56 +0200
> this is a pull request for net-next/master consisting of 21 patches.
Pulled, thanks Marc.
^ permalink raw reply
* Re: [PATCH v2 net-next 00/13] net: stmmac: Improvements for -next
From: David Miller @ 2019-09-05 10:20 UTC (permalink / raw)
To: Jose.Abreu
Cc: netdev, Joao.Pinto, peppe.cavallaro, alexandre.torgue,
mcoquelin.stm32, linux-stm32, linux-arm-kernel, linux-kernel
In-Reply-To: <cover.1567602867.git.joabreu@synopsys.com>
From: Jose Abreu <Jose.Abreu@synopsys.com>
Date: Wed, 4 Sep 2019 15:16:52 +0200
> Couple of improvements for -next tree. More info in commit logs.
Series applied, thank you.
^ permalink raw reply
* [PATCH 2/5] xfrm interface: ifname may be wrong in logs
From: Steffen Klassert @ 2019-09-05 10:21 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <20190905102201.1636-1-steffen.klassert@secunet.com>
From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
The ifname is copied when the interface is created, but is never updated
later. In fact, this property is used only in one error message, where the
netdevice pointer is available, thus let's use it.
Fixes: f203b76d7809 ("xfrm: Add virtual xfrm interfaces")
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
include/net/xfrm.h | 1 -
net/xfrm/xfrm_interface.c | 10 +---------
2 files changed, 1 insertion(+), 10 deletions(-)
diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index b22db30c3d88..ad761ef84797 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -983,7 +983,6 @@ static inline void xfrm_dst_destroy(struct xfrm_dst *xdst)
void xfrm_dst_ifdown(struct dst_entry *dst, struct net_device *dev);
struct xfrm_if_parms {
- char name[IFNAMSIZ]; /* name of XFRM device */
int link; /* ifindex of underlying L2 interface */
u32 if_id; /* interface identifyer */
};
diff --git a/net/xfrm/xfrm_interface.c b/net/xfrm/xfrm_interface.c
index 2310dc9e35c2..68336ee00d72 100644
--- a/net/xfrm/xfrm_interface.c
+++ b/net/xfrm/xfrm_interface.c
@@ -145,8 +145,6 @@ static int xfrmi_create(struct net_device *dev)
if (err < 0)
goto out;
- strcpy(xi->p.name, dev->name);
-
dev_hold(dev);
xfrmi_link(xfrmn, xi);
@@ -294,7 +292,7 @@ xfrmi_xmit2(struct sk_buff *skb, struct net_device *dev, struct flowi *fl)
if (tdev == dev) {
stats->collisions++;
net_warn_ratelimited("%s: Local routing loop detected!\n",
- xi->p.name);
+ dev->name);
goto tx_err_dst_release;
}
@@ -638,12 +636,6 @@ static int xfrmi_newlink(struct net *src_net, struct net_device *dev,
int err;
xfrmi_netlink_parms(data, &p);
-
- if (!tb[IFLA_IFNAME])
- return -EINVAL;
-
- nla_strlcpy(p.name, tb[IFLA_IFNAME], IFNAMSIZ);
-
xi = xfrmi_locate(net, &p);
if (xi)
return -EEXIST;
--
2.17.1
^ permalink raw reply related
* [PATCH 3/5] xfrm interface: fix list corruption for x-netns
From: Steffen Klassert @ 2019-09-05 10:21 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <20190905102201.1636-1-steffen.klassert@secunet.com>
From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
dev_net(dev) is the netns of the device and xi->net is the link netns,
where the device has been linked.
changelink() must operate in the link netns to avoid a corruption of
the xfrm lists.
Note that xi->net and dev_net(xi->physdev) are always the same.
Before the patch, the xfrmi lists may be corrupted and can later trigger a
kernel panic.
Fixes: f203b76d7809 ("xfrm: Add virtual xfrm interfaces")
Reported-by: Julien Floret <julien.floret@6wind.com>
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Tested-by: Julien Floret <julien.floret@6wind.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/xfrm/xfrm_interface.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/xfrm/xfrm_interface.c b/net/xfrm/xfrm_interface.c
index 68336ee00d72..53e5e47b2c55 100644
--- a/net/xfrm/xfrm_interface.c
+++ b/net/xfrm/xfrm_interface.c
@@ -503,7 +503,7 @@ static int xfrmi_change(struct xfrm_if *xi, const struct xfrm_if_parms *p)
static int xfrmi_update(struct xfrm_if *xi, struct xfrm_if_parms *p)
{
- struct net *net = dev_net(xi->dev);
+ struct net *net = xi->net;
struct xfrmi_net *xfrmn = net_generic(net, xfrmi_net_id);
int err;
@@ -663,9 +663,9 @@ static int xfrmi_changelink(struct net_device *dev, struct nlattr *tb[],
struct nlattr *data[],
struct netlink_ext_ack *extack)
{
- struct net *net = dev_net(dev);
+ struct xfrm_if *xi = netdev_priv(dev);
+ struct net *net = xi->net;
struct xfrm_if_parms p;
- struct xfrm_if *xi;
xfrmi_netlink_parms(data, &p);
xi = xfrmi_locate(net, &p);
@@ -707,7 +707,7 @@ static struct net *xfrmi_get_link_net(const struct net_device *dev)
{
struct xfrm_if *xi = netdev_priv(dev);
- return dev_net(xi->phydev);
+ return xi->net;
}
static const struct nla_policy xfrmi_policy[IFLA_XFRM_MAX + 1] = {
--
2.17.1
^ permalink raw reply related
* [PATCH 4/5] xfrm interface: fix management of phydev
From: Steffen Klassert @ 2019-09-05 10:22 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <20190905102201.1636-1-steffen.klassert@secunet.com>
From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
With the current implementation, phydev cannot be removed:
$ ip link add dummy type dummy
$ ip link add xfrm1 type xfrm dev dummy if_id 1
$ ip l d dummy
kernel:[77938.465445] unregister_netdevice: waiting for dummy to become free. Usage count = 1
Manage it like in ip tunnels, ie just keep the ifindex. Not that the side
effect, is that the phydev is now optional.
Fixes: f203b76d7809 ("xfrm: Add virtual xfrm interfaces")
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Tested-by: Julien Floret <julien.floret@6wind.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
include/net/xfrm.h | 1 -
net/xfrm/xfrm_interface.c | 32 +++++++++++++++++---------------
2 files changed, 17 insertions(+), 16 deletions(-)
diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index ad761ef84797..aa08a7a5f6ac 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -990,7 +990,6 @@ struct xfrm_if_parms {
struct xfrm_if {
struct xfrm_if __rcu *next; /* next interface in list */
struct net_device *dev; /* virtual device associated with interface */
- struct net_device *phydev; /* physical device */
struct net *net; /* netns for packet i/o */
struct xfrm_if_parms p; /* interface parms */
diff --git a/net/xfrm/xfrm_interface.c b/net/xfrm/xfrm_interface.c
index 53e5e47b2c55..2ab4859df55a 100644
--- a/net/xfrm/xfrm_interface.c
+++ b/net/xfrm/xfrm_interface.c
@@ -175,7 +175,6 @@ static void xfrmi_dev_uninit(struct net_device *dev)
struct xfrmi_net *xfrmn = net_generic(xi->net, xfrmi_net_id);
xfrmi_unlink(xfrmn, xi);
- dev_put(xi->phydev);
dev_put(dev);
}
@@ -362,7 +361,7 @@ static netdev_tx_t xfrmi_xmit(struct sk_buff *skb, struct net_device *dev)
goto tx_err;
}
- fl.flowi_oif = xi->phydev->ifindex;
+ fl.flowi_oif = xi->p.link;
ret = xfrmi_xmit2(skb, dev, &fl);
if (ret < 0)
@@ -548,7 +547,7 @@ static int xfrmi_get_iflink(const struct net_device *dev)
{
struct xfrm_if *xi = netdev_priv(dev);
- return xi->phydev->ifindex;
+ return xi->p.link;
}
@@ -574,12 +573,14 @@ static void xfrmi_dev_setup(struct net_device *dev)
dev->needs_free_netdev = true;
dev->priv_destructor = xfrmi_dev_free;
netif_keep_dst(dev);
+
+ eth_broadcast_addr(dev->broadcast);
}
static int xfrmi_dev_init(struct net_device *dev)
{
struct xfrm_if *xi = netdev_priv(dev);
- struct net_device *phydev = xi->phydev;
+ struct net_device *phydev = __dev_get_by_index(xi->net, xi->p.link);
int err;
dev->tstats = netdev_alloc_pcpu_stats(struct pcpu_sw_netstats);
@@ -594,13 +595,19 @@ static int xfrmi_dev_init(struct net_device *dev)
dev->features |= NETIF_F_LLTX;
- dev->needed_headroom = phydev->needed_headroom;
- dev->needed_tailroom = phydev->needed_tailroom;
+ if (phydev) {
+ dev->needed_headroom = phydev->needed_headroom;
+ dev->needed_tailroom = phydev->needed_tailroom;
- if (is_zero_ether_addr(dev->dev_addr))
- eth_hw_addr_inherit(dev, phydev);
- if (is_zero_ether_addr(dev->broadcast))
- memcpy(dev->broadcast, phydev->broadcast, dev->addr_len);
+ if (is_zero_ether_addr(dev->dev_addr))
+ eth_hw_addr_inherit(dev, phydev);
+ if (is_zero_ether_addr(dev->broadcast))
+ memcpy(dev->broadcast, phydev->broadcast,
+ dev->addr_len);
+ } else {
+ eth_hw_addr_random(dev);
+ eth_broadcast_addr(dev->broadcast);
+ }
return 0;
}
@@ -644,13 +651,8 @@ static int xfrmi_newlink(struct net *src_net, struct net_device *dev,
xi->p = p;
xi->net = net;
xi->dev = dev;
- xi->phydev = dev_get_by_index(net, p.link);
- if (!xi->phydev)
- return -ENODEV;
err = xfrmi_create(dev);
- if (err < 0)
- dev_put(xi->phydev);
return err;
}
--
2.17.1
^ permalink raw reply related
* [PATCH 5/5] xfrm: policy: avoid warning splat when merging nodes
From: Steffen Klassert @ 2019-09-05 10:22 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <20190905102201.1636-1-steffen.klassert@secunet.com>
From: Florian Westphal <fw@strlen.de>
syzbot reported a splat:
xfrm_policy_inexact_list_reinsert+0x625/0x6e0 net/xfrm/xfrm_policy.c:877
CPU: 1 PID: 6756 Comm: syz-executor.1 Not tainted 5.3.0-rc2+ #57
Call Trace:
xfrm_policy_inexact_node_reinsert net/xfrm/xfrm_policy.c:922 [inline]
xfrm_policy_inexact_node_merge net/xfrm/xfrm_policy.c:958 [inline]
xfrm_policy_inexact_insert_node+0x537/0xb50 net/xfrm/xfrm_policy.c:1023
xfrm_policy_inexact_alloc_chain+0x62b/0xbd0 net/xfrm/xfrm_policy.c:1139
xfrm_policy_inexact_insert+0xe8/0x1540 net/xfrm/xfrm_policy.c:1182
xfrm_policy_insert+0xdf/0xce0 net/xfrm/xfrm_policy.c:1574
xfrm_add_policy+0x4cf/0x9b0 net/xfrm/xfrm_user.c:1670
xfrm_user_rcv_msg+0x46b/0x720 net/xfrm/xfrm_user.c:2676
netlink_rcv_skb+0x1f0/0x460 net/netlink/af_netlink.c:2477
xfrm_netlink_rcv+0x74/0x90 net/xfrm/xfrm_user.c:2684
netlink_unicast_kernel net/netlink/af_netlink.c:1302 [inline]
netlink_unicast+0x809/0x9a0 net/netlink/af_netlink.c:1328
netlink_sendmsg+0xa70/0xd30 net/netlink/af_netlink.c:1917
sock_sendmsg_nosec net/socket.c:637 [inline]
sock_sendmsg net/socket.c:657 [inline]
There is no reproducer, however, the warning can be reproduced
by adding rules with ever smaller prefixes.
The sanity check ("does the policy match the node") uses the prefix value
of the node before its updated to the smaller value.
To fix this, update the prefix earlier. The bug has no impact on tree
correctness, this is only to prevent a false warning.
Reported-by: syzbot+8cc27ace5f6972910b31@syzkaller.appspotmail.com
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/xfrm/xfrm_policy.c | 6 ++++--
tools/testing/selftests/net/xfrm_policy.sh | 7 +++++++
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 8ca637a72697..0fa7c5ce3b2c 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -912,6 +912,7 @@ static void xfrm_policy_inexact_node_reinsert(struct net *net,
} else if (delta > 0) {
p = &parent->rb_right;
} else {
+ bool same_prefixlen = node->prefixlen == n->prefixlen;
struct xfrm_policy *tmp;
hlist_for_each_entry(tmp, &n->hhead, bydst) {
@@ -919,9 +920,11 @@ static void xfrm_policy_inexact_node_reinsert(struct net *net,
hlist_del_rcu(&tmp->bydst);
}
+ node->prefixlen = prefixlen;
+
xfrm_policy_inexact_list_reinsert(net, node, family);
- if (node->prefixlen == n->prefixlen) {
+ if (same_prefixlen) {
kfree_rcu(n, rcu);
return;
}
@@ -929,7 +932,6 @@ static void xfrm_policy_inexact_node_reinsert(struct net *net,
rb_erase(*p, new);
kfree_rcu(n, rcu);
n = node;
- n->prefixlen = prefixlen;
goto restart;
}
}
diff --git a/tools/testing/selftests/net/xfrm_policy.sh b/tools/testing/selftests/net/xfrm_policy.sh
index 5445943bf07f..7a1bf94c5bd3 100755
--- a/tools/testing/selftests/net/xfrm_policy.sh
+++ b/tools/testing/selftests/net/xfrm_policy.sh
@@ -106,6 +106,13 @@ do_overlap()
#
# 10.0.0.0/24 and 10.0.1.0/24 nodes have been merged as 10.0.0.0/23.
ip -net $ns xfrm policy add src 10.1.0.0/24 dst 10.0.0.0/23 dir fwd priority 200 action block
+
+ # similar to above: add policies (with partially random address), with shrinking prefixes.
+ for p in 29 28 27;do
+ for k in $(seq 1 32); do
+ ip -net $ns xfrm policy add src 10.253.1.$((RANDOM%255))/$p dst 10.254.1.$((RANDOM%255))/$p dir fwd priority $((200+k)) action block 2>/dev/null
+ done
+ done
}
do_esp_policy_get_check() {
--
2.17.1
^ permalink raw reply related
* pull request (net): ipsec 2019-09-05
From: Steffen Klassert @ 2019-09-05 10:21 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
1) Several xfrm interface fixes from Nicolas Dichtel:
- Avoid an interface ID corruption on changelink.
- Fix wrong intterface names in the logs.
- Fix a list corruption when changing network namespaces.
- Fix unregistation of the underying phydev.
2) Fix a potential warning when merging xfrm_plocy nodes.
From Florian Westphal.
Please pull or let me know if there are problems.
Thanks!
The following changes since commit 114a5c3240155fdb01bf821c9d326d7bb05bd464:
Merge tag 'mlx5-fixes-2019-07-11' of git://git.kernel.org/pub/scm/linux/kernel/git/saeed/linux (2019-07-11 15:06:37 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec.git master
for you to fetch changes up to 769a807d0b41df4201dbeb01c22eaeb3e5905532:
xfrm: policy: avoid warning splat when merging nodes (2019-08-20 08:09:42 +0200)
----------------------------------------------------------------
Florian Westphal (1):
xfrm: policy: avoid warning splat when merging nodes
Nicolas Dichtel (4):
xfrm interface: avoid corruption on changelink
xfrm interface: ifname may be wrong in logs
xfrm interface: fix list corruption for x-netns
xfrm interface: fix management of phydev
include/net/xfrm.h | 2 --
net/xfrm/xfrm_interface.c | 56 +++++++++++++-----------------
net/xfrm/xfrm_policy.c | 6 ++--
tools/testing/selftests/net/xfrm_policy.sh | 7 ++++
4 files changed, 36 insertions(+), 35 deletions(-)
^ permalink raw reply
* [PATCH 1/5] xfrm interface: avoid corruption on changelink
From: Steffen Klassert @ 2019-09-05 10:21 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <20190905102201.1636-1-steffen.klassert@secunet.com>
From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
The new parameters must not be stored in the netdev_priv() before
validation, it may corrupt the interface. Note also that if data is NULL,
only a memset() is done.
$ ip link add xfrm1 type xfrm dev lo if_id 1
$ ip link add xfrm2 type xfrm dev lo if_id 2
$ ip link set xfrm1 type xfrm dev lo if_id 2
RTNETLINK answers: File exists
$ ip -d link list dev xfrm1
5: xfrm1@lo: <NOARP> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
link/none 00:00:00:00:00:00 brd 00:00:00:00:00:00 promiscuity 0 minmtu 68 maxmtu 1500
xfrm if_id 0x2 addrgenmode eui64 numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535
=> "if_id 0x2"
Fixes: f203b76d7809 ("xfrm: Add virtual xfrm interfaces")
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Tested-by: Julien Floret <julien.floret@6wind.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/xfrm/xfrm_interface.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/net/xfrm/xfrm_interface.c b/net/xfrm/xfrm_interface.c
index 74868f9d81fb..2310dc9e35c2 100644
--- a/net/xfrm/xfrm_interface.c
+++ b/net/xfrm/xfrm_interface.c
@@ -671,12 +671,12 @@ static int xfrmi_changelink(struct net_device *dev, struct nlattr *tb[],
struct nlattr *data[],
struct netlink_ext_ack *extack)
{
- struct xfrm_if *xi = netdev_priv(dev);
struct net *net = dev_net(dev);
+ struct xfrm_if_parms p;
+ struct xfrm_if *xi;
- xfrmi_netlink_parms(data, &xi->p);
-
- xi = xfrmi_locate(net, &xi->p);
+ xfrmi_netlink_parms(data, &p);
+ xi = xfrmi_locate(net, &p);
if (!xi) {
xi = netdev_priv(dev);
} else {
@@ -684,7 +684,7 @@ static int xfrmi_changelink(struct net_device *dev, struct nlattr *tb[],
return -EEXIST;
}
- return xfrmi_update(xi, &xi->p);
+ return xfrmi_update(xi, &p);
}
static size_t xfrmi_get_size(const struct net_device *dev)
--
2.17.1
^ permalink raw reply related
* Re: [PATCH 0/4] gianfar: some assorted cleanup
From: David Miller @ 2019-09-05 10:28 UTC (permalink / raw)
To: asolokha
Cc: claudiu.manoil, ioana.ciornei, linux, andrew, olteanv, f.fainelli,
netdev
In-Reply-To: <20190904135223.31754-1-asolokha@kb.kras.ru>
From: Arseny Solokha <asolokha@kb.kras.ru>
Date: Wed, 4 Sep 2019 20:52:18 +0700
> This is a cleanup series for the gianfar Ethernet driver, following up a
> discussion in [1]. It is intended to precede a conversion of gianfar from
> PHYLIB to PHYLINK API, which will be submitted later in its version 2.
> However, it won't make a conversion cleaner, except for the last patch in
> this series. Obviously this series is not intended for -stable.
>
> The first patch looks super controversial to me, as it moves lots of code
> around for the sole purpose of getting rid of static forward declarations
> in two translation units. On the other hand, this change is purely
> mechanical and cannot do any harm other than cluttering git blame output.
> I can prepare an alternative patch for only swapping adjacent functions
> around, if necessary.
>
> The second patch is a trivial follow-up to the first one, making functions
> that are only called from the same translation unit static.
>
> The third patch removes some now unused macro and structure definitions
> from gianfar.h, slipped away from various cleanups in the past.
>
> The fourth patch, also suggested in [1], makes the driver consistently use
> PHY connection type value obtained from a Device Tree node, instead of
> ignoring it and using the one auto-detected by MAC, when connecting to PHY.
> Obviously a value has to be specified correctly in DT source, or omitted
> altogether, in which case the driver will fall back to auto-detection. When
> querying a DT node, the driver will also take both applicable properties
> into account by making a proper API call instead of open-coding the lookup
> half-way correctly.
>
> [1] https://lore.kernel.org/netdev/CA+h21hruqt6nGG5ksDSwrGH_w5GtGF4fjAMCWJne7QJrjusERQ@mail.gmail.com/
Series applied, thanks.
^ permalink raw reply
* Re: [PATCH v2 0/2] Fix GMII2RGMII private field
From: David Miller @ 2019-09-05 10:32 UTC (permalink / raw)
To: harini.katakam
Cc: andrew, f.fainelli, hkallweit1, michal.simek, netdev,
linux-arm-kernel, linux-kernel, harinikatakamlinux,
radhey.shyam.pandey
In-Reply-To: <1567605621-6818-1-git-send-email-harini.katakam@xilinx.com>
From: Harini Katakam <harini.katakam@xilinx.com>
Date: Wed, 4 Sep 2019 19:30:19 +0530
> Fix the usage of external phy's priv field by gmii2rgmii driver.
>
> Based on net-next.
Series applied to net-next.
^ permalink raw reply
* Re: [PATCH v1] pppoatm: use %*ph to print small buffer
From: David Miller @ 2019-09-05 10:33 UTC (permalink / raw)
To: andriy.shevchenko; +Cc: mitch, netdev
In-Reply-To: <20190904174459.77067-1-andriy.shevchenko@linux.intel.com>
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: Wed, 4 Sep 2019 20:44:59 +0300
> Use %*ph format to print small buffer as hex string.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Applied to net-next.
^ permalink raw reply
* Re: [PATCH v3 net] net: Properly update v4 routes with v6 nexthop
From: David Miller @ 2019-09-05 10:36 UTC (permalink / raw)
To: sharpd; +Cc: netdev, dsahern, sworley
In-Reply-To: <20190904141158.17021-1-sharpd@cumulusnetworks.com>
From: Donald Sharp <sharpd@cumulusnetworks.com>
Date: Wed, 4 Sep 2019 10:11:58 -0400
> When creating a v4 route that uses a v6 nexthop from a nexthop group.
> Allow the kernel to properly send the nexthop as v6 via the RTA_VIA
> attribute.
>
> Broken behavior:
>
> $ ip nexthop add via fe80::9 dev eth0
> $ ip nexthop show
> id 1 via fe80::9 dev eth0 scope link
> $ ip route add 4.5.6.7/32 nhid 1
> $ ip route show
> default via 10.0.2.2 dev eth0
> 4.5.6.7 nhid 1 via 254.128.0.0 dev eth0
> 10.0.2.0/24 dev eth0 proto kernel scope link src 10.0.2.15
> $
>
> Fixed behavior:
>
> $ ip nexthop add via fe80::9 dev eth0
> $ ip nexthop show
> id 1 via fe80::9 dev eth0 scope link
> $ ip route add 4.5.6.7/32 nhid 1
> $ ip route show
> default via 10.0.2.2 dev eth0
> 4.5.6.7 nhid 1 via inet6 fe80::9 dev eth0
> 10.0.2.0/24 dev eth0 proto kernel scope link src 10.0.2.15
> $
>
> v2, v3: Addresses code review comments from David Ahern
>
> Fixes: dcb1ecb50edf (“ipv4: Prepare for fib6_nh from a nexthop object”)
> Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH] net: qed: Move static keyword to the front of declaration
From: David Miller @ 2019-09-05 10:39 UTC (permalink / raw)
To: kw; +Cc: aelior, GR-everest-linux-l2, netdev, linux-kernel
In-Reply-To: <20190904141730.31497-1-kw@linux.com>
From: Krzysztof Wilczynski <kw@linux.com>
Date: Wed, 4 Sep 2019 16:17:30 +0200
> Move the static keyword to the front of declaration of iwarp_state_names,
> and resolve the following compiler warning that can be seen when building
> with warnings enabled (W=1):
>
> drivers/net/ethernet/qlogic/qed/qed_iwarp.c:385:1: warning:
> ‘static’ is not at beginning of declaration [-Wold-style-declaration]
>
> Also, resolve checkpatch.pl script warning:
>
> WARNING: static const char * array should probably be
> static const char * const
>
> Signed-off-by: Krzysztof Wilczynski <kw@linux.com>
Applied to net-next.
^ permalink raw reply
* Re: [PATCH] net: hns: Move static keyword to the front of declaration
From: David Miller @ 2019-09-05 10:39 UTC (permalink / raw)
To: kw
Cc: yisen.zhuang, salil.mehta, liuyonglong, lipeng321, gregkh,
colin.king, huang.zijiang, tglx, netdev, linux-kernel
In-Reply-To: <20190904142116.31884-1-kw@linux.com>
From: Krzysztof Wilczynski <kw@linux.com>
Date: Wed, 4 Sep 2019 16:21:16 +0200
> Move the static keyword to the front of declaration of g_dsaf_mode_match,
> and resolve the following compiler warning that can be seen when building
> with warnings enabled (W=1):
>
> drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c:27:1: warning:
> ‘static’ is not at beginning of declaration [-Wold-style-declaration]
>
> Signed-off-by: Krzysztof Wilczynski <kw@linux.com>
Applied to net-next.
^ permalink raw reply
* Re: [PATCH 0/4] gianfar: some assorted cleanup
From: Vladimir Oltean @ 2019-09-05 10:39 UTC (permalink / raw)
To: Arseny Solokha
Cc: Claudiu Manoil, Ioana Ciornei, Russell King, Andrew Lunn,
Florian Fainelli, netdev
In-Reply-To: <20190904135223.31754-1-asolokha@kb.kras.ru>
On Wed, 4 Sep 2019 at 16:53, Arseny Solokha <asolokha@kb.kras.ru> wrote:
>
> This is a cleanup series for the gianfar Ethernet driver, following up a
> discussion in [1]. It is intended to precede a conversion of gianfar from
> PHYLIB to PHYLINK API, which will be submitted later in its version 2.
> However, it won't make a conversion cleaner, except for the last patch in
> this series. Obviously this series is not intended for -stable.
>
> The first patch looks super controversial to me, as it moves lots of code
> around for the sole purpose of getting rid of static forward declarations
> in two translation units. On the other hand, this change is purely
> mechanical and cannot do any harm other than cluttering git blame output.
> I can prepare an alternative patch for only swapping adjacent functions
> around, if necessary.
>
> The second patch is a trivial follow-up to the first one, making functions
> that are only called from the same translation unit static.
>
> The third patch removes some now unused macro and structure definitions
> from gianfar.h, slipped away from various cleanups in the past.
>
> The fourth patch, also suggested in [1], makes the driver consistently use
> PHY connection type value obtained from a Device Tree node, instead of
> ignoring it and using the one auto-detected by MAC, when connecting to PHY.
> Obviously a value has to be specified correctly in DT source, or omitted
> altogether, in which case the driver will fall back to auto-detection. When
> querying a DT node, the driver will also take both applicable properties
> into account by making a proper API call instead of open-coding the lookup
> half-way correctly.
>
> [1] https://lore.kernel.org/netdev/CA+h21hruqt6nGG5ksDSwrGH_w5GtGF4fjAMCWJne7QJrjusERQ@mail.gmail.com/
>
> Arseny Solokha (4):
> gianfar: remove forward declarations
> gianfar: make five functions static
> gianfar: cleanup gianfar.h
> gianfar: use DT more consistently when selecting PHY connection type
>
> drivers/net/ethernet/freescale/gianfar.c | 4647 ++++++++---------
> drivers/net/ethernet/freescale/gianfar.h | 45 -
> .../net/ethernet/freescale/gianfar_ethtool.c | 13 -
> 3 files changed, 2303 insertions(+), 2402 deletions(-)
>
> --
> 2.23.0
>
Thanks for the cleanup!
-Vladimir
^ permalink raw reply
* Re: [PATCH v2 2/2] PTP: add support for one-shot output
From: Felipe Balbi @ 2019-09-05 10:40 UTC (permalink / raw)
To: Richard Cochran; +Cc: Christopher S Hall, netdev, linux-kernel, davem
In-Reply-To: <20190831150149.GB1692@localhost>
[-- Attachment #1: Type: text/plain, Size: 8864 bytes --]
Hi,
Richard Cochran <richardcochran@gmail.com> writes:
> On Fri, Aug 30, 2019 at 11:00:20AM +0300, Felipe Balbi wrote:
>> seems like this should be defined together with the other flags? If
>> that's the case, it seems like we would EXTTS and PEROUT masks.
>
> Yes, let's make the meanings of the bit fields clear...
>
> --- ptp_clock.h ---
>
> /*
> * Bits of the ptp_extts_request.flags field:
> */
> #define PTP_ENABLE_FEATURE BIT(0)
> #define PTP_RISING_EDGE BIT(1)
> #define PTP_FALLING_EDGE BIT(2)
> #define PTP_EXTTS_VALID_FLAGS (PTP_ENABLE_FEATURE | \
> PTP_RISING_EDGE | \
> PTP_FALLING_EDGE)
>
> /*
> * Bits of the ptp_perout_request.flags field:
> */
> #define PTP_PEROUT_ONE_SHOT BIT(0)
> #define PTP_PEROUT_VALID_FLAGS (PTP_PEROUT_ONE_SHOT)
>
> struct ptp_extts_request {
> unsigned int flags; /* Bit field of PTP_EXTTS_VALID_FLAGS. */
> };
>
> struct ptp_perout_request {
> unsigned int flags; /* Bit field of PTP_PEROUT_VALID_FLAGS. */
> };
Below you can find the combined patch. Locally, I have it split into two
patches, but this lets us look at the full picture. I'll send it as v3
series of two patches on Monday if you like the result. Let me know if
you prefer that I convert the flags to BIT() macro calls instead.
8<------------------------------------------------------------------------
From 633a8214c86a43dcf880d7aed33758b576933369 Mon Sep 17 00:00:00 2001
From: Felipe Balbi <felipe.balbi@linux.intel.com>
Date: Wed, 14 Aug 2019 10:31:08 +0300
Subject: [PATCH 1/5] PTP: introduce new versions of IOCTLs
The current version of the IOCTL have a small problem which prevents us
from extending the API by making use of reserved fields. In these new
IOCTLs, we are now making sure that flags and rsv fields are zero which
will allow us to extend the API in the future.
Reviewed-by: Richard Cochran <richardcochran@gmail.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
---
drivers/ptp/ptp_chardev.c | 63 ++++++++++++++++++++++++++++++++++
include/uapi/linux/ptp_clock.h | 26 ++++++++++++--
2 files changed, 87 insertions(+), 2 deletions(-)
diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
index 18ffe449efdf..9c18476d8d10 100644
--- a/drivers/ptp/ptp_chardev.c
+++ b/drivers/ptp/ptp_chardev.c
@@ -126,7 +126,9 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
switch (cmd) {
case PTP_CLOCK_GETCAPS:
+ case PTP_CLOCK_GETCAPS2:
memset(&caps, 0, sizeof(caps));
+
caps.max_adj = ptp->info->max_adj;
caps.n_alarm = ptp->info->n_alarm;
caps.n_ext_ts = ptp->info->n_ext_ts;
@@ -139,11 +141,24 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
break;
case PTP_EXTTS_REQUEST:
+ case PTP_EXTTS_REQUEST2:
+ memset(&req, 0, sizeof(req));
+
if (copy_from_user(&req.extts, (void __user *)arg,
sizeof(req.extts))) {
err = -EFAULT;
break;
}
+ if (((req.extts.flags & ~PTP_EXTTS_VALID_FLAGS) ||
+ req.extts.rsv[0] || req.extts.rsv[1]) &&
+ cmd == PTP_EXTTS_REQUEST2) {
+ err = -EINVAL;
+ break;
+ } else if (cmd == PTP_EXTTS_REQUEST) {
+ req.extts.flags &= ~PTP_EXTTS_VALID_FLAGS;
+ req.extts.rsv[0] = 0;
+ req.extts.rsv[1] = 0;
+ }
if (req.extts.index >= ops->n_ext_ts) {
err = -EINVAL;
break;
@@ -154,11 +169,27 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
break;
case PTP_PEROUT_REQUEST:
+ case PTP_PEROUT_REQUEST2:
+ memset(&req, 0, sizeof(req));
+
if (copy_from_user(&req.perout, (void __user *)arg,
sizeof(req.perout))) {
err = -EFAULT;
break;
}
+ if (((req.perout.flags & ~PTP_PEROUT_VALID_FLAGS) ||
+ req.perout.rsv[0] || req.perout.rsv[1] ||
+ req.perout.rsv[2] || req.perout.rsv[3]) &&
+ cmd == PTP_PEROUT_REQUEST2) {
+ err = -EINVAL;
+ break;
+ } else if (cmd == PTP_PEROUT_REQUEST) {
+ req.perout.flags &= ~PTP_PEROUT_VALID_FLAGS;
+ req.perout.rsv[0] = 0;
+ req.perout.rsv[1] = 0;
+ req.perout.rsv[2] = 0;
+ req.perout.rsv[3] = 0;
+ }
if (req.perout.index >= ops->n_per_out) {
err = -EINVAL;
break;
@@ -169,6 +200,9 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
break;
case PTP_ENABLE_PPS:
+ case PTP_ENABLE_PPS2:
+ memset(&req, 0, sizeof(req));
+
if (!capable(CAP_SYS_TIME))
return -EPERM;
req.type = PTP_CLK_REQ_PPS;
@@ -177,6 +211,7 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
break;
case PTP_SYS_OFFSET_PRECISE:
+ case PTP_SYS_OFFSET_PRECISE2:
if (!ptp->info->getcrosststamp) {
err = -EOPNOTSUPP;
break;
@@ -201,6 +236,7 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
break;
case PTP_SYS_OFFSET_EXTENDED:
+ case PTP_SYS_OFFSET_EXTENDED2:
if (!ptp->info->gettimex64) {
err = -EOPNOTSUPP;
break;
@@ -232,6 +268,7 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
break;
case PTP_SYS_OFFSET:
+ case PTP_SYS_OFFSET2:
sysoff = memdup_user((void __user *)arg, sizeof(*sysoff));
if (IS_ERR(sysoff)) {
err = PTR_ERR(sysoff);
@@ -266,10 +303,23 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
break;
case PTP_PIN_GETFUNC:
+ case PTP_PIN_GETFUNC2:
if (copy_from_user(&pd, (void __user *)arg, sizeof(pd))) {
err = -EFAULT;
break;
}
+ if ((pd.rsv[0] || pd.rsv[1] || pd.rsv[2]
+ || pd.rsv[3] || pd.rsv[4])
+ && cmd == PTP_PIN_GETFUNC2) {
+ err = -EINVAL;
+ break;
+ } else if (cmd == PTP_PIN_GETFUNC) {
+ pd.rsv[0] = 0;
+ pd.rsv[1] = 0;
+ pd.rsv[2] = 0;
+ pd.rsv[3] = 0;
+ pd.rsv[4] = 0;
+ }
pin_index = pd.index;
if (pin_index >= ops->n_pins) {
err = -EINVAL;
@@ -285,10 +335,23 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
break;
case PTP_PIN_SETFUNC:
+ case PTP_PIN_SETFUNC2:
if (copy_from_user(&pd, (void __user *)arg, sizeof(pd))) {
err = -EFAULT;
break;
}
+ if ((pd.rsv[0] || pd.rsv[1] || pd.rsv[2]
+ || pd.rsv[3] || pd.rsv[4])
+ && cmd == PTP_PIN_SETFUNC2) {
+ err = -EINVAL;
+ break;
+ } else if (cmd == PTP_PIN_SETFUNC) {
+ pd.rsv[0] = 0;
+ pd.rsv[1] = 0;
+ pd.rsv[2] = 0;
+ pd.rsv[3] = 0;
+ pd.rsv[4] = 0;
+ }
pin_index = pd.index;
if (pin_index >= ops->n_pins) {
err = -EINVAL;
diff --git a/include/uapi/linux/ptp_clock.h b/include/uapi/linux/ptp_clock.h
index 1bc794ad957a..cbdc0d97b471 100644
--- a/include/uapi/linux/ptp_clock.h
+++ b/include/uapi/linux/ptp_clock.h
@@ -25,11 +25,21 @@
#include <linux/ioctl.h>
#include <linux/types.h>
-/* PTP_xxx bits, for the flags field within the request structures. */
+/*
+ * Bits of the ptp_extts_request.flags field:
+ */
#define PTP_ENABLE_FEATURE (1<<0)
#define PTP_RISING_EDGE (1<<1)
#define PTP_FALLING_EDGE (1<<2)
+#define PTP_EXTTS_VALID_FLAGS (PTP_ENABLE_FEATURE | \
+ PTP_RISING_EDGE | \
+ PTP_FALLING_EDGE)
+/*
+ * Bits of the ptp_perout_request.flags field:
+ */
+#define PTP_PEROUT_ONE_SHOT (1<<0)
+#define PTP_PEROUT_VALID_FLAGS (~PTP_PEROUT_ONE_SHOT)
/*
* struct ptp_clock_time - represents a time value
*
@@ -67,7 +77,7 @@ struct ptp_perout_request {
struct ptp_clock_time start; /* Absolute start time. */
struct ptp_clock_time period; /* Desired period, zero means disable. */
unsigned int index; /* Which channel to configure. */
- unsigned int flags; /* Reserved for future use. */
+ unsigned int flags;
unsigned int rsv[4]; /* Reserved for future use. */
};
@@ -149,6 +159,18 @@ struct ptp_pin_desc {
#define PTP_SYS_OFFSET_EXTENDED \
_IOWR(PTP_CLK_MAGIC, 9, struct ptp_sys_offset_extended)
+#define PTP_CLOCK_GETCAPS2 _IOR(PTP_CLK_MAGIC, 10, struct ptp_clock_caps)
+#define PTP_EXTTS_REQUEST2 _IOW(PTP_CLK_MAGIC, 11, struct ptp_extts_request)
+#define PTP_PEROUT_REQUEST2 _IOW(PTP_CLK_MAGIC, 12, struct ptp_perout_request)
+#define PTP_ENABLE_PPS2 _IOW(PTP_CLK_MAGIC, 13, int)
+#define PTP_SYS_OFFSET2 _IOW(PTP_CLK_MAGIC, 14, struct ptp_sys_offset)
+#define PTP_PIN_GETFUNC2 _IOWR(PTP_CLK_MAGIC, 15, struct ptp_pin_desc)
+#define PTP_PIN_SETFUNC2 _IOW(PTP_CLK_MAGIC, 16, struct ptp_pin_desc)
+#define PTP_SYS_OFFSET_PRECISE2 \
+ _IOWR(PTP_CLK_MAGIC, 17, struct ptp_sys_offset_precise)
+#define PTP_SYS_OFFSET_EXTENDED2 \
+ _IOWR(PTP_CLK_MAGIC, 18, struct ptp_sys_offset_extended)
+
struct ptp_extts_event {
struct ptp_clock_time t; /* Time event occured. */
unsigned int index; /* Which channel produced the event. */
--
2.23.0
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply related
* Re: [PATCH net-next v2] r8152: adjust the settings of ups flags
From: David Miller @ 2019-09-05 10:41 UTC (permalink / raw)
To: hayeswang; +Cc: netdev, nic_swsd, linux-kernel
In-Reply-To: <1394712342-15778-328-Taiwan-albertk@realtek.com>
From: Hayes Wang <hayeswang@realtek.com>
Date: Thu, 5 Sep 2019 10:46:20 +0800
> The UPS feature only works for runtime suspend, so UPS flags only
> need to be set before enabling runtime suspend. Therefore, I create
> a struct to record relative information, and use it before runtime
> suspend.
>
> All chips could record such information, even though not all of
> them support the feature of UPS. Then, some functions could be
> combined.
>
> Signed-off-by: Hayes Wang <hayeswang@realtek.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next 4/7] net: hns3: add client node validity judgment
From: tanhuazhong @ 2019-09-05 11:01 UTC (permalink / raw)
To: Sergei Shtylyov, davem
Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm,
jakub.kicinski, Peng Li
In-Reply-To: <b0aa6da6-cd42-dd31-8ff7-ca3f48de58ff@cogentembedded.com>
On 2019/9/5 18:12, Sergei Shtylyov wrote:
> On 04.09.2019 17:06, Huazhong Tan wrote:
>
>> From: Peng Li <lipeng321@huawei.com>
>>
>> HNS3 driver can only unregister client which included in
>> hnae3_client_list.
>> This patch adds the client node validity judgment.
>>
>> Signed-off-by: Peng Li <lipeng321@huawei.com>
>> Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
>> ---
>> drivers/net/ethernet/hisilicon/hns3/hnae3.c | 16 ++++++++++++++++
>> 1 file changed, 16 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.c
>> b/drivers/net/ethernet/hisilicon/hns3/hnae3.c
>> index 528f624..6aa5257 100644
>> --- a/drivers/net/ethernet/hisilicon/hns3/hnae3.c
>> +++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.c
>> @@ -138,12 +138,28 @@ EXPORT_SYMBOL(hnae3_register_client);
>> void hnae3_unregister_client(struct hnae3_client *client)
>> {
>> + struct hnae3_client *client_tmp;
>> struct hnae3_ae_dev *ae_dev;
>> + bool existed = false;
>> if (!client)
>> return;
>> mutex_lock(&hnae3_common_lock);
>> +
>> + list_for_each_entry(client_tmp, &hnae3_client_list, node) {
>> + if (client_tmp->type == client->type) {
>> + existed = true;
>> + break;
>> + }
>> + }
>> +
>> + if (!existed) {
>> + mutex_unlock(&hnae3_common_lock);
>> + pr_err("client %s not existed!\n", client->name);
>
> Did not exist, you mean?
>
yes
> [...]
>
> MBR, Sergei
>
> .
>
^ permalink raw reply
* Re: [Bridge] [PATCH v3 1/2] net: bridge: use mac_len in bridge forwarding
From: Toshiaki Makita @ 2019-09-05 11:20 UTC (permalink / raw)
To: Zahari Doychev
Cc: netdev, makita.toshiaki, jiri, nikolay, simon.horman, roopa,
bridge, jhs, dsahern, xiyou.wangcong, johannes,
alexei.starovoitov
In-Reply-To: <20190904143227.5jpn2gnu3fed55wg@tycho>
On 2019/09/04 23:32, Zahari Doychev wrote:
> On Wed, Sep 04, 2019 at 04:14:28PM +0900, Toshiaki Makita wrote:
>> On 2019/09/03 22:36, Zahari Doychev wrote:
>>> On Tue, Sep 03, 2019 at 08:37:36PM +0900, Toshiaki Makita wrote:
>>>> Hi Zahari,
>>>>
>>>> Sorry for reviewing this late.
>>>>
>>>> On 2019/09/03 3:09, Zahari Doychev wrote:
>>>> ...
>>>>> @@ -466,13 +466,14 @@ static bool __allowed_ingress(const struct net_bridge *br,
>>>>> /* Tagged frame */
>>>>> if (skb->vlan_proto != br->vlan_proto) {
>>>>> /* Protocol-mismatch, empty out vlan_tci for new tag */
>>>>> - skb_push(skb, ETH_HLEN);
>>>>> + skb_push(skb, skb->mac_len);
>>>>> skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
>>>>> skb_vlan_tag_get(skb));
>>>>
>>>> I think we should insert vlan at skb->data, i.e. mac_header + mac_len, while this
>>>> function inserts the tag at mac_header + ETH_HLEN which is not always the correct
>>>> offset.
>>>
>>> Maybe I am misunderstanding the concern here but this should make sure that
>>> the VLAN tag from the skb is move back in the payload as the outer most tag.
>>> So it should follow the ethernet header. It looks like this e.g.,:
>>>
>>> VLAN1 in skb:
>>> +------+------+-------+
>>> | DMAC | SMAC | ETYPE |
>>> +------+------+-------+
>>>
>>> VLAN1 moved to payload:
>>> +------+------+-------+-------+
>>> | DMAC | SMAC | VLAN1 | ETYPE |
>>> +------+------+-------+-------+
>>>
>>> VLAN2 in skb:
>>> +------+------+-------+-------+
>>> | DMAC | SMAC | VLAN1 | ETYPE |
>>> +------+------+-------+-------+
>>>
>>> VLAN2 moved to payload:
>>>
>>> +------+------+-------+-------+
>>> | DMAC | SMAC | VLAN2 | VLAN1 | ....
>>> +------+------+-------+-------+
>>>
>>> Doing the skb push with mac_len makes sure that VLAN tag is inserted in the
>>> correct offset. For mac_len == ETH_HLEN this does not change the current
>>> behaviour.
>>
>> Reordering VLAN headers here does not look correct to me. If skb->data points to ETH+VLAN,
>> then we should insert the vlan at the offset.
>> Vlan devices with reorder_hdr disabled produce packets whose mac_len includes ETH+VLAN header,
>> and they expects vlan insertion after the outer vlan header.
>
> I see so in this case we should handle differently as it seems sometimes
> we have to insert after or before the tag in the packet. I am not quite sure
> if this is possible to be detected here. I was trying to do bridging with VLAN
> devices with reorder_hdr disabled working but somehow I was not able to get
> mac_len longer then ETH_HLEN in all cases that I tried. Can you provide some
> example how can I try this out? It will really help me to understand the
> problem better.
I'm not sure if there is a case where we should insert tags before data pointer.
Your case does not look valid to me because skb is already broken in TC (I think I
explained this in the previous discussion). Bridge should not workaround the broken skb.
>> Also I'm not sure there is standard ethernet header in mac_len, as mac_len is not ETH_HLEN.
>> E.g. tun devices can produce vlan packets without ehternet header.
>
> How is the bridge forwarding decision done in this case when there are no
> MAC addresses, vlan based only?
Tun is just an example for header shorter than we expect. It's more like an attack vector.
So maybe it's sufficient to make sure we don't crash or write data to unexpected offset
for such packets. Or if such packets cannot make it to this point, that's ok.
>
>>
>>>
>>>>
>>>>> if (unlikely(!skb))
>>>>> return false;
>>>>> skb_pull(skb, ETH_HLEN);
>>>>
>>>> Now skb->data is mac_header + ETH_HLEN which would be broken when mac_len is not
>>>> ETH_HLEN?
>>>
>>> I thought it would be better to point in this case to the outer tag as otherwise
>>> if mac_len is used the skb->data will point to the next tag which I find somehow
>>> inconsistent or do you see some case where this can cause problems?
>>
>> Vlan devices with reorder_hdr off will break because it relies on skb->data offset
>> as I described in the previous discussion.
>
> I also see in vlan_do_receive that the VLAN tag is moved to the payload when
> reorder_hdr is off and the vlan_dev is not a bridge port. So it seems that
> I am misunderstanding the reorder_hdr option so if you can give me some more
> details about how it is supposed to be used will be highly appreciated.
No, you don't misunderstand it. I just forgot the condition was added.
Now reorder_hdr does not look like a problem, I lost the reason to handle
mac_len != ETH_HLEN case, as I'm thinking this change should not be a workaround for your problem.
If we fix the broken data pointer in TC, there should not be problems with mac_len in bridge.
Do you have any other possible cases this works for?
Toshiaki Makita
^ permalink raw reply
* Re: WARNING in hso_free_net_device
From: Andrey Konovalov @ 2019-09-05 11:24 UTC (permalink / raw)
To: Hui Peng
Cc: Stephen Hemminger, syzbot+44d53c7255bb1aea22d2, alexios.zavras,
David S. Miller, Greg Kroah-Hartman, LKML, USB list,
Mathias Payer, netdev, rfontana, syzkaller-bugs, Thomas Gleixner,
Oliver Neukum
In-Reply-To: <285edb24-01f9-3f9d-4946-b2f41ccd0774@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1328 bytes --]
On Thu, Sep 5, 2019 at 4:20 AM Hui Peng <benquike@gmail.com> wrote:
>
> Can you guys have a look at the attached patch?
Let's try it:
#syz test: https://github.com/google/kasan.git eea39f24
FYI: there are two more reports coming from this driver, which might
(or might not) have the same root cause. One of them has a suggested
fix by Oliver.
https://syzkaller.appspot.com/bug?extid=67b2bd0e34f952d0321e
https://syzkaller.appspot.com/bug?extid=93f2f45b19519b289613
>
> On 9/4/19 6:41 PM, Stephen Hemminger wrote:
> > On Wed, 4 Sep 2019 16:27:50 -0400
> > Hui Peng <benquike@gmail.com> wrote:
> >
> >> Hi, all:
> >>
> >> I looked at the bug a little.
> >>
> >> The issue is that in the error handling code, hso_free_net_device
> >> unregisters
> >>
> >> the net_device (hso_net->net) by calling unregister_netdev. In the
> >> error handling code path,
> >>
> >> hso_net->net has not been registered yet.
> >>
> >> I think there are two ways to solve the issue:
> >>
> >> 1. fix it in drivers/net/usb/hso.c to avoiding unregistering the
> >> net_device when it is still not registered
> >>
> >> 2. fix it in unregister_netdev. We can add a field in net_device to
> >> record whether it is registered, and make unregister_netdev return if
> >> the net_device is not registered yet.
> >>
> >> What do you guys think ?
> > #1
[-- Attachment #2: 0001-Fix-a-wrong-unregistering-bug-in-hso_free_net_device.patch --]
[-- Type: text/x-patch, Size: 2399 bytes --]
From f3fdee8fc03aa2bc982f22da1d29bbf6bca72935 Mon Sep 17 00:00:00 2001
From: Hui Peng <benquike@gmail.com>
Date: Wed, 4 Sep 2019 21:38:35 -0400
Subject: [PATCH] Fix a wrong unregistering bug in hso_free_net_device
As shown below, hso_create_net_device may call hso_free_net_device
before the net_device is registered. hso_free_net_device will
unregister the network device no matter it is registered or not,
unregister_netdev is not able to handle unregistered net_device,
resulting in the bug reported by the syzbot.
```
static struct hso_device *hso_create_net_device(struct usb_interface *interface,
int port_spec)
{
......
net = alloc_netdev(sizeof(struct hso_net), "hso%d", NET_NAME_UNKNOWN,
hso_net_init);
......
if (!hso_net->out_endp) {
dev_err(&interface->dev, "Can't find BULK OUT endpoint\n");
goto exit;
}
......
result = register_netdev(net);
......
exit:
hso_free_net_device(hso_dev);
return NULL;
}
static void hso_free_net_device(struct hso_device *hso_dev)
{
......
if (hso_net->net)
unregister_netdev(hso_net->net);
......
}
```
This patch adds a net_registered field in struct hso_net to record whether
the containing net_device is registered or not, and avoid unregistering it
if it is not registered yet.
Reported-by: syzbot+44d53c7255bb1aea22d2@syzkaller.appspotmail.com
Signed-off-by: Hui Peng <benquike@gmail.com>
---
drivers/net/usb/hso.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index ce78714..5b3df33 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -128,6 +128,7 @@ struct hso_shared_int {
struct hso_net {
struct hso_device *parent;
struct net_device *net;
+ bool net_registered;
struct rfkill *rfkill;
char name[24];
@@ -2362,7 +2363,7 @@ static void hso_free_net_device(struct hso_device *hso_dev)
remove_net_device(hso_net->parent);
- if (hso_net->net)
+ if (hso_net->net && hso_net->net_registered)
unregister_netdev(hso_net->net);
/* start freeing */
@@ -2544,6 +2545,7 @@ static struct hso_device *hso_create_net_device(struct usb_interface *interface,
dev_err(&interface->dev, "Failed to register device\n");
goto exit;
}
+ hso_net->net_registered = true;
hso_log_port(hso_dev);
--
2.7.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox