linux-kernel - Re: [PATCH] pcmcia: synclink_cs: Fix use-after-free in mgslpc

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220915020834.GA110086@ubuntu>
Date:   Wed, 14 Sep 2022 19:08:34 -0700
From:   Hyunwoo Kim <imv4bel@...il.com>
To:     Arnd Bergmann <arnd@...db.de>, laforge@...monks.org,
        gregkh@...uxfoundation.org
Cc:     Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
        linux-kernel@...r.kernel.org,
        Dominik Brodowski <linux@...inikbrodowski.net>,
        Paul Fulghum <paulkf@...rogate.com>, akpm@...l.org,
        imv4bel@...il.com
Subject: Re: [PATCH] pcmcia: synclink_cs: Fix use-after-free in mgslpc_ioctl()

The previous mailing list is here:
https://lore.kernel.org/lkml/20220913052020.GA85241@ubuntu/#r


There are 3 other pcmica drivers in the path "drivers/char/pcmcia/synclink_cs.c", 
the path of the "synclink_cs.c" driver I reported the UAF to.
A similar UAF occurs in the "cm4000_cs.c" and "cm4040_cs.c" drivers. 
(this does not happen in scr24x_cs.c)

The flow of UAF occurrence in cm4040_cs.c driver is as follows:
```
                cpu0                                                cpu1
       1. open()
          cm4040_open()
                                                             2. reader_detach()
                                                                reader_release()
                                                                cm4040_reader_release()
                                                                while (link->open) { ...
       3. link->open = 1;
                                                             4. kfree(dev);
                                                                device_destroy()
       5. read()   <- device_destroy() was called, but read() can be called because fd is open
          cm4040_read()
          int iobase = dev->p_dev->resource[0]->start;   <- UAF
```
In cm4040_open() function, link->open is set to 1.
And in the .remove callback reader_detach() function, if link->open is 1, 
cm4040_close() is called and wait()s until link->open becomes 0.
However, if the above race condition occurs in these two functions, 
the link->open check in reader_detach() can be bypassed.
After that, you can call read() on the task that acquired fd to raise a 
UAF for the kfree()d "dev".


The flow of UAF occurrence in cm4000_cs.c driver is as follows:

```
                cpu0                                                cpu1
       1. open()
          cmm_open()
                                                             2. cm4000_detach()
                                                                stop_monitor()
                                                                if (dev->monitor_running) { ...
       3. start_monitor()
          dev->monitor_running = 1;
                                                             4. cm4000_release()
                                                                cmm_cm4000_release()
                                                                while (link->open) { ...
       5. link->open = 1;
                                                             6. kfree(dev);
                                                                device_destroy()
       7. read()   <- device_destroy() was called, but read() can be called because fd is open
          cmm_read()
          unsigned int iobase = dev->p_dev->resource[0]->start;   <- UAF
```
In the cm4000_cs.c driver, the race condition flow is tricky because of 
the start/stop_monitor() functions.

The overall flow is similar to cm4040_cs.c.
Added one race condition to bypass the "dev->monitor_running" check.


So, should the above two drivers be removed from the kernel like the synclink_cs.c driver?

Or should I submit a patch that fixes the UAF?


Best Regards,
Hyunwoo Kim.