[PATCH v2] mtd: sm_ftl: replace strncpy with memcpy

[Rahul Kumar]

Hi all,

Thanks a lot for the detailed feedback on my patch. I understand now
that this change does not add much value, and I’ll keep your points in
mind for future contributions. I really appreciate the guidance.

Best,
Rahul

On Tue, Sep 9, 2025 at 5:15 PM Pratyush Yadav <pratyush at kernel.org> wrote:
>
> On Mon, Sep 08 2025, Richard Weinberger wrote:
>
> > ----- Ursprüngliche Mail -----
> >> Von: "Miquel Raynal" <miquel.raynal at bootlin.com>
> >> An: "Rahul Kumar" <rk0006818 at gmail.com>
> >>> -   strncpy(buf, sm_attr->data, sm_attr->len);
> >>> -   return sm_attr->len;
> >>> +   memcpy(buf, sm_attr->data, sm_attr->len);
> >>> +   buf[sm_attr->len] = '\0';
> >>> +   return sm_attr->len + 1;
> >>
> >> Are we sure the buffer is always sm_attr->len + 1 long?
>
> Yeah, that was what I was wondering on the original patch too. Poking
> around the code, I see in dev_attr_show() that:
>
>         if (dev_attr->show)
>                 ret = dev_attr->show(dev, dev_attr, buf);
>         if (ret >= (ssize_t)PAGE_SIZE) {
>                 printk("dev_attr_show: %pS returned bad count\n",
>                                 dev_attr->show);
>         }
>
> So I suppose the buffer is PAGE_SIZE long, though the show() API is
> kinda poor for not reporting the size explicitly. If we do really want
> to make this safer, I suppose this is what should be checked for. The
> strncpy with length being limited to the string length instead of buffer
> length is completely useless.
>
> Anyway, here sm_attr->data is (SM_SMALL_PAGE - SM_CIS_VENDOR_OFFSET + 1)
> bytes long, which should add up to 168 bytes, which is perfectly safe to
> just copy.
>
> >
> > Can we please just stop messing with perfectly fine code?
> > I'm sick of the war on string functions.
> > First we had to replace everything with strncpy(), then strlcpy(),
> > then strscpy(), ...
>
> I had a similar reaction initally too. But TBH if the patch made the
> code actually safer I reckon it would be fine. Long term, these kind of
> things can help prevent unexpected bugs. But here we don't even know how
> large buf is so there is no point in using any of the fancy string
> functions. A plain strcpy(buf, str) is just as "safe" as a
> strncpy(buf, str, strlen(str)), or any other fancy variation.
>
> So yeah, I don't think this patch does much...
>
> That said, Rahul you seem like a new contributor. I would say don't be
> too discouraged. Not every patch ends up going through. I have had my
> fair share of rejected patches. Take this as a learning and keep looking
> for other improvements :-)
>
> >
> > Don't get me wrong, I'm all for hardening code paths where
> > strings are arbitrary input, but in many of these cases all strings
> > are no input or already sanitized.
>
> --
> Regards,
> Pratyush Yadav