Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit a518be57 authored by Duoming Zhou's avatar Duoming Zhou Committed by Greg Kroah-Hartman
Browse files

ax25: fix reference count leaks of ax25_dev 



commit 87563a043cef044fed5db7967a75741cc16ad2b1 upstream.

The previous commit d01ffb9eee4a ("ax25: add refcount in ax25_dev
to avoid UAF bugs") introduces refcount into ax25_dev, but there
are reference leak paths in ax25_ctl_ioctl(), ax25_fwd_ioctl(),
ax25_rt_add(), ax25_rt_del() and ax25_rt_opt().

This patch uses ax25_dev_put() and adjusts the position of
ax25_addr_ax25dev() to fix reference cout leaks of ax25_dev.

Fixes: d01ffb9eee4a ("ax25: add refcount in ax25_dev to avoid UAF bugs")
Signed-off-by: default avatarDuoming Zhou <duoming@zju.edu.cn>
Reviewed-by: default avatarDan Carpenter <dan.carpenter@oracle.com>
Link: https://lore.kernel.org/r/20220203150811.42256-1-duoming@zju.edu.cn


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
[OP: backport to 4.19: adjust context]
Signed-off-by: default avatarOvidiu Panait <ovidiu.panait@windriver.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent e2b558fe

include/net/ax25.h

+5 −3
Original line number Diff line number Diff line
@@ -291,10 +291,12 @@ static __inline__ void ax25_cb_put(ax25_cb *ax25) 
	}

}



#define ax25_dev_hold(__ax25_dev) \

	refcount_inc(&((__ax25_dev)->refcount))

static inline void ax25_dev_hold(ax25_dev *ax25_dev)

{

	refcount_inc(&ax25_dev->refcount);

}



static __inline__ void ax25_dev_put(ax25_dev *ax25_dev)

static inline void ax25_dev_put(ax25_dev *ax25_dev)

