[PATCH 0/2] ax25: fix NPD and UAF bugs when detaching ax25 device

[PATCH 0/2] ax25: fix NPD and UAF bugs when detaching ax25 device

[Duoming Zhou]

There are NPD and UAF bugs when detaching ax25 device, we
use lock and refcount to mitigate these bugs.

Duoming Zhou (2):
  ax25: improve the incomplete fix to avoid UAF and NPD bugs
  ax25: add refcount in ax25_dev to avoid UAF bugs

 include/net/ax25.h    | 10 ++++++++++
 net/ax25/af_ax25.c    | 11 ++++++++---
 net/ax25/ax25_dev.c   | 12 ++++++++++--
 net/ax25/ax25_route.c |  3 +++
 4 files changed, 31 insertions(+), 5 deletions(-)

-- 
2.17.1

[PATCH 1/2] ax25: improve the incomplete fix to avoid UAF and NPD bugs

[Duoming Zhou]

The previous commit 1ade48d0c27d ("ax25: NPD bug when detaching
AX25 device") introduce lock_sock() into ax25_kill_by_device to
prevent NPD bug. But the concurrency NPD or UAF bug will occur,
when lock_sock() or release_sock() dereferences the ax25_cb->sock.

The NULL pointer dereference bug can be shown as below:

ax25_kill_by_device()        | ax25_release()
                             |   ax25_destroy_socket()
                             |     ax25_cb_del()
  ...                        |     ...
                             |     ax25->sk=NULL;
  lock_sock(s->sk); //(1)    |
  s->ax25_dev = NULL;        |     ...
  release_sock(s->sk); //(2) |
  ...                        |

The root cause is that the sock is set to null before dereference
site (1) or (2). Therefore, this patch extracts the ax25_cb->sock
in advance, and uses ax25_list_lock to protect it, which can synchronize
with ax25_cb_del() and ensure the value of sock is not null before
dereference sites.

The concurrency UAF bug can be shown as below:

ax25_kill_by_device()        | ax25_release()
                             |   ax25_destroy_socket()
  ...                        |   ...
                             |   sock_put(sk); //FREE
  lock_sock(s->sk); //(1)    |
  s->ax25_dev = NULL;        |   ...
  release_sock(s->sk); //(2) |
  ...                        |

The root cause is that the sock is released before dereference
site (1) or (2). Therefore, this patch uses sock_hold() to increase
the refcount of sock and uses ax25_list_lock to protect it, which
can synchronize with ax25_cb_del() in ax25_destroy_socket() and
ensure the sock wil not be released before dereference sites.

Signed-off-by: Duoming Zhou <duoming@zju.edu.cn>
---
 net/ax25/af_ax25.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
index 02f43f3e2c5..44a8730c26a 100644
--- a/net/ax25/af_ax25.c
+++ b/net/ax25/af_ax25.c
@@ -77,6 +77,7 @@ static void ax25_kill_by_device(struct net_device *dev)
 {
 	ax25_dev *ax25_dev;
 	ax25_cb *s;
+	struct sock *sk;
 
 	if ((ax25_dev = ax25_dev_ax25dev(dev)) == NULL)
 		return;
@@ -85,13 +86,15 @@ static void ax25_kill_by_device(struct net_device *dev)
 again:
 	ax25_for_each(s, &ax25_list) {
 		if (s->ax25_dev == ax25_dev) {
+			sk = s->sk;
+			sock_hold(sk);
 			spin_unlock_bh(&ax25_list_lock);
-			lock_sock(s->sk);
+			lock_sock(sk);
 			s->ax25_dev = NULL;
-			release_sock(s->sk);
+			release_sock(sk);
 			ax25_disconnect(s, ENETUNREACH);
 			spin_lock_bh(&ax25_list_lock);
-
+			sock_put(sk);
 			/* The entry could have been deleted from the
 			 * list meanwhile and thus the next pointer is
 			 * no longer valid.  Play it safe and restart
-- 
2.17.1

[PATCH 2/2] ax25: add refcount in ax25_dev to avoid UAF bugs

[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    | 10 ++++++++++
 net/ax25/af_ax25.c    |  2 ++
 net/ax25/ax25_dev.c   | 12 ++++++++++--
 net/ax25/ax25_route.c |  3 +++
 4 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/include/net/ax25.h b/include/net/ax25.h
index 526e4958919..50b417df622 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,15 @@ 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_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..32f61978ff2 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);
@@ -439,6 +440,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..770b787fb7b 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);
@@ -112,20 +115,22 @@ 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);
 		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) {
 			s->next = ax25_dev->next;
+			ax25_dev_put(ax25_dev);
 			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,6 +138,7 @@ 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)
@@ -149,6 +155,7 @@ int ax25_fwd_ioctl(unsigned int cmd, struct ax25_fwd_struct *fwd)
 		if (ax25_dev->forward != NULL)
 			return -EINVAL;
 		ax25_dev->forward = fwd_dev->dev;
+		ax25_dev_put(fwd_dev);
 		break;
 
 	case SIOCAX25DELFWD:
@@ -161,6 +168,7 @@ int ax25_fwd_ioctl(unsigned int cmd, struct ax25_fwd_struct *fwd)
 		return -EINVAL;
 	}
 
+	ax25_dev_put(ax25_dev);
 	return 0;
 }
 
diff --git a/net/ax25/ax25_route.c b/net/ax25/ax25_route.c
index d0b2e094bd5..1e32693833e 100644
--- a/net/ax25/ax25_route.c
+++ b/net/ax25/ax25_route.c
@@ -116,6 +116,7 @@ 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);
@@ -172,6 +173,7 @@ static int ax25_rt_del(struct ax25_routes_struct *route)
 			}
 		}
 	}
+	ax25_dev_put(ax25_dev);
 	write_unlock_bh(&ax25_route_lock);
 
 	return 0;
@@ -214,6 +216,7 @@ static int ax25_rt_opt(struct ax25_route_opt_struct *rt_option)
 	}
 
 out:
+	ax25_dev_put(ax25_dev);
 	write_unlock_bh(&ax25_route_lock);
 	return err;
 }
-- 
2.17.1

Re: [PATCH 0/2] ax25: fix NPD and UAF bugs when detaching ax25 device

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (master)
by David S. Miller <davem@davemloft.net>:

On Fri, 28 Jan 2022 12:47:14 +0800 you wrote:
> There are NPD and UAF bugs when detaching ax25 device, we
> use lock and refcount to mitigate these bugs.
> 
> Duoming Zhou (2):
>   ax25: improve the incomplete fix to avoid UAF and NPD bugs
>   ax25: add refcount in ax25_dev to avoid UAF bugs
> 
> [...]

Here is the summary with links:
  - [1/2] ax25: improve the incomplete fix to avoid UAF and NPD bugs
    https://git.kernel.org/netdev/net/c/4e0f718daf97
  - [2/2] ax25: add refcount in ax25_dev to avoid UAF bugs
    https://git.kernel.org/netdev/net/c/d01ffb9eee4a

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH 2/2] ax25: add refcount in ax25_dev to avoid UAF bugs

[Dan Carpenter]

On Fri, Jan 28, 2022 at 12:47:16PM +0800, Duoming Zhou wrote:
> diff --git a/include/net/ax25.h b/include/net/ax25.h
> index 526e4958919..50b417df622 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,15 @@ static __inline__ void ax25_cb_put(ax25_cb *ax25)
>  	}
>  }
>  
> +#define ax25_dev_hold(__ax25_dev) \
> +	refcount_inc(&((__ax25_dev)->refcount))

Make this an inline function.

> +
> +static __inline__ void ax25_dev_put(ax25_dev *ax25_dev)

Please run checkpatch.pl --strict on your patches.  s/__inline__/inline/

> +{
> +	if (refcount_dec_and_test(&ax25_dev->refcount)) {
> +		kfree(ax25_dev);
> +	}

Delete the extra curly braces.

> +}
>  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..32f61978ff2 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);
> @@ -439,6 +440,7 @@ static int ax25_ctl_ioctl(const unsigned int cmd, void __user *arg)
>  	  }
>  
>  out_put:
> +	ax25_dev_put(ax25_dev);

The ax25_ctl_ioctl() has a ton of reference leak paths now.  Almost
every return -ESOMETHING needs to be fixed.

>  	ax25_cb_put(ax25);
>  	return ret;
>  
> diff --git a/net/ax25/ax25_dev.c b/net/ax25/ax25_dev.c
> index 256fadb94df..770b787fb7b 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);
> @@ -112,20 +115,22 @@ 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);

Do we not have to call ax25_dev_hold(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) {
>  			s->next = ax25_dev->next;
> +			ax25_dev_put(ax25_dev);

ax25_dev_hold(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,6 +138,7 @@ 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)
> @@ -149,6 +155,7 @@ int ax25_fwd_ioctl(unsigned int cmd, struct ax25_fwd_struct *fwd)
>  		if (ax25_dev->forward != NULL)
>  			return -EINVAL;

Every return -ERROR; in this function leaks reference counts.  This one
should drop the reference for both fwd_dev and ax25_dev.


>  		ax25_dev->forward = fwd_dev->dev;
> +		ax25_dev_put(fwd_dev);
>  		break;
>  
>  	case SIOCAX25DELFWD:

regards,
dan carpenter

Re: [PATCH 2/2] ax25: add refcount in ax25_dev to avoid UAF bugs

[Dan Carpenter]

On Fri, Jan 28, 2022 at 12:47:16PM +0800, Duoming Zhou wrote:
> 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>

I pointed out a few bugs in my previous email.  I've had more time to
look at it now.

Basically you just want to audit all the calls sites which call
ax25_dev_ax25dev() and make sure all the error paths decrement.  Most
of them are buggy.  I'm testing a new Smatch check which is supposed to
detect these sorts of bugs.

I think the refcount in ax25_bind() needs a matching decrement.  Where
is that?  I don't know networking well enough to know the answer to
this...

> @@ -112,20 +115,22 @@ 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);

It would be more readable to do ax25_dev_put(ax25_dev_list).  It's weird
to put ax25_dev here and then a couple lines later

>  		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);

Here

>  		return;
>  	}
>  
>  	while (s != NULL && s->next != NULL) {
>  		if (s->next == ax25_dev) {
>  			s->next = ax25_dev->next;
> +			ax25_dev_put(ax25_dev);

Same.

>  			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;
>  		}
>  

regards,
dan carpenter

Re: Re: [PATCH 2/2] ax25: add refcount in ax25_dev to avoid UAF bugs

[周多明]

Thank you very much for your time and pointing out problems in my patch.

The decrement of ax25_bind() is in ax25_kill_by_device(). If we don't
call ax25_bind() before ax25_kill_by_device(), the ax25_list will be
empty and ax25_dev_put() in ax25_kill_by_device() will not execute.

> @@ -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);

I will send the improved patch as soon as possible.


Best wishes,
Duoming Zhou

Re: Re: [PATCH 2/2] ax25: add refcount in ax25_dev to avoid UAF bugs

[周多明]

Thank you very much for your time and pointing out problems in my patch.
Another two questions you asked is shown below:

> @@ -112,20 +115,22 @@ 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);

Do we not have to call ax25_dev_hold(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) {
>  			s->next = ax25_dev->next;
> +			ax25_dev_put(ax25_dev);

ax25_dev_hold(ax25_dev->next)?

Answer:
We don't have to call ax25_dev_hold(s->next) or ax25_dev_hold(ax25_dev->next)
in ax25_dev_device_down() because we have already increased the refcount when 
we insert ax25_dev into the linked list in ax25_dev_device_up().

> @@ -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);

Best wishes,
Duoming Zhou