Currently, applications that wish to get or set security descriptors have
to explicitly open the volume's security descriptor index ("$Secure")
using ntfs_open_secure(). Applications are also responsible for closing
the index when done with it. However, the cleanup function for doing so,
ntfs_close_secure(), cannot be called easily by all applications because
it requires a SECURITY_CONTEXT argument, not simply the ntfs_volume.
Some applications therefore have to close the inode and its index
contexts manually in order to clean up properly.
This proposal updates libntfs-3g to open $Secure unconditonally as part
of ntfs_mount(), so that applications do not have to worry about it.
ntfs_close_secure() is updated to take in a ntfs_volume for internal use,
and ntfs_destroy_security_context() is now the function to call to free
memory associated with a SECURITY_CONTEXT rather than a ntfs_volume.
Some memory leaks in error paths of ntfs_open_secure() are also fixed.
[v2: call ntfs_close_secure() earlier, check for error, and other cleanups]
Signed-off-by: Eric Biggers <[email protected]>
---
include/ntfs-3g/security.h | 4 +-
libntfs-3g/security.c | 97 +++++++++++++++++++++++++++++-----------------
libntfs-3g/volume.c | 9 +++++
src/lowntfs-3g.c | 6 +--
src/ntfs-3g.c | 6 +--
5 files changed, 75 insertions(+), 47 deletions(-)
diff --git a/include/ntfs-3g/security.h b/include/ntfs-3g/security.h
index b5c6375..c76ce9f 100644
--- a/include/ntfs-3g/security.h
+++ b/include/ntfs-3g/security.h
@@ -256,7 +256,9 @@ int ntfs_set_owner_mode(struct SECURITY_CONTEXT *scx,
le32 ntfs_inherited_id(struct SECURITY_CONTEXT *scx,
ntfs_inode *dir_ni, BOOL fordir);
int ntfs_open_secure(ntfs_volume *vol);
-void ntfs_close_secure(struct SECURITY_CONTEXT *scx);
+int ntfs_close_secure(ntfs_volume *vol);
+
+void ntfs_destroy_security_context(struct SECURITY_CONTEXT *scx);
#if POSIXACLS
diff --git a/libntfs-3g/security.c b/libntfs-3g/security.c
index ef036af..b87745a 100644
--- a/libntfs-3g/security.c
+++ b/libntfs-3g/security.c
@@ -4466,55 +4466,81 @@ int ntfs_set_ntfs_attrib(ntfs_inode *ni,
/*
- * Open $Secure once for all
- * returns zero if it succeeds
- * non-zero if it fails. This is not an error (on NTFS v1.x)
+ * Open the volume's security descriptor index ($Secure)
+ *
+ * returns 0 if it succeeds
+ * -1 with errno set if it fails and the volume is NTFS v3.0+
*/
-
-
int ntfs_open_secure(ntfs_volume *vol)
{
ntfs_inode *ni;
- int res;
+ ntfs_index_context *sii;
+ ntfs_index_context *sdh;
- res = -1;
- vol->secure_ni = (ntfs_inode*)NULL;
- vol->secure_xsii = (ntfs_index_context*)NULL;
- vol->secure_xsdh = (ntfs_index_context*)NULL;
- if (vol->major_ver >= 3) {
- /* make sure this is a genuine $Secure inode 9 */
- ni = ntfs_pathname_to_inode(vol, NULL, "$Secure");
- if (ni && (ni->mft_no == 9)) {
- vol->secure_reentry = 0;
- vol->secure_xsii = ntfs_index_ctx_get(ni,
- sii_stream, 4);
- vol->secure_xsdh = ntfs_index_ctx_get(ni,
- sdh_stream, 4);
- if (ni && vol->secure_xsii && vol->secure_xsdh) {
- vol->secure_ni = ni;
- res = 0;
- }
- }
+ if (vol->secure_ni) /* Already open? */
+ return 0;
+
+ ni = ntfs_pathname_to_inode(vol, NULL, "$Secure");
+ if (!ni)
+ goto err;
+
+ if (ni->mft_no != FILE_Secure) {
+ ntfs_log_error("$Secure does not have expected inode number!");
+ errno = EINVAL;
+ goto err_close_ni;
}
- return (res);
+
+ /* Allocate the needed index contexts. */
+ sii = ntfs_index_ctx_get(ni, sii_stream, 4);
+ if (!sii)
+ goto err_close_ni;
+
+ sdh = ntfs_index_ctx_get(ni, sdh_stream, 4);
+ if (!sdh)
+ goto err_close_sii;
+
+ vol->secure_xsdh = sdh;
+ vol->secure_xsii = sii;
+ vol->secure_ni = ni;
+ return 0;
+
+err_close_sii:
+ ntfs_index_ctx_put(sii);
+err_close_ni:
+ ntfs_inode_close(ni);
+err:
+ /* Failing on NTFS pre-v3.0 is expected. */
+ if (vol->major_ver < 3)
+ return 0;
+ ntfs_log_perror("Failed to open $Secure");
+ return -1;
}
/*
- * Final cleaning
- * Allocated memory is freed to facilitate the detection of memory leaks
+ * Close the volume's security descriptor index ($Secure)
+ *
+ * returns 0 if it succeeds
+ * -1 with errno set if it fails
*/
-
-void ntfs_close_secure(struct SECURITY_CONTEXT *scx)
+int ntfs_close_secure(ntfs_volume *vol)
{
- ntfs_volume *vol;
+ int res = 0;
- vol = scx->vol;
if (vol->secure_ni) {
- ntfs_index_ctx_put(vol->secure_xsii);
ntfs_index_ctx_put(vol->secure_xsdh);
- ntfs_inode_close(vol->secure_ni);
-
+ ntfs_index_ctx_put(vol->secure_xsii);
+ res = ntfs_inode_close(vol->secure_ni);
+ vol->secure_ni = NULL;
}
+ return res;
+}
+
+/*
+ * Destroy a security context
+ * Allocated memory is freed to facilitate the detection of memory leaks
+ */
+void ntfs_destroy_security_context(struct SECURITY_CONTEXT *scx)
+{
ntfs_free_mapping(scx->mapping);
free_caches(scx);
}
@@ -5333,7 +5359,6 @@ struct SECURITY_API *ntfs_initialize_file_security(const
char *device,
scx->vol->secure_flags = 0;
/* accept no mapping and no $Secure */
ntfs_build_mapping(scx,(const char*)NULL,TRUE);
- ntfs_open_secure(vol);
} else {
if (scapi)
free(scapi);
@@ -5365,7 +5390,7 @@ BOOL ntfs_leave_file_security(struct SECURITY_API *scapi)
ok = FALSE;
if (scapi && (scapi->magic == MAGIC_API) && scapi->security.vol) {
vol = scapi->security.vol;
- ntfs_close_secure(&scapi->security);
+ ntfs_destroy_security_context(&scapi->security);
free(scapi);
if (!ntfs_umount(vol, 0))
ok = TRUE;
diff --git a/libntfs-3g/volume.c b/libntfs-3g/volume.c
index 4861d2f..68b8ee1 100644
--- a/libntfs-3g/volume.c
+++ b/libntfs-3g/volume.c
@@ -74,6 +74,7 @@
#include "cache.h"
#include "realpath.h"
#include "misc.h"
+#include "security.h"
const char *ntfs_home =
"News, support and information: http://tuxera.com\n";;
@@ -172,6 +173,9 @@ static int __ntfs_volume_release(ntfs_volume *v)
{
int err = 0;
+ if (ntfs_close_secure(v))
+ ntfs_error_set(&err);
+
if (ntfs_inode_free(&v->vol_ni))
ntfs_error_set(&err);
/*
@@ -1234,6 +1238,11 @@ ntfs_volume *ntfs_device_mount(struct ntfs_device *dev,
ntfs_mount_flags flags)
ntfs_log_perror("Failed to close $AttrDef");
goto error_exit;
}
+
+ /* Open $Secure. */
+ if (ntfs_open_secure(vol))
+ goto error_exit;
+
/*
* Check for dirty logfile and hibernated Windows.
* We care only about read-write mounts.
diff --git a/src/lowntfs-3g.c b/src/lowntfs-3g.c
index 08529a4..a91d123 100644
--- a/src/lowntfs-3g.c
+++ b/src/lowntfs-3g.c
@@ -3852,7 +3852,7 @@ static void ntfs_close(void)
/ ctx->seccache->head.p_reads % 10);
}
}
- ntfs_close_secure(&security);
+ ntfs_destroy_security_context(&security);
}
if (ntfs_umount(ctx->vol, FALSE))
@@ -4363,10 +4363,6 @@ int main(int argc, char *argv[])
#ifdef HAVE_SETXATTR /* extended attributes interface required */
ctx->vol->efs_raw = ctx->efs_raw;
#endif /* HAVE_SETXATTR */
- /* JPA open $Secure, (whatever NTFS version !) */
- /* to initialize security data */
- if (ntfs_open_secure(ctx->vol) && (ctx->vol->major_ver >= 3))
- failed_secure = "Could not open file $Secure";
if (!ntfs_build_mapping(&ctx->security,ctx->usermap_path,
(ctx->vol->secure_flags
& ((1 << SECURITY_DEFAULT) | (1 << SECURITY_ACL)))
diff --git a/src/ntfs-3g.c b/src/ntfs-3g.c
index 640cfc4..2578fef 100644
--- a/src/ntfs-3g.c
+++ b/src/ntfs-3g.c
@@ -3651,7 +3651,7 @@ static void ntfs_close(void)
/ ctx->seccache->head.p_reads % 10);
}
}
- ntfs_close_secure(&security);
+ ntfs_destroy_security_context(&security);
}
if (ntfs_umount(ctx->vol, FALSE))
@@ -4168,10 +4168,6 @@ int main(int argc, char *argv[])
#ifdef HAVE_SETXATTR /* extended attributes interface required */
ctx->vol->efs_raw = ctx->efs_raw;
#endif /* HAVE_SETXATTR */
- /* JPA open $Secure, (whatever NTFS version !) */
- /* to initialize security data */
- if (ntfs_open_secure(ctx->vol) && (ctx->vol->major_ver >= 3))
- failed_secure = "Could not open file $Secure";
if (!ntfs_build_mapping(&ctx->security,ctx->usermap_path,
(ctx->vol->secure_flags
& ((1 << SECURITY_DEFAULT) | (1 << SECURITY_ACL)))
--
2.9.0
------------------------------------------------------------------------------
_______________________________________________
ntfs-3g-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ntfs-3g-devel
