sergiotarxz/nuttx
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

PTY: Fix a race condition in test-fifo-empty-before-read logic

Browse Source
This commit is contained in:
Gregory Nutt 2016-07-19 06:45:02 -06:00
parent 2119c5ce19
commit a4458c5016
2 changed files with 38 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
drivers/Kconfig
View File

@ -29,6 +29,7 @@ config DEV_RANDOM
bool "Enable /dev/random"
default y
depends on ARCH_HAVE_RNG
Enable support for /dev/urandom provided by a hardware TRNG.
config DEV_URANDOM
bool "Enable /dev/urandom"
@ -52,7 +53,7 @@ choice
config DEV_URANDOM_XORSHIFT128
bool "xorshift128"
---help---
xorshift128 is a pseudorandom number generator that's simple,
xorshift128 is a pseudorandom number generator that is simple,
portable, and can also be used on 8-bit and 16-bit MCUs.
NOTE: Not cyptographically secure
@ -74,9 +75,10 @@ config DEV_URANDOM_ARCH
The implementation of /dev/urandom is provided in archtecture-
specific logic using hardware TRNG logic. architecture-specific
logic must provide the whole implementation in this case, including
the function devurandom_register().
the function devurandom_register(). In this case, /dev/urandom may
refer to the same driver as /dev/random.
May or may not be cyptographically secure, depending upon the
NOTE: May or may not be cyptographically secure, depending upon the
implementation.
endchoice # /dev/urandom algorithm

44
drivers/serial/pty.c
View File

@ -43,6 +43,7 @@
#include <sys/ioctl.h>
#include <stdbool.h>
#include <unistd.h>
#include <sched.h>
#include <semaphore.h>
#include <termios.h>
#include <stdio.h>
@ -421,45 +422,66 @@ static ssize_t pty_read(FAR struct file *filep, FAR char *buffer, size_t len)
* bytes were read from the pipe. If we have already read some
* data, we use the FIONREAD ioctl to test if there are more bytes
* in the pipe.
*
* There is an inherent race condition in this test, but leaving
* a few bytes unnecessarily in the pipe should not be harmful.
* (we could lock the scheduler before the test and after the
* file_read() below if we wanted to eliminate the race)
*/
if (ntotal > 0)
{
int nsrc;
/* There are inherent race conditions in this test. We lock
* the scheduler before the test and after the file_read()
* below to eliminate one race: (a) We detect that there is
* data in the source file, (b) we are suspended and another
* thread reads the data, emptying the fifo, then (c) we
* resume and call file_read(), blocking indefinitely.
*/
sched_lock();
/* Check how many bytes are waiting in the pipe */
ret = file_ioctl(&dev->pd_src, FIONREAD,
(unsigned long)((uintptr_t)&nsrc));
if (ret < 0)
{
sched_unlock();
ntotal = ret;
break;
}
/* Break out of the loop and return ntotal if the pipe is
* empty. This is the race: The fifo was emtpy when we
* empty. This is another race: There fifo was empty when we
* called file_ioctl() above, but it might not be empty right
* now.
* now. Losing that race should not lead to any bad behaviors,
* however, we the caller will get those bytes on the next
* read.
*/
if (nsrc < 1)
{
sched_unlock();
break;
}
/* Read one byte from the source the byte. This should not
* block.
*/
nread = file_read(&dev->pd_src, &ch, 1);
sched_unlock();
}
else
#endif
{
/* Read one byte from the source the byte. This call will block
* if the source pipe is empty.
*/
/* Read one byte from the source the byte. This call will block
* if the source pipe is empty.
*/
nread = file_read(&dev->pd_src, &ch, 1);
}
/* Check if file_read was successful */
nread = file_read(&dev->pd_src, &ch, 1);
if (nread < 0)
{
ntotal = nread;