* [PATCH 0/2] ax25: fix NPD and UAF bugs when detaching ax25 device
@ 2022-01-28 4:47 Duoming Zhou
2022-01-28 4:47 ` [PATCH 1/2] ax25: improve the incomplete fix to avoid UAF and NPD bugs Duoming Zhou
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Duoming Zhou @ 2022-01-28 4:47 UTC (permalink / raw)
To: linux-hams; +Cc: jreuter, ralf, davem, kuba, netdev, linux-kernel, 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
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 1/2] ax25: improve the incomplete fix to avoid UAF and NPD bugs 2022-01-28 4:47 [PATCH 0/2] ax25: fix NPD and UAF bugs when detaching ax25 device Duoming Zhou @ 2022-01-28 4:47 ` Duoming Zhou 2022-01-28 4:47 ` [PATCH 2/2] ax25: add refcount in ax25_dev to avoid UAF bugs Duoming Zhou 2022-01-28 15:10 ` [PATCH 0/2] ax25: fix NPD and UAF bugs when detaching ax25 device patchwork-bot+netdevbpf 2 siblings, 0 replies; 8+ messages in thread From: Duoming Zhou @ 2022-01-28 4:47 UTC (permalink / raw) To: linux-hams; +Cc: jreuter, ralf, davem, kuba, netdev, linux-kernel, 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] ax25: add refcount in ax25_dev to avoid UAF bugs 2022-01-28 4:47 [PATCH 0/2] ax25: fix NPD and UAF bugs when detaching ax25 device Duoming Zhou 2022-01-28 4:47 ` [PATCH 1/2] ax25: improve the incomplete fix to avoid UAF and NPD bugs Duoming Zhou @ 2022-01-28 4:47 ` Duoming Zhou 2022-01-31 13:22 ` Dan Carpenter 2022-01-31 17:37 ` Dan Carpenter 2022-01-28 15:10 ` [PATCH 0/2] ax25: fix NPD and UAF bugs when detaching ax25 device patchwork-bot+netdevbpf 2 siblings, 2 replies; 8+ messages in thread From: Duoming Zhou @ 2022-01-28 4:47 UTC (permalink / raw) To: linux-hams; +Cc: 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 | 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] ax25: add refcount in ax25_dev to avoid UAF bugs 2022-01-28 4:47 ` [PATCH 2/2] ax25: add refcount in ax25_dev to avoid UAF bugs Duoming Zhou @ 2022-01-31 13:22 ` Dan Carpenter 2022-02-01 14:53 ` 周多明 2022-01-31 17:37 ` Dan Carpenter 1 sibling, 1 reply; 8+ messages in thread From: Dan Carpenter @ 2022-01-31 13:22 UTC (permalink / raw) To: Duoming Zhou; +Cc: linux-hams, jreuter, ralf, davem, kuba, netdev, linux-kernel 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Re: [PATCH 2/2] ax25: add refcount in ax25_dev to avoid UAF bugs 2022-01-31 13:22 ` Dan Carpenter @ 2022-02-01 14:53 ` 周多明 0 siblings, 0 replies; 8+ messages in thread From: 周多明 @ 2022-02-01 14:53 UTC (permalink / raw) To: Dan Carpenter Cc: linux-hams, jreuter, ralf, davem, kuba, netdev, linux-kernel 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] ax25: add refcount in ax25_dev to avoid UAF bugs 2022-01-28 4:47 ` [PATCH 2/2] ax25: add refcount in ax25_dev to avoid UAF bugs Duoming Zhou 2022-01-31 13:22 ` Dan Carpenter @ 2022-01-31 17:37 ` Dan Carpenter 2022-02-01 6:26 ` 周多明 1 sibling, 1 reply; 8+ messages in thread From: Dan Carpenter @ 2022-01-31 17:37 UTC (permalink / raw) To: Duoming Zhou; +Cc: linux-hams, jreuter, ralf, davem, kuba, netdev, linux-kernel 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Re: [PATCH 2/2] ax25: add refcount in ax25_dev to avoid UAF bugs 2022-01-31 17:37 ` Dan Carpenter @ 2022-02-01 6:26 ` 周多明 0 siblings, 0 replies; 8+ messages in thread From: 周多明 @ 2022-02-01 6:26 UTC (permalink / raw) To: Dan Carpenter Cc: linux-hams, jreuter, ralf, davem, kuba, netdev, linux-kernel 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/2] ax25: fix NPD and UAF bugs when detaching ax25 device 2022-01-28 4:47 [PATCH 0/2] ax25: fix NPD and UAF bugs when detaching ax25 device Duoming Zhou 2022-01-28 4:47 ` [PATCH 1/2] ax25: improve the incomplete fix to avoid UAF and NPD bugs Duoming Zhou 2022-01-28 4:47 ` [PATCH 2/2] ax25: add refcount in ax25_dev to avoid UAF bugs Duoming Zhou @ 2022-01-28 15:10 ` patchwork-bot+netdevbpf 2 siblings, 0 replies; 8+ messages in thread From: patchwork-bot+netdevbpf @ 2022-01-28 15:10 UTC (permalink / raw) To: Duoming Zhou; +Cc: linux-hams, jreuter, ralf, davem, kuba, netdev, linux-kernel 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-02-01 14:54 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-01-28 4:47 [PATCH 0/2] ax25: fix NPD and UAF bugs when detaching ax25 device Duoming Zhou 2022-01-28 4:47 ` [PATCH 1/2] ax25: improve the incomplete fix to avoid UAF and NPD bugs Duoming Zhou 2022-01-28 4:47 ` [PATCH 2/2] ax25: add refcount in ax25_dev to avoid UAF bugs Duoming Zhou 2022-01-31 13:22 ` Dan Carpenter 2022-02-01 14:53 ` 周多明 2022-01-31 17:37 ` Dan Carpenter 2022-02-01 6:26 ` 周多明 2022-01-28 15:10 ` [PATCH 0/2] ax25: fix NPD and UAF bugs when detaching ax25 device patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).