From: Lennart Jablonka <humm@ljabl.com>
To: Alejandro Colomar <alx@kernel.org>
Cc: linux-man@vger.kernel.org
Subject: Re: [PATCH] string_copying.7: don't grant strl{cpy,cat} magic
Date: Fri, 28 Jul 2023 23:51:54 +0000 [thread overview]
Message-ID: <ZMRUmgRfRVxL3wEo@fluorine.ljabl.com> (raw)
In-Reply-To: <8fe571c9-eda2-bd1e-0d78-9c5e8f7222fe@kernel.org>
>> A function can't check whether a pointer points to the start of a
>> string. What it certainly can do is to keep reading until you either
>> find a null byte or read the secret key that lies adjacent in memory and
>> post it to your favorite mailing list.
>>
>> strlcpy and strlcat behave the exact same way any other function
>> accepting a string behaves: If you don't pass a string, the behavior is
>> undefined. And that, I believe, does not deserve a special mention
>> here, seeing as all the other string functions don't get such a mention
>> either.
>
>Hmm, you're right.
>
>What I intended to mean is that while most other functions --e.g.,
>strcpy(3)-- overwrite after the buffer, the design of strlcpy(3) is a
>bit more clever and makes it so that when the caller invokes UB, it
>tries to exploit that UB in a way that the input string is entirely
>read before starting to write, which makes it more likely to crash in
>a read, rather than writing to random memory (which might still happen
>if the read is not enough to crash, though).
I’m interested in where you got that from. This is strlcpy:
size_t
strlcpy(char *dst, const char *src, size_t dsize)
{
const char *osrc = src;
size_t nleft = dsize;
/* Copy as many bytes as will fit. */
if (nleft != 0) {
/*
* <humm> This is where reading and
* writing take place. src doesn’t get
* read entirely before writing begins.
*/
while (--nleft != 0) {
if ((*dst++ = *src++) == '\0')
break;
}
}
/* Not enough room in dst, add NUL and traverse rest of src. */
if (nleft == 0) {
if (dsize != 0)
*dst = '\0'; /* NUL-terminate dst */
while (*src++)
;
}
return(src - osrc - 1); /* count does not include NUL */
}
I don’t see what you mean in there.
>> The "definitions" at the top don't make it clear enough that they aren't
>> supposed to be precise definitions used in your usual C jargon; that
>> while string and a string's length and an object's size are defined
>> by C, and while you understand and sometimes use most of these terms,
>> there is no norm that says "When you talk about a pointer to one past
>> a buffer's last byte, you call it 'end'!" That there is no norm that
>> says "When you say 'copy,' you write to the beginning, not to
>> elsewhere!"
>
>True. My intention was to settle the jargon and pseudo-standardize
>these terms (of course in decades, not tomorrow). Every other project
>uses a different term, and I'd like to unify.
I’m not quite sure yet what to think of that, but I do think you
should mention that in the man page. “Here are some terms;
C defines ‘string,’ but you are invited to re-use all these
terms—it’d be great if we were in unity, after all.”
>> Furthermore, the terminology around "character sequences" confused me
>> while reading the page. When do you have a buffer, neither
>> null-terminated nor null-padded, that is defined not to contain null
>> bytes?
>
>NGINX uses these internally:
>
>$ grepc ngx_str_t
>./src/core/ngx_string.h:16:
>typedef struct {
> size_t len;
> u_char *data;
>} ngx_str_t;
>
>
>Basically it's a non-zero buffer plus its length. They have interesting
>properties; for example, you can take a substring (or should I call it
>sub-sequence) just by taking a pointer to somewhere in the middle, and
>the length of the substring, without really copying the string.
same with a buffer without restriction of null bytes
>> And how do functions behave that want a character sequence if
>> that does contain a null byte? Do they take the null byte to signal the
>> character sequence's end? Need they accept the null byte as part of the
>> character sequence? Is the behavior undefined?
>
>NGINX handles these strings by the length stored in the buffer. Any
>null byte in the middle of a string would be treated as any other
>character, although they would be problematic when interfacing libc; in
>general, care is taken to not have null bytes in those strings. NGINX
>uses mempcpy(3) (or rather, ngx_cpymem(), which is the same thing) to
>copy these things, or other more sophisticated functions and macros
>based on mempcpy(3).
>
>$ grepc ngx_cpymem
>./src/core/ngx_string.h:97:
>#define ngx_cpymem(dst, src, n) (((u_char *) ngx_memcpy(dst, src, n)) + (n))
>
>
>./src/core/ngx_string.h:107:
>#define ngx_cpymem(dst, src, n) (((u_char *) memcpy(dst, src, n)) + (n))
I think if you want to pseudo-standardize terminology, you should
mention how functions are supposed to behave when seeing a null
byte in a character sequence. (I plead for undefined behavior.)
Now, do you think character sequences are more common than simple
buffers, than not caring at all about the specific bytes? That’s
what I do; that’s what I think should be usually done. And do you
suppose character sequences are more valuable than simple buffers?
Sure, iff the sequences are null-terminated, you can use them as
strings, too, but that doesn’t seem like much of a benefit.
>> And lastly, the man page doesn't list the functions' standards or who
>> invented them.
>
>That was deliberate. The specific pages of each of those functions
>already documents that. Since the intention was to differentiate the
>use cases of each of the functions, I believe mentioning the standards
>would just deviate from that main point, and so I omitted that info.
>The point is that choosing one of these functions shouldn't depend on
>what standards are available to the programmer. Instead, the
>programmer should use the appropriate function, and then if it's not
>available, it should be written within the project (probably as a
>wrapper around other functions) to be able to use it. That's why I
>provided some naive implementations of some of them.
That’s not a bad idea, but for some functions, this is the man
page. I wondered where stpecpy comes from, I tried to open
stpecpy(3), I got string_copying(7).
next prev parent reply other threads:[~2023-07-28 23:52 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-28 19:22 [PATCH] string_copying.7: don't grant strl{cpy,cat} magic Lennart Jablonka
2023-07-28 22:05 ` Alejandro Colomar
2023-07-28 23:51 ` Lennart Jablonka [this message]
2023-07-29 12:04 ` Alejandro Colomar
2023-07-29 14:38 ` Matthew House
2023-07-29 14:47 ` Lennart Jablonka
2023-07-29 21:06 ` Matthew House
2023-07-29 19:39 ` G. Branden Robinson
2023-07-30 14:05 ` 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=ZMRUmgRfRVxL3wEo@fluorine.ljabl.com \
--to=humm@ljabl.com \
--cc=alx@kernel.org \
--cc=linux-man@vger.kernel.org \
/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