From 180a6a3ee60a7cb69ed1232388460644f6a21f00 Mon Sep 17 00:00:00 2001 From: Ido Schimmel Date: Thu, 28 Jul 2022 14:45:33 +0300 Subject: netdevsim: fib: Fix reference count leak on route deletion failure As part of FIB offload simulation, netdevsim stores IPv4 and IPv6 routes and holds a reference on FIB info structures that in turn hold a reference on the associated nexthop device(s). In the unlikely case where we are unable to allocate memory to process a route deletion request, netdevsim will not release the reference from the associated FIB info structure, thereby preventing the associated nexthop device(s) from ever being removed [1]. Fix this by scheduling a work item that will flush netdevsim's FIB table upon route deletion failure. This will cause netdevsim to release its reference from all the FIB info structures in its table. Reported by Lucas Leong of Trend Micro Zero Day Initiative. Fixes: 0ae3eb7b4611 ("netdevsim: fib: Perform the route programming in a non-atomic context") Signed-off-by: Ido Schimmel Reviewed-by: Amit Cohen Reviewed-by: David Ahern Signed-off-by: David S. Miller --- drivers/net/netdevsim/fib.c | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/drivers/net/netdevsim/fib.c b/drivers/net/netdevsim/fib.c index c8f398f5bc5b8..57371c697d5cf 100644 --- a/drivers/net/netdevsim/fib.c +++ b/drivers/net/netdevsim/fib.c @@ -54,6 +54,7 @@ struct nsim_fib_data { struct rhashtable nexthop_ht; struct devlink *devlink; struct work_struct fib_event_work; + struct work_struct fib_flush_work; struct list_head fib_event_queue; spinlock_t fib_event_queue_lock; /* Protects fib event queue list */ struct mutex nh_lock; /* Protects NH HT */ @@ -978,7 +979,7 @@ static int nsim_fib_event_schedule_work(struct nsim_fib_data *data, fib_event = kzalloc(sizeof(*fib_event), GFP_ATOMIC); if (!fib_event) - return NOTIFY_BAD; + goto err_fib_event_alloc; fib_event->data = data; fib_event->event = event; @@ -1006,6 +1007,9 @@ static int nsim_fib_event_schedule_work(struct nsim_fib_data *data, err_fib_prepare_event: kfree(fib_event); +err_fib_event_alloc: + if (event == FIB_EVENT_ENTRY_DEL) + schedule_work(&data->fib_flush_work); return NOTIFY_BAD; } @@ -1483,6 +1487,24 @@ static void nsim_fib_event_work(struct work_struct *work) mutex_unlock(&data->fib_lock); } +static void nsim_fib_flush_work(struct work_struct *work) +{ + struct nsim_fib_data *data = container_of(work, struct nsim_fib_data, + fib_flush_work); + struct nsim_fib_rt *fib_rt, *fib_rt_tmp; + + /* Process pending work. */ + flush_work(&data->fib_event_work); + + mutex_lock(&data->fib_lock); + list_for_each_entry_safe(fib_rt, fib_rt_tmp, &data->fib_rt_list, list) { + rhashtable_remove_fast(&data->fib_rt_ht, &fib_rt->ht_node, + nsim_fib_rt_ht_params); + nsim_fib_rt_free(fib_rt, data); + } + mutex_unlock(&data->fib_lock); +} + static int nsim_fib_debugfs_init(struct nsim_fib_data *data, struct nsim_dev *nsim_dev) { @@ -1541,6 +1563,7 @@ struct nsim_fib_data *nsim_fib_create(struct devlink *devlink, goto err_rhashtable_nexthop_destroy; INIT_WORK(&data->fib_event_work, nsim_fib_event_work); + INIT_WORK(&data->fib_flush_work, nsim_fib_flush_work); INIT_LIST_HEAD(&data->fib_event_queue); spin_lock_init(&data->fib_event_queue_lock); @@ -1587,6 +1610,7 @@ struct nsim_fib_data *nsim_fib_create(struct devlink *devlink, err_nexthop_nb_unregister: unregister_nexthop_notifier(devlink_net(devlink), &data->nexthop_nb); err_rhashtable_fib_destroy: + cancel_work_sync(&data->fib_flush_work); flush_work(&data->fib_event_work); rhashtable_free_and_destroy(&data->fib_rt_ht, nsim_fib_rt_free, data); @@ -1616,6 +1640,7 @@ void nsim_fib_destroy(struct devlink *devlink, struct nsim_fib_data *data) NSIM_RESOURCE_IPV4_FIB); unregister_fib_notifier(devlink_net(devlink), &data->fib_nb); unregister_nexthop_notifier(devlink_net(devlink), &data->nexthop_nb); + cancel_work_sync(&data->fib_flush_work); flush_work(&data->fib_event_work); rhashtable_free_and_destroy(&data->fib_rt_ht, nsim_fib_rt_free, data); -- cgit v1.2.3 From 974be75f250378a6913c7bb268adbfd9a76a6df9 Mon Sep 17 00:00:00 2001 From: Ido Schimmel Date: Thu, 28 Jul 2022 14:45:34 +0300 Subject: netdevsim: fib: Add debugfs knob to simulate route deletion failure The previous patch ("netdevsim: fib: Fix reference count leak on route deletion failure") fixed a reference count leak that happens on route deletion failure. Such failures can only be simulated by injecting slab allocation failures, which cannot be surgically injected. In order to be able to specifically test this scenario, add a debugfs knob that allows user space to fail route deletion requests when enabled. Signed-off-by: Ido Schimmel Reviewed-by: Amit Cohen Reviewed-by: David Ahern Signed-off-by: David S. Miller --- drivers/net/netdevsim/fib.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/drivers/net/netdevsim/fib.c b/drivers/net/netdevsim/fib.c index 57371c697d5cf..38a1fde8d8862 100644 --- a/drivers/net/netdevsim/fib.c +++ b/drivers/net/netdevsim/fib.c @@ -62,6 +62,7 @@ struct nsim_fib_data { bool fail_route_offload; bool fail_res_nexthop_group_replace; bool fail_nexthop_bucket_replace; + bool fail_route_delete; }; struct nsim_fib_rt_key { @@ -915,6 +916,10 @@ static int nsim_fib4_prepare_event(struct fib_notifier_info *info, } break; case FIB_EVENT_ENTRY_DEL: + if (data->fail_route_delete) { + NL_SET_ERR_MSG_MOD(extack, "Failed to process route deletion"); + return -EINVAL; + } nsim_fib_account(&data->ipv4.fib, false); break; } @@ -953,6 +958,11 @@ static int nsim_fib6_prepare_event(struct fib_notifier_info *info, } break; case FIB_EVENT_ENTRY_DEL: + if (data->fail_route_delete) { + err = -EINVAL; + NL_SET_ERR_MSG_MOD(extack, "Failed to process route deletion"); + goto err_fib6_event_fini; + } nsim_fib_account(&data->ipv6.fib, false); break; } @@ -1526,6 +1536,10 @@ nsim_fib_debugfs_init(struct nsim_fib_data *data, struct nsim_dev *nsim_dev) debugfs_create_file("nexthop_bucket_activity", 0200, data->ddir, data, &nsim_nexthop_bucket_activity_fops); + + data->fail_route_delete = false; + debugfs_create_bool("fail_route_delete", 0600, data->ddir, + &data->fail_route_delete); return 0; } -- cgit v1.2.3 From 40823f3ee05f7f55cbd4419062a1a388249e88da Mon Sep 17 00:00:00 2001 From: Ido Schimmel Date: Thu, 28 Jul 2022 14:45:35 +0300 Subject: selftests: netdevsim: Add test cases for route deletion failure Add IPv4 and IPv6 test cases that ensure that we are not leaking a reference on the nexthop device when we are unable to delete its associated route. Without the fix in a previous patch ("netdevsim: fib: Fix reference count leak on route deletion failure") both test cases get stuck, waiting for the reference to be released from the dummy device [1][2]. [1] unregister_netdevice: waiting for dummy1 to become free. Usage count = 5 leaked reference. fib_check_nh+0x275/0x620 fib_create_info+0x237c/0x4d30 fib_table_insert+0x1dd/0x1d20 inet_rtm_newroute+0x11b/0x200 rtnetlink_rcv_msg+0x43b/0xd20 netlink_rcv_skb+0x15e/0x430 netlink_unicast+0x53b/0x800 netlink_sendmsg+0x945/0xe40 ____sys_sendmsg+0x747/0x960 ___sys_sendmsg+0x11d/0x190 __sys_sendmsg+0x118/0x1e0 do_syscall_64+0x34/0x80 entry_SYSCALL_64_after_hwframe+0x63/0xcd [2] unregister_netdevice: waiting for dummy1 to become free. Usage count = 5 leaked reference. fib6_nh_init+0xc46/0x1ca0 ip6_route_info_create+0x1167/0x19a0 ip6_route_add+0x27/0x150 inet6_rtm_newroute+0x161/0x170 rtnetlink_rcv_msg+0x43b/0xd20 netlink_rcv_skb+0x15e/0x430 netlink_unicast+0x53b/0x800 netlink_sendmsg+0x945/0xe40 ____sys_sendmsg+0x747/0x960 ___sys_sendmsg+0x11d/0x190 __sys_sendmsg+0x118/0x1e0 do_syscall_64+0x34/0x80 entry_SYSCALL_64_after_hwframe+0x63/0xcd Signed-off-by: Ido Schimmel Reviewed-by: Amit Cohen Reviewed-by: David Ahern Signed-off-by: David S. Miller --- .../testing/selftests/drivers/net/netdevsim/fib.sh | 45 ++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/tools/testing/selftests/drivers/net/netdevsim/fib.sh b/tools/testing/selftests/drivers/net/netdevsim/fib.sh index fc794cd303899..6800de816e8bd 100755 --- a/tools/testing/selftests/drivers/net/netdevsim/fib.sh +++ b/tools/testing/selftests/drivers/net/netdevsim/fib.sh @@ -16,6 +16,7 @@ ALL_TESTS=" ipv4_replay ipv4_flush ipv4_error_path + ipv4_delete_fail ipv6_add ipv6_metric ipv6_append_single @@ -29,11 +30,13 @@ ALL_TESTS=" ipv6_replay_single ipv6_replay_multipath ipv6_error_path + ipv6_delete_fail " NETDEVSIM_PATH=/sys/bus/netdevsim/ DEV_ADDR=1337 DEV=netdevsim${DEV_ADDR} SYSFS_NET_DIR=/sys/bus/netdevsim/devices/$DEV/net/ +DEBUGFS_DIR=/sys/kernel/debug/netdevsim/$DEV/ NUM_NETIFS=0 source $lib_dir/lib.sh source $lib_dir/fib_offload_lib.sh @@ -157,6 +160,27 @@ ipv4_error_path() ipv4_error_path_replay } +ipv4_delete_fail() +{ + RET=0 + + echo "y" > $DEBUGFS_DIR/fib/fail_route_delete + + ip -n testns1 link add name dummy1 type dummy + ip -n testns1 link set dev dummy1 up + + ip -n testns1 route add 192.0.2.0/24 dev dummy1 + ip -n testns1 route del 192.0.2.0/24 dev dummy1 &> /dev/null + + # We should not be able to delete the netdev if we are leaking a + # reference. + ip -n testns1 link del dev dummy1 + + log_test "IPv4 route delete failure" + + echo "n" > $DEBUGFS_DIR/fib/fail_route_delete +} + ipv6_add() { fib_ipv6_add_test "testns1" @@ -304,6 +328,27 @@ ipv6_error_path() ipv6_error_path_replay } +ipv6_delete_fail() +{ + RET=0 + + echo "y" > $DEBUGFS_DIR/fib/fail_route_delete + + ip -n testns1 link add name dummy1 type dummy + ip -n testns1 link set dev dummy1 up + + ip -n testns1 route add 2001:db8:1::/64 dev dummy1 + ip -n testns1 route del 2001:db8:1::/64 dev dummy1 &> /dev/null + + # We should not be able to delete the netdev if we are leaking a + # reference. + ip -n testns1 link del dev dummy1 + + log_test "IPv6 route delete failure" + + echo "n" > $DEBUGFS_DIR/fib/fail_route_delete +} + fib_notify_on_flag_change_set() { local notify=$1; shift -- cgit v1.2.3