Skip to contentSkip to conversation

common: state: Add property to protect existing data

PATCH Applied Daniel Schultz 2 participants 3 messages Apr 12, 2018 – Apr 16, 2018

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH v2 1/2 common: state: Add property to protect existing data

After an update to a newer barebox version with an enabled state framework, existing data in storage memories could be overwritten.

Add a new property to check in front of every write task, if the meta magic field only contains the magic number, zeros or ones.

---
Changes:
v2: Added check for circular backend
Improved error message if content was found

 .../devicetree/bindings/barebox/barebox,state.rst     |  3 +++
 common/state/backend_bucket_circular.c                |  8 +++++---
 common/state/backend_bucket_direct.c                  |  2 ++
 common/state/state.c                                  | 19 ++++++++++++++++++-
 common/state/state.h                                  |  2 ++
 5 files changed, 30 insertions(+), 4 deletions(-)
Documentation/devicetree/bindings/barebox/barebox,state.rst +3 -0
index 872bac0..2893937 100644
@@ -55,6 +55,9 @@ Optional Properties
 * ``algo``: A HMAC algorithm used to detect manipulation of the data
   or header, sensible values follow this pattern ``hmac(<HASH>)``,
   e.g. ``hmac(sha256)``. Only available for the ``backend-type`` ``raw``.
+* ``keep-previous-content``: Check if a the bucket meta magic field contains
+  other data than the magic value. If so, the backend will not write the state
+  to prevent unconditionally overwrites of existing data.
 
 .. note:: For the ``backend-storage-type`` the keyword ``noncircular`` is still
    supported as a fall back to an old storage format. Recommendation is to not
common/state/backend_bucket_circular.c +5 -3
index 2324903..933493e 100644
@@ -396,11 +396,13 @@ static int state_backend_bucket_circular_init(
 			meta = (struct state_backend_storage_bucket_circular_meta *)
 					(buf + sub_offset + circ->writesize - sizeof(*meta));
 
-			if (meta->magic != circular_magic)
+			if (meta->magic != circular_magic) {
 				written_length = 0;
-			else
+				if (meta->magic != ~0 && !!meta->magic)
+					bucket->wrong_magic = 1;
+			} else {
 				written_length = meta->written_length;
-
+			}
 			break;
 		}
 	}
