Re: [PATCH 4/5] strbuf_readlink(): support link targets that exceed PATH_MAX
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Dec 30, 2025, 05:00 UTC
- Message-ID
- <xmqqcy3wh8d1.fsf@gitster.g>
- In-Reply-To
- <aUU8O6ltrNj-FmjZ@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
Show 28 quoted lines
>> > This makes me wonder whether we have a better way to figure out the >> > actual size of the buffer that we ultimately need to allocate. But >> > reading through readlink(3p) doesn't indicate anything, and I'm not sure >> > whether we can always rely on lstat(3p) to return the correct size for >> > symlink contents on all platforms. >> > >> > One thing that _is_ noted though is that calling the function with a >> > buffer size larger than SSIZE_MAX is implementation-defined. It does >> > make me a bit uneasy in that light to grow indefinitely. >> > >> > Which makes me wonder whether Windows has a limit for the symlink >> > contents that we could enforce in theory so that we can reasonably turn >> > this into a bounded loop again? >> >> https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation >> suggests that the maximum permissible target path should be 32,768. But >> that's not _quite_ correct, as >> `../t/../Documentation/RelNotes/../../README.md` is a perfectly valid (if >> awkward) symlink target. >> >> Still, I would say that 32,768 would make for a fine (still insanely high, >> but not so high as to allow malicious symlinks to cause memory problems) >> limit. >> >> Sound good? >> Johannes > > Sounds good to me, thanks!
As this is a generic codepath in strbuf.c, platforms that do not honor Microsoft's promise cited above can break the assumption made here by going beyond 32k, no?
I am OK if this infinite loop had our own "we are growing the buffer very long and still getting not-enough-buf error; let's give up" termination condition.
IOW, a simpler alternative may be
---- >8 ---- Subject: strbuf_readlink(): do not trust PATH_MAX
We have been bitten before by platforms that sets PATH_MAX way too low, far below the length of paths they comfortably support. The strbuf_readlink() limits the link targets to PATH_MAX, which is a code path that is broken by such platforms.
Raise the limit to 32kB, which matches the limit of a platform with such a problem [*].
* https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation
strbuf.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git c/strbuf.c w/strbuf.c index 7fb7d12ac0..1c7659bcd2 100644 --- c/strbuf.c +++ w/strbuf.c @@ -566,7 +566,11 @@ ssize_t strbuf_write(struct strbuf *sb, FILE *f) return sb->len ? fwrite(sb->buf, 1, sb->len, f) : 0; } -#define STRBUF_MAXLINK (2*PATH_MAX) +/* + * Do not use PATH_MAX, as some platforms sets it too low; + * 32kB matches what Windows has as the real limit for a pathnname. + */ +#define STRBUF_MAXLINK (2 * (1 << 15)) int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint) {