{

	if (refcount_dec_and_test(&ax25_dev->refcount)) {

		kfree(ax25_dev);

net/ax25/af_ax25.c

+8 −4
Original line number Diff line number Diff line
@@ -369,21 +369,25 @@ static int ax25_ctl_ioctl(const unsigned int cmd, void __user *arg) 
	if (copy_from_user(&ax25_ctl, arg, sizeof(ax25_ctl)))

		return -EFAULT;



	if ((ax25_dev = ax25_addr_ax25dev(&ax25_ctl.port_addr)) == NULL)

		return -ENODEV;



	if (ax25_ctl.digi_count > AX25_MAX_DIGIS)

		return -EINVAL;



	if (ax25_ctl.arg > ULONG_MAX / HZ && ax25_ctl.cmd != AX25_KILL)

		return -EINVAL;



	ax25_dev = ax25_addr_ax25dev(&ax25_ctl.port_addr);

	if (!ax25_dev)

		return -ENODEV;



	digi.ndigi = ax25_ctl.digi_count;

	for (k = 0; k < digi.ndigi; k++)

		digi.calls[k] = ax25_ctl.digi_addr[k];



	if ((ax25 = ax25_find_cb(&ax25_ctl.source_addr, &ax25_ctl.dest_addr, &digi, ax25_dev->dev)) == NULL)

	ax25 = ax25_find_cb(&ax25_ctl.source_addr, &ax25_ctl.dest_addr, &digi, ax25_dev->dev);

	if (!ax25) {

		ax25_dev_put(ax25_dev);

		return -ENOTCONN;

	}



	switch (ax25_ctl.cmd) {

	case AX25_KILL:

net/ax25/ax25_dev.c

+17 −7
Original line number Diff line number Diff line
@@ -88,8 +88,8 @@ void ax25_dev_device_up(struct net_device *dev) 
	spin_lock_bh(&ax25_dev_lock);

	ax25_dev->next = ax25_dev_list;

	ax25_dev_list  = ax25_dev;

	ax25_dev_hold(ax25_dev);

	spin_unlock_bh(&ax25_dev_lock);

	ax25_dev_hold(ax25_dev);



	ax25_register_dev_sysctl(ax25_dev);

}
@@ -118,8 +118,8 @@ void ax25_dev_device_down(struct net_device *dev) 


	if ((s = ax25_dev_list) == ax25_dev) {

		ax25_dev_list = s->next;

		ax25_dev_put(ax25_dev);

		spin_unlock_bh(&ax25_dev_lock);

		ax25_dev_put(ax25_dev);

		dev->ax25_ptr = NULL;

		dev_put(dev);

		ax25_dev_put(ax25_dev);
@@ -129,8 +129,8 @@ void ax25_dev_device_down(struct net_device *dev) 
	while (s != NULL && s->next != NULL) {

		if (s->next == ax25_dev) {

			s->next = ax25_dev->next;

			ax25_dev_put(ax25_dev);

			spin_unlock_bh(&ax25_dev_lock);

			ax25_dev_put(ax25_dev);

			dev->ax25_ptr = NULL;

			dev_put(dev);

			ax25_dev_put(ax25_dev);
@@ -153,25 +153,35 @@ int ax25_fwd_ioctl(unsigned int cmd, struct ax25_fwd_struct *fwd) 


	switch (cmd) {

	case SIOCAX25ADDFWD:

		if ((fwd_dev = ax25_addr_ax25dev(&fwd->port_to)) == NULL)

		fwd_dev = ax25_addr_ax25dev(&fwd->port_to);

		if (!fwd_dev) {

			ax25_dev_put(ax25_dev);

			return -EINVAL;

		if (ax25_dev->forward != NULL)

		}

		if (ax25_dev->forward) {

			ax25_dev_put(fwd_dev);

			ax25_dev_put(ax25_dev);

			return -EINVAL;

		}

		ax25_dev->forward = fwd_dev->dev;

		ax25_dev_put(fwd_dev);

		ax25_dev_put(ax25_dev);

		break;



	case SIOCAX25DELFWD:

		if (ax25_dev->forward == NULL)

		if (!ax25_dev->forward) {

			ax25_dev_put(ax25_dev);

			return -EINVAL;

		}

		ax25_dev->forward = NULL;

		ax25_dev_put(ax25_dev);

		break;



	default:

		ax25_dev_put(ax25_dev);

		return -EINVAL;

	}



	ax25_dev_put(ax25_dev);

	return 0;

}

net/ax25/ax25_route.c

+11 −5
Original line number Diff line number Diff line
@@ -78,11 +78,13 @@ static int __must_check ax25_rt_add(struct ax25_routes_struct *route) 
	ax25_dev *ax25_dev;

	int i;



	if ((ax25_dev = ax25_addr_ax25dev(&route->port_addr)) == NULL)

		return -EINVAL;

	if (route->digi_count > AX25_MAX_DIGIS)

		return -EINVAL;



	ax25_dev = ax25_addr_ax25dev(&route->port_addr);

	if (!ax25_dev)

		return -EINVAL;



	write_lock_bh(&ax25_route_lock);



	ax25_rt = ax25_route_list;
@@ -94,6 +96,7 @@ static int __must_check ax25_rt_add(struct ax25_routes_struct *route) 
			if (route->digi_count != 0) {

				if ((ax25_rt->digipeat = kmalloc(sizeof(ax25_digi), GFP_ATOMIC)) == NULL) {

					write_unlock_bh(&ax25_route_lock);

					ax25_dev_put(ax25_dev);

					return -ENOMEM;

				}

				ax25_rt->digipeat->lastrepeat = -1;
@@ -104,6 +107,7 @@ static int __must_check ax25_rt_add(struct ax25_routes_struct *route) 
				}

			}

			write_unlock_bh(&ax25_route_lock);

			ax25_dev_put(ax25_dev);

			return 0;

		}

		ax25_rt = ax25_rt->next;
@@ -111,6 +115,7 @@ static int __must_check ax25_rt_add(struct ax25_routes_struct *route) 


	if ((ax25_rt = kmalloc(sizeof(ax25_route), GFP_ATOMIC)) == NULL) {

		write_unlock_bh(&ax25_route_lock);

		ax25_dev_put(ax25_dev);

		return -ENOMEM;

	}
@@ -119,11 +124,11 @@ static int __must_check ax25_rt_add(struct ax25_routes_struct *route) 
	ax25_rt->dev          = ax25_dev->dev;

	ax25_rt->digipeat     = NULL;

	ax25_rt->ip_mode      = ' ';

	ax25_dev_put(ax25_dev);

	if (route->digi_count != 0) {

		if ((ax25_rt->digipeat = kmalloc(sizeof(ax25_digi), GFP_ATOMIC)) == NULL) {

			write_unlock_bh(&ax25_route_lock);

			kfree(ax25_rt);

			ax25_dev_put(ax25_dev);

			return -ENOMEM;

		}

		ax25_rt->digipeat->lastrepeat = -1;
@@ -136,6 +141,7 @@ static int __must_check ax25_rt_add(struct ax25_routes_struct *route) 
	ax25_rt->next   = ax25_route_list;

	ax25_route_list = ax25_rt;

	write_unlock_bh(&ax25_route_lock);

	ax25_dev_put(ax25_dev);



	return 0;

}
@@ -176,8 +182,8 @@ static int ax25_rt_del(struct ax25_routes_struct *route) 
			}

		}

	}

	ax25_dev_put(ax25_dev);

	write_unlock_bh(&ax25_route_lock);

	ax25_dev_put(ax25_dev);



	return 0;

}
@@ -219,8 +225,8 @@ static int ax25_rt_opt(struct ax25_route_opt_struct *rt_option) 
	}



out:

	ax25_dev_put(ax25_dev);

	write_unlock_bh(&ax25_route_lock);

	ax25_dev_put(ax25_dev);

	return err;

}