From: Alejandro Colomar <alx@kernel.org>
To: Jonny Grant <jg@jguk.org>
Cc: Paul Eggert <eggert@cs.ucla.edu>,
Matthew House <mattlloydhouse@gmail.com>,
linux-man <linux-man@vger.kernel.org>,
GNU C Library <libc-alpha@sourceware.org>
Subject: Re: strncpy clarify result may not be null terminated
Date: Fri, 10 Nov 2023 14:15:14 +0100 [thread overview]
Message-ID: <ZU4s6Vm-jBjFCcJ2@debian> (raw)
In-Reply-To: <ebccf5c0-f125-4eb9-8820-e71e77f4f7ed@jguk.org>
[-- Attachment #1: Type: text/plain, Size: 3255 bytes --]
Hi Jonny,
On Fri, Nov 10, 2023 at 11:36:20AM +0000, Jonny Grant wrote:
>
>
> On 10/11/2023 05:36, Paul Eggert wrote:
> > On 2023-11-09 15:48, Alejandro Colomar wrote:
> >> I'd then just use strlen(3)+strcpy(3), avoiding
> >> strncpy(3).
> >
> > But that is vulnerable to the same denial-of-service attack that strlcpy is vulnerable to. You'd need strnlen+strcpy instead.
> >
> > The strncpy approach I suggested is simpler, and (though this doesn't matter much in practice) is typically significantly faster than strnlen+strcpy in the typical case where the destination is a small fixed-size buffer.
> >
> > Although strncpy is not a good design, it's often simpler or faster or safer than later "improvements".
>
> As you say, it is a known API. I recall looking for a standardized bounded string copy a few years ago that avoids pitfalls:
>
> 1) cost of any initial strnlen() reading memory to determine input src size
> 2) accepts a src_max_size to actually try to copy from src
> 3) does not truncate by writing anything to the buffer if there isn't enough space in the dest_max_size to fit src_max_size
> 4) check for NULL pointers
> 5) probably other thing I've overlooked
>
> Something like this API:
> int my_str_copy(char *dest, const char *src, size_t dest_max_size, size_t src_max_size, size_t * dest_written);
> These sizes are including any NUL terminating byte.
>
> 0 on success, or an an error code like EINVAL, or ERANGE if would truncate
- Linux kernel's strscpy() returns -E2BIG if it would truncate. You
may want to follow suit if you want such an errno(3) code.
However, I think it's simpler to return the "standard" user-space
error return value: -1
If you'd need to distinguish error reasons, you could distinguish
error codes, but for a string-copying function I think it's not so
useful.
- Why specify the src buffer size? If you're copying strings, then you
know it'll be null-terminated, so strnlen(3) will not overrun. If
you're not copying strings, then you'll need a different function
that reads from a non-string. The only standard such function is
strncat(3), which reads from a fixed-width null-padded buffer, and
writes to a string. You may want to write a function similar to
strncat(3) that doesn't catenate, if you want to just copy; I call
that function zustr2stp(), and you can find an implementation in
string_copying(7).
- You can reuse the return value for the dest_written value with
ssize_t. Just return -1 on error and the string length on success.
That's how most libc functions behave.
- Regarding NULL checks, it depends on how you program. I wouldn't add
them, but if you want to avoid crashes at all costs, it may be
necessary for you. You could do a wrapper over strxcpy():
inline ssize_t
strxcpy0(char *restrict dst, const char *restrict src, size_t dsize)
{
if (dst == NULL || src == NULL)
return -1;
return strxcpy(dst, src, dsize);
}
I used 0 in the name to mark that this function checks for null
pointers.
Cheers,
Alex
>
> All comments welcome.
>
> Kind regards, Jonny
--
<https://www.alejandro-colomar.es/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2023-11-10 13:15 UTC|newest]
Thread overview: 77+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <cfbd8674-fe6a-4430-95f1-ec8bde7da32e@jguk.org>
[not found] ` <ZUacobMq0l_O8gjg@debian>
[not found] ` <aeb55af5-1017-4ffd-9824-30b43d5748e3@jguk.org>
[not found] ` <ZUgl2HPJvUge7XYN@debian>
[not found] ` <d40fffcb-524d-44b6-a252-b55a8ddc9fee@jguk.org>
[not found] ` <ZUo6btEFD_z_3NcF@devuan>
[not found] ` <929865e3-17b4-49c4-8fa9-8383885e9904@jguk.org>
[not found] ` <ZUpjI1AHNOMOjdFk@devuan>
[not found] ` <ZUsoIbhrJar6ojux@dj3ntoo>
2023-11-08 9:51 ` Alejandro Colomar
2023-11-08 9:59 ` Thorsten Kukuk
2023-11-08 15:09 ` Alejandro Colomar
[not found] ` <6bcad2492ab843019aa63895beaea2ce@DB6PR04MB3255.eurprd04.prod.outlook.com>
2023-11-08 15:44 ` Thorsten Kukuk
2023-11-08 17:26 ` Adhemerval Zanella Netto
2023-11-08 14:06 ` Zack Weinberg
2023-11-08 15:07 ` Alejandro Colomar
2023-11-08 21:35 ` Carlos O'Donell
2023-11-08 22:11 ` Alejandro Colomar
2023-11-08 23:31 ` Paul Eggert
2023-11-09 0:29 ` Alejandro Colomar
2023-11-09 10:13 ` Jonny Grant
2023-11-09 11:08 ` catenate vs concatenate (was: strncpy clarify result may not be null terminated) Alejandro Colomar
2023-11-09 14:06 ` catenate vs concatenate Jonny Grant
2023-11-27 14:33 ` catenate vs concatenate (was: strncpy clarify result may not be null terminated) Zack Weinberg
2023-11-27 15:08 ` Alejandro Colomar
2023-11-27 15:13 ` Alejandro Colomar
2023-11-27 16:59 ` G. Branden Robinson
2023-11-27 18:35 ` Zack Weinberg
2023-11-27 23:45 ` G. Branden Robinson
2023-11-09 11:13 ` strncpy clarify result may not be null terminated Alejandro Colomar
2023-11-09 14:05 ` Jonny Grant
2023-11-09 15:04 ` Alejandro Colomar
2023-11-08 19:04 ` DJ Delorie
2023-11-08 19:40 ` Alejandro Colomar
2023-11-08 19:58 ` DJ Delorie
2023-11-08 20:13 ` Alejandro Colomar
2023-11-08 21:07 ` DJ Delorie
2023-11-08 21:50 ` Alejandro Colomar
2023-11-08 22:17 ` [PATCH] stpncpy.3, string_copying.7: Clarify that st[rp]ncpy() do NOT produce a string Alejandro Colomar
2023-11-08 23:06 ` Paul Eggert
2023-11-08 23:28 ` DJ Delorie
2023-11-09 0:24 ` Alejandro Colomar
2023-11-09 14:11 ` Jonny Grant
2023-11-09 14:35 ` Alejandro Colomar
2023-11-09 14:47 ` Jonny Grant
2023-11-09 15:02 ` Alejandro Colomar
2023-11-09 17:30 ` DJ Delorie
2023-11-09 17:54 ` Andreas Schwab
2023-11-09 18:00 ` Alejandro Colomar
2023-11-09 19:42 ` Jonny Grant
2023-11-09 7:23 ` Oskari Pirhonen
2023-11-09 15:20 ` [PATCH v2 1/2] " Alejandro Colomar
2023-11-09 15:20 ` [PATCH v2 2/2] stpncpy.3, string.3, string_copying.7: Clarify that st[rp]ncpy() pad with null bytes Alejandro Colomar
2023-11-10 5:47 ` Oskari Pirhonen
2023-11-10 10:47 ` Alejandro Colomar
[not found] ` <20231108021240.176996-1-mattlloydhouse@gmail.com>
[not found] ` <ZUvilH5kuQfTuZjy@debian>
[not found] ` <20231109031345.245703-1-mattlloydhouse@gmail.com>
2023-11-09 10:31 ` strncpy clarify result may not be null terminated Jonny Grant
2023-11-09 11:38 ` Alejandro Colomar
2023-11-09 12:43 ` Alejandro Colomar
2023-11-09 12:51 ` Xi Ruoyao
2023-11-09 14:01 ` Alejandro Colomar
2023-11-09 18:11 ` Paul Eggert
2023-11-09 23:48 ` Alejandro Colomar
2023-11-10 5:36 ` Paul Eggert
2023-11-10 11:05 ` Alejandro Colomar
2023-11-10 11:47 ` Alejandro Colomar
2023-11-10 17:58 ` Paul Eggert
2023-11-10 18:36 ` Alejandro Colomar
2023-11-10 20:19 ` Alejandro Colomar
2023-11-10 23:44 ` Jonny Grant
2023-11-10 19:52 ` Alejandro Colomar
2023-11-10 22:14 ` Paul Eggert
2023-11-11 21:13 ` Alejandro Colomar
2023-11-11 22:20 ` Paul Eggert
2023-11-12 9:52 ` Jonny Grant
2023-11-12 10:59 ` Alejandro Colomar
2023-11-10 11:36 ` Jonny Grant
2023-11-10 13:15 ` Alejandro Colomar [this message]
2023-11-10 11:23 ` Jonny Grant
[not found] ` <CACKs7VDsTdSNwbC6+2LtQ67J_eJiD814xkw2_5XM1Q=iMjLXJA@mail.gmail.com>
2023-11-10 11:06 ` Jonny Grant
2023-11-12 9:17 ` [PATCH 0/2] Expand BUGS section of string_copying(7) Alejandro Colomar
2023-11-12 9:18 ` [PATCH 1/2] string_copying.7: BUGS: *cat(3) functions aren't always bad Alejandro Colomar
2023-11-12 9:18 ` [PATCH 2/2] string_copying.7: BUGS: Document strl{cpy,cat}(3)'s performance problems Alejandro Colomar
2023-11-12 11:26 ` [PATCH v2 0/3] Improve string_copying(7) Alejandro Colomar
2023-11-12 11:26 ` [PATCH v2 1/3] string_copying.7: BUGS: *cat(3) functions aren't always bad Alejandro Colomar
2023-11-12 11:26 ` [PATCH v2 2/3] string_copying.7: BUGS: Document strl{cpy,cat}(3)'s performance problems Alejandro Colomar
2023-11-12 11:27 ` [PATCH v2 3/3] strtcpy.3, string_copying.7: Add strtcpy(3) Alejandro Colomar
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=ZU4s6Vm-jBjFCcJ2@debian \
--to=alx@kernel.org \
--cc=eggert@cs.ucla.edu \
--cc=jg@jguk.org \
--cc=libc-alpha@sourceware.org \
--cc=linux-man@vger.kernel.org \
--cc=mattlloydhouse@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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).