Skip to contentSkip to conversation

Revert "ext4: apply umask if ACL support is disabled"

PATCH Applied Max Kellermann 6 participants 7 messages Mar 15, 2024 – Feb 19, 2025

Subsystems (2)
Stable branch ext4 file system
lore
Share & tools
lore

Max Kellermann

author
#0 lore raw

PATCH 1/1 Revert "ext4: apply umask if ACL support is disabled"

This reverts commit 484fd6c1de13b336806a967908a927cc0356e312. The commit caused a regression because now the umask was applied to symlinks and the fix is unnecessary because the umask/O_TMPFILE bug has been fixed somewhere else already.

Fixes https://lore.kernel.org/lkml/[email protected]/ open here
Signed-off-by Max Kellermann <[email protected]>
---
 fs/ext4/acl.h | 5 -----
 1 file changed, 5 deletions(-)
fs/ext4/acl.h +0 -5
index ef4c19e5f570..0c5a79c3b5d4 100644
@@ -68,11 +68,6 @@ extern int ext4_init_acl(handle_t *, struct inode *, struct inode *);
 static inline int
 ext4_init_acl(handle_t *handle, struct inode *inode, struct inode *dir)
 {
-	/* usually, the umask is applied by posix_acl_create(), but if
-	   ext4 ACL support is disabled at compile time, we need to do
-	   it here, because posix_acl_create() will never be called */
-	inode->i_mode &= ~current_umask();
-
 	return 0;
 }
 #endif  /* CONFIG_EXT4_FS_POSIX_ACL */
Signature · 1 line
2.39.2
Reviewed-by on v1 1/1
#1 lore raw
Quoted 8 lines · Max Kellermann

This reverts commit 484fd6c1de13b336806a967908a927cc0356e312. The commit caused a regression because now the umask was applied to symlinks and the fix is unnecessary because the umask/O_TMPFILE bug has been fixed somewhere else already.

Fixes https://lore.kernel.org/lkml/[email protected]/ open here
Signed-off-by Max Kellermann <[email protected]>

---

Looks good to me,

Reviewed-by Christian Brauner <[email protected]>
Tested-by on v1 1/1
#2 lore raw
In reply to Max Kellermann
Quoted 4 lines · Max Kellermann

This reverts commit 484fd6c1de13b336806a967908a927cc0356e312. The commit caused a regression because now the umask was applied to symlinks and the fix is unnecessary because the umask/O_TMPFILE bug has been fixed somewhere else already.

Thanks, Max! I've verified that this fixes symlink modes for me, as well as the flatpak corruption error I was getting.

Quoted 2 lines
Fixes https://lore.kernel.org/lkml/[email protected]/ open here
Signed-off-by Max Kellermann <[email protected]>
Tested-by Michael Forney <[email protected]>
#3 lore raw
Quoted 6 lines · Michael Forney
Quoted 4 lines · Max Kellermann

This reverts commit 484fd6c1de13b336806a967908a927cc0356e312. The commit caused a regression because now the umask was applied to symlinks and the fix is unnecessary because the umask/O_TMPFILE bug has been fixed somewhere else already.

Quoted 7 lines

Thanks, Max! I've verified that this fixes symlink modes for me, as well as the flatpak corruption error I was getting.

Quoted 2 lines
Fixes https://lore.kernel.org/lkml/[email protected]/ open here
Signed-off-by Max Kellermann <[email protected]>
Tested-by Michael Forney <[email protected]>

Just checking in on this. I'd really like to see this regression fixed. It currently affects versions 6.5 through 6.9-rc6 as well as all longterm releases.

maintainer Applied
#4 lore raw
In reply to Max Kellermann
Quoted 6 lines · Max Kellermann

This reverts commit 484fd6c1de13b336806a967908a927cc0356e312. The commit caused a regression because now the umask was applied to symlinks and the fix is unnecessary because the umask/O_TMPFILE bug has been fixed somewhere else already.

Applied, thanks!

[1/1] Revert "ext4: apply umask if ACL support is disabled"

      commit: c77194965dd0dcc26f9c1671d2e74e4eb1248af5

Best regards,

Signature · 1 line
Theodore Ts'o <[email protected]>
#5 lore raw

PATCH 1/1 Revert "ext4: apply umask if ACL support is disabled"

From: Max Kellermann <[email protected]>

This reverts commit 484fd6c1de13b336806a967908a927cc0356e312. The commit caused a regression because now the umask was applied to symlinks and the fix is unnecessary because the umask/O_TMPFILE bug has been fixed somewhere else already.

Fixes https://lore.kernel.org/lkml/[email protected]/ open here
Signed-off-by Max Kellermann <[email protected]>
Reviewed-by Christian Brauner <[email protected]>
Tested-by Michael Forney <[email protected]>
Link https://lore.kernel.org/r/[email protected] open here
Signed-off-by Theodore Ts'o <[email protected]>
(cherry picked from commit c77194965dd0dcc26f9c1671d2e74e4eb1248af5)

---
This revert never reached linux-5.15.y, and caused the same regression with symbolic links. The original problem is just as the revert states fixed and the test program for O_TMPFILE works as expected.

---
 fs/ext4/acl.h | 5 -----
 1 file changed, 5 deletions(-)
fs/ext4/acl.h +0 -5
index 0db6ae0ca936..3219669732bf 100644
@@ -68,11 +68,6 @@ extern int ext4_init_acl(handle_t *, struct inode *, struct inode *);
 static inline int
 ext4_init_acl(handle_t *handle, struct inode *inode, struct inode *dir)
 {
-	/* usually, the umask is applied by posix_acl_create(), but if
-	   ext4 ACL support is disabled at compile time, we need to do
-	   it here, because posix_acl_create() will never be called */
-	inode->i_mode &= ~current_umask();
-
 	return 0;
 }
 #endif  /* CONFIG_EXT4_FS_POSIX_ACL */
Signature · 1 line
2.36.0
maintainer
#6 lore raw
Quoted 15 lines · Jesper Nilsson

From: Max Kellermann <[email protected]>

This reverts commit 484fd6c1de13b336806a967908a927cc0356e312. The commit caused a regression because now the umask was applied to symlinks and the fix is unnecessary because the umask/O_TMPFILE bug has been fixed somewhere else already.

Fixes https://lore.kernel.org/lkml/[email protected]/ open here
Signed-off-by Max Kellermann <[email protected]>
Reviewed-by Christian Brauner <[email protected]>
Tested-by Michael Forney <[email protected]>
Link https://lore.kernel.org/r/[email protected] open here
Signed-off-by Theodore Ts'o <[email protected]>
(cherry picked from commit c77194965dd0dcc26f9c1671d2e74e4eb1248af5)
Quoted 4 lines

---
This revert never reached linux-5.15.y, and caused the same regression with symbolic links. The original problem is just as the revert states fixed and the test program for O_TMPFILE works as expected.

This revert never hit any of the other stable kernels either, should it go to all of them?

Also, you didn't sign off on this :(

thanks,

greg k-h