When simple_write_to_buffer() succeeds, it returns the number of bytes actually copied to the buffer, which may be less than the requested 'count' if the buffer size is insufficient. However, the current code incorrectly uses 'count' as the index for null termination instead of the actual bytes copied, leading to out-of-bound write.
Add a check for the count and use the return value as the index.
Found via static analysis. This is similar to the commit da9374819eb3 ("iio: backend: fix out-of-bound write")
Fixes: b1c5d68ea66e ("iio: dac: ad3552r-hs: add support for internal ramp") Cc: stable@vger.kernel.org Signed-off-by: Miaoqian Lin linmq006@gmail.com --- drivers/iio/dac/ad3552r-hs.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c index 41b96b48ba98..a9578afa7015 100644 --- a/drivers/iio/dac/ad3552r-hs.c +++ b/drivers/iio/dac/ad3552r-hs.c @@ -549,12 +549,15 @@ static ssize_t ad3552r_hs_write_data_source(struct file *f,
guard(mutex)(&st->lock);
+ if (count >= sizeof(buf)) + return -ENOSPC; + ret = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf, count); if (ret < 0) return ret;
- buf[count] = '\0'; + buf[ret] = '\0';
ret = match_string(dbgfs_attr_source, ARRAY_SIZE(dbgfs_attr_source), buf);
On 10/27/25 10:07 AM, Miaoqian Lin wrote:
When simple_write_to_buffer() succeeds, it returns the number of bytes actually copied to the buffer, which may be less than the requested 'count' if the buffer size is insufficient. However, the current code incorrectly uses 'count' as the index for null termination instead of the actual bytes copied, leading to out-of-bound write.
Add a check for the count and use the return value as the index.
Found via static analysis. This is similar to the commit da9374819eb3 ("iio: backend: fix out-of-bound write")
Fixes: b1c5d68ea66e ("iio: dac: ad3552r-hs: add support for internal ramp") Cc: stable@vger.kernel.org Signed-off-by: Miaoqian Lin linmq006@gmail.com
Reviewed-by: David Lechner dlechner@baylibre.com
On Mon, 2025-10-27 at 23:07 +0800, Miaoqian Lin wrote:
When simple_write_to_buffer() succeeds, it returns the number of bytes actually copied to the buffer, which may be less than the requested 'count' if the buffer size is insufficient. However, the current code incorrectly uses 'count' as the index for null termination instead of the actual bytes copied, leading to out-of-bound write.
Add a check for the count and use the return value as the index.
Found via static analysis. This is similar to the commit da9374819eb3 ("iio: backend: fix out-of-bound write")
Fixes: b1c5d68ea66e ("iio: dac: ad3552r-hs: add support for internal ramp") Cc: stable@vger.kernel.org Signed-off-by: Miaoqian Lin linmq006@gmail.com
Reviewed-by: Nuno Sá nuno.sa@analog.com
drivers/iio/dac/ad3552r-hs.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c index 41b96b48ba98..a9578afa7015 100644 --- a/drivers/iio/dac/ad3552r-hs.c +++ b/drivers/iio/dac/ad3552r-hs.c @@ -549,12 +549,15 @@ static ssize_t ad3552r_hs_write_data_source(struct file *f, guard(mutex)(&st->lock);
- if (count >= sizeof(buf))
return -ENOSPC;ret = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf, count); if (ret < 0) return ret;
- buf[count] = '\0';
- buf[ret] = '\0';
ret = match_string(dbgfs_attr_source, ARRAY_SIZE(dbgfs_attr_source), buf);
On Mon, Oct 27, 2025 at 11:07:13PM +0800, Miaoqian Lin wrote:
When simple_write_to_buffer() succeeds, it returns the number of bytes actually copied to the buffer, which may be less than the requested 'count' if the buffer size is insufficient. However, the current code incorrectly uses 'count' as the index for null termination instead of the actual bytes copied, leading to out-of-bound write.
Add a check for the count and use the return value as the index.
...
- if (count >= sizeof(buf))
return -ENOSPC;
But this makes the validation too strict now.
ret = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf, count);
You definitely failed to read the code that implements the above.
if (ret < 0) return ret;
- buf[count] = '\0';
- buf[ret] = '\0';
NAK.
This patch is an unneeded churn.
On Tue, Oct 28, 2025 at 10:18:05AM +0200, Andy Shevchenko wrote:
On Mon, Oct 27, 2025 at 11:07:13PM +0800, Miaoqian Lin wrote:
...
- if (count >= sizeof(buf))
return -ENOSPC;But this makes the validation too strict now.
ret = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf, count);
You definitely failed to read the code that implements the above.
if (ret < 0) return ret;
- buf[count] = '\0';
- buf[ret] = '\0';
Maybe this line is what we might need, but I haven't checked deeper if it's a problem.
NAK.
This patch is an unneeded churn.
On Tue, Oct 28, 2025 at 10:19:27AM +0200, Andy Shevchenko wrote:
On Tue, Oct 28, 2025 at 10:18:05AM +0200, Andy Shevchenko wrote:
On Mon, Oct 27, 2025 at 11:07:13PM +0800, Miaoqian Lin wrote:
+Cc: Markus Burri for the da9374819eb3
...
- if (count >= sizeof(buf))
return -ENOSPC;But this makes the validation too strict now.
ret = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf, count);
You definitely failed to read the code that implements the above.
if (ret < 0) return ret;
- buf[count] = '\0';
- buf[ret] = '\0';
Maybe this line is what we might need, but I haven't checked deeper if it's a problem.
So, copy_to_user() and copy_from_user() are always inlined macros. The simple_write_to_buffer() is not. The question here is how the __builit_object_size() will behave on the address given as a parameter to copy_from_user() in simple_write_to_buffer().
If it may detect reliably that the buffer is the size it has. I believe it's easy for the byte arrays on stack.
That said, without proof that compiler is unable to determine the destination buffer size, this patch and the one by Markus are simple noise which actually changes an error code on the overflow condition.
The only line that assigns NUL character might be useful in some cases (definitely when buffer comes through indirect calls from a heap, etc).
NAK.
This patch is an unneeded churn.
Hi, Andy
Thanks for you review.
Andy Shevchenko andriy.shevchenko@intel.com 于2025年10月28日周二 17:07写道：
On Tue, Oct 28, 2025 at 10:19:27AM +0200, Andy Shevchenko wrote:
On Tue, Oct 28, 2025 at 10:18:05AM +0200, Andy Shevchenko wrote:
On Mon, Oct 27, 2025 at 11:07:13PM +0800, Miaoqian Lin wrote:
+Cc: Markus Burri for the da9374819eb3
...
- if (count >= sizeof(buf))
return -ENOSPC;But this makes the validation too strict now.
ret = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf, count);
You definitely failed to read the code that implements the above.
I previously read the simple_write_to_buffer(), but add the check and think it can help to catch error eariler. My mistake.
if (ret < 0) return ret;
- buf[count] = '\0';
- buf[ret] = '\0';
Maybe this line is what we might need, but I haven't checked deeper if it's a problem.
So, copy_to_user() and copy_from_user() are always inlined macros. The simple_write_to_buffer() is not. The question here is how the __builit_object_size() will behave on the address given as a parameter to copy_from_user() in simple_write_to_buffer().
If it may detect reliably that the buffer is the size it has. I believe it's easy for the byte arrays on stack.
That said, without proof that compiler is unable to determine the destination buffer size, this patch and the one by Markus are simple noise which actually changes an error code on the overflow condition.
The only line that assigns NUL character might be useful in some cases (definitely when buffer comes through indirect calls from a heap, etc).
I believe it is still necessray to use buf[ret] = '\0'; intead of buf[count] = '\0'; If you argee with this, I send a v2 with just this fix. Thanks.
NAK.
This patch is an unneeded churn.
-- With Best Regards, Andy Shevchenko
On Tue, Oct 28, 2025 at 05:46:53PM +0800, 林妙倩 wrote:
Andy Shevchenko andriy.shevchenko@intel.com 于2025年10月28日周二 17:07写道：
On Tue, Oct 28, 2025 at 10:19:27AM +0200, Andy Shevchenko wrote:
On Tue, Oct 28, 2025 at 10:18:05AM +0200, Andy Shevchenko wrote:
On Mon, Oct 27, 2025 at 11:07:13PM +0800, Miaoqian Lin wrote:
...
- if (count >= sizeof(buf))
return -ENOSPC;But this makes the validation too strict now.
ret = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf, count);
You definitely failed to read the code that implements the above.
I previously read the simple_write_to_buffer(), but add the check and think it can help to catch error eariler. My mistake.
if (ret < 0) return ret;
- buf[count] = '\0';
- buf[ret] = '\0';
Maybe this line is what we might need, but I haven't checked deeper if it's a problem.
So, copy_to_user() and copy_from_user() are always inlined macros. The simple_write_to_buffer() is not. The question here is how the __builit_object_size() will behave on the address given as a parameter to copy_from_user() in simple_write_to_buffer().
If it may detect reliably that the buffer is the size it has. I believe it's easy for the byte arrays on stack.
That said, without proof that compiler is unable to determine the destination buffer size, this patch and the one by Markus are simple noise which actually changes an error code on the overflow condition.
The only line that assigns NUL character might be useful in some cases (definitely when buffer comes through indirect calls from a heap, etc).
I believe it is still necessray to use buf[ret] = '\0'; intead of buf[count] = '\0'; If you argee with this, I send a v2 with just this fix. Thanks.
As explained above, please try to model the situation and see if current code is buggy, i.e. provide a step-by-step test case and show a traceback that points to a out-of-boundary access in this function. (Note, you don't need to have a HW for that, you might need to create a dummy IIO or other module with the similar interface and run it, in such a case, share also link to the source code of that module.) When you prove the problem exists, I will happily ACK all similar patches, including yours.
NAK.
This patch is an unneeded churn.
On Tue, 2025-10-28 at 11:07 +0200, Andy Shevchenko wrote:
On Tue, Oct 28, 2025 at 10:19:27AM +0200, Andy Shevchenko wrote:
On Tue, Oct 28, 2025 at 10:18:05AM +0200, Andy Shevchenko wrote:
On Mon, Oct 27, 2025 at 11:07:13PM +0800, Miaoqian Lin wrote:
+Cc: Markus Burri for the da9374819eb3
...
- if (count >= sizeof(buf))
return -ENOSPC;But this makes the validation too strict now.
ret = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf, count);
You definitely failed to read the code that implements the above.
if (ret < 0) return ret;
- buf[count] = '\0';
- buf[ret] = '\0';
Maybe this line is what we might need, but I haven't checked deeper if it's a problem.
So, copy_to_user() and copy_from_user() are always inlined macros. The simple_write_to_buffer() is not. The question here is how the __builit_object_size() will behave on the address given as a parameter to copy_from_user() in simple_write_to_buffer().
If it may detect reliably that the buffer is the size it has. I believe it's easy for the byte arrays on stack.
I think the above does not make sense (unless I'm missing your point which might very well be). So, __builit_object_size() is for things known at compile time. Moreover, simple_write_to_buffer() already has all of the gymnastics to make sure copy_from_user() has the proper parameters. The thing is that it does it in a "silent" way which means that if your buffer is not big enough you'll get a concatenated string. Sure, you'll likely get an error down the road (due to an invalid value) but I do see some value in returning back the root cause of the issue.
So, the preliminary check while not being a big deal, it's also not completely useless IMO. I do not have any strong feeling though. However, I think the below is very much needed...
That said, without proof that compiler is unable to determine the destination buffer size, this patch and the one by Markus are simple noise which actually changes an error code on the overflow condition.
The only line that assigns NUL character might be useful in some cases (definitely when buffer comes through indirect calls from a heap, etc).
I think you can easily pass a string >= than 64 bytes (from userspace). AFAIR, you don't really set a size into debugfs files. For sure you can mess things with zero sized binary attributes so I have some confidence you have the same with debugfs.
And even if all the above is not reproducible I'm still of the opinion that
buf[ret] = '\0';
is semantically the correct code.
- Nuno Sá
On Tue, Oct 28, 2025 at 12:31:04PM +0000, Nuno Sá wrote:
On Tue, 2025-10-28 at 11:07 +0200, Andy Shevchenko wrote:
On Tue, Oct 28, 2025 at 10:19:27AM +0200, Andy Shevchenko wrote:
On Tue, Oct 28, 2025 at 10:18:05AM +0200, Andy Shevchenko wrote:
On Mon, Oct 27, 2025 at 11:07:13PM +0800, Miaoqian Lin wrote:
...
- if (count >= sizeof(buf))
return -ENOSPC;But this makes the validation too strict now.
ret = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf, count);
You definitely failed to read the code that implements the above.
if (ret < 0) return ret;
- buf[count] = '\0';
- buf[ret] = '\0';
Maybe this line is what we might need, but I haven't checked deeper if it's a problem.
So, copy_to_user() and copy_from_user() are always inlined macros. The simple_write_to_buffer() is not. The question here is how the __builit_object_size() will behave on the address given as a parameter to copy_from_user() in simple_write_to_buffer().
If it may detect reliably that the buffer is the size it has. I believe it's easy for the byte arrays on stack.
I think the above does not make sense (unless I'm missing your point which might very well be).
It seems I stand corrected. I was staring too much at copy_from_user() without retrieving the validation logic behind simple_write_to_buffer().
So, __builit_object_size() is for things known at compile time. Moreover, simple_write_to_buffer() already has all of the gymnastics to make sure copy_from_user() has the proper parameters. The thing is that it does it in a "silent" way which means that if your buffer is not big enough you'll get a concatenated string. Sure, you'll likely get an error down the road (due to an invalid value) but I do see some value in returning back the root cause of the issue.
So, the preliminary check while not being a big deal, it's also not completely useless IMO. I do not have any strong feeling though. However, I think the below is very much needed...
That said, without proof that compiler is unable to determine the destination buffer size, this patch and the one by Markus are simple noise which actually changes an error code on the overflow condition.
The only line that assigns NUL character might be useful in some cases (definitely when buffer comes through indirect calls from a heap, etc).
I think you can easily pass a string >= than 64 bytes (from userspace). AFAIR, you don't really set a size into debugfs files. For sure you can mess things with zero sized binary attributes so I have some confidence you have the same with debugfs.
And even if all the above is not reproducible I'm still of the opinion that
buf[ret] = '\0';
is semantically the correct code.
Yes, but it should either be explained as just making code robust vs. real bugfix. For the latter I want to see the real traceback and a reproducer. I also wonder why we never had reports from syzkaller on this. It has non-zero chance to stumble over the issue here (if there is an issue to begin with).
On Tue, 2025-10-28 at 16:45 +0200, Andy Shevchenko wrote:
On Tue, Oct 28, 2025 at 12:31:04PM +0000, Nuno Sá wrote:
On Tue, 2025-10-28 at 11:07 +0200, Andy Shevchenko wrote:
On Tue, Oct 28, 2025 at 10:19:27AM +0200, Andy Shevchenko wrote:
On Tue, Oct 28, 2025 at 10:18:05AM +0200, Andy Shevchenko wrote:
On Mon, Oct 27, 2025 at 11:07:13PM +0800, Miaoqian Lin wrote:
...
- if (count >= sizeof(buf))
return -ENOSPC;But this makes the validation too strict now.
ret = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf, count);
You definitely failed to read the code that implements the above.
if (ret < 0) return ret;
- buf[count] = '\0';
- buf[ret] = '\0';
Maybe this line is what we might need, but I haven't checked deeper if it's a problem.
So, copy_to_user() and copy_from_user() are always inlined macros. The simple_write_to_buffer() is not. The question here is how the __builit_object_size() will behave on the address given as a parameter to copy_from_user() in simple_write_to_buffer().
If it may detect reliably that the buffer is the size it has. I believe it's easy for the byte arrays on stack.
I think the above does not make sense (unless I'm missing your point which might very well be).
It seems I stand corrected. I was staring too much at copy_from_user() without retrieving the validation logic behind simple_write_to_buffer().
:)
...
I think you can easily pass a string >= than 64 bytes (from userspace). AFAIR, you don't really set a size into debugfs files. For sure you can mess things with zero sized binary attributes so I have some confidence you have the same with debugfs.
And even if all the above is not reproducible I'm still of the opinion that
buf[ret] = '\0';
is semantically the correct code.
Yes, but it should either be explained as just making code robust vs. real bugfix.
Agreed. If we find it's the former, the commit message should be updated.
For the latter I want to see the real traceback and a reproducer. I also wonder why we never had reports from syzkaller on this. It has non-zero chance to stumble over the issue here (if there is an issue to begin with).
If I have the time, I might do it. If my suspicious are correct, it should be fairly easy to reproduce.
- Nuno Sá
On Tue, Oct 28, 2025 at 03:12:29PM +0000, Nuno Sá wrote:
On Tue, 2025-10-28 at 16:45 +0200, Andy Shevchenko wrote:
On Tue, Oct 28, 2025 at 12:31:04PM +0000, Nuno Sá wrote:
...
For the latter I want to see the real traceback and a reproducer. I also wonder why we never had reports from syzkaller on this. It has non-zero chance to stumble over the issue here (if there is an issue to begin with).
If I have the time, I might do it. If my suspicious are correct, it should be fairly easy to reproduce.
My suspicious is also like this, if you have a working setup for one of such a user (like this chip) already, it's ~15 minutes to get it done without writing an additional code.
Hi,
I don’t have the actual hardware, so I built a similar demo module to mirror the bug and ran it in QEMU. With KASAN enabled, the PoC triggers BUG: KASAN: stack-out-of-bounds.
Pattern of the bug: - A fixed 64-byte stack buffer is filled using count. - If count > 64, the code still does buf[count] = '\0', causing an out-of-bounds write on the stack.
PoC (what it does): - Opens the device node. - Writes 128 bytes of A to it. - This overflows the 64-byte stack buffer and KASAN reports the stack OOB.
If you have the real device, you may run the similar PoC on your driver to validate—just ensure KASAN is enabled to see the report. I also tested the straightforward fix buf[ret] = '\0'; with that change, the issue no longer reproduces. Below are the trace, the demo module, and the PoC for reference.
[ 11.824318] overflow_demo: copied 63 bytes (stack buf 64, user count 128) [ 11.825125] ================================================================== [ 11.825806] BUG: KASAN: stack-out-of-bounds in mas_walk+0x466/0x510 [ 11.825806] Write of size 8 at addr ffff888011017e48 by task overflow_demo_p/75 [ 11.825806] [ 11.825806] CPU: 0 PID: 75 Comm: overflow_demo_p Tainted: G O 6.6.9 #4 [ 11.825806] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014 [ 11.825806] Call Trace: [ 11.825806] <TASK> [ 11.825806] dump_stack_lvl+0x36/0x50 [ 11.825806] print_report+0xcf/0x670 [ 11.825806] ? __pfx__raw_spin_lock_irqsave+0x10/0x10 [ 11.825806] kasan_report+0xc7/0x100 [ 11.825806] ? mas_walk+0x466/0x510 [ 11.825806] ? mas_walk+0x466/0x510 [ 11.825806] mas_walk+0x466/0x510 [ 11.825806] ? __pfx_rebalance_domains+0x10/0x10 [ 11.825806] lock_vma_under_rcu+0xbe/0x420 [ 11.825806] ? __pfx_lock_vma_under_rcu+0x10/0x10 [ 11.825806] ? __do_softirq+0x1bf/0x5b2 [ 11.825806] ? hrtimer_interrupt+0x313/0x7c0 [ 11.825806] do_user_addr_fault+0x1c6/0x940 [ 11.825806] exc_page_fault+0x5d/0xd0 [ 11.825806] asm_exc_page_fault+0x26/0x30 [ 11.825806] RIP: 0033:0x43f430 [ 11.825806] Code: Unable to access opcode bytes at 0x43f406. [ 11.825806] RSP: 002b:00007ffe2f3b53b8 EFLAGS: 00000202 [ 11.825806] RAX: 00007ffe2f3b53e0 RBX: 0000000000000003 RCX: 0000000000000002 [ 11.825806] RDX: 00007ffe2f3b53c0 RSI: 0000000000486029 RDI: 00000000004b2320 [ 11.825806] RBP: 00007ffe2f3b5490 R08: 00000000004b2820 R09: 0000000000000110 [ 11.825806] R10: 0000000000000000 R11: 0000000000000202 R12: 00007ffe2f3b54a0 [ 11.825806] R13: 00007ffe2f3b5668 R14: 00000000004ad868 R15: 0000000000000001 [ 11.825806] </TASK> [ 11.825806] [ 11.825806] The buggy address belongs to stack of task overflow_demo_p/75 [ 11.825806] and is located at offset 56 in frame: [ 11.825806] lock_vma_under_rcu+0x0/0x420 [ 11.825806] [ 11.825806] This frame has 1 object: [ 11.825806] [32, 96) 'mas' [ 11.825806] [ 11.825806] The buggy address belongs to the physical page: [ 11.825806] page:(____ptrval____) refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x11017 [ 11.825806] flags: 0x100000000000000(node=0|zone=1) [ 11.825806] page_type: 0xffffffff() [ 11.825806] raw: 0100000000000000 dead000000000100 dead000000000122 0000000000000000 [ 11.825806] raw: 0000000000000000 0000000000000000 00000000ffffffff 0000000000000000 [ 11.825806] page dumped because: kasan: bad access detected [ 11.825806] [ 11.825806] Memory state around the buggy address: [ 11.825806] ffff888011017d00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 11.825806] ffff888011017d80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 11.825806] >ffff888011017e00: 00 00 f1 f1 f1 f1 00 00 00 f3 00 00 00 00 f3 f3 [ 11.825806] ^ [ 11.825806] ffff888011017e80: f3 f3 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 11.825806] ffff888011017f00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 11.825806] ================================================================== [ 11.858618] Disabling lock debugging due to kernel taint [ 11.892349] overflow_demo_p (75) used greatest stack depth: 27192 bytes left
Andy Shevchenko andriy.shevchenko@intel.com 于2025年10月28日周二 23:19写道：
On Tue, Oct 28, 2025 at 03:12:29PM +0000, Nuno Sá wrote:
On Tue, 2025-10-28 at 16:45 +0200, Andy Shevchenko wrote:
On Tue, Oct 28, 2025 at 12:31:04PM +0000, Nuno Sá wrote:
...
For the latter I want to see the real traceback and a reproducer. I also wonder why we never had reports from syzkaller on this. It has non-zero chance to stumble over the issue here (if there is an issue to begin with).
If I have the time, I might do it. If my suspicious are correct, it should be fairly easy to reproduce.
My suspicious is also like this, if you have a working setup for one of such a user (like this chip) already, it's ~15 minutes to get it done without writing an additional code.
-- With Best Regards, Andy Shevchenko
On Wed, Dec 17, 2025 at 02:47:17PM +0800, 林妙倩 wrote:
Hi,
I don’t have the actual hardware, so I built a similar demo module to mirror the bug and ran it in QEMU. With KASAN enabled, the PoC triggers BUG: KASAN: stack-out-of-bounds.
Pattern of the bug:
- A fixed 64-byte stack buffer is filled using count.
- If count > 64, the code still does buf[count] = '\0', causing an
out-of-bounds write on the stack.
PoC (what it does):
- Opens the device node.
- Writes 128 bytes of A to it.
- This overflows the 64-byte stack buffer and KASAN reports the stack OOB.
If you have the real device, you may run the similar PoC on your driver to validate—just ensure KASAN is enabled to see the report. I also tested the straightforward fix buf[ret] = '\0'; with that change, the issue no longer reproduces. Below are the trace, the demo module, and the PoC for reference.
Thanks for the additional information, I think it would be good to have a summary of it in the commit message of the fix.
On Mon, Oct 27, 2025 at 11:07:13PM +0800, Miaoqian Lin wrote:
When simple_write_to_buffer() succeeds, it returns the number of bytes actually copied to the buffer, which may be less than the requested 'count' if the buffer size is insufficient. However, the current code incorrectly uses 'count' as the index for null termination instead of the actual bytes copied, leading to out-of-bound write.
Add a check for the count and use the return value as the index.
...
- if (count >= sizeof(buf))
return -ENOSPC;
This is already done below.
ret = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, userbuf, count); if (ret < 0) return ret;
...
- buf[count] = '\0';
- buf[ret] = '\0';
Do we have an actual issue right now? Can you model it and show a real traceback?
linux-stable-mirror@lists.linaro.org
-
Andy Shevchenko -
David Lechner -
Miaoqian Lin -
Nuno Sá -
林妙倩