* [PATCH v5] char: pcmcia: synclink_cs: Fix use-after-free in mgslpc_ops
@ 2022-09-19 4:02 Hyunwoo Kim
2022-09-19 5:51 ` Arnd Bergmann
0 siblings, 1 reply; 2+ messages in thread
From: Hyunwoo Kim @ 2022-09-19 4:02 UTC (permalink / raw)
To: arnd, gregkh; +Cc: linux-kernel, imv4bel, ilpo.jarvinen, paulkf, linux
A race condition may occur if the user physically removes
the pcmcia device while calling ioctl() for this tty device node.
This is a race condition between the mgslpc_ioctl() function and
the mgslpc_detach() function, which may eventually result in UAF.
So, add a refcount check to mgslpc_detach() to free the structure
after the tty device node is close()d.
Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
---
drivers/char/pcmcia/synclink_cs.c | 35 +++++++++++++++++++++++++------
1 file changed, 29 insertions(+), 6 deletions(-)
diff --git a/drivers/char/pcmcia/synclink_cs.c b/drivers/char/pcmcia/synclink_cs.c
index 8fc49b038372..0dfba8833a67 100644
--- a/drivers/char/pcmcia/synclink_cs.c
+++ b/drivers/char/pcmcia/synclink_cs.c
@@ -216,7 +216,8 @@ typedef struct _mgslpc_info {
/* PCMCIA support */
struct pcmcia_device *p_dev;
- int stop;
+ int stop;
+ struct kref refcnt;
/* SPPP/Cisco HDLC device parts */
int netcount;
@@ -228,6 +229,8 @@ typedef struct _mgslpc_info {
} MGSLPC_INFO;
+static DEFINE_MUTEX(remove_mutex);
+
#define MGSLPC_MAGIC 0x5402
/*
@@ -468,10 +471,21 @@ static void mgslpc_wait_until_sent(struct tty_struct *tty, int timeout);
/* PCMCIA prototypes */
+static void mgslpc_delete(struct kref *kref);
static int mgslpc_config(struct pcmcia_device *link);
static void mgslpc_release(u_long arg);
static void mgslpc_detach(struct pcmcia_device *p_dev);
+static void mgslpc_delete(struct kref *kref)
+{
+ MGSLPC_INFO *info = container_of(kref, MGSLPC_INFO, refcnt);
+ struct pcmcia_device *link = info->p_dev;
+
+ mgslpc_release((u_long)link);
+
+ mgslpc_remove_device(info);
+}
+
/*
* 1st function defined in .text section. Calling this function in
* init_module() followed by a breakpoint allows a remote debugger
@@ -534,6 +548,7 @@ static int mgslpc_probe(struct pcmcia_device *link)
init_waitqueue_head(&info->event_wait_q);
spin_lock_init(&info->lock);
spin_lock_init(&info->netlock);
+ kref_init(&info->refcnt);
memcpy(&info->params,&default_params,sizeof(MGSL_PARAMS));
info->idle_mode = HDLC_TXIDLE_FLAGS;
info->imra_value = 0xffff;
@@ -620,13 +635,15 @@ static void mgslpc_release(u_long arg)
static void mgslpc_detach(struct pcmcia_device *link)
{
+ MGSLPC_INFO *info = link->priv;
+
+ mutex_lock(&remove_mutex);
if (debug_level >= DEBUG_LEVEL_INFO)
printk("mgslpc_detach(0x%p)\n", link);
- ((MGSLPC_INFO *)link->priv)->stop = 1;
- mgslpc_release((u_long)link);
-
- mgslpc_remove_device((MGSLPC_INFO *)link->priv);
+ info->stop = 1;
+ kref_put(&info->refcnt, mgslpc_delete);
+ mutex_unlock(&remove_mutex);
}
static int mgslpc_suspend(struct pcmcia_device *link)
@@ -2341,10 +2358,13 @@ static void mgslpc_close(struct tty_struct *tty, struct file * filp)
tty_port_close_end(port, tty);
tty_port_tty_set(port, NULL);
+
cleanup:
if (debug_level >= DEBUG_LEVEL_INFO)
printk("%s(%d):mgslpc_close(%s) exit, count=%d\n", __FILE__, __LINE__,
tty->driver->name, port->count);
+
+ kref_put(&info->refcnt, mgslpc_delete);
}
/* Wait until the transmitter is empty.
@@ -2465,6 +2485,8 @@ static int mgslpc_open(struct tty_struct *tty, struct file * filp)
int retval, line;
unsigned long flags;
+ mutex_lock(&remove_mutex);
+
/* verify range of specified line number */
line = tty->index;
if (line >= mgslpc_device_count) {
@@ -2517,9 +2539,10 @@ static int mgslpc_open(struct tty_struct *tty, struct file * filp)
if (debug_level >= DEBUG_LEVEL_INFO)
printk("%s(%d):mgslpc_open(%s) success\n",
__FILE__, __LINE__, info->device_name);
- retval = 0;
+ kref_get(&info->refcnt);
cleanup:
+ mutex_unlock(&remove_mutex);
return retval;
}
--
2.25.1
Dear,
I fixed the wrong patch referencing "info" after kref_put() in the previous version of the patch.
Regards,
Hyunwoo Kim.
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v5] char: pcmcia: synclink_cs: Fix use-after-free in mgslpc_ops
2022-09-19 4:02 [PATCH v5] char: pcmcia: synclink_cs: Fix use-after-free in mgslpc_ops Hyunwoo Kim
@ 2022-09-19 5:51 ` Arnd Bergmann
0 siblings, 0 replies; 2+ messages in thread
From: Arnd Bergmann @ 2022-09-19 5:51 UTC (permalink / raw)
To: Hyunwoo Kim, Greg Kroah-Hartman
Cc: linux-kernel, Ilpo Järvinen, Paul Fulghum, Dominik Brodowski
On Mon, Sep 19, 2022, at 6:02 AM, Hyunwoo Kim wrote:
> A race condition may occur if the user physically removes
> the pcmcia device while calling ioctl() for this tty device node.
>
> This is a race condition between the mgslpc_ioctl() function and
> the mgslpc_detach() function, which may eventually result in UAF.
>
> So, add a refcount check to mgslpc_detach() to free the structure
> after the tty device node is close()d.
>
> Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-09-19 5:52 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-19 4:02 [PATCH v5] char: pcmcia: synclink_cs: Fix use-after-free in mgslpc_ops Hyunwoo Kim
2022-09-19 5:51 ` Arnd Bergmann
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).