hrev49631 adds 3 changesets to branch 'master'
old head: 16844b92fc2f3f6cec988f0bb873c7a5da436fd0
new head: 41082c87acc30fe80e4b86b90c1dfe0bbabfea38
overview:
http://cgit.haiku-os.org/haiku/log/?qt=range&q=41082c87acc3+%5E16844b92fc2f
----------------------------------------------------------------------------
6adf7a0c75be: VFS: prevent FD inheritance for the kernel completely.
* Each io_context now has a "inherit_fds" member that decides whether
or not this context allows to inherit FDs to its children.
* This replaces the former O_CLOEXEC mechanism.
eb62d3337b82: VFS: Slight rework of the FD disconnect feature.
* This should fix ticket #4157, although I probably have missed
something.
* In any case, it no longer messes with the ref counts of the
file descriptor, and the race condition in put_fd() should be
gone.
* It's still rather messy all in all.
41082c87acc3: AHCI: only perform the reset when ST is not set.
* As the specs say, this causes undefined behavior.
* Should help with the recently introduced boot issues, but cannot
be considered a full fix -- as mmlr pointed out, one has to detect
unsolicited COMINITs.
[ Axel Dörfler <axeld@xxxxxxxxxxxxxxxx> ]
----------------------------------------------------------------------------
5 files changed, 56 insertions(+), 49 deletions(-)
headers/private/kernel/vfs.h | 3 +-
.../kernel/busses/scsi/ahci/ahci_port.cpp | 11 +++--
src/add-ons/kernel/busses/scsi/ahci/ahci_port.h | 5 +-
src/system/kernel/fs/fd.cpp | 48 +++++++++-----------
src/system/kernel/fs/vfs.cpp | 38 +++++++++-------
############################################################################
Commit: 6adf7a0c75beebd9a9bd48602f50ac7c7ae8056e
URL: http://cgit.haiku-os.org/haiku/commit/?id=6adf7a0c75be
Author: Axel Dörfler <axeld@xxxxxxxxxxxxxxxx>
Date: Fri Sep 11 15:08:35 2015 UTC
VFS: prevent FD inheritance for the kernel completely.
* Each io_context now has a "inherit_fds" member that decides whether
or not this context allows to inherit FDs to its children.
* This replaces the former O_CLOEXEC mechanism.
----------------------------------------------------------------------------
diff --git a/headers/private/kernel/vfs.h b/headers/private/kernel/vfs.h
index c9e6a28..f108cd0 100644
--- a/headers/private/kernel/vfs.h
+++ b/headers/private/kernel/vfs.h
@@ -1,5 +1,5 @@
/*
- * Copyright 2002-2011, Axel Dörfler, axeld@xxxxxxxxxxxxxxxx.
+ * Copyright 2002-2015, Axel Dörfler, axeld@xxxxxxxxxxxxxxxx.
* Distributed under the terms of the MIT License.
*
* Copyright 2001-2002, Travis Geiselbrecht. All rights reserved.
@@ -58,6 +58,7 @@ typedef struct io_context {
struct list node_monitors;
uint32 num_monitors;
uint32 max_monitors;
+ bool inherit_fds;
} io_context;
diff --git a/src/system/kernel/fs/vfs.cpp b/src/system/kernel/fs/vfs.cpp
index 94f91b3..34bf87f 100644
--- a/src/system/kernel/fs/vfs.cpp
+++ b/src/system/kernel/fs/vfs.cpp
@@ -1,6 +1,6 @@
/*
* Copyright 2005-2013, Ingo Weinhold, ingo_weinhold@xxxxxx.
- * Copyright 2002-2014, Axel Dörfler, axeld@xxxxxxxxxxxxxxxx.
+ * Copyright 2002-2015, Axel Dörfler, axeld@xxxxxxxxxxxxxxxx.
* Distributed under the terms of the MIT License.
*
* Copyright 2001-2002, Travis Geiselbrecht. All rights reserved.
@@ -4820,7 +4820,7 @@ vfs_new_io_context(io_context* parentContext, bool
purgeCloseOnExec)
MutexLocker parentLocker;
size_t tableSize;
- if (parentContext) {
+ if (parentContext != NULL) {
parentLocker.SetTo(parentContext->io_mutex, false);
tableSize = parentContext->table_size;
} else
@@ -4847,7 +4847,7 @@ vfs_new_io_context(io_context* parentContext, bool
purgeCloseOnExec)
// Copy all parent file descriptors
- if (parentContext) {
+ if (parentContext != NULL) {
size_t i;
mutex_lock(&sIOContextRootLock);
@@ -4860,23 +4860,25 @@ vfs_new_io_context(io_context* parentContext, bool
purgeCloseOnExec)
if (context->cwd)
inc_vnode_ref_count(context->cwd);
- for (i = 0; i < tableSize; i++) {
- struct file_descriptor* descriptor =
parentContext->fds[i];
+ if (parentContext->inherit_fds) {
+ for (i = 0; i < tableSize; i++) {
+ struct file_descriptor* descriptor =
parentContext->fds[i];
- if (descriptor != NULL) {
- bool closeOnExec =
fd_close_on_exec(parentContext, i);
- if (closeOnExec && purgeCloseOnExec)
- continue;
+ if (descriptor != NULL) {
+ bool closeOnExec =
fd_close_on_exec(parentContext, i);
+ if (closeOnExec && purgeCloseOnExec)
+ continue;
- TFD(InheritFD(context, i, descriptor,
parentContext));
+ TFD(InheritFD(context, i, descriptor,
parentContext));
- context->fds[i] = descriptor;
- context->num_used_fds++;
- atomic_add(&descriptor->ref_count, 1);
- atomic_add(&descriptor->open_count, 1);
+ context->fds[i] = descriptor;
+ context->num_used_fds++;
+ atomic_add(&descriptor->ref_count, 1);
+ atomic_add(&descriptor->open_count, 1);
- if (closeOnExec)
- fd_set_close_on_exec(context, i, true);
+ if (closeOnExec)
+ fd_set_close_on_exec(context,
i, true);
+ }
}
}
@@ -4893,6 +4895,7 @@ vfs_new_io_context(io_context* parentContext, bool
purgeCloseOnExec)
}
context->table_size = tableSize;
+ context->inherit_fds = parentContext != NULL;
list_init(&context->node_monitors);
context->max_monitors = DEFAULT_NODE_MONITORS;
@@ -8143,7 +8146,7 @@ _kern_open(int fd, const char* path, int openMode, int
perms)
if (openMode & O_CREAT)
return file_create(fd, pathBuffer.LockBuffer(), openMode,
perms, true);
- return file_open(fd, pathBuffer.LockBuffer(), openMode | O_CLOEXEC,
true);
+ return file_open(fd, pathBuffer.LockBuffer(), openMode, true);
}
############################################################################
Commit: eb62d3337b82f4dce6b1a0b3f116e38491c66f14
URL: http://cgit.haiku-os.org/haiku/commit/?id=eb62d3337b82
Author: Axel Dörfler <axeld@xxxxxxxxxxxxxxxx>
Date: Fri Sep 11 15:10:23 2015 UTC
Ticket: https://dev.haiku-os.org/ticket/4157
VFS: Slight rework of the FD disconnect feature.
* This should fix ticket #4157, although I probably have missed
something.
* In any case, it no longer messes with the ref counts of the
file descriptor, and the race condition in put_fd() should be
gone.
* It's still rather messy all in all.
----------------------------------------------------------------------------
diff --git a/src/system/kernel/fs/fd.cpp b/src/system/kernel/fs/fd.cpp
index 18292f0..4e50e90 100644
--- a/src/system/kernel/fs/fd.cpp
+++ b/src/system/kernel/fs/fd.cpp
@@ -1,6 +1,6 @@
/*
* Copyright 2009-2011, Ingo Weinhold, ingo_weinhold@xxxxxx.
- * Copyright 2002-2010, Axel Dörfler, axeld@xxxxxxxxxxxxxxxx.
+ * Copyright 2002-2015, Axel Dörfler, axeld@xxxxxxxxxxxxxxxx.
* Distributed under the terms of the MIT License.
*/
@@ -224,13 +224,11 @@ put_fd(struct file_descriptor* descriptor)
descriptor->ops->fd_free(descriptor);
// prevent this descriptor from being closed/freed again
- descriptor->open_count = -1;
- descriptor->ref_count = -1;
descriptor->ops = NULL;
descriptor->u.vnode = NULL;
// the file descriptor is kept intact, so that it's not
- // reused until someone explicetly closes it
+ // reused until someone explicitly closes it
}
}
@@ -295,13 +293,8 @@ get_fd_locked(struct io_context* context, int fd)
struct file_descriptor* descriptor = context->fds[fd];
if (descriptor != NULL) {
- // Disconnected descriptors cannot be accessed anymore
- if (descriptor->open_mode & O_DISCONNECTED)
- descriptor = NULL;
- else {
- TFD(GetFD(context, fd, descriptor));
- inc_fd_ref_count(descriptor);
- }
+ TFD(GetFD(context, fd, descriptor));
+ inc_fd_ref_count(descriptor);
}
return descriptor;
@@ -323,7 +316,7 @@ get_open_fd(struct io_context* context, int fd)
MutexLocker _(context->io_mutex);
file_descriptor* descriptor = get_fd_locked(context, fd);
- if (descriptor == NULL)
+ if (descriptor == NULL || (descriptor->open_mode & O_DISCONNECTED) != 0)
return NULL;
atomic_add(&descriptor->open_count, 1);
@@ -427,7 +420,8 @@ dup2_fd(int oldfd, int newfd, bool kernel)
// the table size could be changed)
if ((uint32)oldfd >= context->table_size
|| (uint32)newfd >= context->table_size
- || context->fds[oldfd] == NULL) {
+ || context->fds[oldfd] == NULL
+ || (context->fds[oldfd]->open_mode & O_DISCONNECTED) != 0) {
mutex_unlock(&context->io_mutex);
return B_FILE_ERROR;
}
@@ -508,10 +502,10 @@ fd_ioctl(bool kernelFD, int fd, uint32 op, void* buffer,
size_t length)
int status;
descriptor = get_fd(get_current_io_context(kernelFD), fd);
- if (descriptor == NULL)
+ if (descriptor == NULL || (descriptor->open_mode & O_DISCONNECTED) != 0)
return B_FILE_ERROR;
- if (descriptor->ops->fd_ioctl)
+ if (descriptor->ops->fd_ioctl != NULL)
status = descriptor->ops->fd_ioctl(descriptor, op, buffer,
length);
else
status = B_DEV_INVALID_IOCTL;
@@ -567,7 +561,7 @@ select_fd(int32 fd, struct select_info* info, bool kernel)
MutexLocker locker(context->io_mutex);
struct file_descriptor* descriptor = fdGetter.SetTo(context, fd, true);
- if (descriptor == NULL)
+ if (descriptor == NULL || (descriptor->open_mode & O_DISCONNECTED) != 0)
return B_FILE_ERROR;
uint16 eventsToSelect = info->selected_events & ~B_EVENT_INVALID;
@@ -685,7 +679,7 @@ fd_is_valid(int fd, bool kernel)
{
struct file_descriptor* descriptor
= get_fd(get_current_io_context(kernel), fd);
- if (descriptor == NULL)
+ if (descriptor == NULL || (descriptor->open_mode & O_DISCONNECTED) != 0)
return false;
put_fd(descriptor);
@@ -726,7 +720,7 @@ common_user_io(int fd, off_t pos, void* buffer, size_t
length, bool write)
FDGetter fdGetter;
struct file_descriptor* descriptor = fdGetter.SetTo(fd, false);
- if (!descriptor)
+ if (descriptor == NULL || (descriptor->open_mode & O_DISCONNECTED) != 0)
return B_FILE_ERROR;
if (write ? (descriptor->open_mode & O_RWMASK) == O_RDONLY
@@ -778,7 +772,7 @@ common_user_vector_io(int fd, off_t pos, const iovec*
userVecs, size_t count,
FDGetter fdGetter;
struct file_descriptor* descriptor = fdGetter.SetTo(fd, false);
- if (!descriptor)
+ if (descriptor == NULL || (descriptor->open_mode & O_DISCONNECTED) != 0)
return B_FILE_ERROR;
if (write ? (descriptor->open_mode & O_RWMASK) == O_RDONLY
@@ -891,12 +885,12 @@ _user_seek(int fd, off_t pos, int seekType)
struct file_descriptor* descriptor;
descriptor = get_fd(get_current_io_context(false), fd);
- if (!descriptor)
+ if (descriptor == NULL || (descriptor->open_mode & O_DISCONNECTED) != 0)
return B_FILE_ERROR;
TRACE(("user_seek(descriptor = %p)\n", descriptor));
- if (descriptor->ops->fd_seek)
+ if (descriptor->ops->fd_seek != NULL)
pos = descriptor->ops->fd_seek(descriptor, pos, seekType);
else
pos = ESPIPE;
@@ -937,7 +931,7 @@ _user_read_dir(int fd, struct dirent* userBuffer, size_t
bufferSize,
io_context* ioContext = get_current_io_context(false);
FDGetter fdGetter;
struct file_descriptor* descriptor = fdGetter.SetTo(ioContext, fd,
false);
- if (descriptor == NULL)
+ if (descriptor == NULL || (descriptor->open_mode & O_DISCONNECTED) != 0)
return B_FILE_ERROR;
if (descriptor->ops->fd_read_dir == NULL)
@@ -983,10 +977,10 @@ _user_rewind_dir(int fd)
TRACE(("user_rewind_dir(fd = %d)\n", fd));
descriptor = get_fd(get_current_io_context(false), fd);
- if (descriptor == NULL)
+ if (descriptor == NULL || (descriptor->open_mode & O_DISCONNECTED) != 0)
return B_FILE_ERROR;
- if (descriptor->ops->fd_rewind_dir)
+ if (descriptor->ops->fd_rewind_dir != NULL)
status = descriptor->ops->fd_rewind_dir(descriptor);
else
status = B_UNSUPPORTED;
@@ -1124,7 +1118,7 @@ _kern_write(int fd, off_t pos, const void* buffer, size_t
length)
FDGetter fdGetter;
struct file_descriptor* descriptor = fdGetter.SetTo(fd, true);
- if (descriptor == NULL)
+ if (descriptor == NULL || (descriptor->open_mode & O_DISCONNECTED) != 0)
return B_FILE_ERROR;
if ((descriptor->open_mode & O_RWMASK) == O_RDONLY)
return B_FILE_ERROR;
@@ -1252,7 +1246,7 @@ _kern_read_dir(int fd, struct dirent* buffer, size_t
bufferSize,
struct io_context* ioContext = get_current_io_context(true);
descriptor = get_fd(ioContext, fd);
- if (descriptor == NULL)
+ if (descriptor == NULL || (descriptor->open_mode & O_DISCONNECTED) != 0)
return B_FILE_ERROR;
if (descriptor->ops->fd_read_dir) {
@@ -1278,7 +1272,7 @@ _kern_rewind_dir(int fd)
TRACE(("sys_rewind_dir(fd = %d)\n",fd));
descriptor = get_fd(get_current_io_context(true), fd);
- if (descriptor == NULL)
+ if (descriptor == NULL || (descriptor->open_mode & O_DISCONNECTED) != 0)
return B_FILE_ERROR;
if (descriptor->ops->fd_rewind_dir)
diff --git a/src/system/kernel/fs/vfs.cpp b/src/system/kernel/fs/vfs.cpp
index 34bf87f..3cc6483 100644
--- a/src/system/kernel/fs/vfs.cpp
+++ b/src/system/kernel/fs/vfs.cpp
@@ -4864,7 +4864,8 @@ vfs_new_io_context(io_context* parentContext, bool
purgeCloseOnExec)
for (i = 0; i < tableSize; i++) {
struct file_descriptor* descriptor =
parentContext->fds[i];
- if (descriptor != NULL) {
+ if (descriptor != NULL
+ && (descriptor->open_mode &
O_DISCONNECTED) == 0) {
bool closeOnExec =
fd_close_on_exec(parentContext, i);
if (closeOnExec && purgeCloseOnExec)
continue;
############################################################################
Revision: hrev49631
Commit: 41082c87acc30fe80e4b86b90c1dfe0bbabfea38
URL: http://cgit.haiku-os.org/haiku/commit/?id=41082c87acc3
Author: Axel Dörfler <axeld@xxxxxxxxxxxxxxxx>
Date: Fri Sep 11 15:34:27 2015 UTC
AHCI: only perform the reset when ST is not set.
* As the specs say, this causes undefined behavior.
* Should help with the recently introduced boot issues, but cannot
be considered a full fix -- as mmlr pointed out, one has to detect
unsolicited COMINITs.
----------------------------------------------------------------------------
diff --git a/src/add-ons/kernel/busses/scsi/ahci/ahci_port.cpp
b/src/add-ons/kernel/busses/scsi/ahci/ahci_port.cpp
index 82efc5b..94a0034 100644
--- a/src/add-ons/kernel/busses/scsi/ahci/ahci_port.cpp
+++ b/src/add-ons/kernel/busses/scsi/ahci/ahci_port.cpp
@@ -218,7 +218,8 @@ void
AHCIPort::ResetDevice()
{
// perform a hard reset
- _HardReset();
+ if (!_HardReset())
+ return;
if (wait_until_set(&fRegs->ssts, 0x1, 100000) < B_OK)
TRACE("AHCIPort::ResetDevice port %d no device detected\n",
fIndex);
@@ -418,7 +419,8 @@ AHCIPort::InterruptErrorHandler(uint32 is)
// Spec v1.3, §6.2.2.3 Recovery of Unsolicited COMINIT
// perform a hard reset
- _HardReset();
+ if (!_HardReset())
+ return;
// clear error bits to clear PxSERR.DIAG.X
_ClearErrorRegister();
@@ -1319,12 +1321,13 @@ AHCIPort::ScsiGetRestrictions(bool* isATAPI, bool*
noAutoSense,
}
-void
+bool
AHCIPort::_HardReset()
{
if ((fRegs->cmd & PORT_CMD_ST) != 0) {
// We shouldn't perform a reset, but at least document it
TRACE("AHCIPort::_HardReset() PORT_CMD_ST set, behaviour
undefined\n");
+ return false;
}
fRegs->sctl = (fRegs->sctl & ~SATA_CONTROL_DET_MASK)
@@ -1335,6 +1338,8 @@ AHCIPort::_HardReset()
fRegs->sctl = (fRegs->sctl & ~SATA_CONTROL_DET_MASK)
| DET_NO_INITIALIZATION << SATA_CONTROL_DET_SHIFT;
FlushPostedWrites();
+
+ return true;
}
diff --git a/src/add-ons/kernel/busses/scsi/ahci/ahci_port.h
b/src/add-ons/kernel/busses/scsi/ahci/ahci_port.h
index 34cee69..0bc4e67 100644
--- a/src/add-ons/kernel/busses/scsi/ahci/ahci_port.h
+++ b/src/add-ons/kernel/busses/scsi/ahci/ahci_port.h
@@ -57,7 +57,7 @@ private:
status_t WaitForTransfer(int *tfd, bigtime_t timeout);
void FinishTransfer();
- inline void _HardReset();
+ inline bool _HardReset();
inline void _ClearErrorRegister();
// uint8 * SetCommandFis(volatile command_list_entry *cmd,
volatile fis *fis, const void *data, size_t dataSize);
@@ -90,10 +90,13 @@ private:
volatile prd * fPRDTable;
};
+
inline void
AHCIPort::FlushPostedWrites()
{
volatile uint32 dummy = fRegs->cmd;
dummy = dummy;
}
+
+
#endif // _AHCI_PORT_H
