On 2019-07-14, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
> On Sat, Jul 13, 2019 at 03:41:53AM +0100, Al Viro wrote:
> > On Fri, Jul 12, 2019 at 04:00:26PM +0100, Al Viro wrote:
> > > On Fri, Jul 12, 2019 at 02:25:53PM +0100, Al Viro wrote:
> > >
> > > > if (flags & LOOKUP_BENEATH) {
> > > > nd->root = nd->path;
> > > > if (!(flags & LOOKUP_RCU))
> > > > path_get(&nd->root);
> > > > else
> > > > nd->root_seq = nd->seq;
> > >
> > > BTW, this assignment is needed for LOOKUP_RCU case. Without it
> > > you are pretty much guaranteed that lazy pathwalk will fail,
> > > when it comes to complete_walk().
> > >
> > > Speaking of which, what would happen if LOOKUP_ROOT/LOOKUP_BENEATH
> > > combination would someday get passed?
> >
> > I don't understand what's going on with ->r_seq in there - your
> > call of path_is_under() is after having (re-)sampled rename_lock,
> > but if that was the only .. in there, who's going to recheck
> > the value? For that matter, what's to guarantee that the thing
> > won't get moved just as you are returning from handle_dots()?
> >
> > IOW, what does LOOKUP_IN_ROOT guarantee for caller (openat2())?
>
> Sigh... Usual effects of trying to document things:
>
> 1) LOOKUP_NO_EVAL looks bogus. It had been introduced by commit 57d4657716ac
> (audit: ignore fcaps on umount) and AFAICS it's crap. It is set in
> ksys_umount() and nowhere else. It's ignored by everything except
> filename_mountpoint(). The thing is, call graph for filename_mountpoint()
> is
> filename_mountpoint()
> <- user_path_mountpoint_at()
> <- ksys_umount()
> <- kern_path_mountpoint()
> <- autofs_dev_ioctl_ismountpoint()
> <- find_autofs_mount()
> <- autofs_dev_ioctl_open_mountpoint()
> <- autofs_dev_ioctl_requester()
> <- autofs_dev_ioctl_ismountpoint()
> In other words, that flag is basically "was filename_mountpoint()
> been called by umount(2) or has it come from an autofs ioctl?".
> And looking at the rationale in that commit, autofs ioctls need
> it just as much as umount(2) does. Why is it not set for those
> as well? And why is it conditional at all?
In addition, LOOKUP_NO_EVAL == LOOKUP_OPEN (0x100). Is that meant to be
the case? Also I just saw you have a patch in work.namei that fixes this
up -- do you want me to rebase on top of that?
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>
Attachment:
signature.asc
Description: PGP signature
