On Tue, Feb 4, 2025 at 10:32 PM Mateusz Guzik <mjguzik@xxxxxxxxx> wrote: > > This costs a strlen() call when instatianating a symlink. > > Preferably it would be hidden behind VFS_WARN_ON (or compatible), but > there is no such facility at the moment. With the facility in place the > call can be patched out in production kernels. > > In the meantime, since the cost is being paid unconditionally, use the > result to a fixup the bad caller. > > This is not expected to persist in the long run (tm). > > Sample splat: > bad length passed for symlink [/tmp/syz-imagegen43743633/file0/file0] (got 131109, expected 37) > [rest of WARN blurp goes here] > > Signed-off-by: Mateusz Guzik <mjguzik@xxxxxxxxx> > --- > > This has a side effect of working around the panic reported in: > https://lore.kernel.org/all/67a1e1f4.050a0220.163cdc.0063.GAE@xxxxxxxxxx/ > > I'm confident this merely exposed a bug in ext4, see: > https://lore.kernel.org/all/CAGudoHEv+Diti3r0x9VmF5ixgRVKk4trYnX_skVJNkQoTMaDHg@xxxxxxxxxxxxxx/#t > > Nonethelss, should help catch future problems. > > include/linux/fs.h | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/include/linux/fs.h b/include/linux/fs.h > index be3ad155ec9f..1437a3323731 100644 > --- a/include/linux/fs.h > +++ b/include/linux/fs.h > @@ -791,6 +791,19 @@ struct inode { > > static inline void inode_set_cached_link(struct inode *inode, char *link, int linklen) > { > + int testlen; > + > + /* > + * TODO: patch it into a debug-only check if relevant macros show up. > + * In the meantime, since we are suffering strlen even on production kernels > + * to find the right length, do a fixup if the wrong value got passed. > + */ > + testlen = strlen(link); > + if (testlen != linklen) { > + WARN_ONCE(1, "bad length passed for symlink [%s] (got %d, expected %d)", > + link, linklen, testlen); I just remembered that path_noexec thing got a CVE. I suspect someone may try to assign one here for user-mountable disk images triggering this warn. Can be patched to "printk_once", but then it does not implicitly tell which fs is messing up due to lack of a backtrace. > + linklen = testlen; > + } > inode->i_link = link; > inode->i_linklen = linklen; > inode->i_opflags |= IOP_CACHED_LINK; > -- > 2.43.0 > -- Mateusz Guzik <mjguzik gmail.com>
