Add EOF handling to 'read_all' function in the io module, and TCTIs.
This fixes a possible infinite loop in the TCTIs if / when the other end
of the IPC mechanism shuts down (for whatever reason). We add tests for
this scenario to the unit test for the io module.
Additionally the device and mssim TCTIs now handle this situation and
unit tests added for this scenario too.
Signed-off-by: Philip Tricca <flihp@twobit.us>
diff --git a/Makefile-test.am b/Makefile-test.am
index b51db52..af93dd4 100644
--- a/Makefile-test.am
+++ b/Makefile-test.am
@@ -170,8 +170,8 @@
src/tss2-tcti/tcti-mssim.c src/tss2-tcti/tcti-mssim.h
test_unit_io_CFLAGS = $(CMOCKA_CFLAGS) $(TESTS_CFLAGS)
-test_unit_io_LDADD = $(CMOCKA_LIBS) $(libutil) $(libtss2_mu)
-test_unit_io_LDFLAGS = -Wl,--wrap=connect,--wrap=socket,--wrap=write
+test_unit_io_LDADD = $(CMOCKA_LIBS) $(libtss2_mu) $(libutil)
+test_unit_io_LDFLAGS = -Wl,--wrap=connect,--wrap=read,--wrap=socket,--wrap=write
test_unit_io_SOURCES = test/unit/io.c
test_unit_CommonPreparePrologue_CFLAGS = $(CMOCKA_CFLAGS) $(TESTS_CFLAGS)
diff --git a/src/tss2-tcti/tcti-device.c b/src/tss2-tcti/tcti-device.c
index 806fedf..b674b97 100644
--- a/src/tss2-tcti/tcti-device.c
+++ b/src/tss2-tcti/tcti-device.c
@@ -194,7 +194,7 @@
if ((size_t)size != tcti_common->header.size) {
LOG_WARNING ("TPM2 header size disagrees with number of bytes read "
"from fd %d. Header says %u but we read %zu bytes.",
- tcti_dev->fd, tcti_common->header.size, *response_size);
+ tcti_dev->fd, tcti_common->header.size, size);
}
if (*response_size < tcti_common->header.size) {
LOG_WARNING ("TPM2 response header size is larger than the provided "
diff --git a/src/tss2-tcti/tcti-mssim.c b/src/tss2-tcti/tcti-mssim.c
index 659d735..3de7499 100644
--- a/src/tss2-tcti/tcti-mssim.c
+++ b/src/tss2-tcti/tcti-mssim.c
@@ -371,7 +371,7 @@
ret = socket_recv_buf (tcti_mssim->tpm_sock,
(unsigned char *)response_buffer,
tcti_common->header.size);
- if (ret < 0) {
+ if (ret < (ssize_t)tcti_common->header.size) {
rc = TSS2_TCTI_RC_IO_ERROR;
goto out;
}
diff --git a/src/util/io.c b/src/util/io.c
index ea07f23..e8e2def 100644
--- a/src/util/io.c
+++ b/src/util/io.c
@@ -40,7 +40,13 @@
#include "util/log.h"
#define MAX_PORT_STR_LEN sizeof("65535")
-
+/*
+ * The 'read_all' function attempts to read all of the 'size' bytes requested
+ * from the 'fd' provided into the buffer 'data'. This function will continue
+ * to retry after temporary failures and "short reads". It will only stop
+ * once all of the requested data has been read, an error occurs, or EOF.
+ * On error or EOF, the number of bytes read (if any) will be returned.
+ */
ssize_t
read_all (
int fd,
@@ -59,6 +65,11 @@
fd, errno, strerror (errno));
return recvd_total;
}
+ if (recvd == 0) {
+ LOG_WARNING ("Attempted read %zu bytes from fd %d, but EOF "
+ "returned", size, fd);
+ return recvd_total;
+ }
LOGBLOB_DEBUG (&data [recvd_total], recvd, "read %zd bytes from fd %d:", recvd, fd);
recvd_total += recvd;
size -= recvd;
diff --git a/test/unit/io.c b/test/unit/io.c
index cf442b4..3efaab7 100644
--- a/test/unit/io.c
+++ b/test/unit/io.c
@@ -56,6 +56,19 @@
return mock_type (int);
}
+/*
+ * Wrap the 'recv' system call. The mock queue for this function must have an
+ * integer return value (the number of byts recv'd), as well as a pointer to
+ * a buffer to copy data from to return to the caller.
+ */
+ssize_t
+__wrap_read (int fd, void *buffer, size_t count)
+{
+ LOG_DEBUG ("%s: reading %zu bytes from fd: %d to buffer at 0x%" PRIxPTR,
+ __func__, count, fd, (uintptr_t)buffer);
+ return mock_type (ssize_t);
+}
+
ssize_t
__wrap_write (int fd, const void *buffer, size_t buffer_size)
{
@@ -81,6 +94,35 @@
ret = write_all (99, buf, sizeof (buf));
assert_int_equal(ret, sizeof (buf));
}
+/*
+ * This test causes the underlying 'read' operation to return '0' bytes
+ * indicating EOF.
+ */
+static void
+read_all_eof_test (void **state)
+{
+ ssize_t ret;
+ uint8_t buf [10];
+
+ will_return (__wrap_read, 0);
+ ret = read_all (10, buf, sizeof (buf));
+ assert_int_equal (ret, 0);
+}
+/*
+ * This test is a minor variation on the 'read_all_eof_test'. We still get
+ * an EOF from the underlying read but only after we get a good read, but one
+ * that's less than what was requested.
+ */
+static void
+read_all_twice_eof (void **state)
+{
+ ssize_t ret;
+
+ will_return (__wrap_read, 5);
+ will_return (__wrap_read, 0);
+ ret = read_all (10, NULL, 10);
+ assert_int_equal (ret, 5);
+}
/* When passed all NULL values ensure that we get back the expected RC. */
static void
socket_connect_test (void **state)
@@ -175,6 +217,8 @@
{
const struct CMUnitTest tests[] = {
cmocka_unit_test (write_all_simple_success_test),
+ cmocka_unit_test (read_all_eof_test),
+ cmocka_unit_test (read_all_twice_eof),
cmocka_unit_test (socket_connect_test),
cmocka_unit_test (socket_connect_null_test),
cmocka_unit_test (socket_connect_socket_fail_test),
diff --git a/test/unit/tcti-device.c b/test/unit/tcti-device.c
index 964f90a..395b206 100644
--- a/test/unit/tcti-device.c
+++ b/test/unit/tcti-device.c
@@ -269,6 +269,9 @@
cmocka_unit_test_setup_teardown (tcti_device_transmit_success,
tcti_device_setup,
tcti_device_teardown),
+ cmocka_unit_test_setup_teardown (tcti_device_transmit_success,
+ tcti_device_setup,
+ tcti_device_teardown),
};
return cmocka_run_group_tests (tests, NULL, NULL);
}
diff --git a/test/unit/tcti-mssim.c b/test/unit/tcti-mssim.c
index 7ebdfe6..56246b9 100644
--- a/test/unit/tcti-mssim.c
+++ b/test/unit/tcti-mssim.c
@@ -416,6 +416,67 @@
assert_memory_equal (response_in, response_out, response_size);
}
/*
+ * This test causes the underlying 'read' call to return 0 / EOF when we
+ * call the TCTI 'receive' function. In this case the TCTI should return an
+ * IO error.
+ */
+static void
+tcti_mssim_receive_eof_first_read_test (void **state)
+{
+ TSS2_TCTI_CONTEXT *ctx = (TSS2_TCTI_CONTEXT*)*state;
+ TSS2_TCTI_COMMON_CONTEXT *tcti_common = tcti_common_context_cast (ctx);
+ TSS2_RC rc;
+ /* output buffer for response */
+ uint8_t buf [TPM_HEADER_SIZE] = { 0 };
+ size_t size = sizeof (buf);
+
+ /* Keep state machine check in `receive` from returning error. */
+ tcti_common->state = TCTI_STATE_RECEIVE;
+ will_return (__wrap_read, 0);
+ will_return (__wrap_read, buf);
+ rc = Tss2_Tcti_Receive (ctx,
+ &size,
+ buf,
+ TSS2_TCTI_TIMEOUT_BLOCK);
+ assert_true (rc == TSS2_TCTI_RC_IO_ERROR);
+}
+/*
+ * This test causes the underlying 'read' call to return EOF but only after
+ * a successful read that gets us the response size. This results in the
+ * an IO_ERROR RC being returned.
+ */
+static void
+tcti_mssim_receive_eof_second_read_test (void **state)
+{
+ TSS2_TCTI_CONTEXT *ctx = (TSS2_TCTI_CONTEXT*)*state;
+ TSS2_TCTI_COMMON_CONTEXT *tcti_common = tcti_common_context_cast (ctx);
+ TSS2_RC rc;
+ /* input response buffer */
+ uint8_t response_in [] = { 0x80, 0x02,
+ 0x00, 0x00, 0x00, 0x0c,
+ 0x00, 0x00, 0x00, 0x00,
+ 0x01, 0x02,
+ /* simulator appends 4 bytes of 0's to every response */
+ 0x00, 0x00, 0x00, 0x00 };
+ /* output response buffer */
+ uint8_t response_out [12] = { 0 };
+ size_t size = sizeof (response_out);
+
+ /* Keep state machine check in `receive` from returning error. */
+ tcti_common->state = TCTI_STATE_RECEIVE;
+ /* setup response size for first read */
+ will_return (__wrap_read, 4);
+ will_return (__wrap_read, &response_in [2]);
+ /* setup 0 for EOF on second read */
+ will_return (__wrap_read, 0);
+ will_return (__wrap_read, response_in);
+ rc = Tss2_Tcti_Receive (ctx,
+ &size,
+ response_out,
+ TSS2_TCTI_TIMEOUT_BLOCK);
+ assert_true (rc == TSS2_TCTI_RC_IO_ERROR);
+}
+/*
* This test exercises the successful code path through the transmit function.
*/
static void
@@ -467,6 +528,12 @@
cmocka_unit_test_setup_teardown (tcti_socket_receive_size_success_test,
tcti_socket_setup,
tcti_socket_teardown),
+ cmocka_unit_test_setup_teardown (tcti_mssim_receive_eof_first_read_test,
+ tcti_socket_setup,
+ tcti_socket_teardown),
+ cmocka_unit_test_setup_teardown (tcti_mssim_receive_eof_second_read_test,
+ tcti_socket_setup,
+ tcti_socket_teardown),
cmocka_unit_test_setup_teardown (tcti_socket_transmit_success_test,
tcti_socket_setup,
tcti_socket_teardown)