[PATCH] acpi: nfit: vmalloc-out-of-bounds Read in acpi_nfit_ctl
Fix an issue detected by syzbot with KASAN:
BUG: KASAN: vmalloc-out-of-bounds in cmd_to_func drivers/acpi/nfit/
core.c:416 [inline]
BUG: KASAN: vmalloc-out-of-bounds in acpi_nfit_ctl+0x20e8/0x24a0
drivers/acpi/nfit/core.c:459
The issue occurs in `cmd_to_func` when the `call_pkg->nd_reserved2`
array is accessed without verifying that `call_pkg` points to a
buffer that is sized appropriately as a `struct nd_cmd_pkg`. This
could lead to out-of-bounds access and undefined behavior if the
buffer does not have sufficient space.
To address this issue, a check was added in `acpi_nfit_ctl()` to
ensure that `buf` is not `NULL` and `buf_len` is greater than or
equal to `sizeof(struct nd_cmd_pkg)` before casting `buf` to
`struct nd_cmd_pkg *`. This ensures safe access to the members of
`call_pkg`, including the `nd_reserved2` array.
This change preventing out-of-bounds reads.
Reported-by: syzbot+7534f060ebda6b8b5...@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=7534f060ebda6b8b51b3
Tested-by: syzbot+7534f060ebda6b8b5...@syzkaller.appspotmail.com
Fixes: 906bd684e4b1 ("Merge tag 'spi-fix-v6.12-rc6'")
Signed-off-by: Suraj Sonawane
---
drivers/acpi/nfit/core.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 5429ec9ef..4a2997b60 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -454,8 +454,13 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
struct nvdimm *nvdimm,
if (cmd_rc)
*cmd_rc = -EINVAL;
- if (cmd == ND_CMD_CALL)
- call_pkg = buf;
+ if (cmd == ND_CMD_CALL) {
+ if (buf == NULL || buf_len < sizeof(struct nd_cmd_pkg)) {
+ rc = -EINVAL;
+ goto out;
+ }
+ call_pkg = (struct nd_cmd_pkg *)buf;
+ }
func = cmd_to_func(nfit_mem, cmd, call_pkg, &family);
if (func < 0)
return func;
--
2.34.1
[PATCH v2] acpi: nfit: vmalloc-out-of-bounds Read in acpi_nfit_ctl
Fix an issue detected by syzbot with KASAN:
BUG: KASAN: vmalloc-out-of-bounds in cmd_to_func drivers/acpi/nfit/
core.c:416 [inline]
BUG: KASAN: vmalloc-out-of-bounds in acpi_nfit_ctl+0x20e8/0x24a0
drivers/acpi/nfit/core.c:459
The issue occurs in `cmd_to_func` when the `call_pkg->nd_reserved2`
array is accessed without verifying that `call_pkg` points to a
buffer that is sized appropriately as a `struct nd_cmd_pkg`. This
could lead to out-of-bounds access and undefined behavior if the
buffer does not have sufficient space.
To address this issue, a check was added in `acpi_nfit_ctl()` to
ensure that `buf` is not `NULL` and `buf_len` is greater than or
equal to `sizeof(struct nd_cmd_pkg)` before casting `buf` to
`struct nd_cmd_pkg *`. This ensures safe access to the members of
`call_pkg`, including the `nd_reserved2` array.
This change preventing out-of-bounds reads.
Reported-by: syzbot+7534f060ebda6b8b5...@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=7534f060ebda6b8b51b3
Tested-by: syzbot+7534f060ebda6b8b5...@syzkaller.appspotmail.com
Fixes: 2d5404caa8c7 ("Linux 6.12-rc7")
Signed-off-by: Suraj Sonawane
---
V1:
https://lore.kernel.org/lkml/2024080429.9861-1-surajsonawane0...@gmail.com/
V2: Initialized `out_obj` to `NULL` in `acpi_nfit_ctl()` to prevent
potential uninitialized variable usage if condition is true.
drivers/acpi/nfit/core.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 5429ec9ef..d0e655a9c 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -439,7 +439,7 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
struct nvdimm *nvdimm,
{
struct acpi_nfit_desc *acpi_desc = to_acpi_desc(nd_desc);
struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
- union acpi_object in_obj, in_buf, *out_obj;
+ union acpi_object in_obj, in_buf, *out_obj = NULL;
const struct nd_cmd_desc *desc = NULL;
struct device *dev = acpi_desc->dev;
struct nd_cmd_pkg *call_pkg = NULL;
@@ -454,8 +454,14 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
struct nvdimm *nvdimm,
if (cmd_rc)
*cmd_rc = -EINVAL;
- if (cmd == ND_CMD_CALL)
- call_pkg = buf;
+ if (cmd == ND_CMD_CALL) {
+ if (buf == NULL || buf_len < sizeof(struct nd_cmd_pkg)) {
+ rc = -EINVAL;
+ goto out;
+ }
+ call_pkg = (struct nd_cmd_pkg *)buf;
+ }
+
func = cmd_to_func(nfit_mem, cmd, call_pkg, &family);
if (func < 0)
return func;
--
2.34.1
Re: [syzbot] [acpi?] [nvdimm?] KASAN: vmalloc-out-of-bounds Read in acpi_nfit_ctl (2)
noprof+0x79/0x90 mm/vmalloc.c:3926
> __nd_ioctl drivers/nvdimm/bus.c:1169 [inline]
> nd_ioctl+0x1594/0x1fd0 drivers/nvdimm/bus.c:1264
> vfs_ioctl fs/ioctl.c:51 [inline]
> __do_sys_ioctl fs/ioctl.c:907 [inline]
> __se_sys_ioctl+0xf9/0x170 fs/ioctl.c:893
> do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
> page last free pid 5312 tgid 5312 stack trace:
> reset_page_owner include/linux/page_owner.h:25 [inline]
> free_pages_prepare mm/page_alloc.c:1108 [inline]
> free_unref_page+0xcfb/0xf20 mm/page_alloc.c:2638
> __folio_put+0x2c7/0x440 mm/swap.c:126
> pipe_buf_release include/linux/pipe_fs_i.h:219 [inline]
> pipe_update_tail fs/pipe.c:224 [inline]
> pipe_read+0x6ed/0x13e0 fs/pipe.c:344
> new_sync_read fs/read_write.c:488 [inline]
> vfs_read+0x991/0xb70 fs/read_write.c:569
> ksys_read+0x183/0x2b0 fs/read_write.c:712
> do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> Memory state around the buggy address:
> c9e0df00: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
> c9e0df80: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
> >c9e0e000: 00 00 00 00 00 00 00 03 f8 f8 f8 f8 f8 f8 f8 f8
> ^
> c9e0e080: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
> c9e0e100: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
> ==
>
>
> ---
> This report is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkal...@googlegroups.com.
>
> syzbot will keep track of this issue. See:
> https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
>
> If the report is already addressed, let syzbot know by replying with:
> #syz fix: exact-commit-title
>
> If you want syzbot to run the reproducer, reply with:
> #syz test: git://repo/address.git branch-or-commit-hash
> If you attach or paste a git patch, syzbot will apply it before testing.
>
> If you want to overwrite report's subsystems, reply with:
> #syz set subsystems: new-subsystem
> (See the list of subsystem names on the web dashboard)
>
> If the report is a duplicate of another one, reply with:
> #syz dup: exact-subject-of-another-report
>
> If you want to undo deduplication, reply with:
> #syz undup
>
> --
> You received this message because you are subscribed to the Google Groups
> "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to syzkaller-bugs+unsubscr...@googlegroups.com.
> To view this discussion visit
> https://groups.google.com/d/msgid/syzkaller-bugs/672a3997.050a0220.2a847.11f7.GAE%40google.com
> .
>
From 6c091558d74b19dbd1888aed9338b4d0fd3396da Mon Sep 17 00:00:00 2001
From: Suraj Sonawane
Date: Wed, 13 Nov 2024 17:35:32 +0530
Subject: [PATCH] [PATCH v4] KASAN: vmalloc-out-of-bounds Read in acpi_nfit_ctl(2)
syz test
Signed-off-by: Suraj Sonawane
---
drivers/acpi/nfit/core.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 5429ec9ef..eb5349606 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -439,7 +439,7 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm,
{
struct acpi_nfit_desc *acpi_desc = to_acpi_desc(nd_desc);
struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
- union acpi_object in_obj, in_buf, *out_obj;
+ union acpi_object in_obj, in_buf, *out_obj = NULL;
const struct nd_cmd_desc *desc = NULL;
struct device *dev = acpi_desc->dev;
struct nd_cmd_pkg *call_pkg = NULL;
@@ -454,8 +454,14 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm,
if (cmd_rc)
*cmd_rc = -EINVAL;
- if (cmd == ND_CMD_CALL)
- call_pkg = buf;
+ if (cmd == ND_CMD_CALL) {
+ if (!buf || buf_len < sizeof(*call_pkg)) {
+ rc = -EINVAL;
+ goto out;
+ }
+ call_pkg = (struct nd_cmd_pkg *)buf;
+ }
+
func = cmd_to_func(nfit_mem, cmd, call_pkg, &family);
if (func < 0)
return func;
--
2.34.1
[PATCH v3] acpi: nfit: vmalloc-out-of-bounds Read in acpi_nfit_ctl
Fix an issue detected by syzbot with KASAN:
BUG: KASAN: vmalloc-out-of-bounds in cmd_to_func drivers/acpi/nfit/
core.c:416 [inline]
BUG: KASAN: vmalloc-out-of-bounds in acpi_nfit_ctl+0x20e8/0x24a0
drivers/acpi/nfit/core.c:459
The issue occurs in cmd_to_func when the call_pkg->nd_reserved2
array is accessed without verifying that call_pkg points to a buffer
that is appropriately sized as a struct nd_cmd_pkg. This can lead
to out-of-bounds access and undefined behavior if the buffer does not
have sufficient space.
To address this, a check was added in acpi_nfit_ctl() to ensure that
buf is not NULL and that buf_len is greater than sizeof(*call_pkg)
before casting buf to struct nd_cmd_pkg *. This ensures safe access
to the members of call_pkg, including the nd_reserved2 array.
Reported-by: syzbot+7534f060ebda6b8b5...@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=7534f060ebda6b8b51b3
Tested-by: syzbot+7534f060ebda6b8b5...@syzkaller.appspotmail.com
Fixes: ebe9f6f19d80 ("acpi/nfit: Fix bus command validation")
Signed-off-by: Suraj Sonawane
---
V1:
https://lore.kernel.org/lkml/2024080429.9861-1-surajsonawane0...@gmail.com/
V2: Initialized `out_obj` to `NULL` in `acpi_nfit_ctl()` to prevent
potential uninitialized variable usage if condition is true.
V3: Changed the condition to if (!buf || buf_len < sizeof(*call_pkg))
and updated the Fixes tag to reference the correct commit.
drivers/acpi/nfit/core.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 5429ec9ef..eb5349606 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -439,7 +439,7 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
struct nvdimm *nvdimm,
{
struct acpi_nfit_desc *acpi_desc = to_acpi_desc(nd_desc);
struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
- union acpi_object in_obj, in_buf, *out_obj;
+ union acpi_object in_obj, in_buf, *out_obj = NULL;
const struct nd_cmd_desc *desc = NULL;
struct device *dev = acpi_desc->dev;
struct nd_cmd_pkg *call_pkg = NULL;
@@ -454,8 +454,14 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
struct nvdimm *nvdimm,
if (cmd_rc)
*cmd_rc = -EINVAL;
- if (cmd == ND_CMD_CALL)
- call_pkg = buf;
+ if (cmd == ND_CMD_CALL) {
+ if (!buf || buf_len < sizeof(*call_pkg)) {
+ rc = -EINVAL;
+ goto out;
+ }
+ call_pkg = (struct nd_cmd_pkg *)buf;
+ }
+
func = cmd_to_func(nfit_mem, cmd, call_pkg, &family);
if (func < 0)
return func;
--
2.34.1
Re: [PATCH v2] acpi: nfit: vmalloc-out-of-bounds Read in acpi_nfit_ctl
On 13/11/24 10:08, Alison Schofield wrote:
On Tue, Nov 12, 2024 at 10:50:35AM +0530, Suraj Sonawane wrote:
Fix an issue detected by syzbot with KASAN:
BUG: KASAN: vmalloc-out-of-bounds in cmd_to_func drivers/acpi/nfit/
core.c:416 [inline]
BUG: KASAN: vmalloc-out-of-bounds in acpi_nfit_ctl+0x20e8/0x24a0
drivers/acpi/nfit/core.c:459
The issue occurs in `cmd_to_func` when the `call_pkg->nd_reserved2`
array is accessed without verifying that `call_pkg` points to a
buffer that is sized appropriately as a `struct nd_cmd_pkg`. This
could lead to out-of-bounds access and undefined behavior if the
buffer does not have sufficient space.
To address this issue, a check was added in `acpi_nfit_ctl()` to
ensure that `buf` is not `NULL` and `buf_len` is greater than or
equal to `sizeof(struct nd_cmd_pkg)` before casting `buf` to
`struct nd_cmd_pkg *`. This ensures safe access to the members of
`call_pkg`, including the `nd_reserved2` array.
That all sounds good! A couple of coding conventions fixups suggested
below -
snip
@@ -439,7 +439,7 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
struct nvdimm *nvdimm,
{
struct acpi_nfit_desc *acpi_desc = to_acpi_desc(nd_desc);
struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
- union acpi_object in_obj, in_buf, *out_obj;
+ union acpi_object in_obj, in_buf, *out_obj = NULL;
const struct nd_cmd_desc *desc = NULL;
struct device *dev = acpi_desc->dev;
struct nd_cmd_pkg *call_pkg = NULL;
@@ -454,8 +454,14 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
struct nvdimm *nvdimm,
if (cmd_rc)
*cmd_rc = -EINVAL;
- if (cmd == ND_CMD_CALL)
- call_pkg = buf;
+ if (cmd == ND_CMD_CALL) {
+ if (buf == NULL || buf_len < sizeof(struct nd_cmd_pkg)) {
Comparison to NULL and sizeof() usage preferred like this:
if (!buf || buf_len < sizeof(*call_pkg))
-snip
Thank you for the feedback and your time.
I appreciate your review and coding convention insights. I have studied
the suggested change and updated the patch.
I will test the updated patch with syzbot and submit a new version very
shortly.
Best regards,
Suraj Sonawane
Re: [PATCH v2] acpi: nfit: vmalloc-out-of-bounds Read in acpi_nfit_ctl
On 13/11/24 10:21, Alison Schofield wrote:
On Tue, Nov 12, 2024 at 10:50:35AM +0530, Suraj Sonawane wrote:
Fix an issue detected by syzbot with KASAN:
BUG: KASAN: vmalloc-out-of-bounds in cmd_to_func drivers/acpi/nfit/
core.c:416 [inline]
BUG: KASAN: vmalloc-out-of-bounds in acpi_nfit_ctl+0x20e8/0x24a0
drivers/acpi/nfit/core.c:459
The issue occurs in `cmd_to_func` when the `call_pkg->nd_reserved2`
array is accessed without verifying that `call_pkg` points to a
buffer that is sized appropriately as a `struct nd_cmd_pkg`. This
could lead to out-of-bounds access and undefined behavior if the
buffer does not have sufficient space.
To address this issue, a check was added in `acpi_nfit_ctl()` to
ensure that `buf` is not `NULL` and `buf_len` is greater than or
equal to `sizeof(struct nd_cmd_pkg)` before casting `buf` to
`struct nd_cmd_pkg *`. This ensures safe access to the members of
`call_pkg`, including the `nd_reserved2` array.
This change preventing out-of-bounds reads.
Reported-by: syzbot+7534f060ebda6b8b5...@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=7534f060ebda6b8b51b3
Tested-by: syzbot+7534f060ebda6b8b5...@syzkaller.appspotmail.com
Fixes: 2d5404caa8c7 ("Linux 6.12-rc7")
Signed-off-by: Suraj Sonawane
Suraj,
The fixes tag needs to be where the issue originated, not
where you discovered it (which I'm guessing was using 6.12-rc7).
Thank you for your feedback.
Here's how I find the tag:
$ git blame drivers/acpi/nfit/core.c | grep call_pkg | grep buf
ebe9f6f19d80d drivers/acpi/nfit/core.c (Dan Williams 2019-02-07 14:56:50
-0800 458) call_pkg = buf;
$ git log -1 --pretty=fixes ebe9f6f19d80d
Thank you for this detailed explaination.
Fixes: ebe9f6f19d80 ("acpi/nfit: Fix bus command validation")
I think ^ should be your Fixes tag.
Yes, I ran the provided steps to verify the commit ID ebe9f6f19d80d and
verified it.
snip
I'll update the Fixes tag in the next version of the patch.
Additionally, I will re-test with syzbot and submit the revised patch
shortly.
Thank you for your time and feedback!
Best,
Suraj Sonawane
Re: [syzbot] [acpi?] [nvdimm?] KASAN: vmalloc-out-of-bounds Read in acpi_nfit_ctl (2)
noprof+0x79/0x90 mm/vmalloc.c:3926
> __nd_ioctl drivers/nvdimm/bus.c:1169 [inline]
> nd_ioctl+0x1594/0x1fd0 drivers/nvdimm/bus.c:1264
> vfs_ioctl fs/ioctl.c:51 [inline]
> __do_sys_ioctl fs/ioctl.c:907 [inline]
> __se_sys_ioctl+0xf9/0x170 fs/ioctl.c:893
> do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
> page last free pid 5312 tgid 5312 stack trace:
> reset_page_owner include/linux/page_owner.h:25 [inline]
> free_pages_prepare mm/page_alloc.c:1108 [inline]
> free_unref_page+0xcfb/0xf20 mm/page_alloc.c:2638
> __folio_put+0x2c7/0x440 mm/swap.c:126
> pipe_buf_release include/linux/pipe_fs_i.h:219 [inline]
> pipe_update_tail fs/pipe.c:224 [inline]
> pipe_read+0x6ed/0x13e0 fs/pipe.c:344
> new_sync_read fs/read_write.c:488 [inline]
> vfs_read+0x991/0xb70 fs/read_write.c:569
> ksys_read+0x183/0x2b0 fs/read_write.c:712
> do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> Memory state around the buggy address:
> c9e0df00: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
> c9e0df80: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
> >c9e0e000: 00 00 00 00 00 00 00 03 f8 f8 f8 f8 f8 f8 f8 f8
> ^
> c9e0e080: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
> c9e0e100: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
> ==
>
>
> ---
> This report is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkal...@googlegroups.com.
>
> syzbot will keep track of this issue. See:
> https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
>
> If the report is already addressed, let syzbot know by replying with:
> #syz fix: exact-commit-title
>
> If you want syzbot to run the reproducer, reply with:
> #syz test: git://repo/address.git branch-or-commit-hash
> If you attach or paste a git patch, syzbot will apply it before testing.
>
> If you want to overwrite report's subsystems, reply with:
> #syz set subsystems: new-subsystem
> (See the list of subsystem names on the web dashboard)
>
> If the report is a duplicate of another one, reply with:
> #syz dup: exact-subject-of-another-report
>
> If you want to undo deduplication, reply with:
> #syz undup
>
> --
> You received this message because you are subscribed to the Google Groups
> "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to syzkaller-bugs+unsubscr...@googlegroups.com.
> To view this discussion visit
> https://groups.google.com/d/msgid/syzkaller-bugs/672a3997.050a0220.2a847.11f7.GAE%40google.com
> .
>
From 443b5c366694650f9c771863556433270fff8bc2 Mon Sep 17 00:00:00 2001
From: Suraj Sonawane
Date: Sun, 10 Nov 2024 16:01:24 +0530
Subject: [PATCH v3] KASAN: vmalloc-out-of-bounds Read in acpi_nfit_ctl (2)
syz test
Signed-off-by: Suraj Sonawane
---
drivers/acpi/nfit/core.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 5429ec9ef..4a2997b60 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -454,8 +454,13 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm,
if (cmd_rc)
*cmd_rc = -EINVAL;
- if (cmd == ND_CMD_CALL)
- call_pkg = buf;
+ if (cmd == ND_CMD_CALL) {
+ if (buf == NULL || buf_len < sizeof(struct nd_cmd_pkg)) {
+ rc = -EINVAL;
+ goto out;
+ }
+ call_pkg = (struct nd_cmd_pkg *)buf;
+ }
func = cmd_to_func(nfit_mem, cmd, call_pkg, &family);
if (func < 0)
return func;
--
2.34.1
Re: [PATCH v4] acpi: nfit: vmalloc-out-of-bounds Read in acpi_nfit_ctl
On 16/11/24 01:00, Ira Weiny wrote:
Suraj Sonawane wrote:
Fix an issue detected by syzbot with KASAN:
BUG: KASAN: vmalloc-out-of-bounds in cmd_to_func drivers/acpi/nfit/
core.c:416 [inline]
BUG: KASAN: vmalloc-out-of-bounds in acpi_nfit_ctl+0x20e8/0x24a0
drivers/acpi/nfit/core.c:459
The issue occurs in cmd_to_func when the call_pkg->nd_reserved2
array is accessed without verifying that call_pkg points to a buffer
that is appropriately sized as a struct nd_cmd_pkg. This can lead
to out-of-bounds access and undefined behavior if the buffer does not
have sufficient space.
To address this, a check was added in acpi_nfit_ctl() to ensure that
buf is not NULL and that buf_len is less than sizeof(*call_pkg)
before accessing it. This ensures safe access to the members of
call_pkg, including the nd_reserved2 array.
Reported-by: syzbot+7534f060ebda6b8b5...@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=7534f060ebda6b8b51b3
Tested-by: syzbot+7534f060ebda6b8b5...@syzkaller.appspotmail.com
Fixes: ebe9f6f19d80 ("acpi/nfit: Fix bus command validation")
Signed-off-by: Suraj Sonawane
---
V1:
https://lore.kernel.org/lkml/2024080429.9861-1-surajsonawane0...@gmail.com/
V2: Initialized `out_obj` to `NULL` in `acpi_nfit_ctl()` to prevent
potential uninitialized variable usage if condition is true.
V3: Changed the condition to if (!buf || buf_len < sizeof(*call_pkg))
and updated the Fixes tag to reference the correct commit.
V4: Removed the explicit cast to maintain the original code style.
drivers/acpi/nfit/core.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 5429ec9ef..84d8eef2a 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -454,8 +454,15 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
struct nvdimm *nvdimm,
if (cmd_rc)
*cmd_rc = -EINVAL;
- if (cmd == ND_CMD_CALL)
+ if (cmd == ND_CMD_CALL) {
+ if (!buf || buf_len < sizeof(*call_pkg)) {
+ rc = -EINVAL;
+ goto out;
This goto is wrong. This will result in ACPI_FREE() being called on an
undefined out_obj.
This should be:
return -EINVAL;
Thanks for catching that. You’re correct about the goto issue. I had
initialized out_obj to NULL in previous versions but missed it in this
patch. In v5, I’ve re-initialized out_obj to NULL, to avoid unintended
access. Here’s the updated patch link:
https://lore.kernel.org/lkml/20241116114027.19303-1-surajsonawane0...@gmail.com/
Ira
+ }
+
call_pkg = buf;
+ }
+
func = cmd_to_func(nfit_mem, cmd, call_pkg, &family);
if (func < 0)
return func;
--
2.34.1
Best Regards,
Suraj Sonawane
[PATCH v6] acpi: nfit: vmalloc-out-of-bounds Read in acpi_nfit_ctl
Fix an issue detected by syzbot with KASAN:
BUG: KASAN: vmalloc-out-of-bounds in cmd_to_func drivers/acpi/nfit/
core.c:416 [inline]
BUG: KASAN: vmalloc-out-of-bounds in acpi_nfit_ctl+0x20e8/0x24a0
drivers/acpi/nfit/core.c:459
The issue occurs in cmd_to_func when the call_pkg->nd_reserved2
array is accessed without verifying that call_pkg points to a buffer
that is appropriately sized as a struct nd_cmd_pkg. This can lead
to out-of-bounds access and undefined behavior if the buffer does not
have sufficient space.
To address this, a check was added in acpi_nfit_ctl() to ensure that
buf is not NULL and that buf_len is less than sizeof(*call_pkg)
before accessing it. This ensures safe access to the members of
call_pkg, including the nd_reserved2 array.
Reported-by: syzbot+7534f060ebda6b8b5...@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=7534f060ebda6b8b51b3
Tested-by: syzbot+7534f060ebda6b8b5...@syzkaller.appspotmail.com
Fixes: ebe9f6f19d80 ("acpi/nfit: Fix bus command validation")
Signed-off-by: Suraj Sonawane
---
V1:
https://lore.kernel.org/lkml/2024080429.9861-1-surajsonawane0...@gmail.com/
V2: Initialized `out_obj` to `NULL` in `acpi_nfit_ctl()` to prevent
potential uninitialized variable usage if condition is true.
V3: Changed the condition to if (!buf || buf_len < sizeof(*call_pkg))
and updated the Fixes tag to reference the correct commit.
V4: Removed the explicit cast to maintain the original code style.
V5: Re-Initialized `out_obj` to NULL. To prevent
potential uninitialized variable usage if condition is true.
V6: Remove the goto out condition from the error handling and directly
returned -EINVAL in the check for buf and buf_len
drivers/acpi/nfit/core.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 5429ec9ef..a5d47819b 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -454,8 +454,13 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
struct nvdimm *nvdimm,
if (cmd_rc)
*cmd_rc = -EINVAL;
- if (cmd == ND_CMD_CALL)
+ if (cmd == ND_CMD_CALL) {
+ if (!buf || buf_len < sizeof(*call_pkg))
+ return -EINVAL;
+
call_pkg = buf;
+ }
+
func = cmd_to_func(nfit_mem, cmd, call_pkg, &family);
if (func < 0)
return func;
--
2.34.1
Re: [PATCH v3] acpi: nfit: vmalloc-out-of-bounds Read in acpi_nfit_ctl
On 13/11/24 22:32, Dave Jiang wrote:
On 11/13/24 5:51 AM, Suraj Sonawane wrote:
Fix an issue detected by syzbot with KASAN:
BUG: KASAN: vmalloc-out-of-bounds in cmd_to_func drivers/acpi/nfit/
core.c:416 [inline]
BUG: KASAN: vmalloc-out-of-bounds in acpi_nfit_ctl+0x20e8/0x24a0
drivers/acpi/nfit/core.c:459
The issue occurs in cmd_to_func when the call_pkg->nd_reserved2
array is accessed without verifying that call_pkg points to a buffer
that is appropriately sized as a struct nd_cmd_pkg. This can lead
to out-of-bounds access and undefined behavior if the buffer does not
have sufficient space.
To address this, a check was added in acpi_nfit_ctl() to ensure that
buf is not NULL and that buf_len is greater than sizeof(*call_pkg)
before casting buf to struct nd_cmd_pkg *. This ensures safe access
to the members of call_pkg, including the nd_reserved2 array.
Reported-by: syzbot+7534f060ebda6b8b5...@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=7534f060ebda6b8b51b3
Tested-by: syzbot+7534f060ebda6b8b5...@syzkaller.appspotmail.com
Fixes: ebe9f6f19d80 ("acpi/nfit: Fix bus command validation")
Signed-off-by: Suraj Sonawane
---
V1:
https://lore.kernel.org/lkml/2024080429.9861-1-surajsonawane0...@gmail.com/
V2: Initialized `out_obj` to `NULL` in `acpi_nfit_ctl()` to prevent
potential uninitialized variable usage if condition is true.
V3: Changed the condition to if (!buf || buf_len < sizeof(*call_pkg))
and updated the Fixes tag to reference the correct commit.
drivers/acpi/nfit/core.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 5429ec9ef..eb5349606 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -439,7 +439,7 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
struct nvdimm *nvdimm,
{
struct acpi_nfit_desc *acpi_desc = to_acpi_desc(nd_desc);
struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
- union acpi_object in_obj, in_buf, *out_obj;
+ union acpi_object in_obj, in_buf, *out_obj = NULL;
Looking at the code later, out_obj is always assigned before access. I'm not
seeing a path where out_obj would be accessed unitialized...
I initialized out_obj to NULL to prevent potential issues where goto out
might access an uninitialized pointer, ensuring ACPI_FREE(out_obj)
handles NULL safely in the cleanup section. This covers cases where the
condition !buf || buf_len < sizeof(*call_pkg) triggers an early exit,
preventing unintended behavior.
https://elixir.bootlin.com/linux/v6.12-rc7/source/drivers/acpi/nfit/core.c#L538
const struct nd_cmd_desc *desc = NULL;
struct device *dev = acpi_desc->dev;
struct nd_cmd_pkg *call_pkg = NULL;
@@ -454,8 +454,14 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
struct nvdimm *nvdimm,
if (cmd_rc)
*cmd_rc = -EINVAL;
- if (cmd == ND_CMD_CALL)
- call_pkg = buf;
+ if (cmd == ND_CMD_CALL) {
+ if (!buf || buf_len < sizeof(*call_pkg)) {
+ rc = -EINVAL;
+ goto out;
+ }
+ call_pkg = (struct nd_cmd_pkg *)buf;
Is the casting needed? It wasn't in the old code
I tested the code both with and without the cast using syzbot, and it
didn't result in any errors in either case. Since the buffer (buf) is
being used as a pointer to struct nd_cmd_pkg, and the casting works in
both scenarios, it appears that the cast may not be strictly necessary
for this particular case.
I can remove the cast and retain the original code structure, as it does
not seem to affect functionality. However, the cast was added for
clarity and type safety to ensure that buf is explicitly treated as a
struct nd_cmd_pkg *.
Would you prefer to remove the cast, or should I keep it as is for type
safety and clarity?
+ }
+
func = cmd_to_func(nfit_mem, cmd, call_pkg, &family);
if (func < 0)
return func;
Thank you for your feedback and your time.
Re: [PATCH v3] acpi: nfit: vmalloc-out-of-bounds Read in acpi_nfit_ctl
On 14/11/24 21:12, Dave Jiang wrote:
On 11/14/24 2:19 AM, Suraj Sonawane wrote:
On 13/11/24 22:32, Dave Jiang wrote:
On 11/13/24 5:51 AM, Suraj Sonawane wrote:
Fix an issue detected by syzbot with KASAN:
BUG: KASAN: vmalloc-out-of-bounds in cmd_to_func drivers/acpi/nfit/
core.c:416 [inline]
BUG: KASAN: vmalloc-out-of-bounds in acpi_nfit_ctl+0x20e8/0x24a0
drivers/acpi/nfit/core.c:459
The issue occurs in cmd_to_func when the call_pkg->nd_reserved2
array is accessed without verifying that call_pkg points to a buffer
that is appropriately sized as a struct nd_cmd_pkg. This can lead
to out-of-bounds access and undefined behavior if the buffer does not
have sufficient space.
To address this, a check was added in acpi_nfit_ctl() to ensure that
buf is not NULL and that buf_len is greater than sizeof(*call_pkg)
before casting buf to struct nd_cmd_pkg *. This ensures safe access
to the members of call_pkg, including the nd_reserved2 array.
Reported-by: syzbot+7534f060ebda6b8b5...@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=7534f060ebda6b8b51b3
Tested-by: syzbot+7534f060ebda6b8b5...@syzkaller.appspotmail.com
Fixes: ebe9f6f19d80 ("acpi/nfit: Fix bus command validation")
Signed-off-by: Suraj Sonawane
---
V1:
https://lore.kernel.org/lkml/2024080429.9861-1-surajsonawane0...@gmail.com/
V2: Initialized `out_obj` to `NULL` in `acpi_nfit_ctl()` to prevent
potential uninitialized variable usage if condition is true.
V3: Changed the condition to if (!buf || buf_len < sizeof(*call_pkg))
and updated the Fixes tag to reference the correct commit.
drivers/acpi/nfit/core.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 5429ec9ef..eb5349606 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -439,7 +439,7 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
struct nvdimm *nvdimm,
{
struct acpi_nfit_desc *acpi_desc = to_acpi_desc(nd_desc);
struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
- union acpi_object in_obj, in_buf, *out_obj;
+ union acpi_object in_obj, in_buf, *out_obj = NULL;
Looking at the code later, out_obj is always assigned before access. I'm not
seeing a path where out_obj would be accessed unitialized...
I initialized out_obj to NULL to prevent potential issues where goto out might
access an uninitialized pointer, ensuring ACPI_FREE(out_obj) handles NULL safely
in the cleanup section. This covers cases where the condition !buf || buf_len <
sizeof(*call_pkg) triggers an early exit, preventing unintended behavior.
ok
https://elixir.bootlin.com/linux/v6.12-rc7/source/drivers/acpi/nfit/core.c#L538
const struct nd_cmd_desc *desc = NULL;
struct device *dev = acpi_desc->dev;
struct nd_cmd_pkg *call_pkg = NULL;
@@ -454,8 +454,14 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
struct nvdimm *nvdimm,
if (cmd_rc)
*cmd_rc = -EINVAL;
- if (cmd == ND_CMD_CALL)
- call_pkg = buf;
+ if (cmd == ND_CMD_CALL) {
+ if (!buf || buf_len < sizeof(*call_pkg)) {
+ rc = -EINVAL;
+ goto out;
+ }
+ call_pkg = (struct nd_cmd_pkg *)buf;
Is the casting needed? It wasn't in the old code
I tested the code both with and without the cast using syzbot, and it didn't
result in any errors in either case. Since the buffer (buf) is being used as a
pointer to struct nd_cmd_pkg, and the casting works in both scenarios, it
appears that the cast may not be strictly necessary for this particular case.
I can remove the cast and retain the original code structure, as it does not
seem to affect functionality. However, the cast was added for clarity and type
safety to ensure that buf is explicitly treated as a struct nd_cmd_pkg *.
Would you prefer to remove the cast, or should I keep it as is for type safety
and clarity?
I would just leave it as it was.
I have submitted the patch with the original code unchanged(without
casting) by testing with syzbot. You can view it
here:https://lore.kernel.org/lkml/20241115164223.20854-1-surajsonawane0...@gmail.com/
+ }
+
func = cmd_to_func(nfit_mem, cmd, call_pkg, &family);
if (func < 0)
return func;
Thank you for your feedback and your time.
[PATCH v4] acpi: nfit: vmalloc-out-of-bounds Read in acpi_nfit_ctl
Fix an issue detected by syzbot with KASAN:
BUG: KASAN: vmalloc-out-of-bounds in cmd_to_func drivers/acpi/nfit/
core.c:416 [inline]
BUG: KASAN: vmalloc-out-of-bounds in acpi_nfit_ctl+0x20e8/0x24a0
drivers/acpi/nfit/core.c:459
The issue occurs in cmd_to_func when the call_pkg->nd_reserved2
array is accessed without verifying that call_pkg points to a buffer
that is appropriately sized as a struct nd_cmd_pkg. This can lead
to out-of-bounds access and undefined behavior if the buffer does not
have sufficient space.
To address this, a check was added in acpi_nfit_ctl() to ensure that
buf is not NULL and that buf_len is less than sizeof(*call_pkg)
before accessing it. This ensures safe access to the members of
call_pkg, including the nd_reserved2 array.
Reported-by: syzbot+7534f060ebda6b8b5...@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=7534f060ebda6b8b51b3
Tested-by: syzbot+7534f060ebda6b8b5...@syzkaller.appspotmail.com
Fixes: ebe9f6f19d80 ("acpi/nfit: Fix bus command validation")
Signed-off-by: Suraj Sonawane
---
V1:
https://lore.kernel.org/lkml/2024080429.9861-1-surajsonawane0...@gmail.com/
V2: Initialized `out_obj` to `NULL` in `acpi_nfit_ctl()` to prevent
potential uninitialized variable usage if condition is true.
V3: Changed the condition to if (!buf || buf_len < sizeof(*call_pkg))
and updated the Fixes tag to reference the correct commit.
V4: Removed the explicit cast to maintain the original code style.
drivers/acpi/nfit/core.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 5429ec9ef..84d8eef2a 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -454,8 +454,15 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
struct nvdimm *nvdimm,
if (cmd_rc)
*cmd_rc = -EINVAL;
- if (cmd == ND_CMD_CALL)
+ if (cmd == ND_CMD_CALL) {
+ if (!buf || buf_len < sizeof(*call_pkg)) {
+ rc = -EINVAL;
+ goto out;
+ }
+
call_pkg = buf;
+ }
+
func = cmd_to_func(nfit_mem, cmd, call_pkg, &family);
if (func < 0)
return func;
--
2.34.1
[PATCH v5] acpi: nfit: vmalloc-out-of-bounds Read in acpi_nfit_ctl
Fix an issue detected by syzbot with KASAN:
BUG: KASAN: vmalloc-out-of-bounds in cmd_to_func drivers/acpi/nfit/
core.c:416 [inline]
BUG: KASAN: vmalloc-out-of-bounds in acpi_nfit_ctl+0x20e8/0x24a0
drivers/acpi/nfit/core.c:459
The issue occurs in cmd_to_func when the call_pkg->nd_reserved2
array is accessed without verifying that call_pkg points to a buffer
that is appropriately sized as a struct nd_cmd_pkg. This can lead
to out-of-bounds access and undefined behavior if the buffer does not
have sufficient space.
To address this, a check was added in acpi_nfit_ctl() to ensure that
buf is not NULL and that buf_len is less than sizeof(*call_pkg)
before accessing it. This ensures safe access to the members of
call_pkg, including the nd_reserved2 array.
Reported-by: syzbot+7534f060ebda6b8b5...@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=7534f060ebda6b8b51b3
Tested-by: syzbot+7534f060ebda6b8b5...@syzkaller.appspotmail.com
Fixes: ebe9f6f19d80 ("acpi/nfit: Fix bus command validation")
Signed-off-by: Suraj Sonawane
---
V1:
https://lore.kernel.org/lkml/2024080429.9861-1-surajsonawane0...@gmail.com/
V2: Initialized `out_obj` to `NULL` in `acpi_nfit_ctl()` to prevent
potential uninitialized variable usage if condition is true.
V3: Changed the condition to if (!buf || buf_len < sizeof(*call_pkg))
and updated the Fixes tag to reference the correct commit.
V4: Removed the explicit cast to maintain the original code style.
V5: Re-Initialized `out_obj` to NULL. To prevent
potential uninitialized variable usage if condition is true.
drivers/acpi/nfit/core.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 5429ec9ef..573ed264c 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -439,7 +439,7 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
struct nvdimm *nvdimm,
{
struct acpi_nfit_desc *acpi_desc = to_acpi_desc(nd_desc);
struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
- union acpi_object in_obj, in_buf, *out_obj;
+ union acpi_object in_obj, in_buf, *out_obj = NULL;
const struct nd_cmd_desc *desc = NULL;
struct device *dev = acpi_desc->dev;
struct nd_cmd_pkg *call_pkg = NULL;
@@ -454,8 +454,15 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
struct nvdimm *nvdimm,
if (cmd_rc)
*cmd_rc = -EINVAL;
- if (cmd == ND_CMD_CALL)
+ if (cmd == ND_CMD_CALL) {
+ if (!buf || buf_len < sizeof(*call_pkg)) {
+ rc = -EINVAL;
+ goto out;
+ }
+
call_pkg = buf;
+ }
+
func = cmd_to_func(nfit_mem, cmd, call_pkg, &family);
if (func < 0)
return func;
--
2.34.1
Re: [syzbot] [acpi?] [nvdimm?] KASAN: vmalloc-out-of-bounds Read in acpi_nfit_ctl (2)
noprof+0x79/0x90 mm/vmalloc.c:3926
> __nd_ioctl drivers/nvdimm/bus.c:1169 [inline]
> nd_ioctl+0x1594/0x1fd0 drivers/nvdimm/bus.c:1264
> vfs_ioctl fs/ioctl.c:51 [inline]
> __do_sys_ioctl fs/ioctl.c:907 [inline]
> __se_sys_ioctl+0xf9/0x170 fs/ioctl.c:893
> do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
> page last free pid 5312 tgid 5312 stack trace:
> reset_page_owner include/linux/page_owner.h:25 [inline]
> free_pages_prepare mm/page_alloc.c:1108 [inline]
> free_unref_page+0xcfb/0xf20 mm/page_alloc.c:2638
> __folio_put+0x2c7/0x440 mm/swap.c:126
> pipe_buf_release include/linux/pipe_fs_i.h:219 [inline]
> pipe_update_tail fs/pipe.c:224 [inline]
> pipe_read+0x6ed/0x13e0 fs/pipe.c:344
> new_sync_read fs/read_write.c:488 [inline]
> vfs_read+0x991/0xb70 fs/read_write.c:569
> ksys_read+0x183/0x2b0 fs/read_write.c:712
> do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> Memory state around the buggy address:
> c9e0df00: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
> c9e0df80: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
> >c9e0e000: 00 00 00 00 00 00 00 03 f8 f8 f8 f8 f8 f8 f8 f8
> ^
> c9e0e080: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
> c9e0e100: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
> ==
>
>
> ---
> This report is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkal...@googlegroups.com.
>
> syzbot will keep track of this issue. See:
> https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
>
> If the report is already addressed, let syzbot know by replying with:
> #syz fix: exact-commit-title
>
> If you want syzbot to run the reproducer, reply with:
> #syz test: git://repo/address.git branch-or-commit-hash
> If you attach or paste a git patch, syzbot will apply it before testing.
>
> If you want to overwrite report's subsystems, reply with:
> #syz set subsystems: new-subsystem
> (See the list of subsystem names on the web dashboard)
>
> If the report is a duplicate of another one, reply with:
> #syz dup: exact-subject-of-another-report
>
> If you want to undo deduplication, reply with:
> #syz undup
>
> --
> You received this message because you are subscribed to the Google Groups
> "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to syzkaller-bugs+unsubscr...@googlegroups.com.
> To view this discussion visit
> https://groups.google.com/d/msgid/syzkaller-bugs/672a3997.050a0220.2a847.11f7.GAE%40google.com
> .
>
From cd70ead500c4498914af578fd65fb446854cd9ca Mon Sep 17 00:00:00 2001
From: Suraj Sonawane
Date: Sat, 16 Nov 2024 15:44:28 +0530
Subject: [PATCH v5] acpi: nfit: vmalloc-out-of-bounds Read in acpi_nfit_ctl
Fix an issue detected by syzbot with KASAN:
BUG: KASAN: vmalloc-out-of-bounds in cmd_to_func drivers/acpi/nfit/
core.c:416 [inline]
BUG: KASAN: vmalloc-out-of-bounds in acpi_nfit_ctl+0x20e8/0x24a0
drivers/acpi/nfit/core.c:459
The issue occurs in cmd_to_func when the call_pkg->nd_reserved2
array is accessed without verifying that call_pkg points to a buffer
that is appropriately sized as a struct nd_cmd_pkg. This can lead
to out-of-bounds access and undefined behavior if the buffer does not
have sufficient space.
To address this, a check was added in acpi_nfit_ctl() to ensure that
buf is not NULL and that buf_len is less than sizeof(*call_pkg)
before accessing it. This ensures safe access to the members of
call_pkg, including the nd_reserved2 array.
Reported-by: syzbot+7534f060ebda6b8b5...@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=7534f060ebda6b8b51b3
Tested-by: syzbot+7534f060ebda6b8b5...@syzkaller.appspotmail.com
Fixes: ebe9f6f19d80 ("acpi/nfit: Fix bus command validation")
Signed-off-by: Suraj Sonawane
---
V1: https://lore.kernel.org/lkml/2024080429.9861-1-surajsonawane0...@gmail.com/
V2: Initialized `out_obj` to `NULL` in `acpi_nfit_ctl()` to prevent
potential uninitialized variable usage if condition is true.
V3: Changed the condition to if (!buf || buf_len < sizeof(*call_pkg))
and updated the Fixes tag to reference the correct commit.
V4: Removed the explicit cast to maintain the original code style.
V5: Re-Initialized `out_obj` to NULL.
drivers/acpi/nfit/core.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
Re: [PATCH v6] acpi: nfit: vmalloc-out-of-bounds Read in acpi_nfit_ctl
On 11/18/24 21:56, Suraj Sonawane wrote:
Fix an issue detected by syzbot with KASAN:
BUG: KASAN: vmalloc-out-of-bounds in cmd_to_func drivers/acpi/nfit/
core.c:416 [inline]
BUG: KASAN: vmalloc-out-of-bounds in acpi_nfit_ctl+0x20e8/0x24a0
drivers/acpi/nfit/core.c:459
The issue occurs in cmd_to_func when the call_pkg->nd_reserved2
array is accessed without verifying that call_pkg points to a buffer
that is appropriately sized as a struct nd_cmd_pkg. This can lead
to out-of-bounds access and undefined behavior if the buffer does not
have sufficient space.
To address this, a check was added in acpi_nfit_ctl() to ensure that
buf is not NULL and that buf_len is less than sizeof(*call_pkg)
before accessing it. This ensures safe access to the members of
call_pkg, including the nd_reserved2 array.
Reported-by: syzbot+7534f060ebda6b8b5...@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=7534f060ebda6b8b51b3
Tested-by: syzbot+7534f060ebda6b8b5...@syzkaller.appspotmail.com
Fixes: ebe9f6f19d80 ("acpi/nfit: Fix bus command validation")
Signed-off-by: Suraj Sonawane
---
V1:
https://lore.kernel.org/lkml/2024080429.9861-1-surajsonawane0...@gmail.com/
V2: Initialized `out_obj` to `NULL` in `acpi_nfit_ctl()` to prevent
potential uninitialized variable usage if condition is true.
V3: Changed the condition to if (!buf || buf_len < sizeof(*call_pkg))
and updated the Fixes tag to reference the correct commit.
V4: Removed the explicit cast to maintain the original code style.
V5: Re-Initialized `out_obj` to NULL. To prevent
potential uninitialized variable usage if condition is true.
V6: Remove the goto out condition from the error handling and directly
returned -EINVAL in the check for buf and buf_len
drivers/acpi/nfit/core.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 5429ec9ef..a5d47819b 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -454,8 +454,13 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
struct nvdimm *nvdimm,
if (cmd_rc)
*cmd_rc = -EINVAL;
- if (cmd == ND_CMD_CALL)
+ if (cmd == ND_CMD_CALL) {
+ if (!buf || buf_len < sizeof(*call_pkg))
+ return -EINVAL;
+
call_pkg = buf;
+ }
+
func = cmd_to_func(nfit_mem, cmd, call_pkg, &family);
if (func < 0)
return func;
Hello!
I wanted to follow up on the patch I submitted. I have incorporated all
the suggested changes up to v6. I was wondering if you had a chance to
review it and if there are any comments or feedback.
Here are the links to the discussion for all versions:
https://lore.kernel.org/lkml/?q=acpi%3A+nfit%3A+vmalloc-out-of-bounds+Read+in+acpi_nfit_ctl
Thank you for your time and consideration. I look forward to your response.
Best regards,
Suraj Sonawane
Re: [PATCH v6] acpi: nfit: vmalloc-out-of-bounds Read in acpi_nfit_ctl
On 12/2/24 21:56, Ira Weiny wrote:
Suraj Sonawane wrote:
On 11/18/24 21:56, Suraj Sonawane wrote:
[snip]
drivers/acpi/nfit/core.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 5429ec9ef..a5d47819b 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -454,8 +454,13 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc,
struct nvdimm *nvdimm,
if (cmd_rc)
*cmd_rc = -EINVAL;
- if (cmd == ND_CMD_CALL)
+ if (cmd == ND_CMD_CALL) {
+ if (!buf || buf_len < sizeof(*call_pkg))
+ return -EINVAL;
+
call_pkg = buf;
+ }
+
func = cmd_to_func(nfit_mem, cmd, call_pkg, &family);
if (func < 0)
return func;
Hello!
I wanted to follow up on the patch I submitted. I have incorporated all
the suggested changes up to v6. I was wondering if you had a chance to
review it and if there are any comments or feedback.
It just missed the soak period for the merge. But I'll be looking at it
for an rc pull request.
Thanks for sticking with it,
Ira
[snip]
Thank you for the update.
I also appreciate everyone's efforts in reviewing the patch.
Thank you for reviewing the patch.
Best regards,
Suraj
