Commit b65a1534 authored by David S. Miller's avatar David S. Miller
Browse files

Merge branch 'netdevsim-fib-route-delete-leak' 



Ido Schimmel says:

====================
netdevsim: fib: Fix reference count leak on route deletion failure

Fix a recently reported netdevsim bug found using syzkaller.

Patch #1 fixes the bug.

Patch #2 adds a debugfs knob to allow us to test the fix.

Patch #3 adds test cases.
====================

Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents 944fd1ae 40823f3e

drivers/net/netdevsim/fib.c

+40 −1
Original line number Diff line number Diff line
@@ -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 */
@@ -61,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 {
@@ -914,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;

	}
@@ -952,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;

	}
@@ -978,7 +989,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 +1017,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 +1497,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)

{
@@ -1504,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;

}
@@ -1541,6 +1577,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 +1624,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 +1654,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);

tools/testing/selftests/drivers/net/netdevsim/fib.sh

+45 −0
Original line number Diff line number Diff line
@@ -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