Bug 4446 - ax25: improve the incomplete fix to avoid UAF and NPD bugs
Summary: ax25: improve the incomplete fix to avoid UAF and NPD bugs
Status: RESOLVED WONTFIX
Alias: None
Product: ANCK 4.19 Dev
Classification: ANCK
Component: net (show other bugs) net
Version: 4.19-023.x
Hardware: All Linux
: P3-Medium S3-normal
Target Milestone: ---
Assignee: XuanZhuo
QA Contact: shuming
URL:
Whiteboard:
Keywords: CVE
Depends on:
Blocks:
 
Reported: 2023-03-08 22:12 UTC by baibai871
Modified: 2023-10-20 16:35 UTC (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description baibai871 2023-03-08 22:12:30 UTC
Description of problem:
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

Version-Release number of selected component (if applicable):


How reproducible:
sometimes,when you the concurrency NPD or UAF bug will occur。

Steps to Reproduce:
1.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) |
  ...                        |

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

3.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.


Actual results:
the sock is set to null before dereference
site (1) or (2)
  lock_sock(s->sk); //(1)    |
  s->ax25_dev = NULL;        |     ...
  release_sock(s->sk); //(2)

Expected results:
ensure the value of sock is not null before
dereference sites.

Additional info:
Comment 1 maqiao alibaba_cloud_group 2023-10-20 16:35:16 UTC
CONFIG_AX25 is not enabled, so ANCK is not affected.