[PATCH v2] ntfs: record an error when the ACL xattr undo fails

[Hongling Zeng]

ntfs_set_acl_ex() commits the new ACL xattr before it rewrites the WSL
mode EA, and removes it again when that rewrite fails.  The removal is
best-effort: its result is discarded and the EA paths record no error
state, so when the removal fails too, the new, typically more
permissive, ACL is left on disk while i_mode, the mode EA and the
cached ACL keep the old values.  Once the stale xattr is loaded again -
after a remount, or when the cached ACL is discarded - a chmod 0640 ->
0777 that reported failure leaves the file accessible to the relaxed
ACL while stat() still reports 0640.

Check the removal and record the failure: NVolSetErrors(), which the
next persistence point persists as VOLUME_IS_DIRTY, and a log message
the errors= policy acts on.  The flag cannot be written to disk here
directly: the target inode's mrec_lock is held and the $Volume
mrec_lock cannot be nested under it.

Fixes: fc053f05ca28 ("ntfs: add reparse and ea operations")
Signed-off-by: Hongling Zeng <zenghongling@xxxxxxxxxx>
---
 Change in v2:
 -undo only a committed set: undoing a removal returned -ENODATA for an
  already-removed xattr and was recorded as a volume error, or re-added
  an empty xattr once the removal had dropped the last EA
---
 fs/ntfs/ea.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/fs/ntfs/ea.c b/fs/ntfs/ea.c
index b4fcfbe2da4c..b3e079e2dc03 100644
--- a/fs/ntfs/ea.c
+++ b/fs/ntfs/ea.c
@@ -1084,8 +1084,34 @@ static noinline int ntfs_set_acl_ex(struct mnt_idmap *idmap,
 		mutex_lock(&NTFS_I(inode)->mrec_lock);
 		err = ntfs_ea_set_wsl_inode(inode, 0, &ea_size, NTFS_EA_MODE);
 		if (err) {
-			ntfs_set_ea(inode, name, name_len, NULL, 0,
-				    XATTR_REPLACE, NULL);
+			/*
+			 * Only a set needs undoing: a removal leaves no
+			 * new ACL behind, and repeating it would either
+			 * return -ENODATA again or, once it dropped the
+			 * last EA, re-create an empty xattr.
+			 */
+			if (acl) {
+				int undo_err = ntfs_set_ea(inode, name, name_len,
+						NULL, 0, XATTR_REPLACE, NULL);
+
+				if (undo_err) {
+					/*
+					 * The ACL xattr is committed
+					 * already, so a failed removal
+					 * leaves the new, typically more
+					 * permissive, ACL on disk while
+					 * i_mode, the mode EA and the
+					 * cached ACL keep the old values.
+					 * Record the error: the next
+					 * persistence point persists it as
+					 * VOLUME_IS_DIRTY.  Run chkdsk.
+					 */
+					NVolSetErrors(NTFS_I(inode)->vol);
+					ntfs_error(inode->i_sb,
+						   "Failed to undo the ACL xattr: %d",
+						   undo_err);
+				}
+			}
 			mutex_unlock(&NTFS_I(inode)->mrec_lock);
 			inode->i_mode = old_mode;
 			goto out;
-- 
2.25.1