Re: [PATCH] precompose_utf8: use a flex array for d_name
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Jul 3, 2026, 08:40 UTC
- Message-ID
- <akd1m6KoUh7N8yyE@pks.im>
- In-Reply-To
- <20260703023554.36577-1-ihar.hrachyshka@gmail.com>
On Thu, Jul 02, 2026 at 10:35:54PM -0400, Ihar Hrachyshka wrote:
Show 21 quoted lines
> On macOS, git status may abort while reading a directory entry > whose UTF-8 name grows past NAME_MAX bytes: > > __chk_fail_overflow > __strlcpy_chk > precompose_utf8_readdir > read_directory_recursive > wt_status_collect > cmd_status > > The precompose wrapper already reallocates dirent_prec_psx for > long names, but d_name is declared as char[NAME_MAX + 1]. A > fortified libc can still see that declared object size and reject a > larger strlcpy bound, even though the allocation was grown. > > Make d_name a FLEX_ARRAY and size allocations from offsetof(). That > matches the actual object layout with the dynamic allocation, so the > fortified copy sees a destination whose size can grow with max_name_len. > > Add a regression test that creates a 261-byte non-ASCII basename and > runs status with core.precomposeunicode enabled.
Hm. Why does macOS even allow you to create a file that has a basename longer than NAME_MAX? Does macOS count unicode characters specially?
Show 24 quoted lines
> diff --git a/compat/precompose_utf8.c b/compat/precompose_utf8.c
> index 1711794..8077f62 100644
> --- a/compat/precompose_utf8.c
> +++ b/compat/precompose_utf8.c
> @@ -19,6 +19,11 @@ typedef char *iconv_ibp;
> static const char *repo_encoding = "UTF-8";
> static const char *path_encoding = "UTF-8-MAC";
>
> +static size_t dirent_prec_psx_size(size_t max_name_len)
> +{
> + return st_add(offsetof(dirent_prec_psx, d_name), max_name_len);
> +}
> +
> static size_t has_non_ascii(const char *s, size_t maxlen, size_t *strlen_c)
> {
> const uint8_t *ptr = (const uint8_t *)s;
> @@ -114,8 +119,8 @@ const char *precompose_argv_prefix(int argc, const char **argv, const char *pref
> PREC_DIR *precompose_utf8_opendir(const char *dirname)
> {
> PREC_DIR *prec_dir = xmalloc(sizeof(PREC_DIR));
> - prec_dir->dirent_nfc = xmalloc(sizeof(dirent_prec_psx));
> - prec_dir->dirent_nfc->max_name_len = sizeof(prec_dir->dirent_nfc->d_name);
> + prec_dir->dirent_nfc = xmalloc(dirent_prec_psx_size(NAME_MAX + 1));
> + prec_dir->dirent_nfc->max_name_len = NAME_MAX + 1;We have the `FLEX_ALLOC_MEM()` macro that would probably be a better fit compared to introducing `dirent_prec_psx_size()`.
Also, when converting this to a flex array, can't we do better here and allocate the structures with the right size? Otherwise, I expect that we overallocate most of the entrise.
Show 10 quoted lines
> @@ -145,8 +150,7 @@ struct dirent_prec_psx *precompose_utf8_readdir(PREC_DIR *prec_dir)
> int ret_errno = errno;
>
> if (new_maxlen > prec_dir->dirent_nfc->max_name_len) {
> - size_t new_len = sizeof(dirent_prec_psx) + new_maxlen -
> - sizeof(prec_dir->dirent_nfc->d_name);
> + size_t new_len = dirent_prec_psx_size(new_maxlen);
>
> prec_dir->dirent_nfc = xrealloc(prec_dir->dirent_nfc, new_len);
> prec_dir->dirent_nfc->max_name_len = new_maxlen;Okay, here we indeed have to realloc though, and thus we can't quite avoid `dirent_prec_psx_size()`. Too bad.
Thanks!
Patrick