* [PATCH v3] efi: capsule-loader: Fix use-after-free in efi_capsule_write
@ 2022-09-07 13:52 Hyunwoo Kim
0 siblings, 0 replies; 2+ messages in thread
From: Hyunwoo Kim @ 2022-09-07 13:52 UTC (permalink / raw)
To: ardb; +Cc: linux-efi, imv4bel
A race condition may occur if the user calls close() on another
thread during a write() operation on the device node of the efi capsule.
This is a race condition that occurs between the efi_capsule_write() and
efi_capsule_flush() functions of efi_capsule_fops,
which ultimately results in UAF.
So I added mutex_lock/unlock to these two functions
to avoid race condition.
Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
---
drivers/firmware/efi/capsule-loader.c | 12 ++++++++++++
include/linux/efi.h | 1 +
2 files changed, 13 insertions(+)
diff --git a/drivers/firmware/efi/capsule-loader.c b/drivers/firmware/efi/capsule-loader.c
index 4dde8edd53b6..ca50cb982371 100644
--- a/drivers/firmware/efi/capsule-loader.c
+++ b/drivers/firmware/efi/capsule-loader.c
@@ -174,6 +174,8 @@ static ssize_t efi_capsule_write(struct file *file, const char __user *buff,
void *kbuff = NULL;
size_t write_byte;
+ mutex_lock(&cap_info->write_lock);
+
if (count == 0)
return 0;
@@ -233,12 +235,16 @@ static ssize_t efi_capsule_write(struct file *file, const char __user *buff,
goto failed;
}
+ mutex_unlock(&cap_info->write_lock);
+
return write_byte;
fail_unmap:
kunmap(page);
failed:
efi_free_all_buff_pages(cap_info);
+ mutex_unlock(&cap_info->write_lock);
+
return ret;
}
@@ -256,12 +262,16 @@ static int efi_capsule_flush(struct file *file, fl_owner_t id)
int ret = 0;
struct capsule_info *cap_info = file->private_data;
+ mutex_lock(&cap_info->write_lock);
+
if (cap_info->index > 0) {
pr_err("capsule upload not complete\n");
efi_free_all_buff_pages(cap_info);
ret = -ECANCELED;
}
+ mutex_unlock(&cap_info->write_lock);
+
return ret;
}
@@ -315,6 +325,8 @@ static int efi_capsule_open(struct inode *inode, struct file *file)
return -ENOMEM;
}
+ mutex_init(&cap_info->write_lock);
+
file->private_data = cap_info;
return 0;
diff --git a/include/linux/efi.h b/include/linux/efi.h
index d2b84c2fec39..f8b92a54d3c9 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -204,6 +204,7 @@ struct efi_image_auth {
struct capsule_info {
efi_capsule_header_t header;
efi_capsule_header_t *capsule;
+ struct mutex write_lock;
int reset_type;
long index;
size_t count;
--
2.25.1
Fixed a typo in the v2 patch "struct mutex write_mutex".
ah... I had a problem with my mutt client. Sent back with v3 patch. Sorry.
Regards,
Hyunwoo Kim.
^ permalink raw reply related [flat|nested] 2+ messages in thread
* [PATCH v3] efi: capsule-loader: Fix use-after-free in efi_capsule_write
@ 2022-09-07 13:46 Hyunwoo Kim
0 siblings, 0 replies; 2+ messages in thread
From: Hyunwoo Kim @ 2022-09-07 13:46 UTC (permalink / raw)
To: ardb; +Cc: linux-efi, imv4bel
A race condition may occur if the user calls close() on another
thread during a write() operation on the device node of the efi capsule.
This is a race condition that occurs between the efi_capsule_write() and
efi_capsule_flush() functions of efi_capsule_fops,
which ultimately results in UAF.
So I added mutex_lock/unlock to these two functions
to avoid race condition.
Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
---
drivers/firmware/efi/capsule-loader.c | 12 ++++++++++++
include/linux/efi.h | 1 +
2 files changed, 13 insertions(+)
diff --git a/drivers/firmware/efi/capsule-loader.c b/drivers/firmware/efi/capsule-loader.c
index 4dde8edd53b6..ca50cb982371 100644
--- a/drivers/firmware/efi/capsule-loader.c
+++ b/drivers/firmware/efi/capsule-loader.c
@@ -174,6 +174,8 @@ static ssize_t efi_capsule_write(struct file *file, const char __user *buff,
void *kbuff = NULL;
size_t write_byte;
+ mutex_lock(&cap_info->write_lock);
+
if (count == 0)
return 0;
@@ -233,12 +235,16 @@ static ssize_t efi_capsule_write(struct file *file, const char __user *buff,
goto failed;
}
+ mutex_unlock(&cap_info->write_lock);
+
return write_byte;
fail_unmap:
kunmap(page);
failed:
efi_free_all_buff_pages(cap_info);
+ mutex_unlock(&cap_info->write_lock);
+
return ret;
}
@@ -256,12 +262,16 @@ static int efi_capsule_flush(struct file *file, fl_owner_t id)
int ret = 0;
struct capsule_info *cap_info = file->private_data;
+ mutex_lock(&cap_info->write_lock);
+
if (cap_info->index > 0) {
pr_err("capsule upload not complete\n");
efi_free_all_buff_pages(cap_info);
ret = -ECANCELED;
}
+ mutex_unlock(&cap_info->write_lock);
+
return ret;
}
@@ -315,6 +325,8 @@ static int efi_capsule_open(struct inode *inode, struct file *file)
return -ENOMEM;
}
+ mutex_init(&cap_info->write_lock);
+
file->private_data = cap_info;
return 0;
diff --git a/include/linux/efi.h b/include/linux/efi.h
index d2b84c2fec39..f8b92a54d3c9 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -204,6 +204,7 @@ struct efi_image_auth {
struct capsule_info {
efi_capsule_header_t header;
efi_capsule_header_t *capsule;
+ struct mutex write_lock;
int reset_type;
long index;
size_t count;
--
2.25.1
Fixed a typo in the v2 patch "struct mutex write_mutex".
Regards,
Hyunwoo Kim.
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-09-07 13:52 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-07 13:52 [PATCH v3] efi: capsule-loader: Fix use-after-free in efi_capsule_write Hyunwoo Kim
-- strict thread matches above, loose matches on Subject: below --
2022-09-07 13:46 Hyunwoo Kim
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.