On Thu Dec 1 19:21:46 2022 +0000, Zebediah Figura wrote:
At the moment we are storing DOS attributes (`FILE_ATTRIBUTE_HIDDEN`
and `FILE_ATTRIBUTE_SYSTEM`) in the Samba-style extended attributes only, however depending on the OS and filesystem type these may not be available, or other methods of storage may be available instead.
The full list as I currently understand it is:
- Samba-style user extended attributes - works on Linux and MacOS with
modern filesystems.
- The `system.dos_attrib` extended attribute for the Linux ntfs3 driver
- The `FAT_IOCTL_SET_ATTRIBUTES` `ioctl` for the Linux FAT filesystem driver.
- `fchflags`/`st_flags` on MacOs, FreeBSD and NetBSD can represent
`FILE_ATTRIBUTE_READONLY` with `UF_IMMUTABLE` and `FILE_ATTRIBUTE_HIDDEN` with `UF_HIDDEN`. The show up in the UI on MacOS. (See Wine Bug [#39366](https://bugs.winehq.org/show_bug.cgi?id=39366))
I'm working on patches that take an opportunitistic approach of
storing the attributes in any place that might be available. When reloading the attributes, my plan was to load them from all the places that are available and OR them together. I'm not sure that it makes sense to use multiple approaches. Consider ntfs in particular, which IIRC has a dedicated DOS attribute field for all files, but also supports extended attributes. We probably don't want to store extended attributes as that'd be redundant. Same with Apple, whatever file system that is.
I also think we should revisit `FILE_ATTRIBUTE_READONLY`. Currently
this is excluded from the set of attributes that is stored in the Samba attribute. However, I think this design might have issues with interoperability - it would strip off the attribute if one was already present on a file. I think the proper solution to that is to not remove the attributes currently on a file, i.e. basically what this patch does for FAT, but for xattrs as well. (Along the same lines we should probably introduce support for parsing other Samba attribute versions, and avoid writing over the file times. But one thing at a time.)
As you know `FILE_ATTRIBUTE_READONLY` is currently stored in the
file's access mod flags. However, I think it would be better to add this to the mix for the `READONLY` attribute along with the other backends listed above.
At the same time I want to polish up this patch: [server: Allow to
open files without any permission bits.](https://github.com/wine-staging/wine-staging/blob/master/patches/server-File...). We spoke about this one previously - it temporarily `chmod`s a read-only file to allow it to be openned. (This is fixes Wine Bug [#50771](https://bugs.winehq.org/show_bug.cgi?id=50771))
The biggest catch I can see with my concept for the `READONLY`
attribute is that some of the backends would add additional layers of locking to a file, so to open read-only file, we would need to try to unlock it via `chmod`, `fchflags` and `FAT_IOCTL_SET_ATTRIBUTES` - just because we don't know how the attribute might have been set. Forunately, `FAT_IOCTL_SET_ATTRIBUTES` is Linux-only, and `fchflags` is BSD-only, but still it would add to code complexity.
Even though what I'm proposing is fairly complex, the problem is that
the current implementation is limited to Linux and MacOS, and will only work on modern filesystems, and won't properly interopate with FAT and NTFS filesystems. I think chmod is definitely the wrong approach. I don't understand why we'd need to remove the READONLY attribute in order to open a file without any access bits, though? I suspect the right answer is to open the file with O_PATH if available, and possibly to open it without a backing FD (i.e. with only a path name) if not. That latter might take some extra work, though.
Hi Zeb, thanks for your thoughts. Sorry for the delay in getting back to you - I've been rather busy these last few days.
I'm not sure that it makes sense to use multiple approaches. Consider ntfs in particular, which IIRC has a dedicated DOS attribute field for all files, but also supports extended attributes. We probably don't want to store extended attributes as that'd be redundant. Same with Apple, whatever file system that is.
Yes - ideally every file would only ever have the single most optimal form of attribute marking. However, it seems to me that it's going to be difficult to ensure this without doing a lot of probing. This will involve a lot more OS-specific code, and I'm not thrilled about the possibility of TOCTOU issues.
It seems more elegant to simply try various methods, and see what works. You are correct to say that on NTFS on Linux, every file will be labbelled by both a user extended attribute, and a system DOS attribute extended attrbite. But aside from being slightly inelegant, is this really a problem? It would mean that the file would retain its attribute if it were moved to another filesystem.
I think the proper solution to that is to not remove the attributes currently on a file, i.e. basically what this patch does for FAT, but for xattrs as well.
(Along the same lines we should probably introduce support for parsing other Samba attribute versions, and avoid writing over the file times. But one thing at a time.)
Completely agree.
I think chmod is definitely the wrong approach. I don't understand why we'd need to remove the READONLY attribute in order to open a file without any access bits, though?
This is my partially updated version of the patch from staging: https://gitlab.winehq.org/jhol/wine/-/commit/2d980a3c45b12d707b1a21f30e7af47...
Just looking at the logic, the only scenario that could trigger the chmod behaviour is `FILE_WRITE_ATTRIBUTES` on a read-only file.
I think `O_PATH` should work in this scenario? But we would need a fallback path for the other UNIXs.