mirror of
https://github.com/samba-team/samba.git
synced 2025-02-26 21:57:41 +03:00
s3: smbd: Don't loop infinitely on bad-symlink resolution.
In the FILE_OPEN_IF case we have O_CREAT, but not O_EXCL. Previously we went into a loop trying first ~(O_CREAT|O_EXCL), and if that returned ENOENT try (O_CREAT|O_EXCL). We kept looping indefinately until we got an error, or the file was created or opened. The big problem here is dangling symlinks. Opening without O_NOFOLLOW means both bad symlink and missing path return -1, ENOENT from open(). As POSIX is pathname based it's not possible to tell the difference between these two cases in a non-racy way, so change to try only two attempts before giving up. We don't have this problem for the O_NOFOLLOW case as we just return NT_STATUS_OBJECT_PATH_NOT_FOUND mapped from the ELOOP POSIX error and immediately returned. Unroll the loop logic to two tries instead. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12572 Pair-programmed-with: Ralph Boehme <slow@samba.org> Signed-off-by: Jeremy Allison <jra@samba.org> Signed-off-by: Ralph Boehme <slow@samba.org> Reviewed-by: Ralph Boehme <slow@samba.org> (cherry picked from commit 10c3e3923022485c720f322ca4f0aca5d7501310)
This commit is contained in:
parent
fe31f48ace
commit
e6eb8807bf
@ -639,7 +639,9 @@ static NTSTATUS fd_open_atomic(struct connection_struct *conn,
|
||||
bool *file_created)
|
||||
{
|
||||
NTSTATUS status = NT_STATUS_UNSUCCESSFUL;
|
||||
NTSTATUS retry_status;
|
||||
bool file_existed = VALID_STAT(fsp->fsp_name->st);
|
||||
int curr_flags;
|
||||
|
||||
*file_created = false;
|
||||
|
||||
@ -671,59 +673,65 @@ static NTSTATUS fd_open_atomic(struct connection_struct *conn,
|
||||
* 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).
|
||||
* O_CREAT|O_EXCL.
|
||||
*
|
||||
* The big problem here is dangling symlinks. Opening
|
||||
* without O_NOFOLLOW means both bad symlink
|
||||
* and missing path return -1, ENOENT from open(). As POSIX
|
||||
* is pathname based it's not possible to tell
|
||||
* the difference between these two cases in a
|
||||
* non-racy way, so change to try only two attempts before
|
||||
* giving up.
|
||||
*
|
||||
* We don't have this problem for the O_NOFOLLOW
|
||||
* case as it just returns NT_STATUS_OBJECT_PATH_NOT_FOUND
|
||||
* mapped from the ELOOP POSIX error.
|
||||
*/
|
||||
|
||||
while(1) {
|
||||
int curr_flags = flags;
|
||||
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;
|
||||
if (file_existed) {
|
||||
curr_flags &= ~(O_CREAT);
|
||||
retry_status = NT_STATUS_OBJECT_NAME_NOT_FOUND;
|
||||
} else {
|
||||
curr_flags |= O_EXCL;
|
||||
retry_status = NT_STATUS_OBJECT_NAME_COLLISION;
|
||||
}
|
||||
|
||||
status = fd_open(conn, fsp, curr_flags, mode);
|
||||
if (NT_STATUS_IS_OK(status)) {
|
||||
if (!file_existed) {
|
||||
*file_created = true;
|
||||
}
|
||||
return NT_STATUS_OK;
|
||||
}
|
||||
if (!NT_STATUS_EQUAL(status, retry_status)) {
|
||||
return status;
|
||||
}
|
||||
|
||||
curr_flags = flags;
|
||||
|
||||
/*
|
||||
* Keep file_existed up to date for clarity.
|
||||
*/
|
||||
if (NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND)) {
|
||||
file_existed = false;
|
||||
curr_flags |= O_EXCL;
|
||||
DBG_DEBUG("file %s did not exist. Retry.\n",
|
||||
smb_fname_str_dbg(fsp->fsp_name));
|
||||
} else {
|
||||
file_existed = true;
|
||||
curr_flags &= ~(O_CREAT);
|
||||
DBG_DEBUG("file %s existed. Retry.\n",
|
||||
smb_fname_str_dbg(fsp->fsp_name));
|
||||
}
|
||||
|
||||
status = fd_open(conn, fsp, curr_flags, mode);
|
||||
|
||||
if (NT_STATUS_IS_OK(status) && (!file_existed)) {
|
||||
*file_created = true;
|
||||
}
|
||||
|
||||
return status;
|
||||
}
|
||||
|
||||
|
Loading…
x
Reference in New Issue
Block a user