| Message ID | cover.1680693362.git.fweimer@redhat.com |
|---|---|
| Headers | show |
| Series | strlcpy/strlcat/wcslcpy/wcscat implementation | expand |
Hi Florian, On 4/5/23 13:20, Florian Weimer via Libc-alpha wrote: > These two patches add support for four functions planned for addition to > POSIX, plus their fortified variants. They are available with > _DEFAULT_SOURCE because the BSDs expose them by default, oto. > > Tested on i686-linux-gnu and x86_64-linux-gnu. Built with > build-many-glibcs.py (on all ABIs, but not all targets because disk > space is no longer sufficient on the lab machines I have access to). > > Paul, the Austin Groups issue you raised has been closed. Do you keep > your sustained objection to adding these functions to glibc? Just adding a data point about this: In shadow, we improved existing code going from strncpy(3) to strlcpy(3). See <https://github.com/shadow-maint/shadow/pull/609>. In some cases, as corretly pointed out by Paul, we could futher improve some cases by just calling strcpy(3). However, in some others, strlcpy(3) was the sanest API, and while we could check prior to strcpy(3)/memcpy(3) with strlen(3), that was unnecessarily cluttering the code. See <https://github.com/shadow-maint/shadow/pull/681>. So I like the idea of adding this API to glibc. In fact, it would reduce the need for libbsd in some packages like shadow (although we still need it for readpassphrase(3)). Cheers, Alex > > Thanks, > Florian > > Florian Weimer (2): > Implement strlcpy and strlcat [BZ #178] > Add the wcslcpy, wcslcat functions >
On 4/5/23 04:20, Florian Weimer wrote: > Paul, the Austin Groups issue you raised has been closed. Do you keep > your sustained objection to adding these functions to glibc? Yes, I'll withdraw that objection if we add the functions in a good way. Compatibility with POSIX is more important than omitting a misguided API, so long as the harm is limited to people who insist on using the API despite being warned. By "good way" I mean the following three things. 1. Fix the manual to agree with draft POSIX (currently it disagrees). Also, the manual should mention the same sort of problems for strlcpy/etc. that it already mentions for strncpy/etc. - truncation can lead to security problems, non-text strings, that sort of thing. 2. Do not implement these functions via memcpy or strnlen or anything like that. Just use simple code like OpenBSD's. Preferably just use OpenBSD code as-is. Otherwise we have the danger of straying from OpenBSD semantics. This danger is already present in the proposed patches, which have undefined behavior in rare cases where OpenBSD's behavior is well-defined and where draft POSIX specifies the OpenBSD behavior. We don't have time to formally verify or otherwise carefully audit this code, so this is no place to bikeshed or optimize. The goal is compatibility, not speed. Besides, strlcpy destinations and sources are almost always small, so in practice using memcpy etc. will likely be slower than just doing things the OpenBSD way. 3. The patch should be more conservative: it should declare these functions in string.h only on user request, i.e., only when compiled with -D_XOPEN_SOURCE=800 (or -D_POSIX_C_SOURCE=2023mmL) without -D_GNU_SOURCE. (We can add a new _STRLCPY_SOURCE macro too if that would be helpful, to selectively make these new functions visible.) This will suffice for POSIX conformance, and it will help avoid bugs when existing applications that already supply their own slightly-different strlcpy substitutes are compiled with new glibc: it will give these apps' maintainers the ability to decide after their own manual inspection whether and when to switch to glibc's strlcpy. I'll follow up with more detailed comments on individual patches. And I'll volunteer to do (1), which is likely the most work anyway. I've already done much of (1) and will attach that to my detailed comments.