* pull request (net): ipsec 2018-07-27
From: Steffen Klassert @ 2018-07-27 6:51 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
1) Fix PMTU handling of vti6. We update the PMTU on
the xfrm dst_entry which is not cached anymore
after the flowchache removal. So update the
PMTU of the original dst_entry instead.
From Eyal Birger.
2) Fix a leak of kernel memory to userspace.
From Eric Dumazet.
3) Fix a possible dst_entry memleak in xfrm_lookup_route.
From Tommi Rantala.
4) Fix a skb leak in case we can't call nlmsg_multicast
from xfrm_nlmsg_multicast. From Florian Westphal.
5) Fix a leak of a temporary buffer in the error path of
esp6_input. From Zhen Lei.
Please pull or let me know if there are problems.
Thanks!
The following changes since commit 1c8c5a9d38f607c0b6fd12c91cbe1a4418762a21:
Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next (2018-06-06 18:39:49 -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 7284fdf39a912322ce97de2d30def3c6068a418c:
esp6: fix memleak on error path in esp6_input (2018-06-27 17:32:11 +0200)
----------------------------------------------------------------
Eric Dumazet (1):
xfrm_user: prevent leaking 2 bytes of kernel memory
Eyal Birger (1):
vti6: fix PMTU caching and reporting on xmit
Florian Westphal (1):
xfrm: free skb if nlsk pointer is NULL
Tommi Rantala (1):
xfrm: fix missing dst_release() after policy blocking lbcast and multicast
Zhen Lei (1):
esp6: fix memleak on error path in esp6_input
net/ipv6/esp6.c | 4 +++-
net/ipv6/ip6_vti.c | 11 ++++++-----
net/xfrm/xfrm_policy.c | 3 +++
net/xfrm/xfrm_user.c | 18 +++++++++++-------
4 files changed, 23 insertions(+), 13 deletions(-)
^ permalink raw reply
* [PATCH 2/5] xfrm_user: prevent leaking 2 bytes of kernel memory
From: Steffen Klassert @ 2018-07-27 6:51 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <20180727065154.28557-1-steffen.klassert@secunet.com>
From: Eric Dumazet <edumazet@google.com>
struct xfrm_userpolicy_type has two holes, so we should not
use C99 style initializer.
KMSAN report:
BUG: KMSAN: kernel-infoleak in copyout lib/iov_iter.c:140 [inline]
BUG: KMSAN: kernel-infoleak in _copy_to_iter+0x1b14/0x2800 lib/iov_iter.c:571
CPU: 1 PID: 4520 Comm: syz-executor841 Not tainted 4.17.0+ #5
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x185/0x1d0 lib/dump_stack.c:113
kmsan_report+0x188/0x2a0 mm/kmsan/kmsan.c:1117
kmsan_internal_check_memory+0x138/0x1f0 mm/kmsan/kmsan.c:1211
kmsan_copy_to_user+0x7a/0x160 mm/kmsan/kmsan.c:1253
copyout lib/iov_iter.c:140 [inline]
_copy_to_iter+0x1b14/0x2800 lib/iov_iter.c:571
copy_to_iter include/linux/uio.h:106 [inline]
skb_copy_datagram_iter+0x422/0xfa0 net/core/datagram.c:431
skb_copy_datagram_msg include/linux/skbuff.h:3268 [inline]
netlink_recvmsg+0x6f1/0x1900 net/netlink/af_netlink.c:1959
sock_recvmsg_nosec net/socket.c:802 [inline]
sock_recvmsg+0x1d6/0x230 net/socket.c:809
___sys_recvmsg+0x3fe/0x810 net/socket.c:2279
__sys_recvmmsg+0x58e/0xe30 net/socket.c:2391
do_sys_recvmmsg+0x2a6/0x3e0 net/socket.c:2472
__do_sys_recvmmsg net/socket.c:2485 [inline]
__se_sys_recvmmsg net/socket.c:2481 [inline]
__x64_sys_recvmmsg+0x15d/0x1c0 net/socket.c:2481
do_syscall_64+0x15b/0x230 arch/x86/entry/common.c:287
entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x446ce9
RSP: 002b:00007fc307918db8 EFLAGS: 00000293 ORIG_RAX: 000000000000012b
RAX: ffffffffffffffda RBX: 00000000006dbc24 RCX: 0000000000446ce9
RDX: 000000000000000a RSI: 0000000020005040 RDI: 0000000000000003
RBP: 00000000006dbc20 R08: 0000000020004e40 R09: 0000000000000000
R10: 0000000040000000 R11: 0000000000000293 R12: 0000000000000000
R13: 00007ffc8d2df32f R14: 00007fc3079199c0 R15: 0000000000000001
Uninit was stored to memory at:
kmsan_save_stack_with_flags mm/kmsan/kmsan.c:279 [inline]
kmsan_save_stack mm/kmsan/kmsan.c:294 [inline]
kmsan_internal_chain_origin+0x12b/0x210 mm/kmsan/kmsan.c:685
kmsan_memcpy_origins+0x11d/0x170 mm/kmsan/kmsan.c:527
__msan_memcpy+0x109/0x160 mm/kmsan/kmsan_instr.c:413
__nla_put lib/nlattr.c:569 [inline]
nla_put+0x276/0x340 lib/nlattr.c:627
copy_to_user_policy_type net/xfrm/xfrm_user.c:1678 [inline]
dump_one_policy+0xbe1/0x1090 net/xfrm/xfrm_user.c:1708
xfrm_policy_walk+0x45a/0xd00 net/xfrm/xfrm_policy.c:1013
xfrm_dump_policy+0x1c0/0x2a0 net/xfrm/xfrm_user.c:1749
netlink_dump+0x9b5/0x1550 net/netlink/af_netlink.c:2226
__netlink_dump_start+0x1131/0x1270 net/netlink/af_netlink.c:2323
netlink_dump_start include/linux/netlink.h:214 [inline]
xfrm_user_rcv_msg+0x8a3/0x9b0 net/xfrm/xfrm_user.c:2577
netlink_rcv_skb+0x37e/0x600 net/netlink/af_netlink.c:2448
xfrm_netlink_rcv+0xb2/0xf0 net/xfrm/xfrm_user.c:2598
netlink_unicast_kernel net/netlink/af_netlink.c:1310 [inline]
netlink_unicast+0x1680/0x1750 net/netlink/af_netlink.c:1336
netlink_sendmsg+0x104f/0x1350 net/netlink/af_netlink.c:1901
sock_sendmsg_nosec net/socket.c:629 [inline]
sock_sendmsg net/socket.c:639 [inline]
___sys_sendmsg+0xec8/0x1320 net/socket.c:2117
__sys_sendmsg net/socket.c:2155 [inline]
__do_sys_sendmsg net/socket.c:2164 [inline]
__se_sys_sendmsg net/socket.c:2162 [inline]
__x64_sys_sendmsg+0x331/0x460 net/socket.c:2162
do_syscall_64+0x15b/0x230 arch/x86/entry/common.c:287
entry_SYSCALL_64_after_hwframe+0x44/0xa9
Local variable description: ----upt.i@dump_one_policy
Variable was created at:
dump_one_policy+0x78/0x1090 net/xfrm/xfrm_user.c:1689
xfrm_policy_walk+0x45a/0xd00 net/xfrm/xfrm_policy.c:1013
Byte 130 of 137 is uninitialized
Memory access starts at ffff88019550407f
Fixes: c0144beaeca42 ("[XFRM] netlink: Use nla_put()/NLA_PUT() variantes")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: syzbot <syzkaller@googlegroups.com>
Cc: Steffen Klassert <steffen.klassert@secunet.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/xfrm/xfrm_user.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 080035f056d9..1e50b70ad668 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -1671,9 +1671,11 @@ static inline unsigned int userpolicy_type_attrsize(void)
#ifdef CONFIG_XFRM_SUB_POLICY
static int copy_to_user_policy_type(u8 type, struct sk_buff *skb)
{
- struct xfrm_userpolicy_type upt = {
- .type = type,
- };
+ struct xfrm_userpolicy_type upt;
+
+ /* Sadly there are two holes in struct xfrm_userpolicy_type */
+ memset(&upt, 0, sizeof(upt));
+ upt.type = type;
return nla_put(skb, XFRMA_POLICY_TYPE, sizeof(upt), &upt);
}
--
2.14.1
^ permalink raw reply related
* [PATCH 1/5] vti6: fix PMTU caching and reporting on xmit
From: Steffen Klassert @ 2018-07-27 6:51 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <20180727065154.28557-1-steffen.klassert@secunet.com>
From: Eyal Birger <eyal.birger@gmail.com>
When setting the skb->dst before doing the MTU check, the route PMTU
caching and reporting is done on the new dst which is about to be
released.
Instead, PMTU handling should be done using the original dst.
This is aligned with IPv4 VTI.
Fixes: ccd740cbc6 ("vti6: Add pmtu handling to vti6_xmit.")
Signed-off-by: Eyal Birger <eyal.birger@gmail.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/ipv6/ip6_vti.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/net/ipv6/ip6_vti.c b/net/ipv6/ip6_vti.c
index b7f28deddaea..c72ae3a4fe09 100644
--- a/net/ipv6/ip6_vti.c
+++ b/net/ipv6/ip6_vti.c
@@ -480,10 +480,6 @@ vti6_xmit(struct sk_buff *skb, struct net_device *dev, struct flowi *fl)
goto tx_err_dst_release;
}
- skb_scrub_packet(skb, !net_eq(t->net, dev_net(dev)));
- skb_dst_set(skb, dst);
- skb->dev = skb_dst(skb)->dev;
-
mtu = dst_mtu(dst);
if (!skb->ignore_df && skb->len > mtu) {
skb_dst_update_pmtu(skb, mtu);
@@ -498,9 +494,14 @@ vti6_xmit(struct sk_buff *skb, struct net_device *dev, struct flowi *fl)
htonl(mtu));
}
- return -EMSGSIZE;
+ err = -EMSGSIZE;
+ goto tx_err_dst_release;
}
+ skb_scrub_packet(skb, !net_eq(t->net, dev_net(dev)));
+ skb_dst_set(skb, dst);
+ skb->dev = skb_dst(skb)->dev;
+
err = dst_output(t->net, skb->sk, skb);
if (net_xmit_eval(err) == 0) {
struct pcpu_sw_netstats *tstats = this_cpu_ptr(dev->tstats);
--
2.14.1
^ permalink raw reply related
* [PATCH 3/5] xfrm: fix missing dst_release() after policy blocking lbcast and multicast
From: Steffen Klassert @ 2018-07-27 6:51 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <20180727065154.28557-1-steffen.klassert@secunet.com>
From: Tommi Rantala <tommi.t.rantala@nokia.com>
Fix missing dst_release() when local broadcast or multicast traffic is
xfrm policy blocked.
For IPv4 this results to dst leak: ip_route_output_flow() allocates
dst_entry via __ip_route_output_key() and passes it to
xfrm_lookup_route(). xfrm_lookup returns ERR_PTR(-EPERM) that is
propagated. The dst that was allocated is never released.
IPv4 local broadcast testcase:
ping -b 192.168.1.255 &
sleep 1
ip xfrm policy add src 0.0.0.0/0 dst 192.168.1.255/32 dir out action block
IPv4 multicast testcase:
ping 224.0.0.1 &
sleep 1
ip xfrm policy add src 0.0.0.0/0 dst 224.0.0.1/32 dir out action block
For IPv6 the missing dst_release() causes trouble e.g. when used in netns:
ip netns add TEST
ip netns exec TEST ip link set lo up
ip link add dummy0 type dummy
ip link set dev dummy0 netns TEST
ip netns exec TEST ip addr add fd00::1111 dev dummy0
ip netns exec TEST ip link set dummy0 up
ip netns exec TEST ping -6 -c 5 ff02::1%dummy0 &
sleep 1
ip netns exec TEST ip xfrm policy add src ::/0 dst ff02::1 dir out action block
wait
ip netns del TEST
After netns deletion we see:
[ 258.239097] unregister_netdevice: waiting for lo to become free. Usage count = 2
[ 268.279061] unregister_netdevice: waiting for lo to become free. Usage count = 2
[ 278.367018] unregister_netdevice: waiting for lo to become free. Usage count = 2
[ 288.375259] unregister_netdevice: waiting for lo to become free. Usage count = 2
Fixes: ac37e2515c1a ("xfrm: release dst_orig in case of error in xfrm_lookup()")
Signed-off-by: Tommi Rantala <tommi.t.rantala@nokia.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/xfrm/xfrm_policy.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 5f48251c1319..7c5e8978aeaa 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -2286,6 +2286,9 @@ struct dst_entry *xfrm_lookup_route(struct net *net, struct dst_entry *dst_orig,
if (IS_ERR(dst) && PTR_ERR(dst) == -EREMOTE)
return make_blackhole(net, dst_orig->ops->family, dst_orig);
+ if (IS_ERR(dst))
+ dst_release(dst_orig);
+
return dst;
}
EXPORT_SYMBOL(xfrm_lookup_route);
--
2.14.1
^ permalink raw reply related
* [PATCH 4/5] xfrm: free skb if nlsk pointer is NULL
From: Steffen Klassert @ 2018-07-27 6:51 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <20180727065154.28557-1-steffen.klassert@secunet.com>
From: Florian Westphal <fw@strlen.de>
nlmsg_multicast() always frees the skb, so in case we cannot call
it we must do that ourselves.
Fixes: 21ee543edc0dea ("xfrm: fix race between netns cleanup and state expire notification")
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/xfrm/xfrm_user.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 1e50b70ad668..33878e6e0d0a 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -1025,10 +1025,12 @@ static inline int xfrm_nlmsg_multicast(struct net *net, struct sk_buff *skb,
{
struct sock *nlsk = rcu_dereference(net->xfrm.nlsk);
- if (nlsk)
- return nlmsg_multicast(nlsk, skb, pid, group, GFP_ATOMIC);
- else
- return -1;
+ if (!nlsk) {
+ kfree_skb(skb);
+ return -EPIPE;
+ }
+
+ return nlmsg_multicast(nlsk, skb, pid, group, GFP_ATOMIC);
}
static inline unsigned int xfrm_spdinfo_msgsize(void)
--
2.14.1
^ permalink raw reply related
* [PATCH] net: intel: igb: Replace mdelay() with msleep() in igb_integrated_phy_loopback()
From: Jia-Ju Bai @ 2018-07-27 8:07 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: intel-wired-lan, netdev, linux-kernel, Jia-Ju Bai
igb_integrated_phy_loopback() is never called in atomic context.
It calls mdelay() to busily wait, which is not necessary.
mdelay() can be replaced with msleep().
This is found by a static analysis tool named DCNS written by myself.
Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
---
drivers/net/ethernet/intel/igb/igb_ethtool.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index e77ba0d5866d..4e64c9a6f1b6 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -1670,7 +1670,7 @@ static int igb_integrated_phy_loopback(struct igb_adapter *adapter)
if (hw->phy.type == e1000_phy_m88)
igb_phy_disable_receiver(adapter);
- mdelay(500);
+ msleep(500);
return 0;
}
--
2.17.0
^ permalink raw reply related
* [PATCH 1/2] net: intel: igb: Replace GFP_ATOMIC with GFP_KERNEL in igb_sw_init()
From: Jia-Ju Bai @ 2018-07-27 8:04 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: intel-wired-lan, netdev, linux-kernel, Jia-Ju Bai
igb_sw_init() is never called in atomic context.
It calls kzalloc() and kcalloc() with GFP_ATOMIC, which is not necessary.
GFP_ATOMIC can be replaced with GFP_KERNEL.
This is found by a static analysis tool named DCNS written by myself.
Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
---
drivers/net/ethernet/intel/igb/igb_main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index cce7ada89255..5332a1b608e6 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -3534,7 +3534,7 @@ static int igb_sw_init(struct igb_adapter *adapter)
adapter->flags |= IGB_FLAG_HAS_MSIX;
adapter->mac_table = kzalloc(sizeof(struct igb_mac_addr) *
- hw->mac.rar_entry_count, GFP_ATOMIC);
+ hw->mac.rar_entry_count, GFP_KERNEL);
if (!adapter->mac_table)
return -ENOMEM;
@@ -3544,7 +3544,7 @@ static int igb_sw_init(struct igb_adapter *adapter)
/* Setup and initialize a copy of the hw vlan table array */
adapter->shadow_vfta = kcalloc(E1000_VLAN_FILTER_TBL_SIZE, sizeof(u32),
- GFP_ATOMIC);
+ GFP_KERNEL);
if (!adapter->shadow_vfta)
return -ENOMEM;
--
2.17.0
^ permalink raw reply related
* [PATCH] net: hisilicon: hns: Replace mdelay() with msleep()
From: Jia-Ju Bai @ 2018-07-27 8:01 UTC (permalink / raw)
To: yisen.zhuang, salil.mehta, davem, linyunsheng, joe, pombredanne,
bianpan2016, shenjian15, keescook
Cc: netdev, linux-kernel, Jia-Ju Bai
hns_ppe_common_init_hw() and hns_xgmac_init() are never
called in atomic context.
They call mdelay() to busily wait, which is not necessary.
mdelay() can be replaced with msleep().
This is found by a static analysis tool named DCNS written by myself.
Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
---
drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c | 4 ++--
drivers/net/ethernet/hisilicon/hns/hns_dsaf_xgmac.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c
index 93e71e27401b..866e4cec423d 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c
@@ -203,9 +203,9 @@ static int hns_ppe_common_init_hw(struct ppe_common_cb *ppe_common)
enum dsaf_mode dsaf_mode = dsaf_dev->dsaf_mode;
dsaf_dev->misc_op->ppe_comm_srst(dsaf_dev, 0);
- mdelay(100);
+ msleep(100);
dsaf_dev->misc_op->ppe_comm_srst(dsaf_dev, 1);
- mdelay(100);
+ msleep(100);
if (ppe_common->ppe_mode == PPE_COMMON_MODE_SERVICE) {
switch (dsaf_mode) {
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_xgmac.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_xgmac.c
index 51e7e9f5af49..fbccd1fd94b0 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_xgmac.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_xgmac.c
@@ -215,10 +215,10 @@ static void hns_xgmac_init(void *mac_drv)
u32 port = drv->mac_id;
dsaf_dev->misc_op->xge_srst(dsaf_dev, port, 0);
- mdelay(100);
+ msleep(100);
dsaf_dev->misc_op->xge_srst(dsaf_dev, port, 1);
- mdelay(100);
+ msleep(100);
hns_xgmac_lf_rf_control_init(drv);
hns_xgmac_exc_irq_en(drv, 0);
--
2.17.0
^ permalink raw reply related
* [PATCH] net: amd: pcnet32: Replace GFP_ATOMIC with GFP_KERNEL in pcnet32_alloc_ring()
From: Jia-Ju Bai @ 2018-07-27 7:57 UTC (permalink / raw)
To: pcnet32; +Cc: netdev, linux-kernel, Jia-Ju Bai
pcnet32_alloc_ring() is never called in atomic context.
It calls kcalloc() with GFP_ATOMIC, which is not necessary.
GFP_ATOMIC can be replaced with GFP_KERNEL.
This is found by a static analysis tool named DCNS written by myself.
Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
---
drivers/net/ethernet/amd/pcnet32.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/amd/pcnet32.c b/drivers/net/ethernet/amd/pcnet32.c
index a561705f232c..e69e810d211d 100644
--- a/drivers/net/ethernet/amd/pcnet32.c
+++ b/drivers/net/ethernet/amd/pcnet32.c
@@ -2032,22 +2032,22 @@ static int pcnet32_alloc_ring(struct net_device *dev, const char *name)
}
lp->tx_dma_addr = kcalloc(lp->tx_ring_size, sizeof(dma_addr_t),
- GFP_ATOMIC);
+ GFP_KERNEL);
if (!lp->tx_dma_addr)
return -ENOMEM;
lp->rx_dma_addr = kcalloc(lp->rx_ring_size, sizeof(dma_addr_t),
- GFP_ATOMIC);
+ GFP_KERNEL);
if (!lp->rx_dma_addr)
return -ENOMEM;
lp->tx_skbuff = kcalloc(lp->tx_ring_size, sizeof(struct sk_buff *),
- GFP_ATOMIC);
+ GFP_KERNEL);
if (!lp->tx_skbuff)
return -ENOMEM;
lp->rx_skbuff = kcalloc(lp->rx_ring_size, sizeof(struct sk_buff *),
- GFP_ATOMIC);
+ GFP_KERNEL);
if (!lp->rx_skbuff)
return -ENOMEM;
--
2.17.0
^ permalink raw reply related
* Re: [patch net-next RFC] net: sched: don't dump chains only held by actions
From: Jiri Pirko @ 2018-07-27 6:13 UTC (permalink / raw)
To: Cong Wang
Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
mlxsw
In-Reply-To: <CAM_iQpXooed3X+K=KcPArAOtHAOMjM13=hr+f9Y+_HayESgOJA@mail.gmail.com>
Fri, Jul 27, 2018 at 06:18:14AM CEST, xiyou.wangcong@gmail.com wrote:
>On Thu, Jul 26, 2018 at 9:33 AM Jiri Pirko <jiri@resnulli.us> wrote:
>>
>> From: Jiri Pirko <jiri@mellanox.com>
>>
>> In case a chain is empty and not explicitly created by a user,
>> such chain should not exist. The only exception is if there is
>> an action "goto chain" pointing to it. In that case, don't show the
>> chain in the dump. Track the chain references held by actions and
>> use them to find out if a chain should or should not be shown
>> in chain dump.
>
>Hiding it makes sense. But you still need to make sure
>user can't find it by ID, hiding it merely means user can't
>see it.
>
>Also, I don't understand why you increase the refcnt
>for a hiding chain either, like Jakub mentioned.
>
>If user can't see it, it must not be found by ID either.
Ack. Will do that.
^ permalink raw reply
* Re: [patch net-next RFC] net: sched: don't dump chains only held by actions
From: Jiri Pirko @ 2018-07-27 6:13 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, davem, jhs, xiyou.wangcong, mlxsw
In-Reply-To: <20180726125930.5a5bcf02@cakuba.netronome.com>
Thu, Jul 26, 2018 at 09:59:30PM CEST, jakub.kicinski@netronome.com wrote:
>On Thu, 26 Jul 2018 18:31:01 +0200, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>>
>> In case a chain is empty and not explicitly created by a user,
>> such chain should not exist. The only exception is if there is
>> an action "goto chain" pointing to it. In that case, don't show the
>> chain in the dump. Track the chain references held by actions and
>> use them to find out if a chain should or should not be shown
>> in chain dump.
>>
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>
>I don't have any better ideas :)
>
>One question below.
>
>> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
>> index 75cce2819de9..76035cd6e3bf 100644
>> --- a/net/sched/cls_api.c
>> +++ b/net/sched/cls_api.c
>> @@ -262,6 +262,25 @@ static void tcf_chain_hold(struct tcf_chain *chain)
>> ++chain->refcnt;
>> }
>>
>> +static void tcf_chain_hold_by_act(struct tcf_chain *chain)
>> +{
>> + ++chain->action_refcnt;
>> +}
>> +
>> +static void tcf_chain_release_by_act(struct tcf_chain *chain)
>> +{
>> + --chain->action_refcnt;
>> +}
>> +
>> +static bool tcf_chain_is_zombie(struct tcf_chain *chain)
>> +{
>> + /* In case all the references are action references, this
>> + * chain is a zombie and should not be listed in the chain
>> + * dump list.
>> + */
>> + return chain->refcnt == chain->action_refcnt;
>> +}
>> +
>> static struct tcf_chain *tcf_chain_lookup(struct tcf_block *block,
>> u32 chain_index)
>> {
>> @@ -298,6 +317,15 @@ struct tcf_chain *tcf_chain_get(struct tcf_block *block, u32 chain_index,
>> }
>> EXPORT_SYMBOL(tcf_chain_get);
>>
>> +struct tcf_chain *tcf_chain_get_by_act(struct tcf_block *block, u32 chain_index)
>> +{
>> + struct tcf_chain *chain = tcf_chain_get(block, chain_index, true);
>> +
>> + tcf_chain_hold_by_act(chain);
>> + return chain;
>> +}
>> +EXPORT_SYMBOL(tcf_chain_get_by_act);
>> +
>> static void tc_chain_tmplt_del(struct tcf_chain *chain);
>>
>> void tcf_chain_put(struct tcf_chain *chain)
>> @@ -310,6 +338,13 @@ void tcf_chain_put(struct tcf_chain *chain)
>> }
>> EXPORT_SYMBOL(tcf_chain_put);
>>
>> +void tcf_chain_put_by_act(struct tcf_chain *chain)
>> +{
>> + tcf_chain_release_by_act(chain);
>> + tcf_chain_put(chain);
>> +}
>> +EXPORT_SYMBOL(tcf_chain_put_by_act);
>> +
>> static void tcf_chain_put_explicitly_created(struct tcf_chain *chain)
>> {
>> if (chain->explicitly_created)
>> @@ -1803,17 +1838,26 @@ static int tc_ctl_chain(struct sk_buff *skb, struct nlmsghdr *n,
>> chain = tcf_chain_lookup(block, chain_index);
>> if (n->nlmsg_type == RTM_NEWCHAIN) {
>> if (chain) {
>> - NL_SET_ERR_MSG(extack, "Filter chain already exists");
>> - return -EEXIST;
>> - }
>> - if (!(n->nlmsg_flags & NLM_F_CREATE)) {
>> - NL_SET_ERR_MSG(extack, "Need both RTM_NEWCHAIN and NLM_F_CREATE to create a new chain");
>> - return -ENOENT;
>> - }
>> - chain = tcf_chain_create(block, chain_index);
>> - if (!chain) {
>> - NL_SET_ERR_MSG(extack, "Failed to create filter chain");
>> - return -ENOMEM;
>> + if (tcf_chain_is_zombie(chain)) {
>> + /* The chain exists only because there is
>> + * some action referencing it, meaning it
>> + * is a zombie.
>> + */
>> + tcf_chain_hold(chain);
>
>I'm not 100% sure why this is needed? In my tree below I see:
>
> switch (n->nlmsg_type) {
> case RTM_NEWCHAIN:
> err = tc_chain_tmplt_add(chain, net, tca, extack);
> if (err)
> goto errout;
> /* In case the chain was successfully added, take a reference
> * to the chain. This ensures that an empty chain
> * does not disappear at the end of this function.
> */
> tcf_chain_hold(chain);
> chain->explicitly_created = true;
>
>so one reference will be taken.. do we need two?
There is a put at the end of this function.
>
>> + } else {
>> + NL_SET_ERR_MSG(extack, "Filter chain already exists");
>> + return -EEXIST;
>> + }
>> + } else {
>> + if (!(n->nlmsg_flags & NLM_F_CREATE)) {
>> + NL_SET_ERR_MSG(extack, "Need both RTM_NEWCHAIN and NLM_F_CREATE to create a new chain");
>> + return -ENOENT;
>> + }
>> + chain = tcf_chain_create(block, chain_index);
>> + if (!chain) {
>> + NL_SET_ERR_MSG(extack, "Failed to create filter chain");
>> + return -ENOMEM;
>> + }
>> }
>> } else {
>> if (!chain) {
>> @@ -1944,6 +1988,8 @@ static int tc_dump_chain(struct sk_buff *skb, struct netlink_callback *cb)
>> index++;
>> continue;
>> }
>> + if (tcf_chain_is_zombie(chain))
>> + continue;
>> err = tc_chain_fill_node(chain, net, skb, block,
>> NETLINK_CB(cb->skb).portid,
>> cb->nlh->nlmsg_seq, NLM_F_MULTI,
>
^ permalink raw reply
* RE: [PATCH net-next] net/tls: Corrected enabling of zero-copy mode
From: Vakul Garg @ 2018-07-27 5:49 UTC (permalink / raw)
To: David Miller
Cc: netdev@vger.kernel.org, borisp@mellanox.com, aviadye@mellanox.com,
davejwatson@fb.com
In-Reply-To: <20180725.132836.2058461941832003627.davem@davemloft.net>
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-
> owner@vger.kernel.org] On Behalf Of David Miller
> Sent: Thursday, July 26, 2018 1:59 AM
> To: Vakul Garg <vakul.garg@nxp.com>
> Cc: netdev@vger.kernel.org; borisp@mellanox.com;
> aviadye@mellanox.com; davejwatson@fb.com
> Subject: Re: [PATCH net-next] net/tls: Corrected enabling of zero-copy mode
>
> From: Vakul Garg <vakul.garg@nxp.com>
> Date: Mon, 23 Jul 2018 21:00:06 +0530
>
> > @@ -787,7 +787,7 @@ int tls_sw_recvmsg(struct sock *sk,
> > target = sock_rcvlowat(sk, flags & MSG_WAITALL, len);
> > timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
> > do {
> > - bool zc = false;
> > + bool zc;
> > int chunk = 0;
> >
> > skb = tls_wait_data(sk, flags, timeo, &err);
> ...
> > @@ -836,6 +835,7 @@ int tls_sw_recvmsg(struct sock *sk,
> > if (err < 0)
> > goto fallback_to_reg_recv;
> >
> > + zc = true;
> > err = decrypt_skb_update(sk, skb, sgin, &zc);
> > for (; pages > 0; pages--)
> > put_page(sg_page(&sgin[pages]));
> @@ -845,6 +845,7 @@ int
> > tls_sw_recvmsg(struct sock *sk,
> > }
> > } else {
> > fallback_to_reg_recv:
> > + zc = false;
> > err = decrypt_skb_update(sk, skb, NULL,
> &zc);
> > if (err < 0) {
> > tls_err_abort(sk, EBADMSG);
> > --
> > 2.13.6
> >
>
> This will leave a code path where 'zc' is evaluated but not initialized to
> any value.
>
> And that's the path taken when ctx->decrypted is true. The code after
> your changes looks like:
>
> bool zc;
> ...
> if (!ctx->decrypted) {
>
> ... assignments to 'zc' happen in this code block
>
> ctx->decrypted = true;
> }
>
> if (!zc) {
>
> So when ctx->decrypted it true, the if(!zc) condition runs on an
> uninitialized value.
>
> I have to say that your TLS changes are becomming quite a time sink
> for two reasons.
>
> First, you are making a lot of changes that seem not so needed, and
> whose value is purely determined by taste. I'd put the
> msg_data_left() multiple evaluation patch into this category.
>
> The rest require deep review and understanding of the complicated
> details of the TLS code, and many of them turn out to be incorrect.
My apologies for sending incorrect patches. I would be more careful next time.
>
> As I find more errors in your submissions, I begin to scrutinize your
> patches even more. Thus, review of your changes takes even more time.
>
> And it isn't helping that there are not a lot of other developers
> helping actively to review your changes.
>
> I would like to just make a small request to you, that you concentrate
> on fixing clear bugs and clear issues that need to be resolved.
>
> Thank you.
^ permalink raw reply
* Re: [PATCH net-next] bnxt_en: combine 'else if' and 'else' into single branche
From: Vasundhara Volam @ 2018-07-27 5:45 UTC (permalink / raw)
To: YueHaibing; +Cc: David Miller, michael.chan@broadcom.com, open list, Netdev
In-Reply-To: <CAACQVJo8zB2H6AtQZEVK-nfg8j0f9oVPrBSMjq+CKoQ+xeVrUA@mail.gmail.com>
On Thu, Jul 26, 2018 at 8:24 PM, Vasundhara Volam
<vasundhara-v.volam@broadcom.com> wrote:
> On Wed, Jul 25, 2018 at 4:44 PM, YueHaibing <yuehaibing@huawei.com> wrote:
>>
>> The else-if branch and else branch set mac_ok to true similarly,
>> so combine the two into single else branch.
>>
>> Aslo add comments to explain the two conditions, which
>> from Michael Chan and Vasundhara Volam.
>>
>> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
>> ---
>> drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c | 8 ++++++--
>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c
>> index a649108..f560845 100644
>> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c
>> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c
>> @@ -956,9 +956,13 @@ static int bnxt_vf_validate_set_mac(struct bnxt *bp, struct bnxt_vf_info *vf)
>> } else if (is_valid_ether_addr(vf->vf_mac_addr)) {
>> if (ether_addr_equal((const u8 *)req->l2_addr, vf->vf_mac_addr))
>> mac_ok = true;
>> - } else if (bp->hwrm_spec_code < 0x10202) {
>> - mac_ok = true;
>> } else {
>> + /* There are two cases:
>> + * 1.If firmware spec < 0x10202,VF MAC address is not forwarded
>> + * to the PF and so it doesn't have to match
>> + * 2.Allow VF to modify it's own MAC when PF has not assigned a
>> + * valid MAC address and firmware spec >= 0x10202
>> + */
> No, this is not correct either. You are not covering both conditions.
> else part with cover if (!is_valid_ether_addr(vf->vf_mac_addr))
> before.
My apologies, I was thinking one more condition was required.
Patch seems fine.
>> mac_ok = true;
>> }
>> if (mac_ok)
>> --
>> 2.7.0
>>
>>
^ permalink raw reply
* Re: [PATCH] xfrm: fix ptr_ret.cocci warnings
From: Steffen Klassert @ 2018-07-27 5:39 UTC (permalink / raw)
To: kbuild test robot; +Cc: Benedict Wong, kbuild-all, netdev
In-Reply-To: <20180726070952.GA135949@lkp-ib03>
On Thu, Jul 26, 2018 at 03:09:52PM +0800, kbuild test robot wrote:
> From: kbuild test robot <fengguang.wu@intel.com>
>
> net/xfrm/xfrm_interface.c:692:1-3: WARNING: PTR_ERR_OR_ZERO can be used
>
>
> Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR
>
> Generated by: scripts/coccinelle/api/ptr_ret.cocci
>
> Fixes: 44e2b838c24d ("xfrm: Return detailed errors from xfrmi_newlink")
> CC: Benedict Wong <benedictwong@google.com>
> Signed-off-by: kbuild test robot <fengguang.wu@intel.com>
Applied, thanks!
^ permalink raw reply
* Re: [PATCH ipsec-next] xfrm: Return detailed errors from xfrmi_newlink
From: Steffen Klassert @ 2018-07-27 5:38 UTC (permalink / raw)
To: Benedict Wong; +Cc: netdev, nharold, lorenzo
In-Reply-To: <20180725204528.106433-1-benedictwong@google.com>
On Wed, Jul 25, 2018 at 01:45:29PM -0700, Benedict Wong wrote:
> Currently all failure modes of xfrm interface creation return EEXIST.
> This change improves the granularity of errnos provided by also
> returning ENODEV or EINVAL if failures happen in looking up the
> underlying interface, or a required parameter is not provided.
>
> This change has been tested against the Android Kernel Networking Tests,
> with additional xfrmi_newlink tests here:
>
> https://android-review.googlesource.com/c/kernel/tests/+/715755
>
> Signed-off-by: Benedict Wong <benedictwong@google.com>
Applied to ipsec-next, thanks Benedict!
^ permalink raw reply
* Re: [PATCH bpf-next 0/4] samples: bpf: convert two more samples to libbpf
From: Daniel Borkmann @ 2018-07-27 5:21 UTC (permalink / raw)
To: Jakub Kicinski, alexei.starovoitov; +Cc: oss-drivers, netdev
In-Reply-To: <20180726213221.1295-1-jakub.kicinski@netronome.com>
On 07/26/2018 11:32 PM, Jakub Kicinski wrote:
> Hi!
>
> This set converts xdpsock_user.c and xdp_fwd_user.c to use libbpf instead
> of bpf_load.o. First two patches are minor improvements to libbpf to make
> the conversion (and use of libbpf in general) nicer.
>
> Jakub Kicinski (4):
> tools: libbpf: handle NULL program gracefully in bpf_program__nth_fd()
> tools: libbpf: add bpf_object__find_program_by_title()
> samples: bpf: convert xdp_fwd_user.c to libbpf
> samples: bpf: convert xdpsock_user.c to libbpf
>
> samples/bpf/Makefile | 4 ++--
> samples/bpf/xdp_fwd_user.c | 34 +++++++++++++++++++++++-----------
> samples/bpf/xdpsock_user.c | 38 +++++++++++++++++++++++++++++---------
> tools/lib/bpf/libbpf.c | 15 +++++++++++++++
> tools/lib/bpf/libbpf.h | 3 +++
> 5 files changed, 72 insertions(+), 22 deletions(-)
>
Applied to bpf-next, thanks Jakub!
^ permalink raw reply
* Re: [PATCH bpf-next 0/6] nfp: bpf: improve efficiency of offloaded perf events
From: Daniel Borkmann @ 2018-07-27 5:17 UTC (permalink / raw)
To: Jakub Kicinski, alexei.starovoitov; +Cc: oss-drivers, netdev
In-Reply-To: <20180726025335.23017-1-jakub.kicinski@netronome.com>
On 07/26/2018 04:53 AM, Jakub Kicinski wrote:
> Hi!
>
> This set is focused on improving the performance of perf events
> reported from BPF offload. Perf events can now be received on
> packet data queues, which significantly improves the performance
> (from total of 0.5 Msps to 5Msps per core). To get to this
> performance we need a fast path for control messages which will
> operate on raw buffers and recycle them immediately.
>
> Patch 5 replaces the map pointers for perf maps with map IDs.
> We look the pointers up in a hashtable, anyway, to validate they
> are correct, so there is no performance difference. Map IDs
> have the advantage of being easier to understand for users in
> case of errors (we no longer print raw pointers to the logs).
>
> Last patch improves info messages about map offload.
>
> Jakub Kicinski (6):
> nfp: move repr handling on RX path
> nfp: allow control message reception on data queues
> nfp: bpf: pass raw data buffer to nfp_bpf_event_output()
> nfp: bpf: allow receiving perf events on data queues
> nfp: bpf: remember maps by ID
> nfp: bpf: improve map offload info messages
>
> drivers/net/ethernet/netronome/nfp/bpf/cmsg.c | 25 +++++++-
> drivers/net/ethernet/netronome/nfp/bpf/jit.c | 12 ++--
> drivers/net/ethernet/netronome/nfp/bpf/main.c | 5 +-
> drivers/net/ethernet/netronome/nfp/bpf/main.h | 9 ++-
> .../net/ethernet/netronome/nfp/bpf/offload.c | 63 +++++++++++--------
> drivers/net/ethernet/netronome/nfp/nfp_app.c | 2 +
> drivers/net/ethernet/netronome/nfp/nfp_app.h | 17 +++++
> .../ethernet/netronome/nfp/nfp_net_common.c | 40 +++++++-----
> .../net/ethernet/netronome/nfp/nfp_net_ctrl.h | 1 +
> 9 files changed, 125 insertions(+), 49 deletions(-)
>
Applied to bpf-next, thanks Jakub!
^ permalink raw reply
* Re: [PATCH v5 bpf-next 2/9] veth: Add driver XDP
From: Toshiaki Makita @ 2018-07-27 4:55 UTC (permalink / raw)
To: John Fastabend
Cc: Toshiaki Makita, netdev, Alexei Starovoitov, Daniel Borkmann,
Jesper Dangaard Brouer, Jakub Kicinski
In-Reply-To: <2dd56ee3-08ff-49ef-8019-09e53ac86395@gmail.com>
Hi John,
On 2018/07/27 12:02, John Fastabend wrote:
> On 07/26/2018 07:40 AM, Toshiaki Makita wrote:
>> From: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
>>
>> This is the basic implementation of veth driver XDP.
>>
>> Incoming packets are sent from the peer veth device in the form of skb,
>> so this is generally doing the same thing as generic XDP.
>>
>> This itself is not so useful, but a starting point to implement other
>> useful veth XDP features like TX and REDIRECT.
>>
>> This introduces NAPI when XDP is enabled, because XDP is now heavily
>> relies on NAPI context. Use ptr_ring to emulate NIC ring. Tx function
>> enqueues packets to the ring and peer NAPI handler drains the ring.
>>
>> Currently only one ring is allocated for each veth device, so it does
>> not scale on multiqueue env. This can be resolved by allocating rings
>> on the per-queue basis later.
>>
>> Note that NAPI is not used but netif_rx is used when XDP is not loaded,
>> so this does not change the default behaviour.
>>
>> v3:
>> - Fix race on closing the device.
>> - Add extack messages in ndo_bpf.
>>
>> v2:
>> - Squashed with the patch adding NAPI.
>> - Implement adjust_tail.
>> - Don't acquire consumer lock because it is guarded by NAPI.
>> - Make poll_controller noop since it is unnecessary.
>> - Register rxq_info on enabling XDP rather than on opening the device.
>>
>> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
>> ---
>
>
> [...]
>
> One nit and one question.
>
>> +
>> +static struct sk_buff *veth_xdp_rcv_skb(struct veth_priv *priv,
>> + struct sk_buff *skb)
>> +{
>> + u32 pktlen, headroom, act, metalen;
>> + void *orig_data, *orig_data_end;
>> + int size, mac_len, delta, off;
>> + struct bpf_prog *xdp_prog;
>> + struct xdp_buff xdp;
>> +
>> + rcu_read_lock();
>> + xdp_prog = rcu_dereference(priv->xdp_prog);
>> + if (unlikely(!xdp_prog)) {
>> + rcu_read_unlock();
>> + goto out;
>> + }
>> +
>> + mac_len = skb->data - skb_mac_header(skb);
>> + pktlen = skb->len + mac_len;
>> + size = SKB_DATA_ALIGN(VETH_XDP_HEADROOM + pktlen) +
>> + SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
>> + if (size > PAGE_SIZE)
>> + goto drop;
>
> I'm not sure why it matters if size > PAGE_SIZE here. Why not
> just consume it and use the correct page order in alloc_page if
> its not linear.
Indeed. We can allow such skbs here at least if we don't need
reallocation (which is highly unlikely though).
But I'm not sure we should allocate multiple pages in atomic context.
It tends to cause random allocation failure which is IMO more
frustrating. We are now prohibiting such a situation by max_mtu and
dropping features, which looks more robust to me.
>> +
>> + headroom = skb_headroom(skb) - mac_len;
>> + if (skb_shared(skb) || skb_head_is_locked(skb) ||
>> + skb_is_nonlinear(skb) || headroom < XDP_PACKET_HEADROOM) {
>> + struct sk_buff *nskb;
>> + void *head, *start;
>> + struct page *page;
>> + int head_off;
>> +
>> + page = alloc_page(GFP_ATOMIC);
>
> Should also have __NO_WARN here as well this can be triggered by
> external events so we don't want DDOS here to flood system logs.
Sure, thanks!
--
Toshiaki Makita
^ permalink raw reply
* Re: [pull request][net-next 00/13] Mellanox, mlx5e updates 2018-07-26 (XDP redirect)
From: David Miller @ 2018-07-27 4:33 UTC (permalink / raw)
To: saeedm; +Cc: netdev
In-Reply-To: <20180726225647.11926-1-saeedm@mellanox.com>
From: Saeed Mahameed <saeedm@mellanox.com>
Date: Thu, 26 Jul 2018 15:56:34 -0700
> This series from Tariq adds the support for device-out XDP redirect.
>
> For more information please see tag log below.
>
> Please pull and let me know if there's any problem.
Looks good, pulled, thanks a lot.
^ permalink raw reply
* Re: [PATCH net] net: ena: Fix use of uninitialized DMA address bits field
From: David Miller @ 2018-07-27 4:31 UTC (permalink / raw)
To: pressmangal; +Cc: netdev, netanel
In-Reply-To: <20180726204033.5753-1-pressmangal@gmail.com>
From: Gal Pressman <pressmangal@gmail.com>
Date: Thu, 26 Jul 2018 23:40:33 +0300
> UBSAN triggers the following undefined behaviour warnings:
> [...]
> [ 13.236124] UBSAN: Undefined behaviour in drivers/net/ethernet/amazon/ena/ena_eth_com.c:468:22
> [ 13.240043] shift exponent 64 is too large for 64-bit type 'long long unsigned int'
> [...]
> [ 13.744769] UBSAN: Undefined behaviour in drivers/net/ethernet/amazon/ena/ena_eth_com.c:373:4
> [ 13.748694] shift exponent 64 is too large for 64-bit type 'long long unsigned int'
> [...]
>
> When splitting the address to high and low, GENMASK_ULL is used to generate
> a bitmask with dma_addr_bits field from io_sq (in ena_com_prepare_tx and
> ena_com_add_single_rx_desc).
> The problem is that dma_addr_bits is not initialized with a proper value
> (besides being cleared in ena_com_create_io_queue).
> Assign dma_addr_bits the correct value that is stored in ena_dev when
> initializing the SQ.
>
> Fixes: 1738cd3ed342 ("net: ena: Add a driver for Amazon Elastic Network Adapters (ENA)")
> Signed-off-by: Gal Pressman <pressmangal@gmail.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH net-next] netdevsim: make debug dirs' dentries static
From: David Miller @ 2018-07-27 4:29 UTC (permalink / raw)
To: jakub.kicinski; +Cc: oss-drivers, netdev
In-Reply-To: <20180726212526.585-1-jakub.kicinski@netronome.com>
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Thu, 26 Jul 2018 14:25:26 -0700
> The root directories of netdevsim should only be used by the core
> to create per-device subdirectories, so limit their visibility to
> the core file.
>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
Applied.
^ permalink raw reply
* Re: [PATCH] isdn: hisax: callc: Replace GFP_ATOMIC with GFP_KERNEL in init_PStack()
From: David Miller @ 2018-07-27 4:25 UTC (permalink / raw)
To: baijiaju1990; +Cc: isdn, netdev, linux-kernel
In-Reply-To: <20180727024530.1849-1-baijiaju1990@gmail.com>
From: Jia-Ju Bai <baijiaju1990@gmail.com>
Date: Fri, 27 Jul 2018 10:45:30 +0800
> init_PStack() is never called in atomic context.
> It calls kmalloc() with GFP_ATOMIC, which is not necessary.
> GFP_ATOMIC can be replaced with GFP_KERNEL.
>
> This is found by a static analysis tool named DCNS written by myself.
>
> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH ipsec-next] xfrm: fix 'passing zero to ERR_PTR()' warning
From: Steffen Klassert @ 2018-07-27 5:37 UTC (permalink / raw)
To: YueHaibing; +Cc: davem, herbert, linux-kernel, netdev
In-Reply-To: <20180725085433.18756-1-yuehaibing@huawei.com>
On Wed, Jul 25, 2018 at 04:54:33PM +0800, YueHaibing wrote:
> Fix a static code checker warning:
>
> net/xfrm/xfrm_policy.c:1836 xfrm_resolve_and_create_bundle() warn: passing zero to 'ERR_PTR'
>
> xfrm_tmpl_resolve return 0 just means no xdst found, return NULL
> instead of passing zero to ERR_PTR.
>
> Fixes: d809ec895505 ("xfrm: do not assume that template resolving always returns xfrms")
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Applied, thanks a lot!
^ permalink raw reply
* Re: [patch net-next RFC] net: sched: don't dump chains only held by actions
From: Cong Wang @ 2018-07-27 4:18 UTC (permalink / raw)
To: Jiri Pirko
Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
mlxsw
In-Reply-To: <20180726163101.19718-1-jiri@resnulli.us>
On Thu, Jul 26, 2018 at 9:33 AM Jiri Pirko <jiri@resnulli.us> wrote:
>
> From: Jiri Pirko <jiri@mellanox.com>
>
> In case a chain is empty and not explicitly created by a user,
> such chain should not exist. The only exception is if there is
> an action "goto chain" pointing to it. In that case, don't show the
> chain in the dump. Track the chain references held by actions and
> use them to find out if a chain should or should not be shown
> in chain dump.
Hiding it makes sense. But you still need to make sure
user can't find it by ID, hiding it merely means user can't
see it.
Also, I don't understand why you increase the refcnt
for a hiding chain either, like Jakub mentioned.
If user can't see it, it must not be found by ID either.
^ permalink raw reply
* Re: [PATCH v3 bpf-next 02/14] bpf: introduce cgroup storage maps
From: Daniel Borkmann @ 2018-07-27 4:11 UTC (permalink / raw)
To: Roman Gushchin, netdev; +Cc: linux-kernel, kernel-team, Alexei Starovoitov
In-Reply-To: <20180720174558.5829-3-guro@fb.com>
On 07/20/2018 07:45 PM, Roman Gushchin wrote:
> This commit introduces BPF_MAP_TYPE_CGROUP_STORAGE maps:
> a special type of maps which are implementing the cgroup storage.
>
> From the userspace point of view it's almost a generic
> hash map with the (cgroup inode id, attachment type) pair
> used as a key.
>
> The only difference is that some operations are restricted:
> 1) a user can't create new entries,
> 2) a user can't remove existing entries.
>
> The lookup from userspace is o(log(n)).
>
> Signed-off-by: Roman Gushchin <guro@fb.com>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Acked-by: Martin KaFai Lau <kafai@fb.com>
(First of all sorry for the late review, only limited availability this week
on my side.)
> ---
> include/linux/bpf-cgroup.h | 38 +++++
> include/linux/bpf.h | 1 +
> include/linux/bpf_types.h | 3 +
> include/uapi/linux/bpf.h | 6 +
> kernel/bpf/Makefile | 1 +
> kernel/bpf/local_storage.c | 367 +++++++++++++++++++++++++++++++++++++++++++++
> kernel/bpf/verifier.c | 12 ++
> 7 files changed, 428 insertions(+)
> create mode 100644 kernel/bpf/local_storage.c
>
> diff --git a/include/linux/bpf-cgroup.h b/include/linux/bpf-cgroup.h
> index 79795c5fa7c3..6b0e7bd4b154 100644
> --- a/include/linux/bpf-cgroup.h
> +++ b/include/linux/bpf-cgroup.h
> @@ -3,19 +3,39 @@
> #define _BPF_CGROUP_H
>
> #include <linux/jump_label.h>
> +#include <linux/rbtree.h>
> #include <uapi/linux/bpf.h>
>
[...]
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 15d69b278277..0b089ba4595d 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -5140,6 +5140,14 @@ static int replace_map_fd_with_map_ptr(struct bpf_verifier_env *env)
> return -E2BIG;
> }
>
> + if (map->map_type == BPF_MAP_TYPE_CGROUP_STORAGE &&
> + bpf_cgroup_storage_assign(env->prog, map)) {
> + verbose(env,
> + "only one cgroup storage is allowed\n");
> + fdput(f);
> + return -EBUSY;
> + }
> +
> /* hold the map. If the program is rejected by verifier,
> * the map will be released by release_maps() or it
> * will be used by the valid program until it's unloaded
> @@ -5148,6 +5156,10 @@ static int replace_map_fd_with_map_ptr(struct bpf_verifier_env *env)
> map = bpf_map_inc(map, false);
> if (IS_ERR(map)) {
> fdput(f);
> + if (map->map_type ==
> + BPF_MAP_TYPE_CGROUP_STORAGE)
> + bpf_cgroup_storage_release(env->prog,
> + map);
I think this behavior is a bit strange, meaning that we would reset the map via
bpf_cgroup_storage_release() in this case, but if we error out and exit in any
later instruction the prior bpf_cgroup_storage_assign() is not undone, meaning
at this point we have no other choice but to destroy the map since any later
BPF prog load with bpf_cgroup_storage_assign() attempt would fail with -EBUSY
even though it's not assigned anywhere, is that correct? Same also on any other
errors along the prog load path. E.g. say, as one example, your verifier buffer
is too small, so any retry with the very same program from loader side would fail
above due to different prog pointers?
> return PTR_ERR(map);
> }
> env->used_maps[env->used_map_cnt++] = map;
>
^ 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