All of lore.kernel.org
 help / color / mirror / Atom feed
From: Richard Weinberger <richard@nod.at>
To: "Miguel García" <miguelgarciaroman8@gmail.com>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>,
	 Vignesh Raghavendra <vigneshr@ti.com>,
	 chengzhihao1 <chengzhihao1@huawei.com>,
	 linux-mtd <linux-mtd@lists.infradead.org>,
	 linux-kernel <linux-kernel@vger.kernel.org>,
	 Shuah Khan <skhan@linuxfoundation.org>
Subject: Re: [PATCH] mtd: ubi: replace strcpy with strscpy in mtd parameter parser
Date: Mon, 11 Aug 2025 14:46:29 +0200 (CEST)	[thread overview]
Message-ID: <1786903963.99788.1754916389068.JavaMail.zimbra@nod.at> (raw)
In-Reply-To: <20250811120912.144720-1-miguelgarciaroman8@gmail.com>

----- Ursprüngliche Mail -----
> Von: "Miguel García" <miguelgarciaroman8@gmail.com>
> An: "richard" <richard@nod.at>, "Miquel Raynal" <miquel.raynal@bootlin.com>, "Vignesh Raghavendra" <vigneshr@ti.com>
> CC: "chengzhihao1" <chengzhihao1@huawei.com>, "linux-mtd" <linux-mtd@lists.infradead.org>, "linux-kernel"
> <linux-kernel@vger.kernel.org>, "Shuah Khan" <skhan@linuxfoundation.org>, "Miguel García"
> <miguelgarciaroman8@gmail.com>
> Gesendet: Montag, 11. August 2025 14:09:12
> Betreff: [PATCH] mtd: ubi: replace strcpy with strscpy in mtd parameter parser

> Replace the strcpy() calls used to copy the 'mtd=' parameter into local
> buffers with strscpy() to avoid potential overflow and guarantee NUL
> termination. Destinations are fixed-size arrays (buf and p->name), so
> use sizeof().
> 
> While this code is currently safe due to prior length checks
> (strnlen(val, MTD_PARAM_LEN_MAX) and early return on overflow),
> replacing strcpy() with strscpy() follows current kernel best practices
> and makes the code more robust to future changes. The sizeof() calls
> correctly compute the buffer sizes, matching MTD_PARAM_LEN_MAX.

TBH, I'm not convinced. We're talking about kernel module parameters,
not hostile user input, etc...

By adding sizeof() you're replacing one foodgun with another one.
If buf is in future changed to a pointer, sizeof(buf) needs a fixup too.

Thanks,
//richard

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

WARNING: multiple messages have this Message-ID (diff)
From: Richard Weinberger <richard@nod.at>
To: "Miguel García" <miguelgarciaroman8@gmail.com>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>,
	 Vignesh Raghavendra <vigneshr@ti.com>,
	 chengzhihao1 <chengzhihao1@huawei.com>,
	 linux-mtd <linux-mtd@lists.infradead.org>,
	 linux-kernel <linux-kernel@vger.kernel.org>,
	 Shuah Khan <skhan@linuxfoundation.org>
Subject: Re: [PATCH] mtd: ubi: replace strcpy with strscpy in mtd parameter parser
Date: Mon, 11 Aug 2025 14:46:29 +0200 (CEST)	[thread overview]
Message-ID: <1786903963.99788.1754916389068.JavaMail.zimbra@nod.at> (raw)
In-Reply-To: <20250811120912.144720-1-miguelgarciaroman8@gmail.com>

----- Ursprüngliche Mail -----
> Von: "Miguel García" <miguelgarciaroman8@gmail.com>
> An: "richard" <richard@nod.at>, "Miquel Raynal" <miquel.raynal@bootlin.com>, "Vignesh Raghavendra" <vigneshr@ti.com>
> CC: "chengzhihao1" <chengzhihao1@huawei.com>, "linux-mtd" <linux-mtd@lists.infradead.org>, "linux-kernel"
> <linux-kernel@vger.kernel.org>, "Shuah Khan" <skhan@linuxfoundation.org>, "Miguel García"
> <miguelgarciaroman8@gmail.com>
> Gesendet: Montag, 11. August 2025 14:09:12
> Betreff: [PATCH] mtd: ubi: replace strcpy with strscpy in mtd parameter parser

> Replace the strcpy() calls used to copy the 'mtd=' parameter into local
> buffers with strscpy() to avoid potential overflow and guarantee NUL
> termination. Destinations are fixed-size arrays (buf and p->name), so
> use sizeof().
> 
> While this code is currently safe due to prior length checks
> (strnlen(val, MTD_PARAM_LEN_MAX) and early return on overflow),
> replacing strcpy() with strscpy() follows current kernel best practices
> and makes the code more robust to future changes. The sizeof() calls
> correctly compute the buffer sizes, matching MTD_PARAM_LEN_MAX.

TBH, I'm not convinced. We're talking about kernel module parameters,
not hostile user input, etc...

By adding sizeof() you're replacing one foodgun with another one.
If buf is in future changed to a pointer, sizeof(buf) needs a fixup too.

Thanks,
//richard

  reply	other threads:[~2025-08-11 12:47 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-11 12:09 [PATCH] mtd: ubi: replace strcpy with strscpy in mtd parameter parser Miguel García
2025-08-11 12:09 ` Miguel García
2025-08-11 12:46 ` Richard Weinberger [this message]
2025-08-11 12:46   ` Richard Weinberger
2025-08-19 20:46   ` Miguel García Román
2025-08-19 20:46     ` Miguel García Román

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1786903963.99788.1754916389068.JavaMail.zimbra@nod.at \
    --to=richard@nod.at \
    --cc=chengzhihao1@huawei.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=miguelgarciaroman8@gmail.com \
    --cc=miquel.raynal@bootlin.com \
    --cc=skhan@linuxfoundation.org \
    --cc=vigneshr@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.