The branch, master has been updated
via 6d903bf Cope with a (non-security) open race we've had for ever as
NTCreateX isn't atomic on POSIX.
via 69a3e94 Now we have a guaranteed indication of a file being
created, use it to set the create disposition correctly.
via 02d42be Add function fd_open_atomic() which uses O_CREAT|O_EXCL to
return a guaranteed indication of creation of a new file.
from 3aa186f Simplify the logic in open_file() some more.
http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 6d903bf1899987adaeaaf6608ac318aca4588590
Author: Jeremy Allison <[email protected]>
Date: Tue Jul 10 10:15:07 2012 -0700
Cope with a (non-security) open race we've had for ever as NTCreateX isn't
atomic on POSIX.
On open without create, the file did exist, but some
other (local or NFS) process either renamed/unlinked
and re-created the file with different dev/ino after
we walked the path, but before we did the open. We
could retry the open but it's a rare enough case it's
easier to just fail the open to prevent creating any
problems in the open file db having the wrong dev/ino
key.
Autobuild-User(master): Jeremy Allison <[email protected]>
Autobuild-Date(master): Tue Jul 10 21:57:33 CEST 2012 on sn-devel-104
commit 69a3e947b60397c9bb9175cf52fe009b6b057350
Author: Jeremy Allison <[email protected]>
Date: Mon Jul 9 17:03:45 2012 -0700
Now we have a guaranteed indication of a file being created, use it to set
the create disposition correctly.
commit 02d42be2589ff821ea9f63140694099d518f3046
Author: Jeremy Allison <[email protected]>
Date: Mon Jul 9 16:59:49 2012 -0700
Add function fd_open_atomic() which uses O_CREAT|O_EXCL to return a
guaranteed indication of creation of a new file.
-----------------------------------------------------------------------
Summary of changes:
source3/smbd/open.c | 156 +++++++++++++++++++++++++++++++++++++++++++++-----
1 files changed, 140 insertions(+), 16 deletions(-)
Changeset truncated at 500 lines:
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 473fd97..0f4a588 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -534,6 +534,107 @@ NTSTATUS change_dir_owner_to_parent(connection_struct
*conn,
}
/****************************************************************************
+ Open a file - returning a guaranteed ATOMIC indication of if the
+ file was created or not.
+****************************************************************************/
+
+static NTSTATUS fd_open_atomic(struct connection_struct *conn,
+ files_struct *fsp,
+ int flags,
+ mode_t mode,
+ bool *file_created)
+{
+ NTSTATUS status = NT_STATUS_UNSUCCESSFUL;
+ bool file_existed = VALID_STAT(fsp->fsp_name->st);
+
+ *file_created = false;
+
+ if (!(flags & O_CREAT)) {
+ /*
+ * We're not creating the file, just pass through.
+ */
+ return fd_open(conn, fsp, flags, mode);
+ }
+
+ if (flags & O_EXCL) {
+ /*
+ * Fail if already exists, just pass through.
+ */
+ status = fd_open(conn, fsp, flags, mode);
+ if (NT_STATUS_IS_OK(status)) {
+ /*
+ * Here we've opened with O_CREAT|O_EXCL
+ * and got success. We *know* we created
+ * this file.
+ */
+ *file_created = true;
+ }
+ return status;
+ }
+
+ /*
+ * Now it gets tricky. We have O_CREAT, but not O_EXCL.
+ * To know absolutely if we created the file or not,
+ * we can never call O_CREAT without O_EXCL. So if
+ * we think the file existed, try without O_CREAT|O_EXCL.
+ * If we think the file didn't exist, try with
+ * O_CREAT|O_EXCL. Keep bouncing between these two
+ * requests until either the file is created, or
+ * opened. Either way, we keep going until we get
+ * a returnable result (error, or open/create).
+ */
+
+ while(1) {
+ int curr_flags = flags;
+
+ if (file_existed) {
+ /* Just try open, do not create. */
+ curr_flags &= ~(O_CREAT);
+ status = fd_open(conn, fsp, curr_flags, mode);
+ if (NT_STATUS_EQUAL(status,
+ NT_STATUS_OBJECT_NAME_NOT_FOUND)) {
+ /*
+ * Someone deleted it in the meantime.
+ * Retry with O_EXCL.
+ */
+ file_existed = false;
+ DEBUG(10,("fd_open_atomic: file %s existed. "
+ "Retry.\n",
+ smb_fname_str_dbg(fsp->fsp_name)));
+ continue;
+ }
+ } else {
+ /* Try create exclusively, fail if it exists. */
+ curr_flags |= O_EXCL;
+ status = fd_open(conn, fsp, curr_flags, mode);
+ if (NT_STATUS_EQUAL(status,
+ NT_STATUS_OBJECT_NAME_COLLISION)) {
+ /*
+ * Someone created it in the meantime.
+ * Retry without O_CREAT.
+ */
+ file_existed = true;
+ DEBUG(10,("fd_open_atomic: file %s "
+ "did not exist. Retry.\n",
+ smb_fname_str_dbg(fsp->fsp_name)));
+ continue;
+ }
+ if (NT_STATUS_IS_OK(status)) {
+ /*
+ * Here we've opened with O_CREAT|O_EXCL
+ * and got success. We *know* we created
+ * this file.
+ */
+ *file_created = true;
+ }
+ }
+ /* Create is done, or failed. */
+ break;
+ }
+ return status;
+}
+
+/****************************************************************************
Open a file.
****************************************************************************/
@@ -544,7 +645,8 @@ static NTSTATUS open_file(files_struct *fsp,
int flags,
mode_t unx_mode,
uint32 access_mask, /* client requested access mask.
*/
- uint32 open_access_mask) /* what we're actually using
in the open. */
+ uint32 open_access_mask, /* what we're actually using
in the open. */
+ bool *p_file_created)
{
struct smb_filename *smb_fname = fsp->fsp_name;
NTSTATUS status = NT_STATUS_OK;
@@ -670,7 +772,8 @@ static NTSTATUS open_file(files_struct *fsp,
}
/* Actually do the open */
- status = fd_open(conn, fsp, local_flags, unx_mode);
+ status = fd_open_atomic(conn, fsp, local_flags,
+ unx_mode, p_file_created);
if (!NT_STATUS_IS_OK(status)) {
DEBUG(3,("Error opening file %s (%s) (local_flags=%d) "
"(flags=%d)\n", smb_fname_str_dbg(smb_fname),
@@ -690,7 +793,7 @@ static NTSTATUS open_file(files_struct *fsp,
return status;
}
- if ((local_flags & O_CREAT) && !file_existed) {
+ if (*p_file_created) {
/* We created this file. */
bool need_re_stat = false;
@@ -1707,6 +1810,7 @@ static NTSTATUS open_file_ntcreate(connection_struct
*conn,
uint32 open_access_mask = access_mask;
NTSTATUS status;
char *parent_dir;
+ SMB_STRUCT_STAT saved_stat = smb_fname->st;
if (conn->printer) {
/*
@@ -2236,7 +2340,7 @@ static NTSTATUS open_file_ntcreate(connection_struct
*conn,
fsp_open = open_file(fsp, conn, req, parent_dir,
flags|flags2, unx_mode, access_mask,
- open_access_mask);
+ open_access_mask, &new_file_created);
if (!NT_STATUS_IS_OK(fsp_open)) {
if (NT_STATUS_EQUAL(fsp_open, NT_STATUS_RETRY)) {
@@ -2246,6 +2350,30 @@ static NTSTATUS open_file_ntcreate(connection_struct
*conn,
return fsp_open;
}
+ if (file_existed && !check_same_dev_ino(&saved_stat, &smb_fname->st)) {
+ /*
+ * The file did exist, but some other (local or NFS)
+ * process either renamed/unlinked and re-created the
+ * file with different dev/ino after we walked the path,
+ * but before we did the open. We could retry the
+ * open but it's a rare enough case it's easier to
+ * just fail the open to prevent creating any problems
+ * in the open file db having the wrong dev/ino key.
+ */
+ TALLOC_FREE(lck);
+ fd_close(fsp);
+ DEBUG(1,("open_file_ntcreate: file %s - dev/ino mismatch. "
+ "Old (dev=0x%llu, ino =0x%llu). "
+ "New (dev=0x%llu, ino=0x%llu). Failing open "
+ " with NT_STATUS_ACCESS_DENIED.\n",
+ smb_fname_str_dbg(smb_fname),
+ (unsigned long long)saved_stat.st_ex_dev,
+ (unsigned long long)saved_stat.st_ex_ino,
+ (unsigned long long)smb_fname->st.st_ex_dev,
+ (unsigned long long)smb_fname->st.st_ex_ino));
+ return NT_STATUS_ACCESS_DENIED;
+ }
+
if (!file_existed) {
struct share_mode_entry *batch_entry = NULL;
struct share_mode_entry *exclusive_entry = NULL;
@@ -2362,7 +2490,7 @@ static NTSTATUS open_file_ntcreate(connection_struct
*conn,
SMB_ASSERT(lck != NULL);
/* Delete streams if create_disposition requires it */
- if (file_existed && clear_ads &&
+ if (!new_file_created && clear_ads &&
!is_ntfs_stream_smb_fname(smb_fname)) {
status = delete_all_streams(conn, smb_fname->base_name);
if (!NT_STATUS_IS_OK(status)) {
@@ -2402,7 +2530,7 @@ static NTSTATUS open_file_ntcreate(connection_struct
*conn,
* If requested, truncate the file.
*/
- if (file_existed && (flags2&O_TRUNC)) {
+ if (!new_file_created && (flags2&O_TRUNC)) {
/*
* We are modifying the file after open - update the stat
* struct..
@@ -2438,14 +2566,16 @@ static NTSTATUS open_file_ntcreate(connection_struct
*conn,
if (is_stat_open(open_access_mask)) {
fsp->oplock_type = NO_OPLOCK;
}
+ }
+ if (new_file_created) {
+ info = FILE_WAS_CREATED;
+ } else {
if (flags2 & O_TRUNC) {
info = FILE_WAS_OVERWRITTEN;
} else {
info = FILE_WAS_OPENED;
}
- } else {
- info = FILE_WAS_CREATED;
}
if (pinfo) {
@@ -2487,13 +2617,7 @@ static NTSTATUS open_file_ntcreate(connection_struct
*conn,
fsp->initial_delete_on_close = True;
}
- if (info == FILE_WAS_OVERWRITTEN
- || info == FILE_WAS_CREATED
- || info == FILE_WAS_SUPERSEDED) {
- new_file_created = True;
- }
-
- if (new_file_created) {
+ if (info != FILE_WAS_OPENED) {
/* Files should be initially set as archive */
if (lp_map_archive(SNUM(conn)) ||
lp_store_dos_attributes(SNUM(conn))) {
@@ -2521,7 +2645,7 @@ static NTSTATUS open_file_ntcreate(connection_struct
*conn,
* selected.
*/
- if (!posix_open && !file_existed && !def_acl) {
+ if (!posix_open && new_file_created && !def_acl) {
int saved_errno = errno; /* We might get ENOSYS in the next
* call.. */
--
Samba Shared Repository
