From a9d48205d0aedda021fc3728972a9e9934c2b9de Mon Sep 17 00:00:00 2001 From: Eric Dumazet Date: Thu, 5 Apr 2018 06:39:26 -0700 Subject: net: fool proof dev_valid_name() We want to use dev_valid_name() to validate tunnel names, so better use strnlen(name, IFNAMSIZ) than strlen(name) to make sure to not upset KASAN. Signed-off-by: Eric Dumazet Signed-off-by: David S. Miller --- net/core/dev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/core') diff --git a/net/core/dev.c b/net/core/dev.c index 9b04a9fd1dfd..969462ebb296 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -1027,7 +1027,7 @@ bool dev_valid_name(const char *name) { if (*name == '\0') return false; - if (strlen(name) >= IFNAMSIZ) + if (strnlen(name, IFNAMSIZ) == IFNAMSIZ) return false; if (!strcmp(name, ".") || !strcmp(name, "..")) return false; -- cgit v1.2.3 From b13dda9f9aa7caceeee61c080c2e544d5f5d85e5 Mon Sep 17 00:00:00 2001 From: Eric Dumazet Date: Sat, 7 Apr 2018 13:42:39 -0700 Subject: net: initialize skb->peeked when cloning syzbot reported __skb_try_recv_from_queue() was using skb->peeked while it was potentially unitialized. We need to clear it in __skb_clone() Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Signed-off-by: Eric Dumazet Reported-by: syzbot Signed-off-by: David S. Miller --- net/core/skbuff.c | 1 + 1 file changed, 1 insertion(+) (limited to 'net/core') diff --git a/net/core/skbuff.c b/net/core/skbuff.c index 1bca1e0fc8f7..345b51837ca8 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -857,6 +857,7 @@ static struct sk_buff *__skb_clone(struct sk_buff *n, struct sk_buff *skb) n->hdr_len = skb->nohdr ? skb_headroom(skb) : skb->hdr_len; n->cloned = 1; n->nohdr = 0; + n->peeked = 0; n->destructor = NULL; C(tail); C(end); -- cgit v1.2.3 From 77d36398d99f2565c0a8d43a86fd520a82e64bb8 Mon Sep 17 00:00:00 2001 From: Eric Dumazet Date: Sat, 7 Apr 2018 13:42:40 -0700 Subject: net: fix uninit-value in __hw_addr_add_ex() syzbot complained : BUG: KMSAN: uninit-value in memcmp+0x119/0x180 lib/string.c:861 CPU: 0 PID: 3 Comm: kworker/0:0 Not tainted 4.16.0+ #82 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Workqueue: ipv6_addrconf addrconf_dad_work Call Trace: __dump_stack lib/dump_stack.c:17 [inline] dump_stack+0x185/0x1d0 lib/dump_stack.c:53 kmsan_report+0x142/0x240 mm/kmsan/kmsan.c:1067 __msan_warning_32+0x6c/0xb0 mm/kmsan/kmsan_instr.c:676 memcmp+0x119/0x180 lib/string.c:861 __hw_addr_add_ex net/core/dev_addr_lists.c:60 [inline] __dev_mc_add+0x1c2/0x8e0 net/core/dev_addr_lists.c:670 dev_mc_add+0x6d/0x80 net/core/dev_addr_lists.c:687 igmp6_group_added+0x2db/0xa00 net/ipv6/mcast.c:662 ipv6_dev_mc_inc+0xe9e/0x1130 net/ipv6/mcast.c:914 addrconf_join_solict net/ipv6/addrconf.c:2078 [inline] addrconf_dad_begin net/ipv6/addrconf.c:3828 [inline] addrconf_dad_work+0x427/0x2150 net/ipv6/addrconf.c:3954 process_one_work+0x12c6/0x1f60 kernel/workqueue.c:2113 worker_thread+0x113c/0x24f0 kernel/workqueue.c:2247 kthread+0x539/0x720 kernel/kthread.c:239 Fixes: f001fde5eadd ("net: introduce a list of device addresses dev_addr_list (v6)") Signed-off-by: Eric Dumazet Reported-by: syzbot Signed-off-by: David S. Miller --- net/core/dev_addr_lists.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'net/core') diff --git a/net/core/dev_addr_lists.c b/net/core/dev_addr_lists.c index c0548d268e1a..e3e6a3e2ca22 100644 --- a/net/core/dev_addr_lists.c +++ b/net/core/dev_addr_lists.c @@ -57,8 +57,8 @@ static int __hw_addr_add_ex(struct netdev_hw_addr_list *list, return -EINVAL; list_for_each_entry(ha, &list->list, list) { - if (!memcmp(ha->addr, addr, addr_len) && - ha->type == addr_type) { + if (ha->type == addr_type && + !memcmp(ha->addr, addr, addr_len)) { if (global) { /* check if addr is already used as global */ if (ha->global_use) -- cgit v1.2.3 From fc56be47da8cb111add373c36230b0139139898f Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Thu, 5 Apr 2018 22:13:21 +0200 Subject: devlink: convert occ_get op to separate registration This resolves race during initialization where the resources with ops are registered before driver and the structures used by occ_get op is initialized. So keep occ_get callbacks registered only when all structs are initialized. The example flows, as it is in mlxsw: 1) driver load/asic probe: mlxsw_core -> mlxsw_sp_resources_register -> mlxsw_sp_kvdl_resources_register -> devlink_resource_register IDX mlxsw_spectrum -> mlxsw_sp_kvdl_init -> mlxsw_sp_kvdl_parts_init -> mlxsw_sp_kvdl_part_init -> devlink_resource_size_get IDX (to get the current setup size from devlink) -> devlink_resource_occ_get_register IDX (register current occupancy getter) 2) reload triggered by devlink command: -> mlxsw_devlink_core_bus_device_reload -> mlxsw_sp_fini -> mlxsw_sp_kvdl_fini -> devlink_resource_occ_get_unregister IDX (struct mlxsw_sp *mlxsw_sp is freed at this point, call to occ get which is using mlxsw_sp would cause use-after free) -> mlxsw_sp_init -> mlxsw_sp_kvdl_init -> mlxsw_sp_kvdl_parts_init -> mlxsw_sp_kvdl_part_init -> devlink_resource_size_get IDX (to get the current setup size from devlink) -> devlink_resource_occ_get_register IDX (register current occupancy getter) Fixes: d9f9b9a4d05f ("devlink: Add support for resource abstraction") Signed-off-by: Jiri Pirko Signed-off-by: David S. Miller --- net/core/devlink.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 65 insertions(+), 9 deletions(-) (limited to 'net/core') diff --git a/net/core/devlink.c b/net/core/devlink.c index 9236e421bd62..ad1317376798 100644 --- a/net/core/devlink.c +++ b/net/core/devlink.c @@ -2405,6 +2405,16 @@ devlink_resource_size_params_put(struct devlink_resource *resource, return 0; } +static int devlink_resource_occ_put(struct devlink_resource *resource, + struct sk_buff *skb) +{ + if (!resource->occ_get) + return 0; + return nla_put_u64_64bit(skb, DEVLINK_ATTR_RESOURCE_OCC, + resource->occ_get(resource->occ_get_priv), + DEVLINK_ATTR_PAD); +} + static int devlink_resource_put(struct devlink *devlink, struct sk_buff *skb, struct devlink_resource *resource) { @@ -2425,11 +2435,8 @@ static int devlink_resource_put(struct devlink *devlink, struct sk_buff *skb, if (resource->size != resource->size_new) nla_put_u64_64bit(skb, DEVLINK_ATTR_RESOURCE_SIZE_NEW, resource->size_new, DEVLINK_ATTR_PAD); - if (resource->resource_ops && resource->resource_ops->occ_get) - if (nla_put_u64_64bit(skb, DEVLINK_ATTR_RESOURCE_OCC, - resource->resource_ops->occ_get(devlink), - DEVLINK_ATTR_PAD)) - goto nla_put_failure; + if (devlink_resource_occ_put(resource, skb)) + goto nla_put_failure; if (devlink_resource_size_params_put(resource, skb)) goto nla_put_failure; if (list_empty(&resource->resource_list)) @@ -3162,15 +3169,13 @@ EXPORT_SYMBOL_GPL(devlink_dpipe_table_unregister); * @resource_id: resource's id * @parent_reosurce_id: resource's parent id * @size params: size parameters - * @resource_ops: resource ops */ int devlink_resource_register(struct devlink *devlink, const char *resource_name, u64 resource_size, u64 resource_id, u64 parent_resource_id, - const struct devlink_resource_size_params *size_params, - const struct devlink_resource_ops *resource_ops) + const struct devlink_resource_size_params *size_params) { struct devlink_resource *resource; struct list_head *resource_list; @@ -3213,7 +3218,6 @@ int devlink_resource_register(struct devlink *devlink, resource->size = resource_size; resource->size_new = resource_size; resource->id = resource_id; - resource->resource_ops = resource_ops; resource->size_valid = true; memcpy(&resource->size_params, size_params, sizeof(resource->size_params)); @@ -3315,6 +3319,58 @@ out: } EXPORT_SYMBOL_GPL(devlink_dpipe_table_resource_set); +/** + * devlink_resource_occ_get_register - register occupancy getter + * + * @devlink: devlink + * @resource_id: resource id + * @occ_get: occupancy getter callback + * @occ_get_priv: occupancy getter callback priv + */ +void devlink_resource_occ_get_register(struct devlink *devlink, + u64 resource_id, + devlink_resource_occ_get_t *occ_get, + void *occ_get_priv) +{ + struct devlink_resource *resource; + + mutex_lock(&devlink->lock); + resource = devlink_resource_find(devlink, NULL, resource_id); + if (WARN_ON(!resource)) + goto out; + WARN_ON(resource->occ_get); + + resource->occ_get = occ_get; + resource->occ_get_priv = occ_get_priv; +out: + mutex_unlock(&devlink->lock); +} +EXPORT_SYMBOL_GPL(devlink_resource_occ_get_register); + +/** + * devlink_resource_occ_get_unregister - unregister occupancy getter + * + * @devlink: devlink + * @resource_id: resource id + */ +void devlink_resource_occ_get_unregister(struct devlink *devlink, + u64 resource_id) +{ + struct devlink_resource *resource; + + mutex_lock(&devlink->lock); + resource = devlink_resource_find(devlink, NULL, resource_id); + if (WARN_ON(!resource)) + goto out; + WARN_ON(!resource->occ_get); + + resource->occ_get = NULL; + resource->occ_get_priv = NULL; +out: + mutex_unlock(&devlink->lock); +} +EXPORT_SYMBOL_GPL(devlink_resource_occ_get_unregister); + static int __init devlink_module_init(void) { return genl_register_family(&devlink_nl_family); -- cgit v1.2.3