common/state/backend_bucket_direct.c +2 -0
index 958696e..9d6a337 100644
@@ -69,6 +69,8 @@ static int state_backend_bucket_direct_read(struct state_backend_storage_bucket
 	if (meta.magic == direct_magic) {
 		read_len = meta.written_length;
 	} else {
+		if (meta.magic != ~0 && !!meta.magic)
+			bucket->wrong_magic = 1;
 		if (!IS_ENABLED(CONFIG_STATE_BACKWARD_COMPATIBLE)) {
 			dev_err(direct->dev, "No meta data header found\n");
 			dev_dbg(direct->dev, "Enable backward compatibility or increase stride size\n");
common/state/state.c +18 -1
index 73dfb58..25d9502 100644
@@ -44,6 +44,8 @@ int state_save(struct state *state)
 	void *buf;
 	ssize_t len;
 	int ret;
+	struct state_backend_storage_bucket *bucket;
+	struct state_backend_storage *storage;
 
 	if (!state->dirty)
 		return 0;
@@ -55,7 +57,19 @@ int state_save(struct state *state)
 		return ret;
 	}
 
-	ret = state_storage_write(&state->storage, buf, len);
+	storage = &state->storage;
+	if (state->keep_prev_content) {
+		bool has_content = 0;
+		list_for_each_entry(bucket, &storage->buckets, bucket_list)
+			has_content |= bucket->wrong_magic;
+		if (has_content) {
+			dev_err(&state->dev, "Found foreign content on backend, won't overwrite.\n");
+			ret = -EPERM;
+			goto out;
+		}
+	}
+
+	ret = state_storage_write(storage, buf, len);
 	if (ret) {
 		dev_err(&state->dev, "Failed to write packed state, %d\n", ret);
 		goto out;
@@ -623,6 +637,9 @@ struct state *state_new_from_node(struct device_node *node, bool readonly)
 
 	of_property_read_string(node, "backend-storage-type", &storage_type);
 
+	state->keep_prev_content = of_property_read_bool(node,
+							"keep-previous-content");
+
 	ret = state_format_init(state, backend_type, node, alias);
 	if (ret)
 		goto out_release_state;
common/state/state.h +2 -0
index 6670523..3a0662f 100644
@@ -46,6 +46,7 @@ struct state_backend_storage_bucket {
 	void *buf;
 	ssize_t len;
 	bool needs_refresh;
+	bool wrong_magic;
 };
 
 /**
@@ -105,6 +106,7 @@ struct state {
 	char *of_path;
 	const char *name;
 	uint32_t magic;
+	bool keep_prev_content;
 
 	struct list_head variables; /* Sorted list of variables */
 
Signature · 7 lines
2.7.4


_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox
#1 lore raw

PATCH v2 2/2 ARM: dts: AM335x: Add keep-previous-content property

Some of our customers may have data in the EEPROM region, which we newly have assigned as state partition. This property should prevent them of data loses.

---
 arch/arm/dts/am335x-phytec-state.dtsi | 1 +
 1 file changed, 1 insertion(+)
arch/arm/dts/am335x-phytec-state.dtsi +1 -0
index fbc35b9..6bca597 100644
@@ -23,6 +23,7 @@
 		backend-type = "raw";
 		backend = <&backend_state_eeprom>;
 		backend-stridesize = <40>;
+		keep-previous-content;
 
 		#address-cells = <1>;
 		#size-cells = <1>;
Signature · 7 lines
2.7.4


_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox
Applied
#2 lore raw
In reply to Daniel Schultz
Quoted 12 lines · Daniel Schultz

After an update to a newer barebox version with an enabled state framework, existing data in storage memories could be overwritten.

Add a new property to check in front of every write task, if the meta magic field only contains the magic number, zeros or ones.

---
Changes:
v2: Added check for circular backend
Improved error message if content was found

Applied, thanks

Sascha

Quoted 128 lines
 .../devicetree/bindings/barebox/barebox,state.rst     |  3 +++
 common/state/backend_bucket_circular.c                |  8 +++++---
 common/state/backend_bucket_direct.c                  |  2 ++
 common/state/state.c                                  | 19 ++++++++++++++++++-
 common/state/state.h                                  |  2 ++
 5 files changed, 30 insertions(+), 4 deletions(-)
Documentation/devicetree/bindings/barebox/barebox,state.rst +3 -0
index 872bac0..2893937 100644
@@ -55,6 +55,9 @@ Optional Properties
 * ``algo``: A HMAC algorithm used to detect manipulation of the data
   or header, sensible values follow this pattern ``hmac(<HASH>)``,
   e.g. ``hmac(sha256)``. Only available for the ``backend-type`` ``raw``.
+* ``keep-previous-content``: Check if a the bucket meta magic field contains
+  other data than the magic value. If so, the backend will not write the state
+  to prevent unconditionally overwrites of existing data.
 
 .. note:: For the ``backend-storage-type`` the keyword ``noncircular`` is still
    supported as a fall back to an old storage format. Recommendation is to not
common/state/backend_bucket_circular.c +5 -3
index 2324903..933493e 100644
@@ -396,11 +396,13 @@ static int state_backend_bucket_circular_init(
 			meta = (struct state_backend_storage_bucket_circular_meta *)
 					(buf + sub_offset + circ->writesize - sizeof(*meta));
 
-			if (meta->magic != circular_magic)
+			if (meta->magic != circular_magic) {
 				written_length = 0;
-			else
+				if (meta->magic != ~0 && !!meta->magic)
+					bucket->wrong_magic = 1;
+			} else {
 				written_length = meta->written_length;
-
+			}
 			break;
 		}
 	}
common/state/backend_bucket_direct.c +2 -0
index 958696e..9d6a337 100644
@@ -69,6 +69,8 @@ static int state_backend_bucket_direct_read(struct state_backend_storage_bucket
 	if (meta.magic == direct_magic) {
 		read_len = meta.written_length;
 	} else {
+		if (meta.magic != ~0 && !!meta.magic)
+			bucket->wrong_magic = 1;
 		if (!IS_ENABLED(CONFIG_STATE_BACKWARD_COMPATIBLE)) {
 			dev_err(direct->dev, "No meta data header found\n");
 			dev_dbg(direct->dev, "Enable backward compatibility or increase stride size\n");
common/state/state.c +18 -1
index 73dfb58..25d9502 100644
@@ -44,6 +44,8 @@ int state_save(struct state *state)
 	void *buf;
 	ssize_t len;
 	int ret;
+	struct state_backend_storage_bucket *bucket;
+	struct state_backend_storage *storage;
 
 	if (!state->dirty)
 		return 0;
@@ -55,7 +57,19 @@ int state_save(struct state *state)
 		return ret;
 	}
 
-	ret = state_storage_write(&state->storage, buf, len);
+	storage = &state->storage;
+	if (state->keep_prev_content) {
+		bool has_content = 0;
+		list_for_each_entry(bucket, &storage->buckets, bucket_list)
+			has_content |= bucket->wrong_magic;
+		if (has_content) {
+			dev_err(&state->dev, "Found foreign content on backend, won't overwrite.\n");
+			ret = -EPERM;
+			goto out;
+		}
+	}
+
+	ret = state_storage_write(storage, buf, len);
 	if (ret) {
 		dev_err(&state->dev, "Failed to write packed state, %d\n", ret);
 		goto out;
@@ -623,6 +637,9 @@ struct state *state_new_from_node(struct device_node *node, bool readonly)
 
 	of_property_read_string(node, "backend-storage-type", &storage_type);
 
+	state->keep_prev_content = of_property_read_bool(node,
+							"keep-previous-content");
+
 	ret = state_format_init(state, backend_type, node, alias);
 	if (ret)
 		goto out_release_state;
common/state/state.h +2 -0
index 6670523..3a0662f 100644
@@ -46,6 +46,7 @@ struct state_backend_storage_bucket {
 	void *buf;
 	ssize_t len;
 	bool needs_refresh;
+	bool wrong_magic;
 };
 
 /**
@@ -105,6 +106,7 @@ struct state {
 	char *of_path;
 	const char *name;
 	uint32_t magic;
+	bool keep_prev_content;
 
 	struct list_head variables; /* Sorted list of variables */
 
Signature · 7 lines
2.7.4


_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox
Signature · 9 lines
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox