Re: [PATCH v2] fs: do not cache a symlink length that disagrees with the string

On Tue 18-08-26 23:35:53, Jan Kara wrote:
> I don't know but to me this looks like overly defensive programming... If
> we call strlen() in inode_set_cached_link(), then why pass the length to it
> as an argument in the first place?

You're right, and so was Mateusz.  Please drop this one.

Going back over the callers: erofs and ext4 both run strlen()/strnlen()
themselves and reject the inode as corrupted before they ever call the
helper, and shmem and ext4's create path pass a length derived from the
string they just wrote.  Every caller already guarantees the contract, and
the two that take the length from untrusted on-disk metadata verify it
independently of this helper.

I cited those same two callers in the commit message as evidence that the
API was fragile.  That was backwards - they are evidence that it works as
documented.  What actually remained was "a future caller might get it
wrong", which does not justify a strlen() on every symlink setup, and, as
you point out, leaves the length parameter with no purpose.

Sorry for the noise.

Thanks,
Narek