Linux Input/HID development
 help / color / mirror / Atom feed
From: Alejandro Colomar <alx+linux-hardening@kernel.org>
To: David Laight <david.laight.linux@gmail.com>
Cc: Mariia Nikitash <nikitash.mariiaw@gmail.com>,
	 dmitry.torokhov@gmail.com, linux-input@vger.kernel.org,
	linux-kernel@vger.kernel.org,  linux-hardening@vger.kernel.org,
	kees@kernel.org, justinstitt@google.com, morbo@google.com
Subject: Re: [PATCH] Input: mouse: vsxxxaa: Replace strlcat() with strscpy()
Date: Thu, 5 Nov 2026 22:39:38 +0100	[thread overview]
Message-ID: <auz0ONYzkWIETv8T@devuan> (raw)
In-Reply-To: <20261105205321.7b303231@pumpkin>

[-- Attachment #1: Type: text/plain, Size: 3198 bytes --]

Hi David,

> Date: 2026-11-05 20:53:21+0000
> From: David Laight <david.laight.linux@gmail.com>
>
> On Thu, 5 Nov 2026 12:49:59 +0100
> Alejandro Colomar <alx+linux-hardening@kernel.org> wrote:
> 
> > > Personally I think that while there are issues with strlcat()
> > > (mostly to do with its return value), and that there are often
> > > much better (and faster - so sprintf is out) ways of getting
> > > the same result, the deprecation really needs to propose a
> > > sensible alternative (not inlined strlen() + strscpy()).  
> > 
> > One alternative would be to add strscat(), with the obvious internal
> > strlen(3).  This would be a drop-in replacement, and wouldn't have the
> > issues of strlcat(3) --mainly, the DoS vulnerability which is
> > consequence of the return value, and also the more difficult error
> > handling of doing '>=sizeof(...)' than '<0'--.
> 
> I've had thoughts of a more general function - but I've not tried to
> write it.
> 
> int str_append(char *dest, size_t dest_sz, size_t dest_off, const char *src, size_t src_max_len);
> 
> Where dest_sz is usually sizeof(*dest) and the header can substitute that for
>   a constant zero.
> dest_off would similarly default to strlen(dest), if -ve the function would
>   return that value immediately.
> src_max_len would be used for an effective strnlen(src, src_max_len).
> 
> The idea would be that calls like:
> 	char tgt[100];
> 	len = str_append(tgt, 0, strscpy(tgt, src_1), src_2, 0);
> would append src_1 and src_2 in tgt returning -ve on overflow.
> (Further chaining would then work.)
> 
> But, as I said, I've not tried to write it or see what it would look like in use.

I think it's too complex for what it does.  See how stpecpy() works:

	char  tgt[100];
	char  *p, *e;

	e = tgt + _Countof(tgt);
	p = tgt;
	p = stpecpy(p, e, src_1);
	p = stpecpy(p, e, src_2);
	p = stpecpy(p, e, src_3);
	p = stpecpy(p, e, src_4);
	p = stpecpy(p, e, src_5);
	...
	p = stpecpy(p, e, src_9);
	if (p == NULL)
		goto truncated;

	// 'p - tgt' is the length of the string.
	// In other words, 'p' points to the '\0'.

It copies all the strings into tgt, with truncation.  If any of the 5
calls resulted in truncation, it will be reported as a NULL.

You can also do the chain in a single line:

	p = stpecpy(stpecpy(stpecpy(p, e, src_1), e, src_2), e, src_3);

But IMO it reads better one per line.

Then, strscpy(9) is trivially implementable as:

	ssize_t
	strscpy(char *dst, const char *src, size_t dsize)
	{
		char  *p;

		BUG_ON(dsize == 0);
		p = stpecpy(dst, dst + dsize, src);
		return p ? p - dst : -E2BIG;
	}

And a hypothetical strscat() would be:

	ssize_t
	strscat(char *dst, const char *src, size_t dsize)
	{
		char  *p;

		BUG_ON(dsize == 0);
		p = stpecpy(dst + strlen(dst), dst + dsize, src);
		return p ? p - dst : -E2BIG;
	}

I have been working literally years on this API, and believe it has
reached perfection.  It includes some feedback I got from Doug McIlroy
about the design, which improved the usability and safety.


Have a lovely night!
Alex

> 
> David

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

      reply	other threads:[~2026-11-05 21:39 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-04  6:23 [PATCH] Input: mouse: vsxxxaa: Replace strlcat() with strscpy() Mariia Nikitash
2026-11-04 11:21 ` Alejandro Colomar
2026-11-04 16:33 ` Shihuang Liu
2026-11-05 11:28 ` David Laight
2026-11-05 11:49   ` Alejandro Colomar
2026-11-05 20:53     ` David Laight
2026-11-05 21:39       ` Alejandro Colomar [this message]

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=auz0ONYzkWIETv8T@devuan \
    --to=alx+linux-hardening@kernel.org \
    --cc=david.laight.linux@gmail.com \
    --cc=dmitry.torokhov@gmail.com \
    --cc=justinstitt@google.com \
    --cc=kees@kernel.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=morbo@google.com \
    --cc=nikitash.mariiaw@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox