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: Duoming Zhou <duoming@zju.edu.cn>
Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
Link: https://lore.kernel.org/r/20220203150811.42256-1-duoming@zju.edu.cn
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
[OP: backport to 4.14: adjust context]
Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/include/net/ax25.h b/include/net/ax25.h
index 390e32103a6e4cbf771e0d6af239fc93b89f6e50..5db7b4c9256d7649269d36a224fa5bdbdb708df3 100644 (file)
--- a/include/net/ax25.h
+++ b/include/net/ax25.h
@@ -290,10 +290,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);

diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
index 3f13c619824b8764aebb7458ecfd88d5136c783a..e699bd80a86137997b2a90155de13ea24bbd0c68 100644 (file)
--- a/net/ax25/af_ax25.c
+++ b/net/ax25/af_ax25.c
@@ -370,21 +370,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:

diff --git a/net/ax25/ax25_dev.c b/net/ax25/ax25_dev.c
index 76d105390706396adaf4bef82ae0bfd430297015..55a611f7239bc3ec23319522e4831efa3f2d183e 100644 (file)
--- a/net/ax25/ax25_dev.c
+++ b/net/ax25/ax25_dev.c
@@ -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;
 }
 

diff --git a/net/ax25/ax25_route.c b/net/ax25/ax25_route.c
index c13f1e897b391c72b6c0e913abeeae1955d94b32..7d4c86f11b0f68471dfbfd5f27b9ba32f288a754 100644 (file)
--- a/net/ax25/ax25_route.c
+++ b/net/ax25/ax25_route.c
@@ -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;
 }