On Jun 3, 2016, at 6:37 PM, Al Viro wrote:
> On Fri, Jun 03, 2016 at 11:23:55PM +0100, Al Viro wrote:
>
>> It's not that. It's explicit put_link() in do_last(), followed by
>> ESTALEOPEN and subsequent misbegotten "retry the last step on ESTALEOPEN"
>> looking at now-freed nd->last.name. IOW, the bug predates delayed_call
>> stuff.
>
> EOPENSTALE, that is... Oleg, could you check if the following works?
Yes, this one lasted for an hour with no crashing, so it must be good.
Thanks.
(note, I am not equipped to verify correctness of NFS operations, though).
> diff --git a/fs/namei.c b/fs/namei.c
> index 4c4f95a..3d9511e 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -3166,9 +3166,7 @@ static int do_last(struct nameidata *nd,
> int acc_mode = op->acc_mode;
> unsigned seq;
> struct inode *inode;
> - struct path save_parent = { .dentry = NULL, .mnt = NULL };
> struct path path;
> - bool retried = false;
> int error;
>
> nd->flags &= ~LOOKUP_PARENT;
> @@ -3211,7 +3209,6 @@ static int do_last(struct nameidata *nd,
> return -EISDIR;
> }
>
> -retry_lookup:
> if (open_flag & (O_CREAT | O_TRUNC | O_WRONLY | O_RDWR)) {
> error = mnt_want_write(nd->path.mnt);
> if (!error)
> @@ -3292,23 +3289,14 @@ finish_lookup:
> if (unlikely(error))
> return error;
>
> - if ((nd->flags & LOOKUP_RCU) || nd->path.mnt != path.mnt) {
> - path_to_nameidata(&path, nd);
> - } else {
> - save_parent.dentry = nd->path.dentry;
> - save_parent.mnt = mntget(path.mnt);
> - nd->path.dentry = path.dentry;
> -
> - }
> + path_to_nameidata(&path, nd);
> nd->inode = inode;
> nd->seq = seq;
> /* Why this, you ask? _Now_ we might have grown LOOKUP_JUMPED... */
> finish_open:
> error = complete_walk(nd);
> - if (error) {
> - path_put(&save_parent);
> + if (error)
> return error;
> - }
> audit_inode(nd->name, nd->path.dentry, 0);
> error = -EISDIR;
> if ((open_flag & O_CREAT) && d_is_dir(nd->path.dentry))
> @@ -3331,13 +3319,9 @@ finish_open_created:
> goto out;
> BUG_ON(*opened & FILE_OPENED); /* once it's opened, it's opened */
> error = vfs_open(&nd->path, file, current_cred());
> - if (!error) {
> - *opened |= FILE_OPENED;
> - } else {
> - if (error == -EOPENSTALE)
> - goto stale_open;
> + if (error)
> goto out;
> - }
> + *opened |= FILE_OPENED;
> opened:
> error = open_check_o_direct(file);
> if (!error)
> @@ -3353,26 +3337,7 @@ out:
> }
> if (got_write)
> mnt_drop_write(nd->path.mnt);
> - path_put(&save_parent);
> return error;
> -
> -stale_open:
> - /* If no saved parent or already retried then can't retry */
> - if (!save_parent.dentry || retried)
> - goto out;
> -
> - BUG_ON(save_parent.dentry != dir);
> - path_put(&nd->path);
> - nd->path = save_parent;
> - nd->inode = dir->d_inode;
> - save_parent.mnt = NULL;
> - save_parent.dentry = NULL;
> - if (got_write) {
> - mnt_drop_write(nd->path.mnt);
> - got_write = false;
> - }
> - retried = true;
> - goto retry_lookup;
> }
>
> static int do_tmpfile(struct nameidata *nd, unsigned flags,
