On Mon, Oct 01, 2018 at 05:23:32PM -0700, Darrick J. Wong wrote: > get_link doesn't need the EFSCORRUPTED return; all two of its callers > handle null pointer returns correctly and they don't return the ->get_link > return value directly to userspace. > > It's just these two functions below whose callers assume they have to > deal an error pointer or that it's totally safe to dereference it. No. The only case when ->get_link() has any business returning NULL is when it has done nd_jump_link(). Those should never come without explicit ->readlink() and they should never be fed to vfs_get_link(), so they are not a problem; anything else is a filesystem driver bug, plain and simple. Check for NULL in fs/namei.c:get_link() is *NOT* "defensive programming" bullshit; there it can legitimately happen (aforementioned procfs-style symlinks). Note that there it is not an error at all. Current calling conventions are: * return ERR_PTR(-E...) on error * return pointer to symlink body to be traversed on success * return NULL when ->get_link() instances has jumped to destination on its own and there's no "symlink body" to be traversed. For such symlinks we obviously need an explicit ->readlink() (for whatever string we want readlink(2) to return). These should not be occur on anything NFS-exported or on overlayfs layers, since neither NFSD nor overlayfs don't know what to do with such. What you are proposing is a weird change along the lines of "if you accidentally return NULL it will be treated as empty body, except when it occurs on NFS exports or overlayfs layers; in such cases it will be interpreted as fs corruption. $DEITY help you if real procfs-style symlink hits one of those, since nd_jump_link() called by those will oops in such conditions". As a mitigation strategy it sucks. As part of calling conventions it's confusing and AFAICS absolutely pointless. NAK. And I'm really curious - what has lead to that? Because procfs-style symlink in such conditions would have oopsed before it returned from ->get_link()...
