> > > > > + nd->intent.open.file = NULL;
> > > >
> > > > Why is this NULL assignment needed? nd will not be used after this.
> > > >
> > > > > + }
> > > > > + path_release(nd);
> > > > > +}
> > > > > +
> > > > >
> > >
> > > It could be dropped. The reason for putting it in is that some parts of
> > > the VFS may restart a path walk operation if it fails (see for instance
> > > the ESTALE handling).
> >
> > If you use the nameidata after path_release_open_intent(), you're
> > screwed anyway, since nd->mnt and nd->dentry have already been
> > released.
>
> There is quite a bit of code out there that assumes it is free to stuff
> things into nd->mnt and nd->dentry. Some of it is Al Viro's code, some
> of it is from other people.
> For instance, the ESTALE handling will just save nd->mnt/nd->dentry
> before calling __link_path_walk(), then restore + retry if the ESTALE
> error comes up.
Yeah, but how is that relevant to the fact, that after
path_release_*() _nothing_ will be valid in the nameidata, not
nd->mnt, not nd->dentry, and not nd->intent.open.file. So what's the
point in setting it to NULL if it must never be used anyway?
> > If there's any chance that the path walk restart thing will invoke the
> > filesystems open code twice (I doubt it), then the filesystem must
> > make sure to check intent.open.file, whether it has already been set,
> > and fput() it before setting it another time.
>
> The only user of that code is NFSv4, and we will never even try to
> allocate a file if the OPEN call returned an ESTALE.
That's why I doubted that this is an issue.
> > > Why do we want to keep this behaviour? It is undocumented, it is
> > > non-posix, and it appears to do nothing you cannot do with the existing
> > > access() call.
> > >
> > > Are there any applications using it? If so, which ones, and why?
> >
> > I have absolutely no idea.
> >
> > Looking closer, there's a problem with O_TRUNC as well:
> >
> > namei_flags = flags;
> > if ((namei_flags+1) & O_ACCMODE)
> > namei_flags++;
> > if (namei_flags & O_TRUNC)
> > namei_flags |= 2;
> >
> > So if flags is O_RDONLY|O_TRUNC, intent.open.flags will be
> > FMODE_WRITE|FMODE_READ|O_TRUNC, but filp->f_mode will be FMODE_READ.
>
> That is a bug that needs to be fixed in the intent.open.flags. We don't
> ever want to be opening the file for writing at the filesystem level
> when the user specified open for read.
No, it's being opened for read. The namei_flags (and hence
intent.open.flags) will have FMODE_WRITE, so that the permission is
checked for write.
So I thing the bug is not in the calculation of namei_flags, rather
the fact that intent.open.flags is set to namei_flags, rather than the
original open flags.
Miklos
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
| |
 |