* [Fixes] ax25: add refcount in ax25_dev to avoid UAF bugs
@ 2022-02-02 16:18 Duoming Zhou
2022-02-02 17:47 ` Dan Carpenter
0 siblings, 1 reply; 2+ messages in thread
From: Duoming Zhou @ 2022-02-02 16:18 UTC (permalink / raw)
To: dan.carpenter
Cc: linux-hams, jreuter, ralf, davem, kuba, netdev, linux-kernel,
Duoming Zhou
If we dereference ax25_dev after we call kfree(ax25_dev) in
ax25_dev_device_down(), it will lead to concurrency UAF bugs.
There are eight syscall functions suffer from UAF bugs, include
ax25_bind(), ax25_release(), ax25_connect(), ax25_ioctl(),
ax25_getname(), ax25_sendmsg(), ax25_getsockopt() and
ax25_info_show().
One of the concurrency UAF can be shown as below:
(USE) | (FREE)
| ax25_device_event
| ax25_dev_device_down
ax25_bind | ...
... | kfree(ax25_dev)
ax25_fillin_cb() | ...
ax25_fillin_cb_from_dev() |
... |
The root cause of UAF bugs is that kfree(ax25_dev) in
ax25_dev_device_down() is not protected by any locks.
When ax25_dev, which there are still pointers point to,
is released, the concurrency UAF bug will happen.
This patch introduces refcount into ax25_dev in order to
guarantee that there are no pointers point to it when ax25_dev
is released.
Signed-off-by: Duoming Zhou <duoming@zju.edu.cn>
---
include/net/ax25.h | 12 ++++++++++++
net/ax25/af_ax25.c | 20 ++++++++++++++++----
net/ax25/ax25_dev.c | 33 +++++++++++++++++++++++++++------
net/ax25/ax25_route.c | 27 ++++++++++++++++++++++-----
4 files changed, 77 insertions(+), 15 deletions(-)
diff --git a/include/net/ax25.h b/include/net/ax25.h
index 526e4958919..1a38b1ad529 100644
--- a/include/net/ax25.h
+++ b/include/net/ax25.h
@@ -239,6 +239,7 @@ typedef struct ax25_dev {
#if defined(CONFIG_AX25_DAMA_SLAVE) || defined(CONFIG_AX25_DAMA_MASTER)
ax25_dama_info dama;
#endif
+ refcount_t refcount;
} ax25_dev;
typedef struct ax25_cb {
@@ -293,6 +294,17 @@ static __inline__ void ax25_cb_put(ax25_cb *ax25)
}
}
+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)
+{
+ if (refcount_dec_and_test(&ax25_dev->refcount))
+ kfree(ax25_dev);
+}
+
static inline __be16 ax25_type_trans(struct sk_buff *skb, struct net_device *dev)
{
skb->dev = dev;
diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
index 44a8730c26a..7463bbd4e63 100644
--- a/net/ax25/af_ax25.c
+++ b/net/ax25/af_ax25.c
@@ -91,6 +91,7 @@ static void ax25_kill_by_device(struct net_device *dev)
spin_unlock_bh(&ax25_list_lock);
lock_sock(sk);
s->ax25_dev = NULL;
+ ax25_dev_put(ax25_dev);
release_sock(sk);
ax25_disconnect(s, ENETUNREACH);
spin_lock_bh(&ax25_list_lock);
@@ -358,21 +359,31 @@ 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)
+ ax25_dev = ax25_addr_ax25dev(&ax25_ctl.port_addr);
+ if (ax25_dev == NULL) {
+ ax25_dev_put(ax25_dev);
return -ENODEV;
+ }
- if (ax25_ctl.digi_count > AX25_MAX_DIGIS)
+ if (ax25_ctl.digi_count > AX25_MAX_DIGIS) {
+ ax25_dev_put(ax25_dev);
return -EINVAL;
+ }
- if (ax25_ctl.arg > ULONG_MAX / HZ && ax25_ctl.cmd != AX25_KILL)
+ if (ax25_ctl.arg > ULONG_MAX / HZ && ax25_ctl.cmd != AX25_KILL) {
+ ax25_dev_put(ax25_dev);
return -EINVAL;
+ }
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 == NULL) {
+ ax25_dev_put(ax25_dev);
return -ENOTCONN;
+ }
switch (ax25_ctl.cmd) {
case AX25_KILL:
@@ -439,6 +450,7 @@ static int ax25_ctl_ioctl(const unsigned int cmd, void __user *arg)
}
out_put:
+ ax25_dev_put(ax25_dev);
ax25_cb_put(ax25);
return ret;
diff --git a/net/ax25/ax25_dev.c b/net/ax25/ax25_dev.c
index 256fadb94df..77d9aa2ccab 100644
--- a/net/ax25/ax25_dev.c
+++ b/net/ax25/ax25_dev.c
@@ -37,6 +37,7 @@ ax25_dev *ax25_addr_ax25dev(ax25_address *addr)
for (ax25_dev = ax25_dev_list; ax25_dev != NULL; ax25_dev = ax25_dev->next)
if (ax25cmp(addr, (const ax25_address *)ax25_dev->dev->dev_addr) == 0) {
res = ax25_dev;
+ ax25_dev_hold(ax25_dev);
}
spin_unlock_bh(&ax25_dev_lock);
@@ -56,6 +57,7 @@ void ax25_dev_device_up(struct net_device *dev)
return;
}
+ refcount_set(&ax25_dev->refcount, 1);
dev->ax25_ptr = ax25_dev;
ax25_dev->dev = dev;
dev_hold_track(dev, &ax25_dev->dev_tracker, GFP_ATOMIC);
@@ -83,6 +85,7 @@ 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_register_dev_sysctl(ax25_dev);
@@ -111,21 +114,23 @@ void ax25_dev_device_down(struct net_device *dev)
s->forward = NULL;
if ((s = ax25_dev_list) == ax25_dev) {
+ ax25_dev_put(ax25_dev_list);
ax25_dev_list = s->next;
spin_unlock_bh(&ax25_dev_lock);
dev->ax25_ptr = NULL;
dev_put_track(dev, &ax25_dev->dev_tracker);
- kfree(ax25_dev);
+ ax25_dev_put(ax25_dev);
return;
}
while (s != NULL && s->next != NULL) {
if (s->next == ax25_dev) {
+ ax25_dev_put(s->next);
s->next = ax25_dev->next;
spin_unlock_bh(&ax25_dev_lock);
dev->ax25_ptr = NULL;
dev_put_track(dev, &ax25_dev->dev_tracker);
- kfree(ax25_dev);
+ ax25_dev_put(ax25_dev);
return;
}
@@ -133,31 +138,47 @@ void ax25_dev_device_down(struct net_device *dev)
}
spin_unlock_bh(&ax25_dev_lock);
dev->ax25_ptr = NULL;
+ ax25_dev_put(ax25_dev);
}
int ax25_fwd_ioctl(unsigned int cmd, struct ax25_fwd_struct *fwd)
{
ax25_dev *ax25_dev, *fwd_dev;
- if ((ax25_dev = ax25_addr_ax25dev(&fwd->port_from)) == NULL)
+ ax25_dev = ax25_addr_ax25dev(&fwd->port_from);
+ if (ax25_dev == NULL) {
+ ax25_dev_put(ax25_dev);
return -EINVAL;
+ }
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 == NULL) {
+ ax25_dev_put(fwd_dev);
+ ax25_dev_put(ax25_dev);
return -EINVAL;
- if (ax25_dev->forward != NULL)
+ }
+ if (ax25_dev->forward != NULL) {
+ 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 == NULL) {
+ 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;
}
diff --git a/net/ax25/ax25_route.c b/net/ax25/ax25_route.c
index d0b2e094bd5..7fe7a83b2a3 100644
--- a/net/ax25/ax25_route.c
+++ b/net/ax25/ax25_route.c
@@ -75,10 +75,15 @@ 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)
+ ax25_dev = ax25_addr_ax25dev(&route->port_addr);
+ if (ax25_dev == NULL) {
+ ax25_dev_put(ax25_dev);
return -EINVAL;
- if (route->digi_count > AX25_MAX_DIGIS)
+ }
+ if (route->digi_count > AX25_MAX_DIGIS) {
+ ax25_dev_put(ax25_dev);
return -EINVAL;
+ }
write_lock_bh(&ax25_route_lock);
@@ -91,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;
@@ -101,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;
@@ -108,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;
}
@@ -120,6 +128,7 @@ static int __must_check ax25_rt_add(struct ax25_routes_struct *route)
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;
@@ -132,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;
}
@@ -147,8 +157,11 @@ static int ax25_rt_del(struct ax25_routes_struct *route)
ax25_route *s, *t, *ax25_rt;
ax25_dev *ax25_dev;
- if ((ax25_dev = ax25_addr_ax25dev(&route->port_addr)) == NULL)
+ ax25_dev = ax25_addr_ax25dev(&route->port_addr);
+ if (ax25_dev == NULL) {
+ ax25_dev_put(ax25_dev);
return -EINVAL;
+ }
write_lock_bh(&ax25_route_lock);
@@ -173,7 +186,7 @@ static int ax25_rt_del(struct ax25_routes_struct *route)
}
}
write_unlock_bh(&ax25_route_lock);
-
+ ax25_dev_put(ax25_dev);
return 0;
}
@@ -183,8 +196,11 @@ static int ax25_rt_opt(struct ax25_route_opt_struct *rt_option)
ax25_dev *ax25_dev;
int err = 0;
- if ((ax25_dev = ax25_addr_ax25dev(&rt_option->port_addr)) == NULL)
+ ax25_dev = ax25_addr_ax25dev(&rt_option->port_addr);
+ if (ax25_dev == NULL) {
+ ax25_dev_put(ax25_dev);
return -EINVAL;
+ }
write_lock_bh(&ax25_route_lock);
@@ -215,6 +231,7 @@ static int ax25_rt_opt(struct ax25_route_opt_struct *rt_option)
out:
write_unlock_bh(&ax25_route_lock);
+ ax25_dev_put(ax25_dev);
return err;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [Fixes] ax25: add refcount in ax25_dev to avoid UAF bugs
2022-02-02 16:18 [Fixes] ax25: add refcount in ax25_dev to avoid UAF bugs Duoming Zhou
@ 2022-02-02 17:47 ` Dan Carpenter
0 siblings, 0 replies; 2+ messages in thread
From: Dan Carpenter @ 2022-02-02 17:47 UTC (permalink / raw)
To: Duoming Zhou; +Cc: linux-hams, jreuter, ralf, davem, kuba, netdev, linux-kernel
Hi Duoming Zhou,
The original patch was already applied so you need to just send the
fixes by themselves on top of the net-next git tree.
> ---
> include/net/ax25.h | 12 ++++++++++++
> net/ax25/af_ax25.c | 20 ++++++++++++++++----
> net/ax25/ax25_dev.c | 33 +++++++++++++++++++++++++++------
> net/ax25/ax25_route.c | 27 ++++++++++++++++++++++-----
> 4 files changed, 77 insertions(+), 15 deletions(-)
>
> diff --git a/include/net/ax25.h b/include/net/ax25.h
> index 526e4958919..1a38b1ad529 100644
> --- a/include/net/ax25.h
> +++ b/include/net/ax25.h
> @@ -239,6 +239,7 @@ typedef struct ax25_dev {
> #if defined(CONFIG_AX25_DAMA_SLAVE) || defined(CONFIG_AX25_DAMA_MASTER)
> ax25_dama_info dama;
> #endif
> + refcount_t refcount;
> } ax25_dev;
>
> typedef struct ax25_cb {
> @@ -293,6 +294,17 @@ static __inline__ void ax25_cb_put(ax25_cb *ax25)
> }
> }
>
> +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)
> +{
> + if (refcount_dec_and_test(&ax25_dev->refcount))
> + kfree(ax25_dev);
> +}
> +
Forget about these cleanups for now. Just fix the reference leaks on
error.
> static inline __be16 ax25_type_trans(struct sk_buff *skb, struct net_device *dev)
> {
> skb->dev = dev;
> diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
> index 44a8730c26a..7463bbd4e63 100644
> --- a/net/ax25/af_ax25.c
> +++ b/net/ax25/af_ax25.c
> @@ -91,6 +91,7 @@ static void ax25_kill_by_device(struct net_device *dev)
> spin_unlock_bh(&ax25_list_lock);
> lock_sock(sk);
> s->ax25_dev = NULL;
> + ax25_dev_put(ax25_dev);
> release_sock(sk);
> ax25_disconnect(s, ENETUNREACH);
> spin_lock_bh(&ax25_list_lock);
> @@ -358,21 +359,31 @@ 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)
> + ax25_dev = ax25_addr_ax25dev(&ax25_ctl.port_addr);
> + if (ax25_dev == NULL) {
> + ax25_dev_put(ax25_dev);
This one will lead to a NULL dereference. These NULL dereferences are
> return -ENODEV;
> + }
>
> - if (ax25_ctl.digi_count > AX25_MAX_DIGIS)
> + if (ax25_ctl.digi_count > AX25_MAX_DIGIS) {
> + ax25_dev_put(ax25_dev);
> return -EINVAL;
> + }
>
> - if (ax25_ctl.arg > ULONG_MAX / HZ && ax25_ctl.cmd != AX25_KILL)
> + if (ax25_ctl.arg > ULONG_MAX / HZ && ax25_ctl.cmd != AX25_KILL) {
> + ax25_dev_put(ax25_dev);
> return -EINVAL;
> + }
>
> 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 == NULL) {
> + ax25_dev_put(ax25_dev);
Don't do this.
> return -ENOTCONN;
> + }
>
> switch (ax25_ctl.cmd) {
> case AX25_KILL:
> @@ -439,6 +450,7 @@ static int ax25_ctl_ioctl(const unsigned int cmd, void __user *arg)
> }
>
> out_put:
> + ax25_dev_put(ax25_dev);
> ax25_cb_put(ax25);
> return ret;
>
> diff --git a/net/ax25/ax25_dev.c b/net/ax25/ax25_dev.c
> index 256fadb94df..77d9aa2ccab 100644
> --- a/net/ax25/ax25_dev.c
> +++ b/net/ax25/ax25_dev.c
> @@ -37,6 +37,7 @@ ax25_dev *ax25_addr_ax25dev(ax25_address *addr)
> for (ax25_dev = ax25_dev_list; ax25_dev != NULL; ax25_dev = ax25_dev->next)
> if (ax25cmp(addr, (const ax25_address *)ax25_dev->dev->dev_addr) == 0) {
> res = ax25_dev;
> + ax25_dev_hold(ax25_dev);
> }
> spin_unlock_bh(&ax25_dev_lock);
>
> @@ -56,6 +57,7 @@ void ax25_dev_device_up(struct net_device *dev)
> return;
> }
>
> + refcount_set(&ax25_dev->refcount, 1);
> dev->ax25_ptr = ax25_dev;
> ax25_dev->dev = dev;
> dev_hold_track(dev, &ax25_dev->dev_tracker, GFP_ATOMIC);
> @@ -83,6 +85,7 @@ 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_register_dev_sysctl(ax25_dev);
> @@ -111,21 +114,23 @@ void ax25_dev_device_down(struct net_device *dev)
> s->forward = NULL;
>
> if ((s = ax25_dev_list) == ax25_dev) {
> + ax25_dev_put(ax25_dev_list);
Just leave the style changes until later. I'm sorry I brought it up.
> ax25_dev_list = s->next;
> spin_unlock_bh(&ax25_dev_lock);
> dev->ax25_ptr = NULL;
> dev_put_track(dev, &ax25_dev->dev_tracker);
> - kfree(ax25_dev);
> + ax25_dev_put(ax25_dev);
> return;
> }
>
> while (s != NULL && s->next != NULL) {
> if (s->next == ax25_dev) {
> + ax25_dev_put(s->next);
> s->next = ax25_dev->next;
> spin_unlock_bh(&ax25_dev_lock);
> dev->ax25_ptr = NULL;
> dev_put_track(dev, &ax25_dev->dev_tracker);
> - kfree(ax25_dev);
> + ax25_dev_put(ax25_dev);
> return;
> }
>
> @@ -133,31 +138,47 @@ void ax25_dev_device_down(struct net_device *dev)
> }
> spin_unlock_bh(&ax25_dev_lock);
> dev->ax25_ptr = NULL;
> + ax25_dev_put(ax25_dev);
> }
>
> int ax25_fwd_ioctl(unsigned int cmd, struct ax25_fwd_struct *fwd)
> {
> ax25_dev *ax25_dev, *fwd_dev;
>
> - if ((ax25_dev = ax25_addr_ax25dev(&fwd->port_from)) == NULL)
> + ax25_dev = ax25_addr_ax25dev(&fwd->port_from);
> + if (ax25_dev == NULL) {
> + ax25_dev_put(ax25_dev);
NULL dereference.
> return -EINVAL;
> + }
>
> 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 == NULL) {
> + ax25_dev_put(fwd_dev);
NULL dereference.
> + ax25_dev_put(ax25_dev);
> return -EINVAL;
> - if (ax25_dev->forward != NULL)
> + }
> + if (ax25_dev->forward != NULL) {
> + ax25_dev_put(ax25_dev);
Drop the fwd_dev reference as well.
> 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 == NULL) {
> + 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;
> }
>
> diff --git a/net/ax25/ax25_route.c b/net/ax25/ax25_route.c
> index d0b2e094bd5..7fe7a83b2a3 100644
> --- a/net/ax25/ax25_route.c
> +++ b/net/ax25/ax25_route.c
> @@ -75,10 +75,15 @@ 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)
> + ax25_dev = ax25_addr_ax25dev(&route->port_addr);
> + if (ax25_dev == NULL) {
> + ax25_dev_put(ax25_dev);
NULL dereference.
> return -EINVAL;
> - if (route->digi_count > AX25_MAX_DIGIS)
> + }
> + if (route->digi_count > AX25_MAX_DIGIS) {
> + ax25_dev_put(ax25_dev);
> return -EINVAL;
> + }
>
> write_lock_bh(&ax25_route_lock);
>
> @@ -91,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;
> @@ -101,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;
> @@ -108,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;
> }
>
> @@ -120,6 +128,7 @@ static int __must_check ax25_rt_add(struct ax25_routes_struct *route)
> 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;
> @@ -132,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;
> }
> @@ -147,8 +157,11 @@ static int ax25_rt_del(struct ax25_routes_struct *route)
> ax25_route *s, *t, *ax25_rt;
> ax25_dev *ax25_dev;
>
> - if ((ax25_dev = ax25_addr_ax25dev(&route->port_addr)) == NULL)
> + ax25_dev = ax25_addr_ax25dev(&route->port_addr);
> + if (ax25_dev == NULL) {
> + ax25_dev_put(ax25_dev);
NULL dereference.
> return -EINVAL;
> + }
>
> write_lock_bh(&ax25_route_lock);
>
> @@ -173,7 +186,7 @@ static int ax25_rt_del(struct ax25_routes_struct *route)
> }
> }
> write_unlock_bh(&ax25_route_lock);
> -
> + ax25_dev_put(ax25_dev);
> return 0;
> }
>
> @@ -183,8 +196,11 @@ static int ax25_rt_opt(struct ax25_route_opt_struct *rt_option)
> ax25_dev *ax25_dev;
> int err = 0;
>
> - if ((ax25_dev = ax25_addr_ax25dev(&rt_option->port_addr)) == NULL)
> + ax25_dev = ax25_addr_ax25dev(&rt_option->port_addr);
> + if (ax25_dev == NULL) {
> + ax25_dev_put(ax25_dev);
NULL dereference.
> return -EINVAL;
> + }
>
> write_lock_bh(&ax25_route_lock);
>
> @@ -215,6 +231,7 @@ static int ax25_rt_opt(struct ax25_route_opt_struct *rt_option)
>
> out:
> write_unlock_bh(&ax25_route_lock);
> + ax25_dev_put(ax25_dev);
> return err;
> }
Thanks for this, but please can you resend with the above issues fixed
and based on top of net-next.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-02-02 17:47 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-02 16:18 [Fixes] ax25: add refcount in ax25_dev to avoid UAF bugs Duoming Zhou
2022-02-02 17:47 ` Dan Carpenter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.