On 12/27/22 23:43, James Bottomley wrote:
On Tue, 2022-12-27 at 22:38 +0100, Helge Deller wrote:
Hi James,
On 12/27/22 13:38, James Bottomley wrote:
On Fri, 2022-12-23 at 08:55 +0100, Helge Deller wrote:
On 12/23/22 03:40, yang.yang29@xxxxxxxxxx wrote:
From: Xu Panda <xu.panda@xxxxxxxxxx>
The implementation of strscpy() is more robust and safer.
That's now the recommended way to copy NUL-terminated strings.
Thanks for your patch, but....
Signed-off-by: Xu Panda <xu.panda@xxxxxxxxxx>
Signed-off-by: Yang Yang <yang.yang29@xxxxxxx>
---
drivers/parisc/pdc_stable.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/parisc/pdc_stable.c
b/drivers/parisc/pdc_stable.c
index d6af5726ddf3..403bca0021c5 100644
--- a/drivers/parisc/pdc_stable.c
+++ b/drivers/parisc/pdc_stable.c
@@ -274,8 +274,7 @@ pdcspath_hwpath_write(struct pdcspath_entry
*entry, const char *buf, size_t coun
/* We'll use a local copy of buf */
count = min_t(size_t, count, sizeof(in)-1);
- strncpy(in, buf, count);
- in[count] = '\0';
+ strscpy(in, buf, count + 1);
could you resend it somewhat simplified, e.g.
strscpy(in, buf, sizeof(in));
I don't think you can: count is the size of buf, if that's <
sizeof(in) you've introduced a write beyond end of buffer. In fact
sysfs tends to pass pages as buffers, so there's no actual problem,
but if that ever changed ...
Huh?... he doesn't change "count", so what's wrong with the latest
patch?
the array buf[] is actually buf[count], so if count < 64 then
sizeof(buf) < sizeof(in) and you're copying whatever is after buf on
the stack or wherever it comes from. The amount you copy into in[]
truly has to be the smaller of count and sizeof(in). These are file
operations, so you shouldn't rely on buf[] being null terminated
Ok, the main point I missed was that buf[] might not be null terminated.
Thanks for the explanation.
Yang & Xu, no need to resend the patch. I'll take your v1 version.
Thanks!
Helge
(kernfs ensures it is, but it's a dangerous thing to rely on in the
face of someone trying to exploit a stack smashing attack).
James
