[PATCH 0/1] MR451: ntdll: Only check casefold flag when filesystem supports it.
Currently the wine function `get_dir_case_sensitivity_stat` calls `ioctl` syscall with command `EXT2_IOC_GETFLAGS` and tries to read a flag `EXT4_CASEFOLD_FL` from the result. It does this on all filesystems without checking if the filesystem supports that flag. This patch adds a check: only do the call on filesystems that are known to support `EXT4_CASEFOLD_FL`. To my knowledge these are EXT4 and F2FS. Advantages: - The `EXT4_CASEFOLD_FL` flag is nonstandard and other filesystems may use the same binary value to represent another flag. This patch prevents flag compatibility issues with other filesystems. - This fixes (or works around) an issue on FUSE filesystems using libfuse 3.10 or older and kernel 5.13 or newer, where unsupported `ioctl` calls always succeed and return random (uninitialized) data (see https://github.com/libfuse/libfuse/issues/640). On such setups Wine randomly fails to read files because it thinks that the `EXT4_CASEFOLD_FL` is set when really it is just random data. Ubuntu 22.04 LTS has settled on libfuse 3.10 and kernel 5.15 so this issue will be around for years to come. Disadvantages: - The flag will not be read on possible other current and future filesystems that support it. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/451
From: Tuupertunut <tuupertunut(a)outlook.com> --- dlls/ntdll/unix/file.c | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c index e09e8cafc82..eaca5297680 100644 --- a/dlls/ntdll/unix/file.c +++ b/dlls/ntdll/unix/file.c @@ -148,6 +148,18 @@ typedef struct /* Case-insensitivity attribute */ #define EXT4_CASEFOLD_FL 0x40000000 +#ifndef EXT4_SUPER_MAGIC +#define EXT4_SUPER_MAGIC 0xef53 +#endif + +#ifndef F2FS_SUPER_MAGIC +#define F2FS_SUPER_MAGIC 0xf2f52010 +#endif + +#ifndef FUSE_SUPER_MAGIC +#define FUSE_SUPER_MAGIC 0x65735546 +#endif + #ifndef O_DIRECTORY # define O_DIRECTORY 0200000 /* must be directory */ #endif @@ -1163,15 +1175,20 @@ static BOOLEAN get_dir_case_sensitivity_stat( const char *dir ) if ((fd = open( dir, O_RDONLY | O_NONBLOCK )) == -1) return TRUE; - if (ioctl( fd, EXT2_IOC_GETFLAGS, &flags ) != -1 && (flags & EXT4_CASEFOLD_FL)) + if (fstatfs( fd, &stfs ) != -1) { - sens = FALSE; - } - else if (fstatfs( fd, &stfs ) == 0 && /* CIOPFS is case insensitive. Instead of */ - stfs.f_type == 0x65735546 /* FUSE_SUPER_MAGIC */ && /* parsing mtab to discover if the FUSE FS */ - fstatat( fd, ".ciopfs", &st, AT_NO_AUTOMOUNT ) == 0) /* is CIOPFS, look for .ciopfs in the dir. */ - { - sens = FALSE; + /* EXT4 and F2FS have a flag that makes a directory case insensitive. */ + if ((stfs.f_type == EXT4_SUPER_MAGIC || stfs.f_type == F2FS_SUPER_MAGIC) && + ioctl( fd, EXT2_IOC_GETFLAGS, &flags ) != -1 && (flags & EXT4_CASEFOLD_FL)) + { + sens = FALSE; + } + /* CIOPFS is case insensitive. Instead of parsing mtab to discover if the FUSE FS is CIOPFS, + * look for .ciopfs in the dir. */ + else if (stfs.f_type == FUSE_SUPER_MAGIC && fstatat( fd, ".ciopfs", &st, AT_NO_AUTOMOUNT ) == 0) + { + sens = FALSE; + } } close( fd ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/451
Disadvantages:
The flag will not be read on possible other current and future filesystems that support it.
It was already rejected for this reason. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/451#note_4287
This merge request was closed by Alexandre Julliard. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/451
On Thu Jul 14 20:22:34 2022 +0000, Alexandre Julliard wrote:
Disadvantages:
The flag will not be read on possible other current and future filesystems that support it. It was already rejected for this reason. Could you link to where this previous rejection happened? Was it a conversation that happened somewhere where I could read it?
Anyway, do you think that we can safely assume that no other filesystem will interpret these flags differently? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/451#note_4335
Could you link to where this previous rejection happened? Was it a conversation that happened somewhere where I could read it?
Probably on the mailing list, but a quick search didn't turn it up.
Anyway, do you think that we can safely assume that no other filesystem will interpret these flags differently?
I think so yes, cf. the comments in https://github.com/torvalds/linux/blob/master/include/uapi/linux/fs.h -- https://gitlab.winehq.org/wine/wine/-/merge_requests/451#note_4338
On Thu Jul 14 20:57:01 2022 +0000, Alexandre Julliard wrote:
Could you link to where this previous rejection happened? Was it a conversation that happened somewhere where I could read it? Probably on the mailing list, but a quick search didn't turn it up. Anyway, do you think that we can safely assume that no other filesystem will interpret these flags differently? I think so yes, cf. the comments in https://github.com/torvalds/linux/blob/master/include/uapi/linux/fs.h Fair enough.
One thing that could be improved though is that it would use those `FS_` constant names from the kernel instead of `EXT2` and `EXT4` to highlight that the flags are used in a generic filesystem independent way. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/451#note_4339
participants (3)
-
Alexandre Julliard (＠julliard) -
Tuupertunut
-
Tuupertunut (＠Tuupertunut)