This was discussed previously in bug 1665863 but it didn't seem high-priority at the time; that may have been a mistake. It looks like [this patch will crash][not-longer] if the actual symlink target is longer than the buffer. I think the previous behavior in that case would have been silently truncating it, so arguably this is an improvement, but it's not great. [not-longer]: https://searchfox.org/mozilla-central/rev/ffa4d00965c5281def6d3ddcbcdf6259d38c9b9a/xpcom/string/nsTSubstring.h#981-982 The way this normally works is to check the return value of `readlink` and increase the buffer size as needed: ``` $ strace -fereadlink,readlinkat readlink /proc/self/fd/4 readlink("/proc/self/fd/4", "/tmp/aaaaaaaaaaaaaaaaaaaaaaaaaaa"..., 64) = 64 readlink("/proc/self/fd/4", "/tmp/aaaaaaaaaaaaaaaaaaaaaaaaaaa"..., 128) = 72 … $ strace -fereadlink,readlinkat ls -l /proc/self/fd/4 readlink("/proc/self/fd/4", "/tmp/aaaaaaaaaaaaaaaaaaaaaaaaaaa"..., 65) = 65 readlink("/proc/self/fd/4", "/tmp/aaaaaaaaaaaaaaaaaaaaaaaaaaa"..., 130) = 72 … ``` And some tools don't even try lstat in the first place: ``` $ strace -fereadlink,readlinkat readlink /tmp/l readlink("/tmp/l", "/tmp/aaaaaaaaaaaaaaaaaaaaaaaaaaa"..., 64) = 64 readlink("/tmp/l", "/tmp/aaaaaaaaaaaaaaaaaaaaaaaaaaa"..., 128) = 72 … $ strace -fereadlink,readlinkat ls -l /tmp/l readlink("/tmp/l", "/tmp/aaaaaaaaaaaaaaaaaaaaaaaaaaa"..., 73) = 72 … ``` Note that `ls`'s first attempt is the expected size plus one, to distinguish the expected result from overflow.
Bug 1791029 Comment 8 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
This was discussed previously in bug 1665863 but it didn't seem high-priority at the time; that may have been a mistake. ~~It looks like [this patch will crash][not-longer] if the actual symlink target is longer than the buffer. I think the previous behavior in that case would have been silently truncating it, so arguably this is an improvement, but it's not great.~~ Edit: no, I was confusing `readlink` with the printf family there; it will still silently truncate the path. [not-longer]: https://searchfox.org/mozilla-central/rev/ffa4d00965c5281def6d3ddcbcdf6259d38c9b9a/xpcom/string/nsTSubstring.h#981-982 The way this normally works is to check the return value of `readlink` and increase the buffer size as needed: ``` $ strace -fereadlink,readlinkat readlink /proc/self/fd/4 readlink("/proc/self/fd/4", "/tmp/aaaaaaaaaaaaaaaaaaaaaaaaaaa"..., 64) = 64 readlink("/proc/self/fd/4", "/tmp/aaaaaaaaaaaaaaaaaaaaaaaaaaa"..., 128) = 72 … $ strace -fereadlink,readlinkat ls -l /proc/self/fd/4 readlink("/proc/self/fd/4", "/tmp/aaaaaaaaaaaaaaaaaaaaaaaaaaa"..., 65) = 65 readlink("/proc/self/fd/4", "/tmp/aaaaaaaaaaaaaaaaaaaaaaaaaaa"..., 130) = 72 … ``` And some tools don't even try lstat in the first place: ``` $ strace -fereadlink,readlinkat readlink /tmp/l readlink("/tmp/l", "/tmp/aaaaaaaaaaaaaaaaaaaaaaaaaaa"..., 64) = 64 readlink("/tmp/l", "/tmp/aaaaaaaaaaaaaaaaaaaaaaaaaaa"..., 128) = 72 … $ strace -fereadlink,readlinkat ls -l /tmp/l readlink("/tmp/l", "/tmp/aaaaaaaaaaaaaaaaaaaaaaaaaaa"..., 73) = 72 … ``` Note that `ls`'s first attempt is the expected size plus one, to distinguish the expected result from overflow.