* [PATCH] cxl/features: Bound Get Feature read count to the output buffer
@ 2026-06-11 9:49 Richard Cheng
2026-06-11 15:28 ` Dave Jiang
0 siblings, 1 reply; 4+ messages in thread
From: Richard Cheng @ 2026-06-11 9:49 UTC (permalink / raw)
To: dave, jic23, dave.jiang, alison.schofield, vishal.l.verma,
ira.weiny, djbw
Cc: shiju.jose, ming.li, alucerop, linux-cxl, linux-kernel, newtonl,
kristinc, kaihengf, kobak, Richard Cheng
cxlctl_get_feature() sizes the output buffer rpc_out from the
user-controlled fwctl_rpc.out_len, but the number of bytes the device is
asked to write into rpc_out->payload comes from a separate
user-controlled filed, cxl_mbox_get_feat_in.count. Nothing ties them
together, so a small out_len with a large count makes cxl_get_feature()
write the device's feature data past the kvzalloc()'d buffer. A heap OOB
write reachable from FWCTL_RPC. Reject requests where count exceeds the
available payload room, before the allocation.
The issue is triggered via the following reproducer [1], with error log
[2].
[1]:
"""
#include <fcntl.h>
#include <stdint.h>
#include <string.h>
#include <sys/ioctl.h>
#define FWCTL_RPC _IO(0x9A, 1)
#define GET_SUP_FEATS 0x0500
#define GET_FEATURE 0x0501
struct fwctl_rpc { uint32_t size, scope, in_len, out_len; uint64_t in, o
struct hdr { uint32_t opcode, flags, op_size, rsvd; }; /*
struct get_feat_in { uint8_t uuid[16]; uint16_t off, count; uint8_t sel;
__attribute__((packed)); /*
struct feat_entry { uint8_t uuid[16]; uint16_t id, get_sz, set_sz;
uint32_t flags; uint8_t gv, sv; uint16_t eff; uint8_t rsvd[18];
__attribute__((packed)); /*
static int fd;
static int rpc(uint32_t il, void *i, uint32_t ol, void *o) {
struct fwctl_rpc r = { sizeof(r), 0, il, ol,
(uint64_t)(uintptr_t)i, (uint64_t)(uintptr_t)o };
return ioctl(fd, FWCTL_RPC, &r);
}
int main(void) {
uint8_t in[48] = {0}, out[8192] = {0}, o8[8] = {0};
struct hdr *h = (void *)in;
fd = open("/dev/fwctl/fwctl0", O_RDWR);
/* 1. enumerate features -> pick a real UUID with readable data
h->opcode = GET_SUP_FEATS; h->op_size = 8;
*(uint32_t *)(in + 16) = 48 * 64; /* get_sup_feats_in.c
rpc(48, in, sizeof(out), out);
int n = *(uint16_t *)(out + 8); /* num_entries */
struct feat_entry *e = (void *)(out + 16);
int b = 0;
for (int k = 0; k < n; k++) if (e[k].get_sz > e[b].get_sz) b = k
/* 2. Get Feature: out_len=8 (zero payload room) + non-zero coun
memset(in, 0, sizeof(in));
h->opcode = GET_FEATURE; h->op_size = 21;
struct get_feat_in *g = (void *)(in + 16);
memcpy(g->uuid, e[b].uuid, 16);
g->count = e[b].get_sz < 32 ? e[b].get_sz : 32;
return rpc(48, in, 8, o8); /* -> OOB write below
}
"""
[2]:
"""
BUG: KASAN: slab-out-of-bounds in memcpy_fromio+0x234/0x730
Write of size 8 at addr ffff00016b629648 by task repro_fwctl_get/3637
CPU: 41 UID: 0 PID: 3637 Comm: repro_fwctl_get Not tainted
7.1.0-rc7-cxltest-kasan+ #1 PREEMPT(full)
Call trace:
show_stack+0x28/0x48 (C)
__dump_stack+0x2c/0x50
dump_stack_lvl+0x80/0xb8
print_address_description+0x84/0x220
print_report+0x54/0x80
kasan_report+0xb4/0x130
__asan_report_store_n_noabort+0x20/0x38
memcpy_fromio+0x234/0x730
cxl_pci_mbox_send+0x6c4/0xc00
cxl_internal_send_cmd+0x114/0x228
cxl_get_feature+0x18c/0x2a0
cxlctl_fw_rpc+0x5f0/0xeb8
fwctl_cmd_rpc+0x2c0/0x598
fwctl_fops_ioctl+0x30c/0x4a0
__arm64_sys_ioctl+0xc08/0x1380
invoke_syscall+0x110/0x188
do_el0_svc+0x110/0x1b0
el0_svc+0x48/0x108
el0t_64_sync_handler+0x88/0x148
el0t_64_sync+0x1b8/0x1c0
Allocated by task 3637:
kasan_save_track+0x44/0x98
kasan_save_alloc_info+0x48/0x70
__kasan_kmalloc+0xa0/0xd0
__kvmalloc_node_noprof+0x3fc/0x6a0
cxlctl_fw_rpc+0x5b8/0xeb8
fwctl_cmd_rpc+0x2c0/0x598
fwctl_fops_ioctl+0x30c/0x4a0
__arm64_sys_ioctl+0xc08/0x1380
invoke_syscall+0x110/0x188
do_el0_svc+0x110/0x1b0
el0_svc+0x48/0x108
el0t_64_sync_handler+0x88/0x148
el0t_64_sync+0x1b8/0x1c0
The buggy address belongs to the object at ffff00016b629640
which belongs to the cache kmalloc-rnd-04-8 of size 8
The buggy address is located 0 bytes to the right of
allocated 8-byte region [ffff00016b629640, ffff00016b629648)
The buggy address belongs to the physical page:
page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x1eb62
flags: 0x17fffc000000000(node=0|zone=2|lastcpupid=0x1ffff)
page_type: f5(slab)
raw: 017fffc000000000 ffff000080016c00 dead000000000100 dead000000000122
raw: 0000000000000000 0000000808000800 00000000f5000000 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff00016b629500: fa fc fc fc fa fc fc fc fa fc fc fc 00 fc fc fc
ffff00016b629580: fa fc fc fc fa fc fc fc fc fc fc fc fa fc fc fc
>ffff00016b629600: fa fc fc fc fa fc fc fc 00 fc fc fc fa fc fc fc
^
ffff00016b629680: fa fc fc fc fa fc fc fc fc fc fc fc fa fc fc fc
ffff00016b629700: 00 fc fc fc fc fc fc fc fa fc fc fc fc fc fc fc
==================================================================
"""
Fixes: 5908f3ed6dc2 ("cxl: Add support to handle user feature commands for get feature")
Reviewed-by: Kai-Heng Feng <kaihengf@nvidia.com>
Reviewed-by: Koba Ko <kobak@nvidia.com>
Signed-off-by: Richard Cheng <icheng@nvidia.com>
---
drivers/cxl/core/features.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
index 3435db9ea6b1..3f949225da6d 100644
--- a/drivers/cxl/core/features.c
+++ b/drivers/cxl/core/features.c
@@ -471,6 +471,10 @@ static void *cxlctl_get_feature(struct cxl_features_state *cxlfs,
if (!count)
return ERR_PTR(-EINVAL);
+ if (out_size < offsetof(struct fwctl_rpc_cxl_out, payload) ||
+ count > out_size - offsetof(struct fwctl_rpc_cxl_out, payload))
+ return ERR_PTR(-EINVAL);
+
struct fwctl_rpc_cxl_out *rpc_out __free(kvfree) =
kvzalloc(out_size, GFP_KERNEL);
if (!rpc_out)
base-commit: 4549871118cf616eecdd2d939f78e3b9e1dddc48
--
2.43.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] cxl/features: Bound Get Feature read count to the output buffer
2026-06-11 9:49 [PATCH] cxl/features: Bound Get Feature read count to the output buffer Richard Cheng
@ 2026-06-11 15:28 ` Dave Jiang
2026-06-23 1:54 ` Richard Cheng
0 siblings, 1 reply; 4+ messages in thread
From: Dave Jiang @ 2026-06-11 15:28 UTC (permalink / raw)
To: Richard Cheng, dave, jic23, alison.schofield, vishal.l.verma, djbw
Cc: shiju.jose, ming.li, alucerop, linux-cxl, linux-kernel, newtonl,
kristinc, kaihengf, kobak
On 6/11/26 2:49 AM, Richard Cheng wrote:
> cxlctl_get_feature() sizes the output buffer rpc_out from the
> user-controlled fwctl_rpc.out_len, but the number of bytes the device is
> asked to write into rpc_out->payload comes from a separate
> user-controlled filed, cxl_mbox_get_feat_in.count. Nothing ties them
> together, so a small out_len with a large count makes cxl_get_feature()
> write the device's feature data past the kvzalloc()'d buffer. A heap OOB
> write reachable from FWCTL_RPC. Reject requests where count exceeds the
> available payload room, before the allocation.
>
>
> The issue is triggered via the following reproducer [1], with error log
> [2].
>
> [1]:
> """
> #include <fcntl.h>
> #include <stdint.h>
> #include <string.h>
> #include <sys/ioctl.h>
>
> #define FWCTL_RPC _IO(0x9A, 1)
> #define GET_SUP_FEATS 0x0500
> #define GET_FEATURE 0x0501
>
> struct fwctl_rpc { uint32_t size, scope, in_len, out_len; uint64_t in, o
> struct hdr { uint32_t opcode, flags, op_size, rsvd; }; /*
> struct get_feat_in { uint8_t uuid[16]; uint16_t off, count; uint8_t sel;
> __attribute__((packed)); /*
> struct feat_entry { uint8_t uuid[16]; uint16_t id, get_sz, set_sz;
> uint32_t flags; uint8_t gv, sv; uint16_t eff; uint8_t rsvd[18];
> __attribute__((packed)); /*
>
> static int fd;
> static int rpc(uint32_t il, void *i, uint32_t ol, void *o) {
> struct fwctl_rpc r = { sizeof(r), 0, il, ol,
> (uint64_t)(uintptr_t)i, (uint64_t)(uintptr_t)o };
> return ioctl(fd, FWCTL_RPC, &r);
> }
>
> int main(void) {
> uint8_t in[48] = {0}, out[8192] = {0}, o8[8] = {0};
> struct hdr *h = (void *)in;
> fd = open("/dev/fwctl/fwctl0", O_RDWR);
>
> /* 1. enumerate features -> pick a real UUID with readable data
> h->opcode = GET_SUP_FEATS; h->op_size = 8;
> *(uint32_t *)(in + 16) = 48 * 64; /* get_sup_feats_in.c
> rpc(48, in, sizeof(out), out);
> int n = *(uint16_t *)(out + 8); /* num_entries */
> struct feat_entry *e = (void *)(out + 16);
> int b = 0;
> for (int k = 0; k < n; k++) if (e[k].get_sz > e[b].get_sz) b = k
>
> /* 2. Get Feature: out_len=8 (zero payload room) + non-zero coun
> memset(in, 0, sizeof(in));
> h->opcode = GET_FEATURE; h->op_size = 21;
> struct get_feat_in *g = (void *)(in + 16);
> memcpy(g->uuid, e[b].uuid, 16);
> g->count = e[b].get_sz < 32 ? e[b].get_sz : 32;
> return rpc(48, in, 8, o8); /* -> OOB write below
> }
> """
>
> [2]:
> """
> BUG: KASAN: slab-out-of-bounds in memcpy_fromio+0x234/0x730
> Write of size 8 at addr ffff00016b629648 by task repro_fwctl_get/3637
>
> CPU: 41 UID: 0 PID: 3637 Comm: repro_fwctl_get Not tainted
> 7.1.0-rc7-cxltest-kasan+ #1 PREEMPT(full)
> Call trace:
> show_stack+0x28/0x48 (C)
> __dump_stack+0x2c/0x50
> dump_stack_lvl+0x80/0xb8
> print_address_description+0x84/0x220
> print_report+0x54/0x80
> kasan_report+0xb4/0x130
> __asan_report_store_n_noabort+0x20/0x38
> memcpy_fromio+0x234/0x730
> cxl_pci_mbox_send+0x6c4/0xc00
> cxl_internal_send_cmd+0x114/0x228
> cxl_get_feature+0x18c/0x2a0
> cxlctl_fw_rpc+0x5f0/0xeb8
> fwctl_cmd_rpc+0x2c0/0x598
> fwctl_fops_ioctl+0x30c/0x4a0
> __arm64_sys_ioctl+0xc08/0x1380
> invoke_syscall+0x110/0x188
> do_el0_svc+0x110/0x1b0
> el0_svc+0x48/0x108
> el0t_64_sync_handler+0x88/0x148
> el0t_64_sync+0x1b8/0x1c0
>
> Allocated by task 3637:
> kasan_save_track+0x44/0x98
> kasan_save_alloc_info+0x48/0x70
> __kasan_kmalloc+0xa0/0xd0
> __kvmalloc_node_noprof+0x3fc/0x6a0
> cxlctl_fw_rpc+0x5b8/0xeb8
> fwctl_cmd_rpc+0x2c0/0x598
> fwctl_fops_ioctl+0x30c/0x4a0
> __arm64_sys_ioctl+0xc08/0x1380
> invoke_syscall+0x110/0x188
> do_el0_svc+0x110/0x1b0
> el0_svc+0x48/0x108
> el0t_64_sync_handler+0x88/0x148
> el0t_64_sync+0x1b8/0x1c0
>
> The buggy address belongs to the object at ffff00016b629640
> which belongs to the cache kmalloc-rnd-04-8 of size 8
> The buggy address is located 0 bytes to the right of
> allocated 8-byte region [ffff00016b629640, ffff00016b629648)
>
> The buggy address belongs to the physical page:
> page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x1eb62
> flags: 0x17fffc000000000(node=0|zone=2|lastcpupid=0x1ffff)
> page_type: f5(slab)
> raw: 017fffc000000000 ffff000080016c00 dead000000000100 dead000000000122
> raw: 0000000000000000 0000000808000800 00000000f5000000 0000000000000000
> page dumped because: kasan: bad access detected
>
> Memory state around the buggy address:
> ffff00016b629500: fa fc fc fc fa fc fc fc fa fc fc fc 00 fc fc fc
> ffff00016b629580: fa fc fc fc fa fc fc fc fc fc fc fc fa fc fc fc
> >ffff00016b629600: fa fc fc fc fa fc fc fc 00 fc fc fc fa fc fc fc
> ^
> ffff00016b629680: fa fc fc fc fa fc fc fc fc fc fc fc fa fc fc fc
> ffff00016b629700: 00 fc fc fc fc fc fc fc fa fc fc fc fc fc fc fc
> ==================================================================
> """
>
> Fixes: 5908f3ed6dc2 ("cxl: Add support to handle user feature commands for get feature")
> Reviewed-by: Kai-Heng Feng <kaihengf@nvidia.com>
> Reviewed-by: Koba Ko <kobak@nvidia.com>
> Signed-off-by: Richard Cheng <icheng@nvidia.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
The oops log can probably be trimmed. And the reproducer isn't necessary in the commit log.
However, can the reproducer be add to CXL CLI unit test?
> ---
> drivers/cxl/core/features.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
> index 3435db9ea6b1..3f949225da6d 100644
> --- a/drivers/cxl/core/features.c
> +++ b/drivers/cxl/core/features.c
> @@ -471,6 +471,10 @@ static void *cxlctl_get_feature(struct cxl_features_state *cxlfs,
> if (!count)
> return ERR_PTR(-EINVAL);
>
> + if (out_size < offsetof(struct fwctl_rpc_cxl_out, payload) ||
> + count > out_size - offsetof(struct fwctl_rpc_cxl_out, payload))
> + return ERR_PTR(-EINVAL);
> +
> struct fwctl_rpc_cxl_out *rpc_out __free(kvfree) =
> kvzalloc(out_size, GFP_KERNEL);
> if (!rpc_out)
>
> base-commit: 4549871118cf616eecdd2d939f78e3b9e1dddc48
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] cxl/features: Bound Get Feature read count to the output buffer
2026-06-11 15:28 ` Dave Jiang
@ 2026-06-23 1:54 ` Richard Cheng
2026-06-23 14:10 ` Dave Jiang
0 siblings, 1 reply; 4+ messages in thread
From: Richard Cheng @ 2026-06-23 1:54 UTC (permalink / raw)
To: Dave Jiang
Cc: dave, jic23, alison.schofield, vishal.l.verma, djbw, shiju.jose,
ming.li, alucerop, linux-cxl, linux-kernel, newtonl, kristinc,
kaihengf, kobak
On Thu, Jun 11, 2026 at 08:28:00AM +0800, Dave Jiang wrote:
>
>
> On 6/11/26 2:49 AM, Richard Cheng wrote:
> > cxlctl_get_feature() sizes the output buffer rpc_out from the
> > user-controlled fwctl_rpc.out_len, but the number of bytes the device is
> > asked to write into rpc_out->payload comes from a separate
> > user-controlled filed, cxl_mbox_get_feat_in.count. Nothing ties them
> > together, so a small out_len with a large count makes cxl_get_feature()
> > write the device's feature data past the kvzalloc()'d buffer. A heap OOB
> > write reachable from FWCTL_RPC. Reject requests where count exceeds the
> > available payload room, before the allocation.
> >
> >
> > The issue is triggered via the following reproducer [1], with error log
> > [2].
> >
> > [1]:
> > """
> > #include <fcntl.h>
> > #include <stdint.h>
> > #include <string.h>
> > #include <sys/ioctl.h>
> >
> > #define FWCTL_RPC _IO(0x9A, 1)
> > #define GET_SUP_FEATS 0x0500
> > #define GET_FEATURE 0x0501
> >
> > struct fwctl_rpc { uint32_t size, scope, in_len, out_len; uint64_t in, o
> > struct hdr { uint32_t opcode, flags, op_size, rsvd; }; /*
> > struct get_feat_in { uint8_t uuid[16]; uint16_t off, count; uint8_t sel;
> > __attribute__((packed)); /*
> > struct feat_entry { uint8_t uuid[16]; uint16_t id, get_sz, set_sz;
> > uint32_t flags; uint8_t gv, sv; uint16_t eff; uint8_t rsvd[18];
> > __attribute__((packed)); /*
> >
> > static int fd;
> > static int rpc(uint32_t il, void *i, uint32_t ol, void *o) {
> > struct fwctl_rpc r = { sizeof(r), 0, il, ol,
> > (uint64_t)(uintptr_t)i, (uint64_t)(uintptr_t)o };
> > return ioctl(fd, FWCTL_RPC, &r);
> > }
> >
> > int main(void) {
> > uint8_t in[48] = {0}, out[8192] = {0}, o8[8] = {0};
> > struct hdr *h = (void *)in;
> > fd = open("/dev/fwctl/fwctl0", O_RDWR);
> >
> > /* 1. enumerate features -> pick a real UUID with readable data
> > h->opcode = GET_SUP_FEATS; h->op_size = 8;
> > *(uint32_t *)(in + 16) = 48 * 64; /* get_sup_feats_in.c
> > rpc(48, in, sizeof(out), out);
> > int n = *(uint16_t *)(out + 8); /* num_entries */
> > struct feat_entry *e = (void *)(out + 16);
> > int b = 0;
> > for (int k = 0; k < n; k++) if (e[k].get_sz > e[b].get_sz) b = k
> >
> > /* 2. Get Feature: out_len=8 (zero payload room) + non-zero coun
> > memset(in, 0, sizeof(in));
> > h->opcode = GET_FEATURE; h->op_size = 21;
> > struct get_feat_in *g = (void *)(in + 16);
> > memcpy(g->uuid, e[b].uuid, 16);
> > g->count = e[b].get_sz < 32 ? e[b].get_sz : 32;
> > return rpc(48, in, 8, o8); /* -> OOB write below
> > }
> > """
> >
> > [2]:
> > """
> > BUG: KASAN: slab-out-of-bounds in memcpy_fromio+0x234/0x730
> > Write of size 8 at addr ffff00016b629648 by task repro_fwctl_get/3637
> >
> > CPU: 41 UID: 0 PID: 3637 Comm: repro_fwctl_get Not tainted
> > 7.1.0-rc7-cxltest-kasan+ #1 PREEMPT(full)
> > Call trace:
> > show_stack+0x28/0x48 (C)
> > __dump_stack+0x2c/0x50
> > dump_stack_lvl+0x80/0xb8
> > print_address_description+0x84/0x220
> > print_report+0x54/0x80
> > kasan_report+0xb4/0x130
> > __asan_report_store_n_noabort+0x20/0x38
> > memcpy_fromio+0x234/0x730
> > cxl_pci_mbox_send+0x6c4/0xc00
> > cxl_internal_send_cmd+0x114/0x228
> > cxl_get_feature+0x18c/0x2a0
> > cxlctl_fw_rpc+0x5f0/0xeb8
> > fwctl_cmd_rpc+0x2c0/0x598
> > fwctl_fops_ioctl+0x30c/0x4a0
> > __arm64_sys_ioctl+0xc08/0x1380
> > invoke_syscall+0x110/0x188
> > do_el0_svc+0x110/0x1b0
> > el0_svc+0x48/0x108
> > el0t_64_sync_handler+0x88/0x148
> > el0t_64_sync+0x1b8/0x1c0
> >
> > Allocated by task 3637:
> > kasan_save_track+0x44/0x98
> > kasan_save_alloc_info+0x48/0x70
> > __kasan_kmalloc+0xa0/0xd0
> > __kvmalloc_node_noprof+0x3fc/0x6a0
> > cxlctl_fw_rpc+0x5b8/0xeb8
> > fwctl_cmd_rpc+0x2c0/0x598
> > fwctl_fops_ioctl+0x30c/0x4a0
> > __arm64_sys_ioctl+0xc08/0x1380
> > invoke_syscall+0x110/0x188
> > do_el0_svc+0x110/0x1b0
> > el0_svc+0x48/0x108
> > el0t_64_sync_handler+0x88/0x148
> > el0t_64_sync+0x1b8/0x1c0
> >
> > The buggy address belongs to the object at ffff00016b629640
> > which belongs to the cache kmalloc-rnd-04-8 of size 8
> > The buggy address is located 0 bytes to the right of
> > allocated 8-byte region [ffff00016b629640, ffff00016b629648)
> >
> > The buggy address belongs to the physical page:
> > page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x1eb62
> > flags: 0x17fffc000000000(node=0|zone=2|lastcpupid=0x1ffff)
> > page_type: f5(slab)
> > raw: 017fffc000000000 ffff000080016c00 dead000000000100 dead000000000122
> > raw: 0000000000000000 0000000808000800 00000000f5000000 0000000000000000
> > page dumped because: kasan: bad access detected
> >
> > Memory state around the buggy address:
> > ffff00016b629500: fa fc fc fc fa fc fc fc fa fc fc fc 00 fc fc fc
> > ffff00016b629580: fa fc fc fc fa fc fc fc fc fc fc fc fa fc fc fc
> > >ffff00016b629600: fa fc fc fc fa fc fc fc 00 fc fc fc fa fc fc fc
> > ^
> > ffff00016b629680: fa fc fc fc fa fc fc fc fc fc fc fc fa fc fc fc
> > ffff00016b629700: 00 fc fc fc fc fc fc fc fa fc fc fc fc fc fc fc
> > ==================================================================
> > """
> >
> > Fixes: 5908f3ed6dc2 ("cxl: Add support to handle user feature commands for get feature")
> > Reviewed-by: Kai-Heng Feng <kaihengf@nvidia.com>
> > Reviewed-by: Koba Ko <kobak@nvidia.com>
> > Signed-off-by: Richard Cheng <icheng@nvidia.com>
>
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
>
> The oops log can probably be trimmed. And the reproducer isn't necessary in the commit log.
>
> However, can the reproducer be add to CXL CLI unit test?
>
Hi Dave,
I want to make sure you want it as a in-tree unit test
or you want a follow up ndctl patch which can cooperate with
cxl_test ?
I suppose the latter will fit better ?
Best regards,
Richard Cheng.
>
> > ---
> > drivers/cxl/core/features.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
> > index 3435db9ea6b1..3f949225da6d 100644
> > --- a/drivers/cxl/core/features.c
> > +++ b/drivers/cxl/core/features.c
> > @@ -471,6 +471,10 @@ static void *cxlctl_get_feature(struct cxl_features_state *cxlfs,
> > if (!count)
> > return ERR_PTR(-EINVAL);
> >
> > + if (out_size < offsetof(struct fwctl_rpc_cxl_out, payload) ||
> > + count > out_size - offsetof(struct fwctl_rpc_cxl_out, payload))
> > + return ERR_PTR(-EINVAL);
> > +
> > struct fwctl_rpc_cxl_out *rpc_out __free(kvfree) =
> > kvzalloc(out_size, GFP_KERNEL);
> > if (!rpc_out)
> >
> > base-commit: 4549871118cf616eecdd2d939f78e3b9e1dddc48
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] cxl/features: Bound Get Feature read count to the output buffer
2026-06-23 1:54 ` Richard Cheng
@ 2026-06-23 14:10 ` Dave Jiang
0 siblings, 0 replies; 4+ messages in thread
From: Dave Jiang @ 2026-06-23 14:10 UTC (permalink / raw)
To: Richard Cheng
Cc: dave, jic23, alison.schofield, vishal.l.verma, djbw, shiju.jose,
ming.li, alucerop, linux-cxl, linux-kernel, newtonl, kristinc,
kaihengf, kobak
On 6/22/26 6:54 PM, Richard Cheng wrote:
> On Thu, Jun 11, 2026 at 08:28:00AM +0800, Dave Jiang wrote:
>>
>>
>> On 6/11/26 2:49 AM, Richard Cheng wrote:
>>> cxlctl_get_feature() sizes the output buffer rpc_out from the
>>> user-controlled fwctl_rpc.out_len, but the number of bytes the device is
>>> asked to write into rpc_out->payload comes from a separate
>>> user-controlled filed, cxl_mbox_get_feat_in.count. Nothing ties them
>>> together, so a small out_len with a large count makes cxl_get_feature()
>>> write the device's feature data past the kvzalloc()'d buffer. A heap OOB
>>> write reachable from FWCTL_RPC. Reject requests where count exceeds the
>>> available payload room, before the allocation.
>>>
>>>
>>> The issue is triggered via the following reproducer [1], with error log
>>> [2].
>>>
>>> [1]:
>>> """
>>> #include <fcntl.h>
>>> #include <stdint.h>
>>> #include <string.h>
>>> #include <sys/ioctl.h>
>>>
>>> #define FWCTL_RPC _IO(0x9A, 1)
>>> #define GET_SUP_FEATS 0x0500
>>> #define GET_FEATURE 0x0501
>>>
>>> struct fwctl_rpc { uint32_t size, scope, in_len, out_len; uint64_t in, o
>>> struct hdr { uint32_t opcode, flags, op_size, rsvd; }; /*
>>> struct get_feat_in { uint8_t uuid[16]; uint16_t off, count; uint8_t sel;
>>> __attribute__((packed)); /*
>>> struct feat_entry { uint8_t uuid[16]; uint16_t id, get_sz, set_sz;
>>> uint32_t flags; uint8_t gv, sv; uint16_t eff; uint8_t rsvd[18];
>>> __attribute__((packed)); /*
>>>
>>> static int fd;
>>> static int rpc(uint32_t il, void *i, uint32_t ol, void *o) {
>>> struct fwctl_rpc r = { sizeof(r), 0, il, ol,
>>> (uint64_t)(uintptr_t)i, (uint64_t)(uintptr_t)o };
>>> return ioctl(fd, FWCTL_RPC, &r);
>>> }
>>>
>>> int main(void) {
>>> uint8_t in[48] = {0}, out[8192] = {0}, o8[8] = {0};
>>> struct hdr *h = (void *)in;
>>> fd = open("/dev/fwctl/fwctl0", O_RDWR);
>>>
>>> /* 1. enumerate features -> pick a real UUID with readable data
>>> h->opcode = GET_SUP_FEATS; h->op_size = 8;
>>> *(uint32_t *)(in + 16) = 48 * 64; /* get_sup_feats_in.c
>>> rpc(48, in, sizeof(out), out);
>>> int n = *(uint16_t *)(out + 8); /* num_entries */
>>> struct feat_entry *e = (void *)(out + 16);
>>> int b = 0;
>>> for (int k = 0; k < n; k++) if (e[k].get_sz > e[b].get_sz) b = k
>>>
>>> /* 2. Get Feature: out_len=8 (zero payload room) + non-zero coun
>>> memset(in, 0, sizeof(in));
>>> h->opcode = GET_FEATURE; h->op_size = 21;
>>> struct get_feat_in *g = (void *)(in + 16);
>>> memcpy(g->uuid, e[b].uuid, 16);
>>> g->count = e[b].get_sz < 32 ? e[b].get_sz : 32;
>>> return rpc(48, in, 8, o8); /* -> OOB write below
>>> }
>>> """
>>>
>>> [2]:
>>> """
>>> BUG: KASAN: slab-out-of-bounds in memcpy_fromio+0x234/0x730
>>> Write of size 8 at addr ffff00016b629648 by task repro_fwctl_get/3637
>>>
>>> CPU: 41 UID: 0 PID: 3637 Comm: repro_fwctl_get Not tainted
>>> 7.1.0-rc7-cxltest-kasan+ #1 PREEMPT(full)
>>> Call trace:
>>> show_stack+0x28/0x48 (C)
>>> __dump_stack+0x2c/0x50
>>> dump_stack_lvl+0x80/0xb8
>>> print_address_description+0x84/0x220
>>> print_report+0x54/0x80
>>> kasan_report+0xb4/0x130
>>> __asan_report_store_n_noabort+0x20/0x38
>>> memcpy_fromio+0x234/0x730
>>> cxl_pci_mbox_send+0x6c4/0xc00
>>> cxl_internal_send_cmd+0x114/0x228
>>> cxl_get_feature+0x18c/0x2a0
>>> cxlctl_fw_rpc+0x5f0/0xeb8
>>> fwctl_cmd_rpc+0x2c0/0x598
>>> fwctl_fops_ioctl+0x30c/0x4a0
>>> __arm64_sys_ioctl+0xc08/0x1380
>>> invoke_syscall+0x110/0x188
>>> do_el0_svc+0x110/0x1b0
>>> el0_svc+0x48/0x108
>>> el0t_64_sync_handler+0x88/0x148
>>> el0t_64_sync+0x1b8/0x1c0
>>>
>>> Allocated by task 3637:
>>> kasan_save_track+0x44/0x98
>>> kasan_save_alloc_info+0x48/0x70
>>> __kasan_kmalloc+0xa0/0xd0
>>> __kvmalloc_node_noprof+0x3fc/0x6a0
>>> cxlctl_fw_rpc+0x5b8/0xeb8
>>> fwctl_cmd_rpc+0x2c0/0x598
>>> fwctl_fops_ioctl+0x30c/0x4a0
>>> __arm64_sys_ioctl+0xc08/0x1380
>>> invoke_syscall+0x110/0x188
>>> do_el0_svc+0x110/0x1b0
>>> el0_svc+0x48/0x108
>>> el0t_64_sync_handler+0x88/0x148
>>> el0t_64_sync+0x1b8/0x1c0
>>>
>>> The buggy address belongs to the object at ffff00016b629640
>>> which belongs to the cache kmalloc-rnd-04-8 of size 8
>>> The buggy address is located 0 bytes to the right of
>>> allocated 8-byte region [ffff00016b629640, ffff00016b629648)
>>>
>>> The buggy address belongs to the physical page:
>>> page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x1eb62
>>> flags: 0x17fffc000000000(node=0|zone=2|lastcpupid=0x1ffff)
>>> page_type: f5(slab)
>>> raw: 017fffc000000000 ffff000080016c00 dead000000000100 dead000000000122
>>> raw: 0000000000000000 0000000808000800 00000000f5000000 0000000000000000
>>> page dumped because: kasan: bad access detected
>>>
>>> Memory state around the buggy address:
>>> ffff00016b629500: fa fc fc fc fa fc fc fc fa fc fc fc 00 fc fc fc
>>> ffff00016b629580: fa fc fc fc fa fc fc fc fc fc fc fc fa fc fc fc
>>> >ffff00016b629600: fa fc fc fc fa fc fc fc 00 fc fc fc fa fc fc fc
>>> ^
>>> ffff00016b629680: fa fc fc fc fa fc fc fc fc fc fc fc fa fc fc fc
>>> ffff00016b629700: 00 fc fc fc fc fc fc fc fa fc fc fc fc fc fc fc
>>> ==================================================================
>>> """
>>>
>>> Fixes: 5908f3ed6dc2 ("cxl: Add support to handle user feature commands for get feature")
>>> Reviewed-by: Kai-Heng Feng <kaihengf@nvidia.com>
>>> Reviewed-by: Koba Ko <kobak@nvidia.com>
>>> Signed-off-by: Richard Cheng <icheng@nvidia.com>
>>
>> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
>>
>> The oops log can probably be trimmed. And the reproducer isn't necessary in the commit log.
>>
>> However, can the reproducer be add to CXL CLI unit test?
>>
>
> Hi Dave,
>
> I want to make sure you want it as a in-tree unit test
> or you want a follow up ndctl patch which can cooperate with
> cxl_test ?
>
> I suppose the latter will fit better ?
Follow up ndctl patch please. Thanks!
>
> Best regards,
> Richard Cheng.
>
>>
>>> ---
>>> drivers/cxl/core/features.c | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/cxl/core/features.c b/drivers/cxl/core/features.c
>>> index 3435db9ea6b1..3f949225da6d 100644
>>> --- a/drivers/cxl/core/features.c
>>> +++ b/drivers/cxl/core/features.c
>>> @@ -471,6 +471,10 @@ static void *cxlctl_get_feature(struct cxl_features_state *cxlfs,
>>> if (!count)
>>> return ERR_PTR(-EINVAL);
>>>
>>> + if (out_size < offsetof(struct fwctl_rpc_cxl_out, payload) ||
>>> + count > out_size - offsetof(struct fwctl_rpc_cxl_out, payload))
>>> + return ERR_PTR(-EINVAL);
>>> +
>>> struct fwctl_rpc_cxl_out *rpc_out __free(kvfree) =
>>> kvzalloc(out_size, GFP_KERNEL);
>>> if (!rpc_out)
>>>
>>> base-commit: 4549871118cf616eecdd2d939f78e3b9e1dddc48
>>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-06-23 14:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-11 9:49 [PATCH] cxl/features: Bound Get Feature read count to the output buffer Richard Cheng
2026-06-11 15:28 ` Dave Jiang
2026-06-23 1:54 ` Richard Cheng
2026-06-23 14:10 ` Dave Jiang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®