afs_linux_lookup: Avoid d_add on afs_lookup error 37/12637/5
authorAndrew Deason <adeason@sinenomine.net>
Thu, 15 Jun 2017 20:29:17 +0000 (15:29 -0500)
committerBenjamin Kaduk <kaduk@mit.edu>
Wed, 5 Jul 2017 16:00:16 +0000 (12:00 -0400)
commitd7211350eec18b30e9ccf30f5e95fb58162c637d
treeda654b5027eb8c79adf730e53aacf90a515293ac
parent5dd2ce2043f53e80e1ded25abcfd565b4071a3ad
afs_linux_lookup: Avoid d_add on afs_lookup error

Currently, afs_linux_lookup looks roughly like this pseudocode:

{
    code = afs_lookup(&vcp);
    if (!code) {
        ip = AFSTOV(vcp);
        error = process_ip(ip);
        if (error) {
            goto done;
        }
    }
    process_dp(dp);
    newdp = d_splice_alias(ip, dp);
 done:
    cleanup();
}

Note that if there is an error while processing the looked-up inode
(ip), we jump over d_splice_alias. But if we encounter an error from
afs_lookup itself, we do not jump over d_splice_alias. This means that
if afs_lookup encounters any error, we initialize the given dentry
(dp) as a negative entry, effectively telling the Linux kernel that
the requested name does not exist.

This is correct for ENOENT errors, of course, but is incorrect for any
other error. For non-ENOENT errors we later return an error from the
function, but this does not invalidate the generated dentry. The
result is that when afs_lookup encounters an error, that error will be
propagated to userspace, but subsequent lookups for the same name will
yield an ENOENT error (until the dentry is invalidated). This can
easily cause a file to seem to mysteriously disappear, if a transient
error like network problems caused the afs_lookup call to fail.

To fix this, treat ENOENT as a non-error, like the comments already
suggest. In our case, ENOENT is not really an error; it just means we
populate the given dentry differently. So if we get ENOENT from
afs_lookup, set our vcache to NULL and clear the error, and continue.

This also has the side effect of not treating ENOENT errors from
afs_CreateAttr identically to ENOENT errors from afs_lookup. That
shouldn't happen, but there have been abuses of the ENOENT error code
in the past, so it is probably better to be cautious.

Many thanks to Gaja Sophie Peters for assistance in tracking down and
testing fixes for this issue, including providing access to test systems
experiencing the buggy behavior.

FIXES 133654

Change-Id: Ia9aab289d5c041557ab6b00f1d41de2edfc97a89
Reviewed-on: https://gerrit.openafs.org/12637
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Joe Gorse <jhgorse@gmail.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Tested-by: Michael Meffie <mmeffie@sinenomine.net>
src/afs/LINUX/osi_vnodeops.c