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
