On Fri, Sep 1, 2023 at 11:02 PM Ruiwen Zhao <ruiwen@xxxxxxxxxx> wrote: > On Fri, Sep 1, 2023 at 12:27 PM Ruiwen Zhao <ruiwen@xxxxxxxxxx> wrote: > > > > Hi all, > > > > Thanks for all the help! I tested the easy fix (i.e. to ignore ENXIO, see git diff here [1]), and can confirm it worked. The setup is the same as the repro steps I mentioned above, so I am only pasting the last step here: > > > > ``` > > ruiwen@instance-1:/tmp # mv merged/home/ruiwen/foolink merged/home/ruiwen/foolink2 > > ruiwen@instance-1:/tmp # ls -l merged/home/ruiwen/ -l > > total 0 > > -rw-r--r-- 1 root root 0 Sep 1 18:46 foo > > lrwxrwxrwx 1 root root 3 Sep 1 18:47 foolink2 -> foo > > ``` > > > > I also checked dmesg and can confirm that there is no error. I can send out a PR for this fix. Meanwhile I have a followup question on the source of ENXIO. > > > > Amir - you mentioned that ENXIO might come from no_open() default ->open() method. I found no_open in fs/inode.c returned ENXIO [2], but cannot connect it to ovl_security_fileattr. If the error happens on every open, then it will happen on even reading the symlink, instead of only moving it, right? Can you elaborate on no_open() default ->open() method? > > > > > > [1] https://gist.github.com/ruiwen-zhao/93ebe0b4ad20fab005d0300e9f0194c2 > > > > [2] https://github.com/torvalds/linux/blob/b84acc11b1c9552c9ca3a099b1610a6018619332/fs/inode.c#L145 > > > > Thanks, > > Ruiwen > > > > On Fri, Sep 1, 2023 at 10:58 AM Amir Goldstein <amir73il@xxxxxxxxx> wrote: > >> > >> On Fri, Sep 1, 2023 at 1:14 PM Miklos Szeredi <miklos@xxxxxxxxxx> wrote: > >> > > >> > On Fri, 1 Sept 2023 at 12:08, Amir Goldstein <amir73il@xxxxxxxxx> wrote: > >> > > > >> > > On Fri, Sep 1, 2023 at 12:59 AM Ruiwen Zhao <ruiwen@xxxxxxxxxx> wrote: > >> > > > > >> > > > Thanks for the reply Amir! Really helps. I will try the easy fix (i.e. ignoring ENXIO) and test it. Meanwhile I have two questions: > >> > > > > >> > > > 1. Since ENXIO comes from ovl_security_fileattr() trying to open the symlink, I was trying to find who returns ENXIO in the first place. I did some code search on libfuse (https://github.com/libfuse/libfuse), but cannot find ENXIO anywhere. Could it be from the kernel side? (I am trying to use this as a justification of the easy fix.) > >> > > > > >> > > > >> > > I think ENXIO comes from no_open() default ->open() method. > >> > > > >> > > > 2. Miklos's commit message says "The reason is that ovl_copy_fileattr() is triggered due to S_NOATIME being > >> > > > set on all inodes (by fuse) regardless of fileattr." Does that mean `ovl_copy_fileattr` should not be triggered on symlink files but it is, and therefore it is getting the errors like ENXIO and ENOTTY? > >> > > > > >> > > > >> > > Miklos' comment explains why ovl_copy_fileattr() passes the gate: > >> > > > >> > > if (inode->i_flags & OVL_COPY_I_FLAGS_MASK) { > >> > > > >> > > S_NOATIME is an indication that the file MAY have fileattr FS_NOATIME_FL, > >> > > but in the case of FUSE and NFS, S_NOATIME is there for another unrelated > >> > > reason. > >> > > > >> > > In any case, S_NOATIME on a symlink is never an indication of fileattr, > >> > > so I think the correct solution is to add the conditions to the gate: > >> > > > >> > > if (inode->i_flags & OVL_COPY_I_FLAGS_MASK && > >> > > ((S_ISDIR(inode->i_mode) || S_ISREG(inode->i_mode))) { > >> > > > >> > > > (Sending again in plain-text mode because previous email was blocked) In the future, when sending to mailing lists, please do not "top post". write your reply below the text, so mailing list readers can follow the conversation. > > Hi all, > > Thanks for all the help! I tested the easy fix (i.e. to ignore ENXIO, > see git diff here [1]), and can confirm it worked. The setup is the For the record, there is no need to check ENXIO on upper. It makes no sense to get ENXIO from upper and not from lower. > same as the repro steps I mentioned above, so I am only pasting the > last step here: Thanks for the report. Could you please also test the alternative I suggested above. > > ``` > ruiwen@instance-1:/tmp # mv merged/home/ruiwen/foolink > merged/home/ruiwen/foolink2 > ruiwen@instance-1:/tmp # ls -l merged/home/ruiwen/ -l > total 0 > -rw-r--r-- 1 root root 0 Sep 1 18:46 foo > lrwxrwxrwx 1 root root 3 Sep 1 18:47 foolink2 -> foo > ``` > Thank you for testing. I will add your Tested-by. If you want to learn how to write an xfstest for this repro I can guide you - if not, I will do it myself. > I also checked dmesg and can confirm that there is no error. I can > send out a PR for this fix. Meanwhile I have a followup question on > the source of ENXIO. > > Amir - you mentioned that ENXIO might come from no_open() default > ->open() method. I found no_open in fs/inode.c returned ENXIO [2], but > cannot connect it to ovl_security_fileattr. If the error happens on > every open, then it will happen on even reading the symlink, instead > of only moving it, right? Can you elaborate on no_open() default > ->open() method? > > [1] https://gist.github.com/ruiwen-zhao/93ebe0b4ad20fab005d0300e9f0194c2 > > [2] https://github.com/torvalds/linux/blob/b84acc11b1c9552c9ca3a099b1610a6018619332/fs/inode.c#L145 > The way that fileattr flags are set and read from a file/dir is via ioctl. It is not possible to open a symlink to perform the ioctl in this manner. The situation with blockdev and chardev is even messier - it is possible to open them for ioctl, but trying to set the fileattr with ioctl is not expected to work and in any case, those are not going to be inode attributes that ovl needs to copy up. Thanks, Amir.
