Mirrors_Framework/rtems
1
0
Fork 0
mirror of https://gitlab.rtems.org/rtems/rtos/rtems.git synced 2026-09-20 11:58:49 +08:00
Code Issues Packages Projects Releases Wiki Activity

libio: Robust file descriptor reference counting

Browse Source 
There was a race conditon in the reference counting of file descriptors
during a close() operation.  After the call to the close handler, the
rtems_libio_free() function cleared the flags to zero.  However, at this
point in time there may still exist some holders of the file descriptor.
With RTEMS_DEBUG enabled this could lead to failed assertions in
rtems_libio_iop_drop().

Change the code to use only atomic read-modify-write operations on the
rtems_libio_iop::flags.
This commit is contained in:
Sebastian Huber
2020-03-13 09:57:04 +01:00
parent 2ff8363466
commit c2287ba2cf
10 changed files with 82 additions and 57 deletions
Download Patch File Download Diff File Expand all files Collapse all files
cpukit/include/rtems/libio.h
+1 -1
View File
@@ -1318,8 +1318,8 @@ extern const rtems_filesystem_limits_and_options_t
* to (eg: offset, driver, pathname should be in that)
*/
struct rtems_libio_tt {
off_t offset; /* current offset into file */
Atomic_Uint flags;
off_t offset; /* current offset into file */
rtems_filesystem_location_info_t pathinfo;
uint32_t data0; /* private to "driver" */
void *data1; /* ... */
cpukit/include/rtems/libio_.h
+1 -23
View File
@@ -84,28 +84,6 @@ extern rtems_filesystem_mount_table_entry_t rtems_filesystem_null_mt_entry;
*/
extern rtems_filesystem_global_location_t rtems_filesystem_global_location_null;
/**
* @brief Sets the iop flags to the specified flags together with
* LIBIO_FLAGS_OPEN.
*
* Use this once a file descriptor allocated via rtems_libio_allocate() is
* fully initialized.
*
* @param[in] iop The iop.
* @param[in] flags The flags.
*/
static inline void rtems_libio_iop_flags_initialize(
rtems_libio_t *iop,
uint32_t flags
)
{
_Atomic_Store_uint(
&iop->flags,
LIBIO_FLAGS_OPEN | flags,
ATOMIC_ORDER_RELEASE
);
}
/**
* @brief Sets the specified flags in the iop.
*
@@ -222,7 +200,7 @@ static inline void rtems_libio_iop_drop( rtems_libio_t *iop )
#define rtems_libio_check_is_open(_iop) \
do { \
if (((_iop)->flags & LIBIO_FLAGS_OPEN) == 0) { \
if ((rtems_libio_iop_flags(_iop) & LIBIO_FLAGS_OPEN) == 0) { \
errno = EBADF; \
return -1; \
} \
cpukit/libcsupport/src/fcntl.c
+2 -2
View File
@@ -45,9 +45,9 @@ static int duplicate_iop( rtems_libio_t *iop )
*/
rv = (*diop->pathinfo.handlers->open_h)( diop, NULL, oflag, 0 );
if ( rv == 0 ) {
rtems_libio_iop_flags_initialize(
rtems_libio_iop_flags_set(
diop,
rtems_libio_fcntl_flags( oflag )
LIBIO_FLAGS_OPEN | rtems_libio_fcntl_flags( oflag )
);
rv = rtems_libio_iop_to_descriptor( diop );
} else {
cpukit/libcsupport/src/libio.c
+14 -1
View File
@@ -134,11 +134,24 @@ void rtems_libio_free(
rtems_libio_t *iop
)
{
size_t zero;
rtems_filesystem_location_free( &iop->pathinfo );
rtems_libio_lock();
iop = memset( iop, 0, sizeof( *iop ) );
/*
* Clear everything except the reference count part. At this point in time
* there may be still some holders of this file descriptor.
*/
rtems_libio_iop_flags_clear( iop, LIBIO_FLAGS_REFERENCE_INC - 1U );
zero = offsetof( rtems_libio_t, offset );
memset( (char *) iop + zero, 0, sizeof( *iop ) - zero );
/*
* Append it to the free list. This increases the likelihood that a use
* after close is detected.
*/
*rtems_libio_iop_free_tail = iop;
rtems_libio_iop_free_tail = &iop->data1;
cpukit/libcsupport/src/open.c
+1 -5
View File
@@ -115,11 +115,7 @@ static int do_open(
rtems_filesystem_eval_path_extract_currentloc( &ctx, &iop->pathinfo );
rtems_filesystem_eval_path_cleanup( &ctx );
_Atomic_Store_uint(
&iop->flags,
rtems_libio_fcntl_flags( oflag ),
ATOMIC_ORDER_RELAXED
);
rtems_libio_iop_flags_set( iop, rtems_libio_fcntl_flags( oflag ) );
rv = (*iop->pathinfo.handlers->open_h)( iop, path, oflag, mode );
cpukit/libcsupport/src/sup_fs_deviceio.c
+3 -3
View File
@@ -34,7 +34,7 @@ int rtems_deviceio_open(
rtems_libio_open_close_args_t args;
args.iop = iop;
args.flags = iop->flags;
args.flags = rtems_libio_iop_flags( iop );
args.mode = mode;
status = rtems_io_open( major, minor, &args );
@@ -75,7 +75,7 @@ ssize_t rtems_deviceio_read(
args.offset = iop->offset;
args.buffer = buf;
args.count = nbyte;
args.flags = iop->flags;
args.flags = rtems_libio_iop_flags( iop );
args.bytes_moved = 0;
status = rtems_io_read( major, minor, &args );
@@ -103,7 +103,7 @@ ssize_t rtems_deviceio_write(
args.offset = iop->offset;
args.buffer = RTEMS_DECONST( void *, buf );
args.count = nbyte;
args.flags = iop->flags;
args.flags = rtems_libio_iop_flags( iop );
args.bytes_moved = 0;
status = rtems_io_write( major, minor, &args );
cpukit/libcsupport/src/termios.c
+3 -3
View File
@@ -2110,7 +2110,7 @@ rtems_termios_imfs_open (rtems_libio_t *iop,
memset (&args, 0, sizeof (args));
args.iop = iop;
args.flags = iop->flags;
args.flags = rtems_libio_iop_flags (iop);
args.mode = mode;
rtems_termios_obtain ();
@@ -2162,7 +2162,7 @@ rtems_termios_imfs_read (rtems_libio_t *iop, void *buffer, size_t count)
args.iop = iop;
args.buffer = buffer;
args.count = count;
args.flags = iop->flags;
args.flags = rtems_libio_iop_flags (iop);
sc = rtems_termios_linesw[tty->t_line].l_read (tty, &args);
tty->tty_rcvwakeup = false;
@@ -2202,7 +2202,7 @@ rtems_termios_imfs_write (rtems_libio_t *iop, const void *buffer, size_t count)
args.iop = iop;
args.buffer = RTEMS_DECONST (void *, buffer);
args.count = count;
args.flags = iop->flags;
args.flags = rtems_libio_iop_flags (iop);
sc = rtems_termios_linesw[tty->t_line].l_write (tty, &args);
rtems_mutex_unlock (&tty->osem);
cpukit/libnetworking/rtems/rtems_syscall.c
+7 -3
View File
@@ -87,7 +87,7 @@ rtems_bsdnet_makeFdForSocket (void *so)
iop->pathinfo.handlers = &socket_handlers;
iop->pathinfo.mt_entry = &rtems_filesystem_null_mt_entry;
rtems_filesystem_location_add_to_mt_entry(&iop->pathinfo);
rtems_libio_iop_flags_initialize(iop, LIBIO_FLAGS_READ_WRITE);
rtems_libio_iop_flags_set(iop, LIBIO_FLAGS_OPEN | LIBIO_FLAGS_READ_WRITE);
return fd;
}
@@ -739,14 +739,18 @@ rtems_bsdnet_write (rtems_libio_t *iop, const void *buffer, size_t count)
static int
so_ioctl (rtems_libio_t *iop, struct socket *so, uint32_t command, void *buffer)
{
unsigned int nonblock;
switch (command) {
case FIONBIO:
nonblock = rtems_libio_fcntl_flags (O_NONBLOCK);
if (*(int *)buffer) {
iop->flags |= O_NONBLOCK;
rtems_libio_iop_flags_set (iop, nonblock);
so->so_state |= SS_NBIO;
}
else {
iop->flags &= ~O_NONBLOCK;
rtems_libio_iop_flags_clear (iop, nonblock);
so->so_state &= ~SS_NBIO;
}
return 0;
cpukit/posix/src/shmopen.c
+2 -2
View File
@@ -286,14 +286,14 @@ int shm_open( const char *name, int oflag, mode_t mode )
iop->pathinfo.mt_entry = &rtems_filesystem_null_mt_entry;
rtems_filesystem_location_add_to_mt_entry( &iop->pathinfo );
flags = LIBIO_FLAGS_CLOSE_ON_EXEC;
flags = LIBIO_FLAGS_OPEN | LIBIO_FLAGS_CLOSE_ON_EXEC;
if ( (oflag & O_ACCMODE) == O_RDONLY ) {
flags |= LIBIO_FLAGS_READ;
} else {
flags |= LIBIO_FLAGS_READ_WRITE;
}
rtems_libio_iop_flags_initialize( iop, flags );
rtems_libio_iop_flags_set( iop, flags );
return fd;
}
testsuites/fstests/fsclose01/init.c
+48 -14
View File
@@ -1,15 +1,26 @@
/*
* Copyright (c) 2012, 2018 embedded brains GmbH. All rights reserved.
* Copyright (C) 2012, 2020 embedded brains GmbH (http://www.embedded-brains.de)
*
* embedded brains GmbH
* Dornierstr. 4
* 82178 Puchheim
* Germany
* <rtems@embedded-brains.de>
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
* 1. Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* 2. Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in the
* documentation and/or other materials provided with the distribution.
*
* The license and distribution terms for this file may be
* found in the file LICENSE in this distribution or at
* http://www.rtems.org/license/LICENSE.
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
* AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
* ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
* LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
* CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
* SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
* INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
* CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
* POSSIBILITY OF SUCH DAMAGE.
*/
#ifdef HAVE_CONFIG_H
@@ -32,7 +43,7 @@
const char rtems_test_name[] = "FSCLOSE 1";
typedef enum {
ACTION_ClOSE,
ACTION_CLOSE,
ACTION_FCNTL,
ACTION_FDATASYNC,
ACTION_FCHDIR,
@@ -328,7 +339,7 @@ static void worker_task(rtems_task_argument arg)
wait();
switch (ctx->action) {
case ACTION_ClOSE:
case ACTION_CLOSE:
ctx->wait_in_close = true;
rv = close(ctx->fd);
rtems_test_assert(rv == 0);
@@ -460,25 +471,48 @@ static void test_close(test_context *ctx)
);
rtems_test_assert(sc == RTEMS_SUCCESSFUL);
for (ac = ACTION_ClOSE; ac <= ACTION_WRITEV; ++ac) {
for (ac = ACTION_CLOSE; ac <= ACTION_WRITEV; ++ac) {
rtems_libio_t *iop;
unsigned int flags;
unsigned int expected_flags;
ctx->action = ac;
ctx->fd = open(path, O_RDWR);
rtems_test_assert(ctx->fd >= 0);
iop = rtems_libio_iop(ctx->fd);
wakeup_worker(ctx);
rv = close(ctx->fd);
rtems_test_assert(rv == -1);
if (ac == ACTION_ClOSE) {
if (ac == ACTION_CLOSE) {
rtems_test_assert(errno == EBADF);
flags = rtems_libio_iop_hold(iop);
expected_flags = LIBIO_FLAGS_READ_WRITE;
rtems_test_assert(flags == expected_flags);
flags = rtems_libio_iop_flags(iop);
expected_flags = LIBIO_FLAGS_REFERENCE_INC | LIBIO_FLAGS_READ_WRITE;
rtems_test_assert(flags == expected_flags);
} else {
rtems_test_assert(errno == EBUSY);
}
wakeup_worker(ctx);
if (ac == ACTION_CLOSE) {
flags = rtems_libio_iop_flags(iop);
expected_flags = LIBIO_FLAGS_REFERENCE_INC;
rtems_test_assert(flags == expected_flags);
rtems_libio_iop_drop(iop);
flags = rtems_libio_iop_flags(iop);
expected_flags = 0;
rtems_test_assert(flags == expected_flags);
}
rv = close(ctx->fd);
if (ac == ACTION_ClOSE) {
if (ac == ACTION_CLOSE) {
rtems_test_assert(rv == -1);
rtems_test_assert(errno == EBADF);
} else {