From: Eric Biggers <ebiggers3@gmail.com>
Date: Mon, 13 Jul 2015 00:49:05 +0000 (-0500)
Subject: ntfs-3g_apply.c: only call sd_fixup() if ntfs_set_ntfs_acl() fails
X-Git-Tag: v1.8.2~43
X-Git-Url: https://wimlib.net/git/?p=wimlib;a=commitdiff_plain;h=a5f60b5a884906945b841457874c83e609b016af;ds=sidebyside

ntfs-3g_apply.c: only call sd_fixup() if ntfs_set_ntfs_acl() fails
---

diff --git a/src/ntfs-3g_apply.c b/src/ntfs-3g_apply.c
index dc61e477..ba90fae8 100644
--- a/src/ntfs-3g_apply.c
+++ b/src/ntfs-3g_apply.c
@@ -110,19 +110,18 @@ sid_size(const wimlib_SID *sid)
  *   moving the empty SACL earlier in the security descriptor or by removing the
  *   SACL entirely.  The latter work-around is valid because an empty SACL is
  *   equivalent to a "null", or non-existent, SACL.
- * - Versions up to and including 2013.1.13 reject security descriptors ending
- *   with an empty DACL (Discretionary Access Control List).  This is very
- *   similar to the SACL bug and should be fixed in the next release after
- *   2013.1.13.  However, removing the DACL is not a valid workaround because
- *   this changes the meaning of the security descriptor--- an empty DACL allows
- *   no access, whereas a "null" DACL allows all access.
+ * - Versions before 2014.2.15 reject security descriptors ending with an empty
+ *   DACL (Discretionary Access Control List).  This is very similar to the SACL
+ *   bug.  However, removing the DACL is not a valid workaround because this
+ *   changes the meaning of the security descriptor--- an empty DACL allows no
+ *   access, whereas a "null" DACL allows all access.
  *
  * If the security descriptor was fixed, this function returns an allocated
  * buffer containing the fixed security descriptor, and its size is updated.
- * Otherwise (or if no memory is available) the original descriptor is returned.
+ * Otherwise (or if no memory is available) NULL is returned.
  */
-static u8 *
-sd_fixup(const u8 *_desc, size_t *size_p)
+static void *
+sd_fixup(const void *_desc, size_t *size_p)
 {
 	u32 owner_offset, group_offset, dacl_offset;
 #if !defined(HAVE_NTFS_MNT_RDONLY)
@@ -130,14 +129,13 @@ sd_fixup(const u8 *_desc, size_t *size_p)
 #endif
 	bool owner_valid, group_valid;
 	size_t size = *size_p;
-	const wimlib_SECURITY_DESCRIPTOR_RELATIVE *desc =
-			(const wimlib_SECURITY_DESCRIPTOR_RELATIVE*)_desc;
+	const wimlib_SECURITY_DESCRIPTOR_RELATIVE *desc = _desc;
 	wimlib_SECURITY_DESCRIPTOR_RELATIVE *desc_new;
 	const wimlib_SID *owner, *group, *sid;
 
 	/* Don't attempt to fix clearly invalid security descriptors.  */
 	if (size < sizeof(wimlib_SECURITY_DESCRIPTOR_RELATIVE))
-		return (u8*)_desc;
+		return NULL;
 
 	if (le16_to_cpu(desc->control) & wimlib_SE_DACL_PRESENT)
 		dacl_offset = le32_to_cpu(desc->dacl_offset);
@@ -161,7 +159,7 @@ sd_fixup(const u8 *_desc, size_t *size_p)
 	    (sacl_offset != 0 && sacl_offset == size - sizeof(wimlib_ACL)) ||
 	#endif
 	    (dacl_offset != 0 && dacl_offset == size - sizeof(wimlib_ACL))))
-		return (u8*)_desc;
+		return NULL;
 
 	owner_offset = le32_to_cpu(desc->owner_offset);
 	group_offset = le32_to_cpu(desc->group_offset);
@@ -186,12 +184,12 @@ sd_fixup(const u8 *_desc, size_t *size_p)
 	} else if (group_valid) {
 		sid = group;
 	} else {
-		return (u8*)_desc;
+		return NULL;
 	}
 
 	desc_new = MALLOC(size + sid_size(sid));
 	if (!desc_new)
-		return (u8*)_desc;
+		return NULL;
 
 	memcpy(desc_new, desc, size);
 	if (owner_valid)
@@ -200,7 +198,7 @@ sd_fixup(const u8 *_desc, size_t *size_p)
 		desc_new->group_offset = cpu_to_le32(size);
 	memcpy((u8*)desc_new + size, sid, sid_size(sid));
 	*size_p = size + sid_size(sid);
-	return (u8*)desc_new;
+	return desc_new;
 }
 
 /* Set the security descriptor @desc of size @desc_size on the NTFS inode @ni.
@@ -209,20 +207,25 @@ static int
 ntfs_3g_set_security_descriptor(ntfs_inode *ni, const void *desc, size_t desc_size)
 {
 	struct SECURITY_CONTEXT sec_ctx;
-	u8 *desc_fixed;
+	void *desc_fixed = NULL;
 	int ret = 0;
 
 	memset(&sec_ctx, 0, sizeof(sec_ctx));
 	sec_ctx.vol = ni->vol;
 
-	desc_fixed = sd_fixup(desc, &desc_size);
-
-	if (ntfs_set_ntfs_acl(&sec_ctx, ni, desc_fixed, desc_size, 0))
+retry:
+	if (ntfs_set_ntfs_acl(&sec_ctx, ni, desc, desc_size, 0)) {
+		if (desc_fixed == NULL) {
+			desc_fixed = sd_fixup(desc, &desc_size);
+			if (desc_fixed != NULL) {
+				desc = desc_fixed;
+				goto retry;
+			}
+		}
 		ret = WIMLIB_ERR_SET_SECURITY;
+	}
 
-	if (desc_fixed != desc)
-		FREE(desc_fixed);
-
+	FREE(desc_fixed);
 	return ret;
 }