summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAndrew Jeffery <andrew@aj.id.au>2018-03-27 10:35:31 +1030
committerAndrew Jeffery <andrew@aj.id.au>2018-04-04 17:27:31 +0930
commit52a8319658c8ac3dcce2fed116d20d6014d7b143 (patch)
treec546f26c8d07e3284828b09d85574d3806cd1c1c
parent943aba060f143667d502792f11e95f42bc7da346 (diff)
downloadphosphor-mboxd-52a8319658c8ac3dcce2fed116d20d6014d7b143.zip
phosphor-mboxd-52a8319658c8ac3dcce2fed116d20d6014d7b143.tar.gz
vpnor: Add handler for CREATE_WRITE_WINDOW
The virtual PNOR implementation enforces the read-only attribute of FFS partitions, which is a departure from how things were handled previously. In the past it was purely up to the host to respect the flags set on the partition, but nothing prevented the host from modifying it. Now it's possible for errors to occur when the host attempts to flush changes back to the flash: mboxd can deny the change. This denial can happen in a number of circumstances: 1. An explicit WRITE_FLUSH command from the host 2. An implicit WRITE_FLUSH via an explicit CLOSE_WINDOW command 3. An implicit WRITE_FLUSH via CREATE_{READ,WRITE}_WINDOW, which happens via the implicit CLOSE_WINDOW All of these attempts will fail if the write to the currently open window cannot be allowed to succeed. Failing to open a read window due to failure to flush pending writes is particularly painful, as we are not able to ever successfully open a window again. Instead, detect when the host attempts to open a write window over a anything but a writeable partition. If this case is detected, return an error for the CREATE_WRITE_WINDOW operation to prevent systemic failures later on. Change-Id: I991b6f1570d9b1b384b1024e3bd8a77e5efcd198 Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
-rw-r--r--vpnor/mboxd_flash.cpp13
-rw-r--r--vpnor/mboxd_msg.cpp36
-rw-r--r--vpnor/mboxd_msg.hpp3
-rw-r--r--vpnor/test/Makefile.am.include3
4 files changed, 45 insertions, 10 deletions
diff --git a/vpnor/mboxd_flash.cpp b/vpnor/mboxd_flash.cpp
index ffe75c6..a774ec1 100644
--- a/vpnor/mboxd_flash.cpp
+++ b/vpnor/mboxd_flash.cpp
@@ -203,7 +203,9 @@ int write_flash(struct mbox_context* context, uint32_t offset, void* buf,
const struct pnor_partition& part = table->partition(offset);
if (part.data.user.data[1] & PARTITION_READONLY)
{
- /* FIXME: This should be done on CREATE_WRITE_WINDOW, not here */
+ MSG_ERR("Unreachable: Host attempted to write to read-only "
+ "partition %s\n",
+ part.data.name);
return -MBOX_R_WRITE_ERROR;
}
@@ -214,11 +216,10 @@ int write_flash(struct mbox_context* context, uint32_t offset, void* buf,
}
catch (vpnor::UnmappedOffset& e)
{
- /* Paper over the fact that the write isn't persistent */
- MSG_INFO("Dropping %d bytes host wrote to unmapped offset 0x%" PRIx32
- "\n",
- count, offset);
- return 0;
+ MSG_ERR("Unreachable: Host attempted to write %" PRIu32
+ " bytes to unmapped offset 0x%" PRIx32 "\n",
+ count, offset);
+ return -MBOX_R_WRITE_ERROR;
}
catch (const vpnor::OutOfBoundsOffset& e)
{
diff --git a/vpnor/mboxd_msg.cpp b/vpnor/mboxd_msg.cpp
index 130c098..dd9e64a 100644
--- a/vpnor/mboxd_msg.cpp
+++ b/vpnor/mboxd_msg.cpp
@@ -6,6 +6,7 @@ extern "C" {
};
#include "vpnor/mboxd_msg.hpp"
+#include "vpnor/pnor_partition_table.hpp"
// clang-format off
const mboxd_mbox_handler vpnor_mbox_handlers[NUM_MBOX_CMDS] =
@@ -15,10 +16,43 @@ const mboxd_mbox_handler vpnor_mbox_handlers[NUM_MBOX_CMDS] =
mbox_handle_flash_info,
mbox_handle_read_window,
mbox_handle_close_window,
- mbox_handle_write_window,
+ vpnor_handle_write_window,
mbox_handle_dirty_window,
mbox_handle_flush_window,
mbox_handle_ack,
mbox_handle_erase_window
};
// clang-format on
+
+/* XXX: Maybe this should be a method on a class? */
+static bool vpnor_partition_is_readonly(const pnor_partition& part)
+{
+ return part.data.user.data[1] & PARTITION_READONLY;
+}
+
+int vpnor_handle_write_window(struct mbox_context* context,
+ union mbox_regs* req, struct mbox_msg* resp)
+{
+ size_t offset = get_u16(&req->msg.args[0]);
+ offset <<= context->block_size_shift;
+ try
+ {
+ const pnor_partition& part = context->vpnor->table->partition(offset);
+ if (vpnor_partition_is_readonly(part))
+ {
+ return -MBOX_R_WINDOW_ERROR;
+ }
+ }
+ catch (const openpower::virtual_pnor::UnmappedOffset& e)
+ {
+ /*
+ * Writes to unmapped areas are not meaningful, so deny the request.
+ * This removes the ability for a compromised host to abuse unused
+ * space if any data was to be persisted (which it isn't).
+ */
+ return -MBOX_R_WINDOW_ERROR;
+ }
+
+ /* Defer to the default handler */
+ return mbox_handle_write_window(context, req, resp);
+}
diff --git a/vpnor/mboxd_msg.hpp b/vpnor/mboxd_msg.hpp
index 7485c38..d9a05bb 100644
--- a/vpnor/mboxd_msg.hpp
+++ b/vpnor/mboxd_msg.hpp
@@ -2,4 +2,7 @@ extern "C" {
#include "mbox.h"
extern const mboxd_mbox_handler vpnor_mbox_handlers[NUM_MBOX_CMDS];
+
+int vpnor_handle_write_window(struct mbox_context *context,
+ union mbox_regs *req, struct mbox_msg *resp);
};
diff --git a/vpnor/test/Makefile.am.include b/vpnor/test/Makefile.am.include
index 85b2cbc..819343b 100644
--- a/vpnor/test/Makefile.am.include
+++ b/vpnor/test/Makefile.am.include
@@ -247,7 +247,4 @@ check_PROGRAMS += \
%reldir%/create_write_window_ro_partition \
%reldir%/create_write_window_rw_partition \
%reldir%/create_write_window_unmapped
-
-XFAIL_TESTS += %reldir%/create_write_window_ro_partition
-XFAIL_TESTS += %reldir%/create_write_window_unmapped
endif
OpenPOWER on IntegriCloud