[PATCH] state: guard against empty variable set in DT

[PATCH] state: guard against empty variable set in DT

[Ahmad Fatoum]

We always initialize state->variables, but there are two places in the
code that assume there is at least one entry already.

Change them to use list_first_entry_or_null/list_last_entry_or_null as
appropriate to catch this issue gracefully.

This should have only affected state nodes without children, which is
not a useful device tree description, but nevertheless we should handle
that somehow instead of reading uninitialized values that may trigger
a panic or other misbehavior.

Reported-by: Fabian Pfitzner <f.pfitzner@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/state/backend_format_raw.c | 16 ++++++++++++++--
 common/state/state.c              |  8 +++++---
 include/linux/list.h              | 14 ++++++++++++++
 3 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/common/state/backend_format_raw.c b/common/state/backend_format_raw.c
index 5fb38cd711da..b7b88dd5b94e 100644
--- a/common/state/backend_format_raw.c
+++ b/common/state/backend_format_raw.c
@@ -198,6 +198,18 @@ static int backend_format_raw_unpack(struct state_backend_format *format,
 	return ret;
 }
 
+static inline size_t state_data_size(struct state *state)
+{
+	const struct state_variable *sv;
+
+	/* Make use of the fact that the list is sorted in ascending order */
+	sv = list_last_entry_or_null(&state->variables, struct state_variable, list);
+	if (!sv)
+		return 0;
+
+	return sv->start + sv->size;
+}
+
 static int backend_format_raw_pack(struct state_backend_format *format,
 				   struct state *state, void ** buf_out,
 				   ssize_t * len_out)
@@ -216,8 +228,8 @@ static int backend_format_raw_pack(struct state_backend_format *format,
 			return ret;
 	}
 
-	sv = list_last_entry(&state->variables, struct state_variable, list);
-	size_data = sv->start + sv->size;
+	size_data = state_data_size(state);
+
 	size_full = size_data + sizeof(*header) + backend_raw->digest_length;
 
 	buf = xzalloc(size_full);
diff --git a/common/state/state.c b/common/state/state.c
index ac6cd6e57276..bafc07dfe751 100644
--- a/common/state/state.c
+++ b/common/state/state.c
@@ -405,10 +405,12 @@ int state_from_node(struct state *state, struct device_node *node, bool create)
 	if (create) {
 		const struct state_variable *sv;
 
-		/* start with second entry */
-		sv = list_first_entry(&state->variables, struct state_variable,
-				      list);
+		/* no variable = no variable overlap */
+		sv = list_first_entry_or_null(&state->variables, struct state_variable, list);
+		if (!sv)
+			return 0;
 
+		/* start with second entry */
 		list_for_each_entry_continue(sv, &state->variables, list) {
 			const struct state_variable *last_sv;
 
diff --git a/include/linux/list.h b/include/linux/list.h
index b90ea3e125d0..a036e3d07c07 100644
--- a/include/linux/list.h
+++ b/include/linux/list.h
@@ -529,6 +529,20 @@ static inline void list_splice_tail_init(struct list_head *list,
 	pos__ != head__ ? list_entry(pos__, type, member) : NULL; \
 })
 
+/**
+ * list_last_entry_or_null - get the last element from a list
+ * @ptr:	the list head to take the element from.
+ * @type:	the type of the struct this is embedded in.
+ * @member:	the name of the list_head within the struct.
+ *
+ * Note that if the list is empty, it returns NULL.
+ */
+#define list_last_entry_or_null(ptr, type, member) ({ \
+	struct list_head *head__ = (ptr); \
+	struct list_head *pos__ = READ_ONCE(head__->prev); \
+	pos__ != head__ ? list_entry(pos__, type, member) : NULL; \
+})
+
 /**
  * list_next_entry - get the next element in list
  * @pos:	the type * to cursor
-- 
2.47.3

Re: [PATCH] state: guard against empty variable set in DT

[Sascha Hauer]

On Mon, 03 Nov 2025 12:16:23 +0100, Ahmad Fatoum wrote:
> We always initialize state->variables, but there are two places in the
> code that assume there is at least one entry already.
> 
> Change them to use list_first_entry_or_null/list_last_entry_or_null as
> appropriate to catch this issue gracefully.
> 
> This should have only affected state nodes without children, which is
> not a useful device tree description, but nevertheless we should handle
> that somehow instead of reading uninitialized values that may trigger
> a panic or other misbehavior.
> 
> [...]

Applied, thanks!

[1/1] state: guard against empty variable set in DT
      https://git.pengutronix.de/cgit/barebox/commit/?id=f1b549a97e50 (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>