linux-kernel - [PATCH] gpiolib: cdev: Fix use after free in lineinfo_changed

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date: Wed, 1 May 2024 10:26:12 +0800
From: Zhongqiu Han <quic_zhonhan@...cinc.com>
To: <brgl@...ev.pl>, <warthog618@...il.com>, <linus.walleij@...aro.org>
CC: <linux-gpio@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <quic_zhonhan@...cinc.com>
Subject: [PATCH] gpiolib: cdev: Fix use after free in lineinfo_changed_notify

The use-after-free issue occurs when userspace closes the GPIO chip device
file (e.g., "/dev/gpiochip0") by invoking gpio_chrdev_release(), while one
of its GPIO lines is simultaneously being released. In a stress test
scenario, stress-ng tool starts multi stress-ng-dev threads to continuously
open and close device file in the /dev, the use-after-free issue will occur
with a low reproducibility.

Here is the typical stack when issue happened:

BUG: KASAN: slab-use-after-free in lineinfo_changed_notify+0x84/0x1bc
Read of size 8 at addr ffffff803c06e840 by task stress-ng-dev/5437
Call trace:
 dump_backtrace
 show_stack
 dump_stack_lvl
 print_report
 kasan_report
 __asan_load8
 lineinfo_changed_notify
 notifier_call_chain
 blocking_notifier_call_chain
 gpiod_free_commit
 gpiod_free
 gpio_free
 st54spi_gpio_dev_release
 __fput
 __fput_sync
 __arm64_sys_close
 invoke_syscall
 el0_svc_common
 do_el0_svc
 el0_svc
 el0t_64_sync_handler
 el0t_64_sync
Allocated by task 5425:
 kasan_set_track
 kasan_save_alloc_info
 __kasan_kmalloc
 __kmalloc
 bitmap_zalloc
 gpio_chrdev_open
 chrdev_open
 do_dentry_open
 vfs_open
 path_openat
 do_filp_open
 do_sys_openat2
 __arm64_sys_openat
 invoke_syscall
 el0_svc_common
 do_el0_svc
 el0_svc
 el0t_64_sync_handler
 el0t_64_sync
Freed by task 5425:
 kasan_set_track
 kasan_save_free_info
 ____kasan_slab_free
 __kasan_slab_free
 slab_free_freelist_hook
 __kmem_cache_free
 kfree
 bitmap_free
 gpio_chrdev_release
 __fput
 __fput_sync
 __arm64_sys_close
 invoke_syscall
 el0_svc_common
 do_el0_svc
 el0_svc
 el0t_64_sync_handler
 el0t_64_sync

The use-after-free issue occurs as follows: watched_lines is freed by
bitmap_free(), but the lineinfo_changed_nb notifier chain cannot be
successfully unregistered due to waiting write rwsem when closing the
GPIO chip device file. Additionally, one of the GPIO chip's lines is
also in the release process and holds the notifier chain's read rwsem.
Consequently, a race condition leads to the use-after-free of
watched_lines.

[free]
gpio_chrdev_release()
  --> bitmap_free(cdev->watched_lines)                  <-- freed
  --> blocking_notifier_chain_unregister()
    --> down_write(&nh->rwsem)                          <-- waiting rwsem
          --> __down_write_common()
            --> rwsem_down_write_slowpath()
                  --> schedule_preempt_disabled()
                    --> schedule()

[use]
st54spi_gpio_dev_release()
  --> gpio_free()
    --> gpiod_free()
      --> gpiod_free_commit()
        --> gpiod_line_state_notify()
          --> blocking_notifier_call_chain()
            --> down_read(&nh->rwsem);                  <-- held rwsem
            --> notifier_call_chain()
              --> lineinfo_changed_notify()
                --> test_bit(xxxx, cdev->watched_lines) <-- use after free

To fix this issue, call the bitmap_free() function after successfully
unregistering the notifier chain. This prevents lineinfo_changed_notify()
from being called, thus avoiding the use-after-free race condition.

Fixes: 51c1064e82e7 ("gpiolib: add new ioctl() for monitoring changes in line info")
Signed-off-by: Zhongqiu Han <quic_zhonhan@...cinc.com>
---
 drivers/gpio/gpiolib-cdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index d09c7d728365..6b3a43e3ba47 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -2799,11 +2799,11 @@ static int gpio_chrdev_release(struct inode *inode, struct file *file)
 	struct gpio_chardev_data *cdev = file->private_data;
 	struct gpio_device *gdev = cdev->gdev;
 
-	bitmap_free(cdev->watched_lines);
 	blocking_notifier_chain_unregister(&gdev->device_notifier,
 					   &cdev->device_unregistered_nb);
 	blocking_notifier_chain_unregister(&gdev->line_state_notifier,
 					   &cdev->lineinfo_changed_nb);
+	bitmap_free(cdev->watched_lines);
 	gpio_device_put(gdev);
 	kfree(cdev);
 
-- 
2.25.1