From 03891f820c2117b19e80b370281eb924a09cf79f Mon Sep 17 00:00:00 2001 From: Raed Salem Date: Sun, 2 Feb 2020 13:19:34 +0200 Subject: xfrm: handle NETDEV_UNREGISTER for xfrm device This patch to handle the asynchronous unregister device event so the device IPsec offload resources could be cleanly released. Fixes: e4db5b61c572 ("xfrm: policy: remove pcpu policy cache") Signed-off-by: Raed Salem Reviewed-by: Boris Pismenny Reviewed-by: Saeed Mahameed Signed-off-by: Steffen Klassert --- net/xfrm/xfrm_device.c | 1 + 1 file changed, 1 insertion(+) diff --git a/net/xfrm/xfrm_device.c b/net/xfrm/xfrm_device.c index 50f567a88f45..3231ec628544 100644 --- a/net/xfrm/xfrm_device.c +++ b/net/xfrm/xfrm_device.c @@ -383,6 +383,7 @@ static int xfrm_dev_event(struct notifier_block *this, unsigned long event, void return xfrm_dev_feat_change(dev); case NETDEV_DOWN: + case NETDEV_UNREGISTER: return xfrm_dev_down(dev); } return NOTIFY_DONE; -- cgit From f1ed10264ed6b66b9cd5e8461cffce69be482356 Mon Sep 17 00:00:00 2001 From: Nicolas Dichtel Date: Tue, 4 Feb 2020 17:00:27 +0100 Subject: vti[6]: fix packet tx through bpf_redirect() in XinY cases I forgot the 4in6/6in4 cases in my previous patch. Let's fix them. Fixes: 95224166a903 ("vti[6]: fix packet tx through bpf_redirect()") Signed-off-by: Nicolas Dichtel Signed-off-by: Steffen Klassert --- net/ipv4/Kconfig | 1 + net/ipv4/ip_vti.c | 38 ++++++++++++++++++++++++++++++-------- net/ipv6/ip6_vti.c | 32 +++++++++++++++++++++++++------- 3 files changed, 56 insertions(+), 15 deletions(-) diff --git a/net/ipv4/Kconfig b/net/ipv4/Kconfig index f96bd489b362..6490b845e17b 100644 --- a/net/ipv4/Kconfig +++ b/net/ipv4/Kconfig @@ -303,6 +303,7 @@ config SYN_COOKIES config NET_IPVTI tristate "Virtual (secure) IP: tunneling" + depends on IPV6 || IPV6=n select INET_TUNNEL select NET_IP_TUNNEL select XFRM diff --git a/net/ipv4/ip_vti.c b/net/ipv4/ip_vti.c index 37cddd18f282..1b4e6f298648 100644 --- a/net/ipv4/ip_vti.c +++ b/net/ipv4/ip_vti.c @@ -187,17 +187,39 @@ static netdev_tx_t vti_xmit(struct sk_buff *skb, struct net_device *dev, int mtu; if (!dst) { - struct rtable *rt; - - fl->u.ip4.flowi4_oif = dev->ifindex; - fl->u.ip4.flowi4_flags |= FLOWI_FLAG_ANYSRC; - rt = __ip_route_output_key(dev_net(dev), &fl->u.ip4); - if (IS_ERR(rt)) { + switch (skb->protocol) { + case htons(ETH_P_IP): { + struct rtable *rt; + + fl->u.ip4.flowi4_oif = dev->ifindex; + fl->u.ip4.flowi4_flags |= FLOWI_FLAG_ANYSRC; + rt = __ip_route_output_key(dev_net(dev), &fl->u.ip4); + if (IS_ERR(rt)) { + dev->stats.tx_carrier_errors++; + goto tx_error_icmp; + } + dst = &rt->dst; + skb_dst_set(skb, dst); + break; + } +#if IS_ENABLED(CONFIG_IPV6) + case htons(ETH_P_IPV6): + fl->u.ip6.flowi6_oif = dev->ifindex; + fl->u.ip6.flowi6_flags |= FLOWI_FLAG_ANYSRC; + dst = ip6_route_output(dev_net(dev), NULL, &fl->u.ip6); + if (dst->error) { + dst_release(dst); + dst = NULL; + dev->stats.tx_carrier_errors++; + goto tx_error_icmp; + } + skb_dst_set(skb, dst); + break; +#endif + default: dev->stats.tx_carrier_errors++; goto tx_error_icmp; } - dst = &rt->dst; - skb_dst_set(skb, dst); } dst_hold(dst); diff --git a/net/ipv6/ip6_vti.c b/net/ipv6/ip6_vti.c index 524006aa0d78..56e642efefff 100644 --- a/net/ipv6/ip6_vti.c +++ b/net/ipv6/ip6_vti.c @@ -450,15 +450,33 @@ vti6_xmit(struct sk_buff *skb, struct net_device *dev, struct flowi *fl) int mtu; if (!dst) { - fl->u.ip6.flowi6_oif = dev->ifindex; - fl->u.ip6.flowi6_flags |= FLOWI_FLAG_ANYSRC; - dst = ip6_route_output(dev_net(dev), NULL, &fl->u.ip6); - if (dst->error) { - dst_release(dst); - dst = NULL; + switch (skb->protocol) { + case htons(ETH_P_IP): { + struct rtable *rt; + + fl->u.ip4.flowi4_oif = dev->ifindex; + fl->u.ip4.flowi4_flags |= FLOWI_FLAG_ANYSRC; + rt = __ip_route_output_key(dev_net(dev), &fl->u.ip4); + if (IS_ERR(rt)) + goto tx_err_link_failure; + dst = &rt->dst; + skb_dst_set(skb, dst); + break; + } + case htons(ETH_P_IPV6): + fl->u.ip6.flowi6_oif = dev->ifindex; + fl->u.ip6.flowi6_flags |= FLOWI_FLAG_ANYSRC; + dst = ip6_route_output(dev_net(dev), NULL, &fl->u.ip6); + if (dst->error) { + dst_release(dst); + dst = NULL; + goto tx_err_link_failure; + } + skb_dst_set(skb, dst); + break; + default: goto tx_err_link_failure; } - skb_dst_set(skb, dst); } dst_hold(dst); -- cgit From 171d449a028573b2f0acdc7f31ecbb045391b320 Mon Sep 17 00:00:00 2001 From: Xin Long Date: Sun, 9 Feb 2020 21:15:29 +0800 Subject: xfrm: fix uctx len check in verify_sec_ctx_len It's not sufficient to do 'uctx->len != (sizeof(struct xfrm_user_sec_ctx) + uctx->ctx_len)' check only, as uctx->len may be greater than nla_len(rt), in which case it will cause slab-out-of-bounds when accessing uctx->ctx_str later. This patch is to fix it by return -EINVAL when uctx->len > nla_len(rt). Fixes: df71837d5024 ("[LSM-IPSec]: Security association restriction.") Signed-off-by: Xin Long Signed-off-by: Steffen Klassert --- net/xfrm/xfrm_user.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c index b88ba45ff1ac..38ff02d31402 100644 --- a/net/xfrm/xfrm_user.c +++ b/net/xfrm/xfrm_user.c @@ -110,7 +110,8 @@ static inline int verify_sec_ctx_len(struct nlattr **attrs) return 0; uctx = nla_data(rt); - if (uctx->len != (sizeof(struct xfrm_user_sec_ctx) + uctx->ctx_len)) + if (uctx->len > nla_len(rt) || + uctx->len != (sizeof(struct xfrm_user_sec_ctx) + uctx->ctx_len)) return -EINVAL; return 0; -- cgit From a1a7e3a36e01ca6e67014f8cf673cb8e47be5550 Mon Sep 17 00:00:00 2001 From: Xin Long Date: Sun, 9 Feb 2020 21:16:38 +0800 Subject: xfrm: add the missing verify_sec_ctx_len check in xfrm_add_acquire Without doing verify_sec_ctx_len() check in xfrm_add_acquire(), it may be out-of-bounds to access uctx->ctx_str with uctx->ctx_len, as noticed by syz: BUG: KASAN: slab-out-of-bounds in selinux_xfrm_alloc_user+0x237/0x430 Read of size 768 at addr ffff8880123be9b4 by task syz-executor.1/11650 Call Trace: dump_stack+0xe8/0x16e print_address_description.cold.3+0x9/0x23b kasan_report.cold.4+0x64/0x95 memcpy+0x1f/0x50 selinux_xfrm_alloc_user+0x237/0x430 security_xfrm_policy_alloc+0x5c/0xb0 xfrm_policy_construct+0x2b1/0x650 xfrm_add_acquire+0x21d/0xa10 xfrm_user_rcv_msg+0x431/0x6f0 netlink_rcv_skb+0x15a/0x410 xfrm_netlink_rcv+0x6d/0x90 netlink_unicast+0x50e/0x6a0 netlink_sendmsg+0x8ae/0xd40 sock_sendmsg+0x133/0x170 ___sys_sendmsg+0x834/0x9a0 __sys_sendmsg+0x100/0x1e0 do_syscall_64+0xe5/0x660 entry_SYSCALL_64_after_hwframe+0x6a/0xdf So fix it by adding the missing verify_sec_ctx_len check there. Fixes: 980ebd25794f ("[IPSEC]: Sync series - acquire insert") Reported-by: Hangbin Liu Signed-off-by: Xin Long Signed-off-by: Steffen Klassert --- net/xfrm/xfrm_user.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c index 38ff02d31402..e6cfaa680ef3 100644 --- a/net/xfrm/xfrm_user.c +++ b/net/xfrm/xfrm_user.c @@ -2274,6 +2274,9 @@ static int xfrm_add_acquire(struct sk_buff *skb, struct nlmsghdr *nlh, xfrm_mark_get(attrs, &mark); err = verify_newpolicy_info(&ua->policy); + if (err) + goto free_state; + err = verify_sec_ctx_len(attrs); if (err) goto free_state; -- cgit From edf0d283d988c0c7da53fa2cc1388649263470de Mon Sep 17 00:00:00 2001 From: Madhuparna Bhowmik Date: Fri, 21 Feb 2020 21:54:47 +0530 Subject: ipv6: xfrm6_tunnel.c: Use built-in RCU list checking hlist_for_each_entry_rcu() has built-in RCU and lock checking. Pass cond argument to list_for_each_entry_rcu() to silence false lockdep warning when CONFIG_PROVE_RCU_LIST is enabled by default. Signed-off-by: Madhuparna Bhowmik Signed-off-by: Steffen Klassert --- net/ipv6/xfrm6_tunnel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/ipv6/xfrm6_tunnel.c b/net/ipv6/xfrm6_tunnel.c index e11bdb0aaa15..25b7ebda2fab 100644 --- a/net/ipv6/xfrm6_tunnel.c +++ b/net/ipv6/xfrm6_tunnel.c @@ -78,7 +78,7 @@ static struct xfrm6_tunnel_spi *__xfrm6_tunnel_spi_lookup(struct net *net, const hlist_for_each_entry_rcu(x6spi, &xfrm6_tn->spi_byaddr[xfrm6_tunnel_spi_hash_byaddr(saddr)], - list_byaddr) { + list_byaddr, lockdep_is_held(&xfrm6_tunnel_spi_lock)) { if (xfrm6_addr_equal(&x6spi->addr, saddr)) return x6spi; } -- cgit From d1d17a359ce6901545c075d7401c10179d9cedfd Mon Sep 17 00:00:00 2001 From: Xin Long Date: Wed, 4 Mar 2020 16:51:42 +0800 Subject: esp: remove the skb from the chain when it's enqueued in cryptd_wq Xiumei found a panic in esp offload: BUG: unable to handle kernel NULL pointer dereference at 0000000000000020 RIP: 0010:esp_output_done+0x101/0x160 [esp4] Call Trace: ? esp_output+0x180/0x180 [esp4] cryptd_aead_crypt+0x4c/0x90 cryptd_queue_worker+0x6e/0xa0 process_one_work+0x1a7/0x3b0 worker_thread+0x30/0x390 ? create_worker+0x1a0/0x1a0 kthread+0x112/0x130 ? kthread_flush_work_fn+0x10/0x10 ret_from_fork+0x35/0x40 It was caused by that skb secpath is used in esp_output_done() after it's been released elsewhere. The tx path for esp offload is: __dev_queue_xmit()-> validate_xmit_skb_list()-> validate_xmit_xfrm()-> esp_xmit()-> esp_output_tail()-> aead_request_set_callback(esp_output_done) <--[1] crypto_aead_encrypt() <--[2] In [1], .callback is set, and in [2] it will trigger the worker schedule, later on a kernel thread will call .callback(esp_output_done), as the call trace shows. But in validate_xmit_xfrm(): skb_list_walk_safe(skb, skb2, nskb) { ... err = x->type_offload->xmit(x, skb2, esp_features); [esp_xmit] ... } When the err is -EINPROGRESS, which means this skb2 will be enqueued and later gets encrypted and sent out by .callback later in a kernel thread, skb2 should be removed fromt skb chain. Otherwise, it will get processed again outside validate_xmit_xfrm(), which could release skb secpath, and cause the panic above. This patch is to remove the skb from the chain when it's enqueued in cryptd_wq. While at it, remove the unnecessary 'if (!skb)' check. Fixes: 3dca3f38cfb8 ("xfrm: Separate ESP handling from segmentation for GRO packets.") Reported-by: Xiumei Mu Signed-off-by: Xin Long Signed-off-by: Steffen Klassert --- net/xfrm/xfrm_device.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/net/xfrm/xfrm_device.c b/net/xfrm/xfrm_device.c index 3231ec628544..e2db468cf50e 100644 --- a/net/xfrm/xfrm_device.c +++ b/net/xfrm/xfrm_device.c @@ -78,8 +78,8 @@ struct sk_buff *validate_xmit_xfrm(struct sk_buff *skb, netdev_features_t featur int err; unsigned long flags; struct xfrm_state *x; - struct sk_buff *skb2, *nskb; struct softnet_data *sd; + struct sk_buff *skb2, *nskb, *pskb = NULL; netdev_features_t esp_features = features; struct xfrm_offload *xo = xfrm_offload(skb); struct sec_path *sp; @@ -168,14 +168,14 @@ struct sk_buff *validate_xmit_xfrm(struct sk_buff *skb, netdev_features_t featur } else { if (skb == skb2) skb = nskb; - - if (!skb) - return NULL; + else + pskb->next = nskb; continue; } skb_push(skb2, skb2->data - skb_mac_header(skb2)); + pskb = skb2; } return skb; -- cgit From 2a9de3af21aa8c31cd68b0b39330d69f8c1e59df Mon Sep 17 00:00:00 2001 From: Torsten Hilbrich Date: Wed, 11 Mar 2020 11:19:06 +0100 Subject: vti6: Fix memory leak of skb if input policy check fails The vti6_rcv function performs some tests on the retrieved tunnel including checking the IP protocol, the XFRM input policy, the source and destination address. In all but one places the skb is released in the error case. When the input policy check fails the network packet is leaked. Using the same goto-label discard in this case to fix this problem. Fixes: ed1efb2aefbb ("ipv6: Add support for IPsec virtual tunnel interfaces") Signed-off-by: Torsten Hilbrich Reviewed-by: Nicolas Dichtel Signed-off-by: Steffen Klassert --- net/ipv6/ip6_vti.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/ipv6/ip6_vti.c b/net/ipv6/ip6_vti.c index 56e642efefff..cc6180e08a4f 100644 --- a/net/ipv6/ip6_vti.c +++ b/net/ipv6/ip6_vti.c @@ -311,7 +311,7 @@ static int vti6_rcv(struct sk_buff *skb) if (!xfrm6_policy_check(NULL, XFRM_POLICY_IN, skb)) { rcu_read_unlock(); - return 0; + goto discard; } ipv6h = ipv6_hdr(skb); -- cgit From 8e7ae2518f5265f0ef09d561748098fde5a87ccd Mon Sep 17 00:00:00 2001 From: Martin KaFai Lau Date: Fri, 13 Mar 2020 18:02:09 -0700 Subject: bpf: Sanitize the bpf_struct_ops tcp-cc name The bpf_struct_ops tcp-cc name should be sanitized in order to avoid problematic chars (e.g. whitespaces). This patch reuses the bpf_obj_name_cpy() for accepting the same set of characters in order to keep a consistent bpf programming experience. A "size" param is added. Also, the strlen is returned on success so that the caller (like the bpf_tcp_ca here) can error out on empty name. The existing callers of the bpf_obj_name_cpy() only need to change the testing statement to "if (err < 0)". For all these existing callers, the err will be overwritten later, so no extra change is needed for the new strlen return value. v3: - reverse xmas tree style v2: - Save the orig_src to avoid "end - size" (Andrii) Fixes: 0baf26b0fcd7 ("bpf: tcp: Support tcp_congestion_ops in bpf") Signed-off-by: Martin KaFai Lau Signed-off-by: Daniel Borkmann Acked-by: Andrii Nakryiko Link: https://lore.kernel.org/bpf/20200314010209.1131542-1-kafai@fb.com --- include/linux/bpf.h | 1 + kernel/bpf/syscall.c | 25 ++++++++++++++----------- net/ipv4/bpf_tcp_ca.c | 7 ++----- 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 49b1a70e12c8..212991f6f2a5 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -160,6 +160,7 @@ static inline void copy_map_value(struct bpf_map *map, void *dst, void *src) } void copy_map_value_locked(struct bpf_map *map, void *dst, void *src, bool lock_src); +int bpf_obj_name_cpy(char *dst, const char *src, unsigned int size); struct bpf_offload_dev; struct bpf_offloaded_map; diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index 0c7fb0d4836d..2857b7dda382 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -696,14 +696,15 @@ int bpf_get_file_flag(int flags) offsetof(union bpf_attr, CMD##_LAST_FIELD) - \ sizeof(attr->CMD##_LAST_FIELD)) != NULL -/* dst and src must have at least BPF_OBJ_NAME_LEN number of bytes. - * Return 0 on success and < 0 on error. +/* dst and src must have at least "size" number of bytes. + * Return strlen on success and < 0 on error. */ -static int bpf_obj_name_cpy(char *dst, const char *src) +int bpf_obj_name_cpy(char *dst, const char *src, unsigned int size) { - const char *end = src + BPF_OBJ_NAME_LEN; + const char *end = src + size; + const char *orig_src = src; - memset(dst, 0, BPF_OBJ_NAME_LEN); + memset(dst, 0, size); /* Copy all isalnum(), '_' and '.' chars. */ while (src < end && *src) { if (!isalnum(*src) && @@ -712,11 +713,11 @@ static int bpf_obj_name_cpy(char *dst, const char *src) *dst++ = *src++; } - /* No '\0' found in BPF_OBJ_NAME_LEN number of bytes */ + /* No '\0' found in "size" number of bytes */ if (src == end) return -EINVAL; - return 0; + return src - orig_src; } int map_check_no_btf(const struct bpf_map *map, @@ -810,8 +811,9 @@ static int map_create(union bpf_attr *attr) if (IS_ERR(map)) return PTR_ERR(map); - err = bpf_obj_name_cpy(map->name, attr->map_name); - if (err) + err = bpf_obj_name_cpy(map->name, attr->map_name, + sizeof(attr->map_name)); + if (err < 0) goto free_map; atomic64_set(&map->refcnt, 1); @@ -2098,8 +2100,9 @@ static int bpf_prog_load(union bpf_attr *attr, union bpf_attr __user *uattr) goto free_prog; prog->aux->load_time = ktime_get_boottime_ns(); - err = bpf_obj_name_cpy(prog->aux->name, attr->prog_name); - if (err) + err = bpf_obj_name_cpy(prog->aux->name, attr->prog_name, + sizeof(attr->prog_name)); + if (err < 0) goto free_prog; /* run eBPF verifier */ diff --git a/net/ipv4/bpf_tcp_ca.c b/net/ipv4/bpf_tcp_ca.c index 574972bc7299..2bf3abeb1456 100644 --- a/net/ipv4/bpf_tcp_ca.c +++ b/net/ipv4/bpf_tcp_ca.c @@ -184,7 +184,6 @@ static int bpf_tcp_ca_init_member(const struct btf_type *t, { const struct tcp_congestion_ops *utcp_ca; struct tcp_congestion_ops *tcp_ca; - size_t tcp_ca_name_len; int prog_fd; u32 moff; @@ -199,13 +198,11 @@ static int bpf_tcp_ca_init_member(const struct btf_type *t, tcp_ca->flags = utcp_ca->flags; return 1; case offsetof(struct tcp_congestion_ops, name): - tcp_ca_name_len = strnlen(utcp_ca->name, sizeof(utcp_ca->name)); - if (!tcp_ca_name_len || - tcp_ca_name_len == sizeof(utcp_ca->name)) + if (bpf_obj_name_cpy(tcp_ca->name, utcp_ca->name, + sizeof(tcp_ca->name)) <= 0) return -EINVAL; if (tcp_ca_find(utcp_ca->name)) return -EEXIST; - memcpy(tcp_ca->name, utcp_ca->name, sizeof(tcp_ca->name)); return 1; } -- cgit From 8096f229421f7b22433775e928d506f0342e5907 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Fri, 20 Mar 2020 10:48:13 +0100 Subject: bpf: Explicitly memset the bpf_attr structure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For the bpf syscall, we are relying on the compiler to properly zero out the bpf_attr union that we copy userspace data into. Unfortunately that doesn't always work properly, padding and other oddities might not be correctly zeroed, and in some tests odd things have been found when the stack is pre-initialized to other values. Fix this by explicitly memsetting the structure to 0 before using it. Reported-by: Maciej Żenczykowski Reported-by: John Stultz Reported-by: Alexander Potapenko Reported-by: Alistair Delva Signed-off-by: Greg Kroah-Hartman Signed-off-by: Daniel Borkmann Acked-by: Yonghong Song Link: https://android-review.googlesource.com/c/kernel/common/+/1235490 Link: https://lore.kernel.org/bpf/20200320094813.GA421650@kroah.com --- kernel/bpf/syscall.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index 2857b7dda382..3ac1629cf00e 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -3362,7 +3362,7 @@ err_put: SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, size) { - union bpf_attr attr = {}; + union bpf_attr attr; int err; if (sysctl_unprivileged_bpf_disabled && !capable(CAP_SYS_ADMIN)) @@ -3374,6 +3374,7 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz size = min_t(u32, size, sizeof(attr)); /* copy attributes from user space, may be less than sizeof(bpf_attr) */ + memset(&attr, 0, sizeof(attr)); if (copy_from_user(&attr, uattr, size) != 0) return -EFAULT; -- cgit From 5c6f25887963f15492b604dd25cb149c501bbabf Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Fri, 20 Mar 2020 17:22:58 +0100 Subject: bpf: Explicitly memset some bpf info structures declared on the stack Trying to initialize a structure with "= {};" will not always clean out all padding locations in a structure. So be explicit and call memset to initialize everything for a number of bpf information structures that are then copied from userspace, sometimes from smaller memory locations than the size of the structure. Reported-by: Daniel Borkmann Signed-off-by: Greg Kroah-Hartman Signed-off-by: Daniel Borkmann Acked-by: Yonghong Song Link: https://lore.kernel.org/bpf/20200320162258.GA794295@kroah.com --- kernel/bpf/btf.c | 3 ++- kernel/bpf/syscall.c | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index 32ab9225026e..7787bdcb5d68 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -4564,7 +4564,7 @@ int btf_get_info_by_fd(const struct btf *btf, union bpf_attr __user *uattr) { struct bpf_btf_info __user *uinfo; - struct bpf_btf_info info = {}; + struct bpf_btf_info info; u32 info_copy, btf_copy; void __user *ubtf; u32 uinfo_len; @@ -4573,6 +4573,7 @@ int btf_get_info_by_fd(const struct btf *btf, uinfo_len = attr->info.info_len; info_copy = min_t(u32, uinfo_len, sizeof(info)); + memset(&info, 0, sizeof(info)); if (copy_from_user(&info, uinfo, info_copy)) return -EFAULT; diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index 3ac1629cf00e..966b7b34cde0 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -2795,7 +2795,7 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog, union bpf_attr __user *uattr) { struct bpf_prog_info __user *uinfo = u64_to_user_ptr(attr->info.info); - struct bpf_prog_info info = {}; + struct bpf_prog_info info; u32 info_len = attr->info.info_len; struct bpf_prog_stats stats; char __user *uinsns; @@ -2807,6 +2807,7 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog, return err; info_len = min_t(u32, sizeof(info), info_len); + memset(&info, 0, sizeof(info)); if (copy_from_user(&info, uinfo, info_len)) return -EFAULT; @@ -3070,7 +3071,7 @@ static int bpf_map_get_info_by_fd(struct bpf_map *map, union bpf_attr __user *uattr) { struct bpf_map_info __user *uinfo = u64_to_user_ptr(attr->info.info); - struct bpf_map_info info = {}; + struct bpf_map_info info; u32 info_len = attr->info.info_len; int err; @@ -3079,6 +3080,7 @@ static int bpf_map_get_info_by_fd(struct bpf_map *map, return err; info_len = min_t(u32, sizeof(info), info_len); + memset(&info, 0, sizeof(info)); info.type = map->map_type; info.id = map->id; info.key_size = map->key_size; -- cgit From 4c59406ed00379c8663f8663d82b2537467ce9d7 Mon Sep 17 00:00:00 2001 From: YueHaibing Date: Mon, 23 Mar 2020 15:32:39 +0800 Subject: xfrm: policy: Fix doulbe free in xfrm_policy_timer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After xfrm_add_policy add a policy, its ref is 2, then xfrm_policy_timer read_lock xp->walk.dead is 0 .... mod_timer() xfrm_policy_kill policy->walk.dead = 1 .... del_timer(&policy->timer) xfrm_pol_put //ref is 1 xfrm_pol_put //ref is 0 xfrm_policy_destroy call_rcu xfrm_pol_hold //ref is 1 read_unlock xfrm_pol_put //ref is 0 xfrm_policy_destroy call_rcu xfrm_policy_destroy is called twice, which may leads to double free. Call Trace: RIP: 0010:refcount_warn_saturate+0x161/0x210 ... xfrm_policy_timer+0x522/0x600 call_timer_fn+0x1b3/0x5e0 ? __xfrm_decode_session+0x2990/0x2990 ? msleep+0xb0/0xb0 ? _raw_spin_unlock_irq+0x24/0x40 ? __xfrm_decode_session+0x2990/0x2990 ? __xfrm_decode_session+0x2990/0x2990 run_timer_softirq+0x5c5/0x10e0 Fix this by use write_lock_bh in xfrm_policy_kill. Fixes: ea2dea9dacc2 ("xfrm: remove policy lock when accessing policy->walk.dead") Signed-off-by: YueHaibing Acked-by: Timo Teräs Acked-by: Herbert Xu Signed-off-by: Steffen Klassert --- net/xfrm/xfrm_policy.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c index 297d1eb79e5c..96a8c452b63e 100644 --- a/net/xfrm/xfrm_policy.c +++ b/net/xfrm/xfrm_policy.c @@ -434,7 +434,9 @@ EXPORT_SYMBOL(xfrm_policy_destroy); static void xfrm_policy_kill(struct xfrm_policy *policy) { + write_lock_bh(&policy->lock); policy->walk.dead = 1; + write_unlock_bh(&policy->lock); atomic_inc(&policy->genid); -- cgit From 0016d3201753b59f3ae84b868fe66c86ad256f19 Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Wed, 25 Mar 2020 09:05:32 +0100 Subject: nl80211: fix NL80211_ATTR_CHANNEL_WIDTH attribute type The new opmode notification used this attribute with a u8, when it's documented as a u32 and indeed used in userspace as such, it just happens to work on little-endian systems since userspace isn't doing any strict size validation, and the u8 goes into the lower byte. Fix this. Cc: stable@vger.kernel.org Fixes: 466b9936bf93 ("cfg80211: Add support to notify station's opmode change to userspace") Signed-off-by: Johannes Berg Link: https://lore.kernel.org/r/20200325090531.be124f0a11c7.Iedbf4e197a85471ebd729b186d5365c0343bf7a8@changeid Signed-off-by: Johannes Berg --- net/wireless/nl80211.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index ec5d67794aab..f0af23c1634a 100644 --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -16416,7 +16416,7 @@ void cfg80211_sta_opmode_change_notify(struct net_device *dev, const u8 *mac, goto nla_put_failure; if ((sta_opmode->changed & STA_OPMODE_MAX_BW_CHANGED) && - nla_put_u8(msg, NL80211_ATTR_CHANNEL_WIDTH, sta_opmode->bw)) + nla_put_u32(msg, NL80211_ATTR_CHANNEL_WIDTH, sta_opmode->bw)) goto nla_put_failure; if ((sta_opmode->changed & STA_OPMODE_N_SS_CHANGED) && -- cgit From 575a97acc3b7446094b0dcaf6285c7c6934c2477 Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Wed, 25 Mar 2020 09:09:19 +0100 Subject: ieee80211: fix HE SPR size calculation The he_sr_control field is just a u8, so le32_to_cpu() shouldn't be applied to it; this was evidently copied from ieee80211_he_oper_size(). Fix it, and also adjust the type of the local variable. Fixes: ef11a931bd1c ("mac80211: HE: add Spatial Reuse element parsing support") Signed-off-by: Johannes Berg Link: https://lore.kernel.org/r/20200325090918.dfe483b49e06.Ia53622f23b2610a2ae6ea39a199866196fe946c1@changeid Signed-off-by: Johannes Berg --- include/linux/ieee80211.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h index 7d3f2ced92d1..73c66a3a33ae 100644 --- a/include/linux/ieee80211.h +++ b/include/linux/ieee80211.h @@ -2102,14 +2102,14 @@ ieee80211_he_spr_size(const u8 *he_spr_ie) { struct ieee80211_he_spr *he_spr = (void *)he_spr_ie; u8 spr_len = sizeof(struct ieee80211_he_spr); - u32 he_spr_params; + u8 he_spr_params; /* Make sure the input is not NULL */ if (!he_spr_ie) return 0; /* Calc required length */ - he_spr_params = le32_to_cpu(he_spr->he_sr_control); + he_spr_params = he_spr->he_sr_control; if (he_spr_params & IEEE80211_HE_SPR_NON_SRG_OFFSET_PRESENT) spr_len++; if (he_spr_params & IEEE80211_HE_SPR_SRG_INFORMATION_PRESENT) -- cgit From a0761a301746ec2d92d7fcb82af69c0a6a4339aa Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Thu, 26 Mar 2020 15:09:42 +0200 Subject: mac80211: drop data frames without key on encrypted links If we know that we have an encrypted link (based on having had a key configured for TX in the past) then drop all data frames in the key selection handler if there's no key anymore. This fixes an issue with mac80211 internal TXQs - there we can buffer frames for an encrypted link, but then if the key is no longer there when they're dequeued, the frames are sent without encryption. This happens if a station is disconnected while the frames are still on the TXQ. Detecting that a link should be encrypted based on a first key having been configured for TX is fine as there are no use cases for a connection going from with encryption to no encryption. With extended key IDs, however, there is a case of having a key configured for only decryption, so we can't just trigger this behaviour on a key being configured. Cc: stable@vger.kernel.org Reported-by: Jouni Malinen Signed-off-by: Johannes Berg Signed-off-by: Luca Coelho Link: https://lore.kernel.org/r/iwlwifi.20200326150855.6865c7f28a14.I9fb1d911b064262d33e33dfba730cdeef83926ca@changeid Signed-off-by: Johannes Berg --- net/mac80211/debugfs_sta.c | 3 ++- net/mac80211/key.c | 20 ++++++++++++-------- net/mac80211/sta_info.h | 1 + net/mac80211/tx.c | 12 +++++++++--- 4 files changed, 24 insertions(+), 12 deletions(-) diff --git a/net/mac80211/debugfs_sta.c b/net/mac80211/debugfs_sta.c index c80b1e163ea4..3419ed66c7b0 100644 --- a/net/mac80211/debugfs_sta.c +++ b/net/mac80211/debugfs_sta.c @@ -5,7 +5,7 @@ * Copyright 2007 Johannes Berg * Copyright 2013-2014 Intel Mobile Communications GmbH * Copyright(c) 2016 Intel Deutschland GmbH - * Copyright (C) 2018 - 2019 Intel Corporation + * Copyright (C) 2018 - 2020 Intel Corporation */ #include @@ -78,6 +78,7 @@ static const char * const sta_flag_names[] = { FLAG(MPSP_OWNER), FLAG(MPSP_RECIPIENT), FLAG(PS_DELIVER), + FLAG(USES_ENCRYPTION), #undef FLAG }; diff --git a/net/mac80211/key.c b/net/mac80211/key.c index 0f889b919b06..efc1acc6543c 100644 --- a/net/mac80211/key.c +++ b/net/mac80211/key.c @@ -6,7 +6,7 @@ * Copyright 2007-2008 Johannes Berg * Copyright 2013-2014 Intel Mobile Communications GmbH * Copyright 2015-2017 Intel Deutschland GmbH - * Copyright 2018-2019 Intel Corporation + * Copyright 2018-2020 Intel Corporation */ #include @@ -262,22 +262,29 @@ static void ieee80211_key_disable_hw_accel(struct ieee80211_key *key) sta ? sta->sta.addr : bcast_addr, ret); } -int ieee80211_set_tx_key(struct ieee80211_key *key) +static int _ieee80211_set_tx_key(struct ieee80211_key *key, bool force) { struct sta_info *sta = key->sta; struct ieee80211_local *local = key->local; assert_key_lock(local); + set_sta_flag(sta, WLAN_STA_USES_ENCRYPTION); + sta->ptk_idx = key->conf.keyidx; - if (!ieee80211_hw_check(&local->hw, AMPDU_KEYBORDER_SUPPORT)) + if (force || !ieee80211_hw_check(&local->hw, AMPDU_KEYBORDER_SUPPORT)) clear_sta_flag(sta, WLAN_STA_BLOCK_BA); ieee80211_check_fast_xmit(sta); return 0; } +int ieee80211_set_tx_key(struct ieee80211_key *key) +{ + return _ieee80211_set_tx_key(key, false); +} + static void ieee80211_pairwise_rekey(struct ieee80211_key *old, struct ieee80211_key *new) { @@ -441,11 +448,8 @@ static int ieee80211_key_replace(struct ieee80211_sub_if_data *sdata, if (pairwise) { rcu_assign_pointer(sta->ptk[idx], new); if (new && - !(new->conf.flags & IEEE80211_KEY_FLAG_NO_AUTO_TX)) { - sta->ptk_idx = idx; - clear_sta_flag(sta, WLAN_STA_BLOCK_BA); - ieee80211_check_fast_xmit(sta); - } + !(new->conf.flags & IEEE80211_KEY_FLAG_NO_AUTO_TX)) + _ieee80211_set_tx_key(new, true); } else { rcu_assign_pointer(sta->gtk[idx], new); } diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h index c00e28585f9d..552eed36faca 100644 --- a/net/mac80211/sta_info.h +++ b/net/mac80211/sta_info.h @@ -98,6 +98,7 @@ enum ieee80211_sta_info_flags { WLAN_STA_MPSP_OWNER, WLAN_STA_MPSP_RECIPIENT, WLAN_STA_PS_DELIVER, + WLAN_STA_USES_ENCRYPTION, NUM_WLAN_STA_FLAGS, }; diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c index 87def9cb91ff..7dbfb9e3cd84 100644 --- a/net/mac80211/tx.c +++ b/net/mac80211/tx.c @@ -590,10 +590,13 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx) struct ieee80211_tx_info *info = IEEE80211_SKB_CB(tx->skb); struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)tx->skb->data; - if (unlikely(info->flags & IEEE80211_TX_INTFL_DONT_ENCRYPT)) + if (unlikely(info->flags & IEEE80211_TX_INTFL_DONT_ENCRYPT)) { tx->key = NULL; - else if (tx->sta && - (key = rcu_dereference(tx->sta->ptk[tx->sta->ptk_idx]))) + return TX_CONTINUE; + } + + if (tx->sta && + (key = rcu_dereference(tx->sta->ptk[tx->sta->ptk_idx]))) tx->key = key; else if (ieee80211_is_group_privacy_action(tx->skb) && (key = rcu_dereference(tx->sdata->default_multicast_key))) @@ -654,6 +657,9 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx) if (!skip_hw && tx->key && tx->key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE) info->control.hw_key = &tx->key->conf; + } else if (!ieee80211_is_mgmt(hdr->frame_control) && tx->sta && + test_sta_flag(tx->sta, WLAN_STA_USES_ENCRYPTION)) { + return TX_DROP; } return TX_CONTINUE; -- cgit From 05dcb8bb258575a8dd3499d0d78bd2db633c2b23 Mon Sep 17 00:00:00 2001 From: Ilan Peer Date: Thu, 26 Mar 2020 15:09:43 +0200 Subject: cfg80211: Do not warn on same channel at the end of CSA When cfg80211_update_assoc_bss_entry() is called, there is a verification that the BSS channel actually changed. As some APs use CSA also for bandwidth changes, this would result with a kernel warning. Fix this by removing the WARN_ON(). Signed-off-by: Ilan Peer Signed-off-by: Luca Coelho Link: https://lore.kernel.org/r/iwlwifi.20200326150855.96316ada0e8d.I6710376b1b4257e5f4712fc7ab16e2b638d512aa@changeid Signed-off-by: Johannes Berg --- net/wireless/scan.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/net/wireless/scan.c b/net/wireless/scan.c index aef240fdf8df..328402ab64a3 100644 --- a/net/wireless/scan.c +++ b/net/wireless/scan.c @@ -2022,7 +2022,11 @@ void cfg80211_update_assoc_bss_entry(struct wireless_dev *wdev, spin_lock_bh(&rdev->bss_lock); - if (WARN_ON(cbss->pub.channel == chan)) + /* + * Some APs use CSA also for bandwidth changes, i.e., without actually + * changing the control channel, so no need to update in such a case. + */ + if (cbss->pub.channel == chan) goto done; /* use transmitting bss */ -- cgit From ce2e1ca703071723ca2dd94d492a5ab6d15050da Mon Sep 17 00:00:00 2001 From: Jouni Malinen Date: Thu, 26 Mar 2020 15:51:34 +0100 Subject: mac80211: Check port authorization in the ieee80211_tx_dequeue() case mac80211 used to check port authorization in the Data frame enqueue case when going through start_xmit(). However, that authorization status may change while the frame is waiting in a queue. Add a similar check in the dequeue case to avoid sending previously accepted frames after authorization change. This provides additional protection against potential leaking of frames after a station has been disconnected and the keys for it are being removed. Cc: stable@vger.kernel.org Signed-off-by: Jouni Malinen Link: https://lore.kernel.org/r/20200326155133.ced84317ea29.I34d4c47cd8cc8a4042b38a76f16a601fbcbfd9b3@changeid Signed-off-by: Johannes Berg --- net/mac80211/tx.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c index 7dbfb9e3cd84..455eb8e6a459 100644 --- a/net/mac80211/tx.c +++ b/net/mac80211/tx.c @@ -3604,8 +3604,25 @@ begin: tx.skb = skb; tx.sdata = vif_to_sdata(info->control.vif); - if (txq->sta) + if (txq->sta) { tx.sta = container_of(txq->sta, struct sta_info, sta); + /* + * Drop unicast frames to unauthorised stations unless they are + * EAPOL frames from the local station. + */ + if (unlikely(!ieee80211_vif_is_mesh(&tx.sdata->vif) && + tx.sdata->vif.type != NL80211_IFTYPE_OCB && + !is_multicast_ether_addr(hdr->addr1) && + !test_sta_flag(tx.sta, WLAN_STA_AUTHORIZED) && + (!(info->control.flags & + IEEE80211_TX_CTRL_PORT_CTRL_PROTO) || + !ether_addr_equal(tx.sdata->vif.addr, + hdr->addr2)))) { + I802_DEBUG_INC(local->tx_handlers_drop_unauth_port); + ieee80211_free_txskb(&local->hw, skb); + goto begin; + } + } /* * The key can be removed while the packet was queued, so need to call -- cgit From b16798f5b907733966fd1a558fca823b3c67e4a1 Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Thu, 26 Mar 2020 15:51:35 +0100 Subject: mac80211: mark station unauthorized before key removal If a station is still marked as authorized, mark it as no longer so before removing its keys. This allows frames transmitted to it to be rejected, providing additional protection against leaking plain text data during the disconnection flow. Cc: stable@vger.kernel.org Link: https://lore.kernel.org/r/20200326155133.ccb4fb0bb356.If48f0f0504efdcf16b8921f48c6d3bb2cb763c99@changeid Signed-off-by: Johannes Berg --- net/mac80211/sta_info.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c index 0f5f40678885..e3572be307d6 100644 --- a/net/mac80211/sta_info.c +++ b/net/mac80211/sta_info.c @@ -4,7 +4,7 @@ * Copyright 2006-2007 Jiri Benc * Copyright 2013-2014 Intel Mobile Communications GmbH * Copyright (C) 2015 - 2017 Intel Deutschland GmbH - * Copyright (C) 2018-2019 Intel Corporation + * Copyright (C) 2018-2020 Intel Corporation */ #include @@ -1049,6 +1049,11 @@ static void __sta_info_destroy_part2(struct sta_info *sta) might_sleep(); lockdep_assert_held(&local->sta_mtx); + while (sta->sta_state == IEEE80211_STA_AUTHORIZED) { + ret = sta_info_move_state(sta, IEEE80211_STA_ASSOC); + WARN_ON_ONCE(ret); + } + /* now keys can no longer be reached */ ieee80211_free_sta_keys(local, sta); -- cgit From b95d2ccd2ccb834394d50347d0e40dc38a954e4a Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Thu, 26 Mar 2020 15:53:34 +0100 Subject: mac80211: set IEEE80211_TX_CTRL_PORT_CTRL_PROTO for nl80211 TX When a frame is transmitted via the nl80211 TX rather than as a normal frame, IEEE80211_TX_CTRL_PORT_CTRL_PROTO wasn't set and this will lead to wrong decisions (rate control etc.) being made about the frame; fix this. Fixes: 911806491425 ("mac80211: Add support for tx_control_port") Signed-off-by: Johannes Berg Link: https://lore.kernel.org/r/20200326155333.f183f52b02f0.I4054e2a8c11c2ddcb795a0103c87be3538690243@changeid Signed-off-by: Johannes Berg --- net/mac80211/tx.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c index 455eb8e6a459..d9cca6dbd870 100644 --- a/net/mac80211/tx.c +++ b/net/mac80211/tx.c @@ -5,7 +5,7 @@ * Copyright 2006-2007 Jiri Benc * Copyright 2007 Johannes Berg * Copyright 2013-2014 Intel Mobile Communications GmbH - * Copyright (C) 2018 Intel Corporation + * Copyright (C) 2018, 2020 Intel Corporation * * Transmit and frame generation functions. */ @@ -5149,6 +5149,7 @@ int ieee80211_tx_control_port(struct wiphy *wiphy, struct net_device *dev, struct ieee80211_local *local = sdata->local; struct sk_buff *skb; struct ethhdr *ehdr; + u32 ctrl_flags = 0; u32 flags; /* Only accept CONTROL_PORT_PROTOCOL configured in CONNECT/ASSOCIATE @@ -5158,6 +5159,9 @@ int ieee80211_tx_control_port(struct wiphy *wiphy, struct net_device *dev, proto != cpu_to_be16(ETH_P_PREAUTH)) return -EINVAL; + if (proto == sdata->control_port_protocol) + ctrl_flags |= IEEE80211_TX_CTRL_PORT_CTRL_PROTO; + if (unencrypted) flags = IEEE80211_TX_INTFL_DONT_ENCRYPT; else @@ -5183,7 +5187,7 @@ int ieee80211_tx_control_port(struct wiphy *wiphy, struct net_device *dev, skb_reset_mac_header(skb); local_bh_disable(); - __ieee80211_subif_start_xmit(skb, skb->dev, flags, 0); + __ieee80211_subif_start_xmit(skb, skb->dev, flags, ctrl_flags); local_bh_enable(); return 0; -- cgit From bcaeb886ade124331a6f3a5cef34a3f1484c0a03 Mon Sep 17 00:00:00 2001 From: Xu Wang Date: Thu, 26 Mar 2020 18:14:29 +0800 Subject: qlcnic: Fix bad kzalloc null test In qlcnic_83xx_get_reset_instruction_template, the variable of null test is bad, so correct it. Signed-off-by: Xu Wang Signed-off-by: David S. Miller --- drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_init.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_init.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_init.c index 07f9067affc6..cda5b0a9e948 100644 --- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_init.c +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_init.c @@ -1720,7 +1720,7 @@ static int qlcnic_83xx_get_reset_instruction_template(struct qlcnic_adapter *p_d ahw->reset.seq_error = 0; ahw->reset.buff = kzalloc(QLC_83XX_RESTART_TEMPLATE_SIZE, GFP_KERNEL); - if (p_dev->ahw->reset.buff == NULL) + if (ahw->reset.buff == NULL) return -ENOMEM; p_buff = p_dev->ahw->reset.buff; -- cgit From f6bf1bafdc2152bb22aff3a4e947f2441a1d49e2 Mon Sep 17 00:00:00 2001 From: Ido Schimmel Date: Thu, 26 Mar 2020 16:17:33 +0200 Subject: mlxsw: spectrum_mr: Fix list iteration in error path list_for_each_entry_from_reverse() iterates backwards over the list from the current position, but in the error path we should start from the previous position. Fix this by using list_for_each_entry_continue_reverse() instead. This suppresses the following error from coccinelle: drivers/net/ethernet/mellanox/mlxsw//spectrum_mr.c:655:34-38: ERROR: invalid reference to the index variable of the iterator on line 636 Fixes: c011ec1bbfd6 ("mlxsw: spectrum: Add the multicast routing offloading logic") Signed-off-by: Ido Schimmel Reviewed-by: Jiri Pirko Signed-off-by: David S. Miller --- drivers/net/ethernet/mellanox/mlxsw/spectrum_mr.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr.c index 54275624718b..336e5ecc68f8 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr.c +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr.c @@ -637,12 +637,12 @@ static int mlxsw_sp_mr_vif_resolve(struct mlxsw_sp_mr_table *mr_table, return 0; err_erif_unresolve: - list_for_each_entry_from_reverse(erve, &mr_vif->route_evif_list, - vif_node) + list_for_each_entry_continue_reverse(erve, &mr_vif->route_evif_list, + vif_node) mlxsw_sp_mr_route_evif_unresolve(mr_table, erve); err_irif_unresolve: - list_for_each_entry_from_reverse(irve, &mr_vif->route_ivif_list, - vif_node) + list_for_each_entry_continue_reverse(irve, &mr_vif->route_ivif_list, + vif_node) mlxsw_sp_mr_route_ivif_unresolve(mr_table, irve); mr_vif->rif = NULL; return err; -- cgit From 8262e6f9b1034ede34548a04dec4c302d92c9497 Mon Sep 17 00:00:00 2001 From: Marek Vasut Date: Wed, 25 Mar 2020 15:25:47 +0100 Subject: net: ks8851-ml: Fix IO operations, again This patch reverts 58292104832f ("net: ks8851-ml: Fix 16-bit IO operation") and edacb098ea9c ("net: ks8851-ml: Fix 16-bit data access"), because it turns out these were only necessary due to buggy hardware. This patch adds a check for such a buggy hardware to prevent any such mistakes again. While working further on the KS8851 driver, it came to light that the KS8851-16MLL is capable of switching bus endianness by a hardware strap, EESK pin. If this strap is incorrect, the IO accesses require such endian swapping as is being reverted by this patch. Such swapping also impacts the performance significantly. Hence, in addition to removing it, detect that the hardware is broken, report to user, and fail to bind with such hardware. Fixes: 58292104832f ("net: ks8851-ml: Fix 16-bit IO operation") Fixes: edacb098ea9c ("net: ks8851-ml: Fix 16-bit data access") Signed-off-by: Marek Vasut Cc: David S. Miller Cc: Lukas Wunner Cc: Petr Stetiar Cc: YueHaibing Signed-off-by: David S. Miller --- drivers/net/ethernet/micrel/ks8851_mll.c | 56 +++++++++++++++++++++++++++++--- 1 file changed, 52 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/micrel/ks8851_mll.c b/drivers/net/ethernet/micrel/ks8851_mll.c index 58579baf3f7a..45cc840d8e2e 100644 --- a/drivers/net/ethernet/micrel/ks8851_mll.c +++ b/drivers/net/ethernet/micrel/ks8851_mll.c @@ -156,6 +156,50 @@ static int msg_enable; * chip is busy transferring packet data (RX/TX FIFO accesses). */ +/** + * ks_check_endian - Check whether endianness of the bus is correct + * @ks : The chip information + * + * The KS8851-16MLL EESK pin allows selecting the endianness of the 16bit + * bus. To maintain optimum performance, the bus endianness should be set + * such that it matches the endianness of the CPU. + */ + +static int ks_check_endian(struct ks_net *ks) +{ + u16 cider; + + /* + * Read CIDER register first, however read it the "wrong" way around. + * If the endian strap on the KS8851-16MLL in incorrect and the chip + * is operating in different endianness than the CPU, then the meaning + * of BE[3:0] byte-enable bits is also swapped such that: + * BE[3,2,1,0] becomes BE[1,0,3,2] + * + * Luckily for us, the byte-enable bits are the top four MSbits of + * the address register and the CIDER register is at offset 0xc0. + * Hence, by reading address 0xc0c0, which is not impacted by endian + * swapping, we assert either BE[3:2] or BE[1:0] while reading the + * CIDER register. + * + * If the bus configuration is correct, reading 0xc0c0 asserts + * BE[3:2] and this read returns 0x0000, because to read register + * with bottom two LSbits of address set to 0, BE[1:0] must be + * asserted. + * + * If the bus configuration is NOT correct, reading 0xc0c0 asserts + * BE[1:0] and this read returns non-zero 0x8872 value. + */ + iowrite16(BE3 | BE2 | KS_CIDER, ks->hw_addr_cmd); + cider = ioread16(ks->hw_addr); + if (!cider) + return 0; + + netdev_err(ks->netdev, "incorrect EESK endian strap setting\n"); + + return -EINVAL; +} + /** * ks_rdreg16 - read 16 bit register from device * @ks : The chip information @@ -166,7 +210,7 @@ static int msg_enable; static u16 ks_rdreg16(struct ks_net *ks, int offset) { - ks->cmd_reg_cache = (u16)offset | ((BE3 | BE2) >> (offset & 0x02)); + ks->cmd_reg_cache = (u16)offset | ((BE1 | BE0) << (offset & 0x02)); iowrite16(ks->cmd_reg_cache, ks->hw_addr_cmd); return ioread16(ks->hw_addr); } @@ -181,7 +225,7 @@ static u16 ks_rdreg16(struct ks_net *ks, int offset) static void ks_wrreg16(struct ks_net *ks, int offset, u16 value) { - ks->cmd_reg_cache = (u16)offset | ((BE3 | BE2) >> (offset & 0x02)); + ks->cmd_reg_cache = (u16)offset | ((BE1 | BE0) << (offset & 0x02)); iowrite16(ks->cmd_reg_cache, ks->hw_addr_cmd); iowrite16(value, ks->hw_addr); } @@ -197,7 +241,7 @@ static inline void ks_inblk(struct ks_net *ks, u16 *wptr, u32 len) { len >>= 1; while (len--) - *wptr++ = be16_to_cpu(ioread16(ks->hw_addr)); + *wptr++ = (u16)ioread16(ks->hw_addr); } /** @@ -211,7 +255,7 @@ static inline void ks_outblk(struct ks_net *ks, u16 *wptr, u32 len) { len >>= 1; while (len--) - iowrite16(cpu_to_be16(*wptr++), ks->hw_addr); + iowrite16(*wptr++, ks->hw_addr); } static void ks_disable_int(struct ks_net *ks) @@ -1218,6 +1262,10 @@ static int ks8851_probe(struct platform_device *pdev) goto err_free; } + err = ks_check_endian(ks); + if (err) + goto err_free; + netdev->irq = platform_get_irq(pdev, 0); if ((int)netdev->irq < 0) { -- cgit From 2e8c339b4946490a922a21aa8cd869c6cfad2023 Mon Sep 17 00:00:00 2001 From: Heiner Kallweit Date: Fri, 27 Mar 2020 17:33:32 +0100 Subject: r8169: fix PHY driver check on platforms w/o module softdeps On Android/x86 the module loading infrastructure can't deal with softdeps. Therefore the check for presence of the Realtek PHY driver module fails. mdiobus_register() will try to load the PHY driver module, therefore move the check to after this call and explicitly check that a dedicated PHY driver is bound to the PHY device. Fixes: f32593773549 ("r8169: check that Realtek PHY driver module is loaded") Reported-by: Chih-Wei Huang Signed-off-by: Heiner Kallweit Signed-off-by: David S. Miller --- drivers/net/ethernet/realtek/r8169_main.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c index a9bdafd15a35..791d99b9e1cf 100644 --- a/drivers/net/ethernet/realtek/r8169_main.c +++ b/drivers/net/ethernet/realtek/r8169_main.c @@ -5285,6 +5285,13 @@ static int r8169_mdio_register(struct rtl8169_private *tp) if (!tp->phydev) { mdiobus_unregister(new_bus); return -ENODEV; + } else if (!tp->phydev->drv) { + /* Most chip versions fail with the genphy driver. + * Therefore ensure that the dedicated PHY driver is loaded. + */ + dev_err(&pdev->dev, "realtek.ko not loaded, maybe it needs to be added to initramfs?\n"); + mdiobus_unregister(new_bus); + return -EUNATCH; } /* PHY will be woken up in rtl_open() */ @@ -5446,15 +5453,6 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) int chipset, region; int jumbo_max, rc; - /* Some tools for creating an initramfs don't consider softdeps, then - * r8169.ko may be in initramfs, but realtek.ko not. Then the generic - * PHY driver is used that doesn't work with most chip versions. - */ - if (!driver_find("RTL8201CP Ethernet", &mdio_bus_type)) { - dev_err(&pdev->dev, "realtek.ko not loaded, maybe it needs to be added to initramfs?\n"); - return -ENOENT; - } - dev = devm_alloc_etherdev(&pdev->dev, sizeof (*tp)); if (!dev) return -ENOMEM; -- cgit