From: Duoming Zhou duoming@zju.edu.cn
commit 82e31755e55fbcea6a9dfaae5fe4860ade17cbc0 upstream.
There are race conditions that may lead to UAF bugs in ax25_heartbeat_expiry(), ax25_t1timer_expiry(), ax25_t2timer_expiry(), ax25_t3timer_expiry() and ax25_idletimer_expiry(), when we call ax25_release() to deallocate ax25_dev.
One of the UAF bugs caused by ax25_release() is shown below:
(Thread 1) | (Thread 2) ax25_dev_device_up() //(1) | ... | ax25_kill_by_device() ax25_bind() //(2) | ax25_connect() | ... ax25_std_establish_data_link() | ax25_start_t1timer() | ax25_dev_device_down() //(3) mod_timer(&ax25->t1timer,..) | | ax25_release() (wait a time) | ... | ax25_dev_put(ax25_dev) //(4)FREE ax25_t1timer_expiry() | ax25->ax25_dev->values[..] //USE| ... ... |
We increase the refcount of ax25_dev in position (1) and (2), and decrease the refcount of ax25_dev in position (3) and (4). The ax25_dev will be freed in position (4) and be used in ax25_t1timer_expiry().
The fail log is shown below: ==============================================================
[ 106.116942] BUG: KASAN: use-after-free in ax25_t1timer_expiry+0x1c/0x60 [ 106.116942] Read of size 8 at addr ffff88800bda9028 by task swapper/0/0 [ 106.116942] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.17.0-06123-g0905eec574 [ 106.116942] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-14 [ 106.116942] Call Trace: ... [ 106.116942] ax25_t1timer_expiry+0x1c/0x60 [ 106.116942] call_timer_fn+0x122/0x3d0 [ 106.116942] __run_timers.part.0+0x3f6/0x520 [ 106.116942] run_timer_softirq+0x4f/0xb0 [ 106.116942] __do_softirq+0x1c2/0x651 ...
This patch adds del_timer_sync() in ax25_release(), which could ensure that all timers stop before we deallocate ax25_dev.
Signed-off-by: Duoming Zhou duoming@zju.edu.cn Signed-off-by: Paolo Abeni pabeni@redhat.com Signed-off-by: Ovidiu Panait ovidiu.panait@windriver.com --- net/ax25/af_ax25.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c index f5686c463bc0..363d47f94532 100644 --- a/net/ax25/af_ax25.c +++ b/net/ax25/af_ax25.c @@ -1053,6 +1053,11 @@ static int ax25_release(struct socket *sock) ax25_destroy_socket(ax25); } if (ax25_dev) { + del_timer_sync(&ax25->timer); + del_timer_sync(&ax25->t1timer); + del_timer_sync(&ax25->t2timer); + del_timer_sync(&ax25->t3timer); + del_timer_sync(&ax25->idletimer); dev_put_track(ax25_dev->dev, &ax25_dev->dev_tracker); ax25_dev_put(ax25_dev); }
On Fri, Apr 15, 2022 at 07:13:17PM +0300, Ovidiu Panait wrote:
From: Duoming Zhou duoming@zju.edu.cn
commit 82e31755e55fbcea6a9dfaae5fe4860ade17cbc0 upstream.
<snip>
Any reason you are not cc: the original authors and maintainers of these changes?
thanks,
greg k-h
linux-stable-mirror@lists.linaro.org