Linux userland API discussions
 help / color / mirror / Atom feed
* [PATCH v6 7/9] selftests/futex: Expand for set_robust_list2()
From: André Almeida @ 2025-11-22  5:50 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Peter Zijlstra, Darren Hart,
	Davidlohr Bueso, Arnd Bergmann, Sebastian Andrzej Siewior,
	Waiman Long, Ryan Houdek
  Cc: linux-kernel, linux-kselftest, linux-api, kernel-dev,
	André Almeida
In-Reply-To: <20251122-tonyk-robust_futex-v6-0-05fea005a0fd@igalia.com>

Reuse the same selftest for the original set_robust_list() syscall for
the new set_robust_list2() syscall. Use kselftest variants feature to
run the relevant tests for both interfaces. Create new test cases for
checking invalid parameters, the ability to correctly set multiple lists
for the same task, and to use 32-bit lists in a 64-bit task.

Signed-off-by: André Almeida <andrealmeid@igalia.com>
---
 .../selftests/futex/functional/robust_list.c       | 409 +++++++++++++++++++--
 1 file changed, 387 insertions(+), 22 deletions(-)

diff --git a/tools/testing/selftests/futex/functional/robust_list.c b/tools/testing/selftests/futex/functional/robust_list.c
index e7d1254e18ca..bf47e9ab2951 100644
--- a/tools/testing/selftests/futex/functional/robust_list.c
+++ b/tools/testing/selftests/futex/functional/robust_list.c
@@ -42,6 +42,27 @@
 
 #define SLEEP_US 100
 
+#ifndef SYS_set_robust_list2
+# define SYS_set_robust_list2 470
+
+enum robust_list_cmd {
+	FUTEX_ROBUST_LIST_CMD_SET_64,
+	FUTEX_ROBUST_LIST_CMD_SET_32,
+	FUTEX_ROBUST_LIST_CMD_LIST_LIMIT,
+	FUTEX_ROBUST_LIST_CMD_USER_MAX,
+};
+
+struct robust_list32 {
+	uint32_t next;
+};
+
+struct robust_list_head32 {
+	struct robust_list32	list;
+	int32_t			futex_offset;
+	uint32_t		list_op_pending;
+};
+#endif
+
 static pthread_barrier_t barrier, barrier2;
 
 static int set_robust_list(struct robust_list_head *head, size_t len)
@@ -54,6 +75,58 @@ static int get_robust_list(int pid, struct robust_list_head **head, size_t *len_
 	return syscall(SYS_get_robust_list, pid, head, len_ptr);
 }
 
+static int set_robust_list2(struct robust_list_head *head, int index,
+			    enum robust_list_cmd cmd, unsigned int flags)
+{
+	return syscall(SYS_set_robust_list2, head, index, cmd, flags);
+}
+
+static bool robust_list2_support(void)
+{
+	int ret = set_robust_list2(0, 0, FUTEX_ROBUST_LIST_CMD_LIST_LIMIT, 0);
+
+	if (ret == -1 && errno == ENOSYS)
+		return false;
+
+	return true;
+}
+
+/*
+ * Return the set command according to the app bitness
+ */
+static int get_cmd_set(void)
+{
+	return sizeof(uintptr_t) == 8 ? FUTEX_ROBUST_LIST_CMD_SET_64 :
+	       FUTEX_ROBUST_LIST_CMD_SET_32;
+}
+
+FIXTURE(robust_api) {};
+
+FIXTURE_VARIANT(robust_api)
+{
+	bool robust2;
+};
+
+FIXTURE_SETUP(robust_api)
+{
+	if (!variant->robust2)
+		return;
+
+	ASSERT_NE(robust_list2_support(), false);
+}
+
+FIXTURE_TEARDOWN(robust_api) {}
+
+FIXTURE_VARIANT_ADD(robust_api, robust1)
+{
+	.robust2 = false,
+};
+
+FIXTURE_VARIANT_ADD(robust_api, robust2)
+{
+	.robust2 = true,
+};
+
 /*
  * Basic lock struct, contains just the futex word and the robust list element
  * Real implementations have also a *prev to easily walk in the list
@@ -61,6 +134,12 @@ static int get_robust_list(int pid, struct robust_list_head **head, size_t *len_
 struct lock_struct {
 	_Atomic(unsigned int)	futex;
 	struct robust_list	list;
+	bool			robust2;
+};
+
+struct lock_struct32 {
+	_Atomic(uint32_t)	futex;
+	struct robust_list32	list;
 };
 
 /*
@@ -89,20 +168,17 @@ static int create_child(int (*fn)(void *arg), void *arg)
 /*
  * Helper function to prepare and register a robust list
  */
-static int set_list(struct robust_list_head *head)
+static int set_list(struct robust_list_head *head, bool robust2, int index)
 {
-	int ret;
-
-	ret = set_robust_list(head, sizeof(*head));
-	if (ret)
-		return ret;
-
 	head->futex_offset = (size_t) offsetof(struct lock_struct, futex) -
 			     (size_t) offsetof(struct lock_struct, list);
 	head->list.next = &head->list;
 	head->list_op_pending = NULL;
 
-	return 0;
+	if (!robust2)
+		return set_robust_list(head, sizeof(*head));
+
+	return set_robust_list2(head, index, get_cmd_set(), 0);
 }
 
 /*
@@ -174,7 +250,7 @@ static int child_fn_lock(void *arg)
 	struct robust_list_head head;
 	int ret;
 
-	ret = set_list(&head);
+	ret = set_list(&head, lock->robust2, 0);
 	if (ret) {
 		ksft_test_result_fail("set_robust_list error\n");
 		return ret;
@@ -204,14 +280,16 @@ static int child_fn_lock(void *arg)
  * in the robust list and die. The parent thread will wait on this futex, and
  * should be waken up when the child exits.
  */
-TEST(test_robustness)
+TEST_F(robust_api, test_robustness)
 {
 	struct lock_struct lock = { .futex = 0 };
 	_Atomic(unsigned int) *futex = &lock.futex;
-	struct robust_list_head head;
 	int ret, pid, wstatus;
+	struct robust_list_head head;
 
-	ret = set_list(&head);
+	lock.robust2 = variant->robust2;
+
+	ret = set_list(&head, lock.robust2, 0);
 	ASSERT_EQ(ret, 0);
 
 	/*
@@ -270,6 +348,46 @@ TEST(test_set_robust_list_invalid_size)
 	ksft_test_result_pass("%s\n", __func__);
 }
 
+/*
+ * Test invalid parameters
+ */
+TEST(test_set_robust_list2_inval)
+{
+	struct robust_list_head head;
+	int ret, list_limit;
+
+	if (!robust_list2_support()) {
+		ksft_test_result_skip("robust_list2 not supported\n");
+		return;
+	}
+
+	/* Bad flag */
+	ret = set_robust_list2(&head, 0, get_cmd_set(), 999);
+	ASSERT_EQ(ret, -1);
+	ASSERT_EQ(errno, EINVAL);
+
+	/* Bad index */
+	list_limit = set_robust_list2(NULL, 0, FUTEX_ROBUST_LIST_CMD_LIST_LIMIT, 0);
+	ASSERT_GT(list_limit, 0);
+
+	ret = set_robust_list2(&head, -1, get_cmd_set(), 0);
+	ASSERT_EQ(ret, -1);
+	ASSERT_EQ(errno, EINVAL);
+
+	ret = set_robust_list2(&head, list_limit + 1, get_cmd_set(), 0);
+	ASSERT_EQ(ret, -1);
+	ASSERT_EQ(errno, EINVAL);
+
+	/* Bad command */
+	ret = set_robust_list2(&head, 0, FUTEX_ROBUST_LIST_CMD_USER_MAX, 0);
+	ASSERT_EQ(ret, -1);
+	ASSERT_EQ(errno, EINVAL);
+
+	ret = set_robust_list2(&head, 0, -1, 0);
+	ASSERT_EQ(ret, -1);
+	ASSERT_EQ(errno, EINVAL);
+}
+
 /*
  * Test get_robust_list with pid = 0, getting the list of the running thread
  */
@@ -363,7 +481,7 @@ static int child_fn_lock_with_error(void *arg)
 	struct robust_list_head head;
 	int ret;
 
-	ret = set_list(&head);
+	ret = set_list(&head, false, 0);
 	if (ret) {
 		ksft_test_result_fail("set_robust_list error\n");
 		return -1;
@@ -388,14 +506,16 @@ static int child_fn_lock_with_error(void *arg)
  * earlier, just after setting list_op_pending and taking the lock, to test the
  * list_op_pending mechanism
  */
-TEST(test_set_list_op_pending)
+TEST_F(robust_api, test_set_list_op_pending)
 {
 	struct lock_struct lock = { .futex = 0 };
 	_Atomic(unsigned int) *futex = &lock.futex;
-	struct robust_list_head head;
 	int ret, wstatus;
+	struct robust_list_head head;
+
+	lock.robust2 = variant->robust2;
 
-	ret = set_list(&head);
+	ret = set_list(&head, lock.robust2, 0);
 	ASSERT_EQ(ret, 0);
 
 	ret = pthread_barrier_init(&barrier, NULL, 2);
@@ -429,7 +549,7 @@ static int child_lock_holder(void *arg)
 	struct robust_list_head head;
 	int i;
 
-	set_list(&head);
+	set_list(&head, locks[0].robust2, 0);
 
 	for (i = 0; i < CHILD_NR; i++) {
 		locks[i].futex = 0;
@@ -471,12 +591,19 @@ static int child_wait_lock(void *arg)
  * Test a robust list of more than one element. All the waiters should wake when
  * the holder dies
  */
-TEST(test_robust_list_multiple_elements)
+TEST_F(robust_api, test_robust_list_multiple_elements)
 {
 	struct lock_struct locks[CHILD_NR];
 	pid_t pids[CHILD_NR + 1];
 	int i, ret, wstatus;
 
+	if (!robust_list2_support()) {
+		ksft_test_result_skip("robust_list2 not supported\n");
+		return;
+	}
+
+	locks[0].robust2 = variant->robust2;
+
 	ret = pthread_barrier_init(&barrier, NULL, 2);
 	ASSERT_EQ(ret, 0);
 	ret = pthread_barrier_init(&barrier2, NULL, CHILD_NR + 1);
@@ -507,13 +634,98 @@ TEST(test_robust_list_multiple_elements)
 		ksft_test_result_pass("%s\n", __func__);
 }
 
+static int child_lock_holder_multiple_lists(void *arg)
+{
+	struct lock_struct *locks = arg;
+	struct robust_list_head *heads;
+	int i, list_limit;
+
+	list_limit = set_robust_list2(NULL, 0, FUTEX_ROBUST_LIST_CMD_LIST_LIMIT, 0);
+
+	heads = malloc(list_limit * sizeof(*heads));
+	if (!heads)
+		return -1;
+
+	for (i = 0; i < list_limit; i++) {
+		set_list(&heads[i], true, i);
+		locks[i].futex = 0;
+		mutex_lock(&locks[i], &heads[i], false);
+	}
+
+	pthread_barrier_wait(&barrier);
+	pthread_barrier_wait(&barrier2);
+
+	/* See comment at child_fn_lock() */
+	usleep(SLEEP_US);
+
+	return 0;
+}
+
+/*
+ * Similar to test_robust_list_multiple_elements, but instead of one list with
+ * several elements, create several lists with one element.
+ */
+TEST(test_robust_list_multiple_lists)
+{
+	int i, ret, wstatus, list_limit;
+	struct lock_struct *locks;
+	pid_t *pids;
+
+	if (!robust_list2_support()) {
+		ksft_test_result_skip("robust_list2 not supported\n");
+		return;
+	}
+
+	list_limit = set_robust_list2(NULL, 0, FUTEX_ROBUST_LIST_CMD_LIST_LIMIT, 0);
+	ASSERT_GT(list_limit, 1);
+
+	locks = malloc(list_limit * sizeof(*locks));
+	ASSERT_NE(locks, NULL);
+
+	pids = malloc(list_limit * sizeof(*pids));
+	ASSERT_NE(pids, NULL);
+
+	ret = pthread_barrier_init(&barrier, NULL, 2);
+	ASSERT_EQ(ret, 0);
+	ret = pthread_barrier_init(&barrier2, NULL, list_limit + 1);
+	ASSERT_EQ(ret, 0);
+
+	pids[0] = create_child(&child_lock_holder_multiple_lists, locks);
+
+	/* Wait until the locker thread takes the look */
+	pthread_barrier_wait(&barrier);
+
+	for (i = 0; i < list_limit; i++)
+		pids[i+1] = create_child(&child_wait_lock, &locks[i]);
+
+	/* Wait for all children to return */
+	ret = 0;
+
+	for (i = 0; i < list_limit; i++) {
+		waitpid(pids[i], &wstatus, 0);
+		if (WEXITSTATUS(wstatus))
+			ret = -1;
+	}
+
+	pthread_barrier_destroy(&barrier);
+	pthread_barrier_destroy(&barrier2);
+
+	/* Pass only if the child hasn't return error */
+	if (!ret)
+		ksft_test_result_pass("%s\n", __func__);
+
+	free(locks);
+	free(pids);
+}
+
 static int child_circular_list(void *arg)
 {
-	static struct robust_list_head head;
+	struct robust_list_head head;
 	struct lock_struct a, b, c;
+	bool robust2 = *(bool *) arg;
 	int ret;
 
-	ret = set_list(&head);
+	ret = set_list(&head, robust2, 0);
 	if (ret) {
 		ksft_test_result_fail("set_list error\n");
 		return -1;
@@ -536,11 +748,12 @@ static int child_circular_list(void *arg)
  * while processing it so it won't be trapped in an infinite loop while handling
  * a process exit
  */
-TEST(test_circular_list)
+TEST_F(robust_api, test_circular_list)
 {
 	int wstatus;
+	bool robust2 = variant->robust2;
 
-	create_child(child_circular_list, NULL);
+	create_child(child_circular_list, &robust2);
 
 	wait(&wstatus);
 
@@ -549,4 +762,156 @@ TEST(test_circular_list)
 		ksft_test_result_pass("%s\n", __func__);
 }
 
+/*
+ * 32-bit version of child_lock_holder. 
+ */
+static int child_lock_holder32(void *arg)
+{
+	struct lock_struct32 *locks = arg;
+	struct robust_list_head32 *head;
+	pid_t tid = gettid();
+	int i, ret;
+
+	head = mmap((void *)0x10000, sizeof(*head), PROT_READ | PROT_WRITE,
+		    MAP_FIXED_NOREPLACE | MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+
+	if (!head || ((uint32_t)(uintptr_t) head) > 0x7FFFFFFF) {
+		ksft_test_result_fail("child_lock_holder32 error\n");
+		return -1;
+	}
+
+	head->futex_offset = (uint32_t) ((size_t) offsetof(struct lock_struct32, futex) -
+			     (size_t) offsetof(struct lock_struct32, list));
+	head->list.next = (uint32_t)(uintptr_t) &head->list;
+	head->list_op_pending = (uint32_t)(uintptr_t) NULL;
+
+	ret = set_robust_list2((struct robust_list_head *) head, 0,
+			       FUTEX_ROBUST_LIST_CMD_SET_32, 0);
+	if (ret) {
+		ksft_test_result_fail("set_robust_list2 error\n");
+		return -1;
+	}
+
+	/*
+	 * Take all the locks and insert them in the list
+	 */
+	for (i = 0; i < CHILD_NR; i++) {
+		struct robust_list32 *list = &head->list;
+
+		locks[i].futex = tid;
+
+		while (list->next != (uint32_t)(uintptr_t) &head->list)
+			list = (struct robust_list32 *)(uintptr_t) list->next;
+
+		list->next = (uint32_t)(uintptr_t) &locks[i].list;
+		locks[i].list.next = (uint32_t)(uintptr_t) &head->list;
+	}
+
+	pthread_barrier_wait(&barrier);
+	pthread_barrier_wait(&barrier2);
+
+	/* See comment at child_fn_lock() */
+	usleep(SLEEP_US);
+
+	/* Exit holding all the locks */
+	return 0;
+}
+
+static int child_wait_lock32(void *arg)
+{
+	struct lock_struct32 *lock = arg;
+	_Atomic(unsigned int) *futex;
+	struct timespec to;
+	pid_t tid;
+	int ret;
+
+	futex = &lock->futex;
+
+	pthread_barrier_wait(&barrier2);
+
+	to.tv_sec = FUTEX_TIMEOUT;
+	to.tv_nsec = 0;
+
+	tid = atomic_load(futex);
+
+	/* Kernel ignores futexes without the waiters flag */
+	tid |= FUTEX_WAITERS;
+	atomic_store(futex, tid);
+
+	ret = futex_wait((futex_t *) futex, tid, &to, 0);
+
+	if (ret) {
+		ksft_test_result_fail("futex_wait error\n");
+		return -1;
+	}
+
+	if (!(lock->futex & FUTEX_OWNER_DIED)) {
+		ksft_test_result_fail("futex not marked with FUTEX_OWNER_DIED\n");
+		return -1;
+	}
+
+	return 0;
+}
+
+/*
+ * Test to create a 32-bit robust list in a 64-bit kernel. Replicate
+ * test_robust_list_multiple_elements, but it's simplified: don't do all the
+ * mutex lock dance, just insert futexes in the list and check if the kernel
+ * correctly walks the list and wake the threads
+ */
+TEST(test_32bit_lists)
+{
+	struct lock_struct32 *locks;
+	pid_t pids[CHILD_NR + 1];
+	int i, ret, wstatus;
+
+	if (sizeof(uintptr_t) != 8) {
+		ksft_test_result_skip("Test only for 64-bit\n");
+		return;
+	}
+
+	if (!robust_list2_support()) {
+		ksft_test_result_skip("robust_list2 not supported\n");
+		return;
+	}
+
+	locks = mmap((void *)0x20000, sizeof(*locks) * CHILD_NR,
+		     PROT_READ | PROT_WRITE, MAP_FIXED_NOREPLACE | MAP_PRIVATE | MAP_ANONYMOUS,
+		     -1, 0);
+
+	ASSERT_NE(locks, NULL);
+	ASSERT_LT((uintptr_t) locks, 0x7FFFFFFF);
+
+	ret = pthread_barrier_init(&barrier, NULL, 2);
+	ASSERT_EQ(ret, 0);
+	ret = pthread_barrier_init(&barrier2, NULL, CHILD_NR + 1);
+	ASSERT_EQ(ret, 0);
+
+	pids[0] = create_child(&child_lock_holder32, locks);
+
+	/* Wait until the locker thread takes the look */
+	pthread_barrier_wait(&barrier);
+
+	for (i = 0; i < CHILD_NR; i++)
+		pids[i+1] = create_child(&child_wait_lock32, &locks[i]);
+
+	/* Wait for all children to return */
+	ret = 0;
+
+	for (i = 0; i < CHILD_NR; i++) {
+		waitpid(pids[i], &wstatus, 0);
+		if (WEXITSTATUS(wstatus))
+			ret = -1;
+	}
+
+	pthread_barrier_destroy(&barrier);
+	pthread_barrier_destroy(&barrier2);
+
+	/* Pass only if the child hasn't return error */
+	if (!ret)
+		ksft_test_result_pass("%s\n", __func__);
+
+	munmap(locks, sizeof(*locks) * CHILD_NR);
+}
+
 TEST_HARNESS_MAIN

-- 
2.52.0


^ permalink raw reply related

* [PATCH v6 6/9] futex: Wire up get_robust_list2 syscall
From: André Almeida @ 2025-11-22  5:50 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Peter Zijlstra, Darren Hart,
	Davidlohr Bueso, Arnd Bergmann, Sebastian Andrzej Siewior,
	Waiman Long, Ryan Houdek
  Cc: linux-kernel, linux-kselftest, linux-api, kernel-dev,
	André Almeida
In-Reply-To: <20251122-tonyk-robust_futex-v6-0-05fea005a0fd@igalia.com>

Wire up the new get_robust_list2 syscall in all available architectures.

Signed-off-by: André Almeida <andrealmeid@igalia.com>
---
 arch/alpha/kernel/syscalls/syscall.tbl      | 1 +
 arch/arm/tools/syscall.tbl                  | 1 +
 arch/m68k/kernel/syscalls/syscall.tbl       | 1 +
 arch/microblaze/kernel/syscalls/syscall.tbl | 1 +
 arch/mips/kernel/syscalls/syscall_n32.tbl   | 1 +
 arch/mips/kernel/syscalls/syscall_n64.tbl   | 1 +
 arch/mips/kernel/syscalls/syscall_o32.tbl   | 1 +
 arch/parisc/kernel/syscalls/syscall.tbl     | 1 +
 arch/powerpc/kernel/syscalls/syscall.tbl    | 1 +
 arch/s390/kernel/syscalls/syscall.tbl       | 1 +
 arch/sh/kernel/syscalls/syscall.tbl         | 1 +
 arch/sparc/kernel/syscalls/syscall.tbl      | 1 +
 arch/x86/entry/syscalls/syscall_32.tbl      | 1 +
 arch/x86/entry/syscalls/syscall_64.tbl      | 1 +
 arch/xtensa/kernel/syscalls/syscall.tbl     | 1 +
 include/uapi/asm-generic/unistd.h           | 4 +++-
 kernel/sys_ni.c                             | 1 +
 17 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/arch/alpha/kernel/syscalls/syscall.tbl b/arch/alpha/kernel/syscalls/syscall.tbl
index d0cb7b902cc6..b4a42beda6db 100644
--- a/arch/alpha/kernel/syscalls/syscall.tbl
+++ b/arch/alpha/kernel/syscalls/syscall.tbl
@@ -510,3 +510,4 @@
 578	common	file_getattr			sys_file_getattr
 579	common	file_setattr			sys_file_setattr
 580	common	set_robust_list2		sys_set_robust_list2
+581	common	get_robust_list2		sys_get_robust_list2
diff --git a/arch/arm/tools/syscall.tbl b/arch/arm/tools/syscall.tbl
index 910e6e14ccf0..d4a4d8446cb0 100644
--- a/arch/arm/tools/syscall.tbl
+++ b/arch/arm/tools/syscall.tbl
@@ -485,3 +485,4 @@
 468	common	file_getattr			sys_file_getattr
 469	common	file_setattr			sys_file_setattr
 470	common	set_robust_list2		sys_set_robust_list2
+471	common	get_robust_list2		sys_get_robust_list2
diff --git a/arch/m68k/kernel/syscalls/syscall.tbl b/arch/m68k/kernel/syscalls/syscall.tbl
index eee3f320483d..c2f1c5a3313c 100644
--- a/arch/m68k/kernel/syscalls/syscall.tbl
+++ b/arch/m68k/kernel/syscalls/syscall.tbl
@@ -470,3 +470,4 @@
 468	common	file_getattr			sys_file_getattr
 469	common	file_setattr			sys_file_setattr
 470	common  set_robust_list2		sys_set_robust_list2
+471	common  get_robust_list2		sys_get_robust_list2
diff --git a/arch/microblaze/kernel/syscalls/syscall.tbl b/arch/microblaze/kernel/syscalls/syscall.tbl
index 6c69d8ebbc38..1389dd194eec 100644
--- a/arch/microblaze/kernel/syscalls/syscall.tbl
+++ b/arch/microblaze/kernel/syscalls/syscall.tbl
@@ -476,3 +476,4 @@
 468	common	file_getattr			sys_file_getattr
 469	common	file_setattr			sys_file_setattr
 470	common	set_robust_list2		sys_set_robust_list2
+471	common	get_robust_list2		sys_get_robust_list2
diff --git a/arch/mips/kernel/syscalls/syscall_n32.tbl b/arch/mips/kernel/syscalls/syscall_n32.tbl
index f70db3741b0e..e149d2ddbc2f 100644
--- a/arch/mips/kernel/syscalls/syscall_n32.tbl
+++ b/arch/mips/kernel/syscalls/syscall_n32.tbl
@@ -409,3 +409,4 @@
 468	n32	file_getattr			sys_file_getattr
 469	n32	file_setattr			sys_file_setattr
 470	n32	set_robust_list2		sys_set_robust_list2
+471	n32	get_robust_list2		sys_get_robust_list2
diff --git a/arch/mips/kernel/syscalls/syscall_n64.tbl b/arch/mips/kernel/syscalls/syscall_n64.tbl
index 9480488f9495..7ddddc89a751 100644
--- a/arch/mips/kernel/syscalls/syscall_n64.tbl
+++ b/arch/mips/kernel/syscalls/syscall_n64.tbl
@@ -385,3 +385,4 @@
 468	n64	file_getattr			sys_file_getattr
 469	n64	file_setattr			sys_file_setattr
 470	n64	set_robust_list2		sys_set_robust_list2
+471	n64	get_robust_list2		sys_get_robust_list2
diff --git a/arch/mips/kernel/syscalls/syscall_o32.tbl b/arch/mips/kernel/syscalls/syscall_o32.tbl
index 2761c9cd8946..c0a5ebafed1a 100644
--- a/arch/mips/kernel/syscalls/syscall_o32.tbl
+++ b/arch/mips/kernel/syscalls/syscall_o32.tbl
@@ -458,3 +458,4 @@
 468	o32	file_getattr			sys_file_getattr
 469	o32	file_setattr			sys_file_setattr
 470	o32	set_robust_list2		sys_set_robust_list2
+471	o32	get_robust_list2		sys_get_robust_list2
diff --git a/arch/parisc/kernel/syscalls/syscall.tbl b/arch/parisc/kernel/syscalls/syscall.tbl
index eb37fda5c48f..4c6cb64ec113 100644
--- a/arch/parisc/kernel/syscalls/syscall.tbl
+++ b/arch/parisc/kernel/syscalls/syscall.tbl
@@ -469,3 +469,4 @@
 468	common	file_getattr			sys_file_getattr
 469	common	file_setattr			sys_file_setattr
 470	common	set_robust_list2		sys_set_robust_list2
+471	common	get_robust_list2		sys_get_robust_list2
diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
index 472bebec449d..1475fa6b3ee3 100644
--- a/arch/powerpc/kernel/syscalls/syscall.tbl
+++ b/arch/powerpc/kernel/syscalls/syscall.tbl
@@ -561,3 +561,4 @@
 468	common	file_getattr			sys_file_getattr
 469	common	file_setattr			sys_file_setattr
 470	common	set_robust_list2		sys_set_robust_list2
+470	common	get_robust_list2		sys_get_robust_list2
diff --git a/arch/s390/kernel/syscalls/syscall.tbl b/arch/s390/kernel/syscalls/syscall.tbl
index ba7fac304941..b8161ee922ef 100644
--- a/arch/s390/kernel/syscalls/syscall.tbl
+++ b/arch/s390/kernel/syscalls/syscall.tbl
@@ -473,3 +473,4 @@
 468  common	file_getattr		sys_file_getattr		sys_file_getattr
 469  common	file_setattr		sys_file_setattr		sys_file_setattr
 470  common	set_robust_list2	sys_set_robust_list2		sys_set_robust_list2
+471  common	get_robust_list2	sys_get_robust_list2		sys_get_robust_list2
diff --git a/arch/sh/kernel/syscalls/syscall.tbl b/arch/sh/kernel/syscalls/syscall.tbl
index c05c94a742be..566baa152634 100644
--- a/arch/sh/kernel/syscalls/syscall.tbl
+++ b/arch/sh/kernel/syscalls/syscall.tbl
@@ -474,3 +474,4 @@
 468	common	file_getattr			sys_file_getattr
 469	common	file_setattr			sys_file_setattr
 470	common	set_robust_list2		sys_set_robust_list2
+471	common	get_robust_list2		sys_get_robust_list2
diff --git a/arch/sparc/kernel/syscalls/syscall.tbl b/arch/sparc/kernel/syscalls/syscall.tbl
index 3a59f3008325..fb3844c17711 100644
--- a/arch/sparc/kernel/syscalls/syscall.tbl
+++ b/arch/sparc/kernel/syscalls/syscall.tbl
@@ -516,3 +516,4 @@
 468	common	file_getattr			sys_file_getattr
 469	common	file_setattr			sys_file_setattr
 470	common	set_robust_list2		sys_set_robust_list2
+471	common	get_robust_list2		sys_get_robust_list2
diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
index e9d6e1a1d777..0df93458ef37 100644
--- a/arch/x86/entry/syscalls/syscall_32.tbl
+++ b/arch/x86/entry/syscalls/syscall_32.tbl
@@ -476,3 +476,4 @@
 468	i386	file_getattr		sys_file_getattr
 469	i386	file_setattr		sys_file_setattr
 470	i386	set_robust_list2	sys_set_robust_list2
+471	i386	get_robust_list2	sys_get_robust_list2
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index 8fdcf090300d..e7fdcc3d6e52 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -395,6 +395,7 @@
 468	common	file_getattr		sys_file_getattr
 469	common	file_setattr		sys_file_setattr
 470	common	set_robust_list2	sys_set_robust_list2
+471	common	get_robust_list2	sys_get_robust_list2
 
 #
 # Due to a historical design error, certain syscalls are numbered differently
diff --git a/arch/xtensa/kernel/syscalls/syscall.tbl b/arch/xtensa/kernel/syscalls/syscall.tbl
index d7bb6b9104dd..bd63dbc78c0e 100644
--- a/arch/xtensa/kernel/syscalls/syscall.tbl
+++ b/arch/xtensa/kernel/syscalls/syscall.tbl
@@ -441,3 +441,4 @@
 468	common	file_getattr			sys_file_getattr
 469	common	file_setattr			sys_file_setattr
 470	common	set_robust_list2		sys_set_robust_list2
+471	common	get_robust_list2		sys_get_robust_list2
diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index 44fc87287983..9539e893c9ac 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -860,9 +860,11 @@ __SYSCALL(__NR_file_setattr, sys_file_setattr)
 
 #define __NR_set_robust_list2 470
 __SYSCALL(__NR_set_robust_list2, sys_set_robust_list2)
+#define __NR_get_robust_list2 471
+__SYSCALL(__NR_get_robust_list2, sys_get_robust_list2)
 
 #undef __NR_syscalls
-#define __NR_syscalls 471
+#define __NR_syscalls 472
 
 /*
  * 32 bit systems traditionally used different
diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
index 0ca2cfe69b11..0a7f7634446c 100644
--- a/kernel/sys_ni.c
+++ b/kernel/sys_ni.c
@@ -173,6 +173,7 @@ COND_SYSCALL(lsm_get_self_attr);
 COND_SYSCALL(lsm_set_self_attr);
 COND_SYSCALL(lsm_list_modules);
 COND_SYSCALL(set_robust_list2);
+COND_SYSCALL(get_robust_list2);
 
 /* CONFIG_MMU only */
 COND_SYSCALL(swapon);

-- 
2.52.0


^ permalink raw reply related

* [PATCH v6 5/9] futex: Wire up set_robust_list2 syscall
From: André Almeida @ 2025-11-22  5:50 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Peter Zijlstra, Darren Hart,
	Davidlohr Bueso, Arnd Bergmann, Sebastian Andrzej Siewior,
	Waiman Long, Ryan Houdek
  Cc: linux-kernel, linux-kselftest, linux-api, kernel-dev,
	André Almeida
In-Reply-To: <20251122-tonyk-robust_futex-v6-0-05fea005a0fd@igalia.com>

Wire up the new set_robust_list2 syscall in all available architectures.

Signed-off-by: André Almeida <andrealmeid@igalia.com>
---
 arch/alpha/kernel/syscalls/syscall.tbl      | 1 +
 arch/arm/tools/syscall.tbl                  | 1 +
 arch/m68k/kernel/syscalls/syscall.tbl       | 1 +
 arch/microblaze/kernel/syscalls/syscall.tbl | 1 +
 arch/mips/kernel/syscalls/syscall_n32.tbl   | 1 +
 arch/mips/kernel/syscalls/syscall_n64.tbl   | 1 +
 arch/mips/kernel/syscalls/syscall_o32.tbl   | 1 +
 arch/parisc/kernel/syscalls/syscall.tbl     | 1 +
 arch/powerpc/kernel/syscalls/syscall.tbl    | 1 +
 arch/s390/kernel/syscalls/syscall.tbl       | 1 +
 arch/sh/kernel/syscalls/syscall.tbl         | 1 +
 arch/sparc/kernel/syscalls/syscall.tbl      | 1 +
 arch/x86/entry/syscalls/syscall_32.tbl      | 1 +
 arch/x86/entry/syscalls/syscall_64.tbl      | 1 +
 arch/xtensa/kernel/syscalls/syscall.tbl     | 1 +
 include/uapi/asm-generic/unistd.h           | 5 ++++-
 kernel/sys_ni.c                             | 1 +
 scripts/syscall.tbl                         | 1 +
 18 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/arch/alpha/kernel/syscalls/syscall.tbl b/arch/alpha/kernel/syscalls/syscall.tbl
index 16dca28ebf17..d0cb7b902cc6 100644
--- a/arch/alpha/kernel/syscalls/syscall.tbl
+++ b/arch/alpha/kernel/syscalls/syscall.tbl
@@ -509,3 +509,4 @@
 577	common	open_tree_attr			sys_open_tree_attr
 578	common	file_getattr			sys_file_getattr
 579	common	file_setattr			sys_file_setattr
+580	common	set_robust_list2		sys_set_robust_list2
diff --git a/arch/arm/tools/syscall.tbl b/arch/arm/tools/syscall.tbl
index b07e699aaa3c..910e6e14ccf0 100644
--- a/arch/arm/tools/syscall.tbl
+++ b/arch/arm/tools/syscall.tbl
@@ -484,3 +484,4 @@
 467	common	open_tree_attr			sys_open_tree_attr
 468	common	file_getattr			sys_file_getattr
 469	common	file_setattr			sys_file_setattr
+470	common	set_robust_list2		sys_set_robust_list2
diff --git a/arch/m68k/kernel/syscalls/syscall.tbl b/arch/m68k/kernel/syscalls/syscall.tbl
index f41d38dfbf13..eee3f320483d 100644
--- a/arch/m68k/kernel/syscalls/syscall.tbl
+++ b/arch/m68k/kernel/syscalls/syscall.tbl
@@ -469,3 +469,4 @@
 467	common	open_tree_attr			sys_open_tree_attr
 468	common	file_getattr			sys_file_getattr
 469	common	file_setattr			sys_file_setattr
+470	common  set_robust_list2		sys_set_robust_list2
diff --git a/arch/microblaze/kernel/syscalls/syscall.tbl b/arch/microblaze/kernel/syscalls/syscall.tbl
index 580af574fe73..6c69d8ebbc38 100644
--- a/arch/microblaze/kernel/syscalls/syscall.tbl
+++ b/arch/microblaze/kernel/syscalls/syscall.tbl
@@ -475,3 +475,4 @@
 467	common	open_tree_attr			sys_open_tree_attr
 468	common	file_getattr			sys_file_getattr
 469	common	file_setattr			sys_file_setattr
+470	common	set_robust_list2		sys_set_robust_list2
diff --git a/arch/mips/kernel/syscalls/syscall_n32.tbl b/arch/mips/kernel/syscalls/syscall_n32.tbl
index d824ffe9a014..f70db3741b0e 100644
--- a/arch/mips/kernel/syscalls/syscall_n32.tbl
+++ b/arch/mips/kernel/syscalls/syscall_n32.tbl
@@ -408,3 +408,4 @@
 467	n32	open_tree_attr			sys_open_tree_attr
 468	n32	file_getattr			sys_file_getattr
 469	n32	file_setattr			sys_file_setattr
+470	n32	set_robust_list2		sys_set_robust_list2
diff --git a/arch/mips/kernel/syscalls/syscall_n64.tbl b/arch/mips/kernel/syscalls/syscall_n64.tbl
index 7a7049c2c307..9480488f9495 100644
--- a/arch/mips/kernel/syscalls/syscall_n64.tbl
+++ b/arch/mips/kernel/syscalls/syscall_n64.tbl
@@ -384,3 +384,4 @@
 467	n64	open_tree_attr			sys_open_tree_attr
 468	n64	file_getattr			sys_file_getattr
 469	n64	file_setattr			sys_file_setattr
+470	n64	set_robust_list2		sys_set_robust_list2
diff --git a/arch/mips/kernel/syscalls/syscall_o32.tbl b/arch/mips/kernel/syscalls/syscall_o32.tbl
index d330274f0601..2761c9cd8946 100644
--- a/arch/mips/kernel/syscalls/syscall_o32.tbl
+++ b/arch/mips/kernel/syscalls/syscall_o32.tbl
@@ -457,3 +457,4 @@
 467	o32	open_tree_attr			sys_open_tree_attr
 468	o32	file_getattr			sys_file_getattr
 469	o32	file_setattr			sys_file_setattr
+470	o32	set_robust_list2		sys_set_robust_list2
diff --git a/arch/parisc/kernel/syscalls/syscall.tbl b/arch/parisc/kernel/syscalls/syscall.tbl
index 88a788a7b18d..eb37fda5c48f 100644
--- a/arch/parisc/kernel/syscalls/syscall.tbl
+++ b/arch/parisc/kernel/syscalls/syscall.tbl
@@ -468,3 +468,4 @@
 467	common	open_tree_attr			sys_open_tree_attr
 468	common	file_getattr			sys_file_getattr
 469	common	file_setattr			sys_file_setattr
+470	common	set_robust_list2		sys_set_robust_list2
diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
index b453e80dfc00..472bebec449d 100644
--- a/arch/powerpc/kernel/syscalls/syscall.tbl
+++ b/arch/powerpc/kernel/syscalls/syscall.tbl
@@ -560,3 +560,4 @@
 467	common	open_tree_attr			sys_open_tree_attr
 468	common	file_getattr			sys_file_getattr
 469	common	file_setattr			sys_file_setattr
+470	common	set_robust_list2		sys_set_robust_list2
diff --git a/arch/s390/kernel/syscalls/syscall.tbl b/arch/s390/kernel/syscalls/syscall.tbl
index 8a6744d658db..ba7fac304941 100644
--- a/arch/s390/kernel/syscalls/syscall.tbl
+++ b/arch/s390/kernel/syscalls/syscall.tbl
@@ -472,3 +472,4 @@
 467  common	open_tree_attr		sys_open_tree_attr		sys_open_tree_attr
 468  common	file_getattr		sys_file_getattr		sys_file_getattr
 469  common	file_setattr		sys_file_setattr		sys_file_setattr
+470  common	set_robust_list2	sys_set_robust_list2		sys_set_robust_list2
diff --git a/arch/sh/kernel/syscalls/syscall.tbl b/arch/sh/kernel/syscalls/syscall.tbl
index 5e9c9eff5539..c05c94a742be 100644
--- a/arch/sh/kernel/syscalls/syscall.tbl
+++ b/arch/sh/kernel/syscalls/syscall.tbl
@@ -473,3 +473,4 @@
 467	common	open_tree_attr			sys_open_tree_attr
 468	common	file_getattr			sys_file_getattr
 469	common	file_setattr			sys_file_setattr
+470	common	set_robust_list2		sys_set_robust_list2
diff --git a/arch/sparc/kernel/syscalls/syscall.tbl b/arch/sparc/kernel/syscalls/syscall.tbl
index ebb7d06d1044..3a59f3008325 100644
--- a/arch/sparc/kernel/syscalls/syscall.tbl
+++ b/arch/sparc/kernel/syscalls/syscall.tbl
@@ -515,3 +515,4 @@
 467	common	open_tree_attr			sys_open_tree_attr
 468	common	file_getattr			sys_file_getattr
 469	common	file_setattr			sys_file_setattr
+470	common	set_robust_list2		sys_set_robust_list2
diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
index 4877e16da69a..e9d6e1a1d777 100644
--- a/arch/x86/entry/syscalls/syscall_32.tbl
+++ b/arch/x86/entry/syscalls/syscall_32.tbl
@@ -475,3 +475,4 @@
 467	i386	open_tree_attr		sys_open_tree_attr
 468	i386	file_getattr		sys_file_getattr
 469	i386	file_setattr		sys_file_setattr
+470	i386	set_robust_list2	sys_set_robust_list2
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index ced2a1deecd7..8fdcf090300d 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -394,6 +394,7 @@
 467	common	open_tree_attr		sys_open_tree_attr
 468	common	file_getattr		sys_file_getattr
 469	common	file_setattr		sys_file_setattr
+470	common	set_robust_list2	sys_set_robust_list2
 
 #
 # Due to a historical design error, certain syscalls are numbered differently
diff --git a/arch/xtensa/kernel/syscalls/syscall.tbl b/arch/xtensa/kernel/syscalls/syscall.tbl
index 374e4cb788d8..d7bb6b9104dd 100644
--- a/arch/xtensa/kernel/syscalls/syscall.tbl
+++ b/arch/xtensa/kernel/syscalls/syscall.tbl
@@ -440,3 +440,4 @@
 467	common	open_tree_attr			sys_open_tree_attr
 468	common	file_getattr			sys_file_getattr
 469	common	file_setattr			sys_file_setattr
+470	common	set_robust_list2		sys_set_robust_list2
diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index 04e0077fb4c9..44fc87287983 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -858,8 +858,11 @@ __SYSCALL(__NR_file_getattr, sys_file_getattr)
 #define __NR_file_setattr 469
 __SYSCALL(__NR_file_setattr, sys_file_setattr)
 
+#define __NR_set_robust_list2 470
+__SYSCALL(__NR_set_robust_list2, sys_set_robust_list2)
+
 #undef __NR_syscalls
-#define __NR_syscalls 470
+#define __NR_syscalls 471
 
 /*
  * 32 bit systems traditionally used different
diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
index bf5d05c635ff..0ca2cfe69b11 100644
--- a/kernel/sys_ni.c
+++ b/kernel/sys_ni.c
@@ -172,6 +172,7 @@ COND_SYSCALL_COMPAT(fadvise64_64);
 COND_SYSCALL(lsm_get_self_attr);
 COND_SYSCALL(lsm_set_self_attr);
 COND_SYSCALL(lsm_list_modules);
+COND_SYSCALL(set_robust_list2);
 
 /* CONFIG_MMU only */
 COND_SYSCALL(swapon);
diff --git a/scripts/syscall.tbl b/scripts/syscall.tbl
index d1ae5e92c615..58c334aa8922 100644
--- a/scripts/syscall.tbl
+++ b/scripts/syscall.tbl
@@ -410,3 +410,4 @@
 467	common	open_tree_attr			sys_open_tree_attr
 468	common	file_getattr			sys_file_getattr
 469	common	file_setattr			sys_file_setattr
+470	common	set_robust_list2		sys_set_robust_list2

-- 
2.52.0


^ permalink raw reply related

* [PATCH v6 4/9] futex: Create get_robust_list2() syscall
From: André Almeida @ 2025-11-22  5:50 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Peter Zijlstra, Darren Hart,
	Davidlohr Bueso, Arnd Bergmann, Sebastian Andrzej Siewior,
	Waiman Long, Ryan Houdek
  Cc: linux-kernel, linux-kselftest, linux-api, kernel-dev,
	André Almeida
In-Reply-To: <20251122-tonyk-robust_futex-v6-0-05fea005a0fd@igalia.com>

As in the original robust list interface, to pair with
set_robust_list2(), create a get_robust_list2() syscall with the
following signature:

  get_robust_list2(int pid, void __user **head_ptr, unsigned int index,
		   unsigned int flags)

  - `pid` sets with task's list should be returned. If is 0, it gets the
    list of the calling task.
  - `index` is the index of the list to get
  - `flags` is unused but can be used for expanding the interface

Signed-off-by: André Almeida <andrealmeid@igalia.com>
---
For some reason I wasn't able to use put_user() for 32-bit lists.. it
kept corrupting the value due to wrong write size I believe.
copy_to_user() worked fine nonetheless.
---
 kernel/futex/syscalls.c | 61 +++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 57 insertions(+), 4 deletions(-)

diff --git a/kernel/futex/syscalls.c b/kernel/futex/syscalls.c
index 0b7fa88aa34c..f730d16632fc 100644
--- a/kernel/futex/syscalls.c
+++ b/kernel/futex/syscalls.c
@@ -48,7 +48,7 @@ static inline void __user *futex_task_robust_list(struct task_struct *p, bool co
 	return p->robust_list;
 }
 
-static void __user *futex_get_robust_list_common(int pid, bool compat)
+static void __user *futex_get_robust_list_common(int pid, bool compat, int index)
 {
 	struct task_struct *p = current;
 	void __user *head;
@@ -75,7 +75,15 @@ static void __user *futex_get_robust_list_common(int pid, bool compat)
 	if (!ptrace_may_access(p, PTRACE_MODE_READ_REALCREDS))
 		goto err_unlock;
 
-	head = futex_task_robust_list(p, compat);
+	if (index >= 0) {
+		scoped_guard(mutex, &p->futex_exit_mutex) {
+			uintptr_t *rl = p->futex_robust_lists;
+
+			head = rl ? (void __user *) rl[index] : NULL;
+		}
+	} else {
+		head = futex_task_robust_list(p, compat);
+	}
 
 	up_read(&p->signal->exec_update_lock);
 	put_task_struct(p);
@@ -99,7 +107,7 @@ SYSCALL_DEFINE3(get_robust_list, int, pid,
 		struct robust_list_head __user * __user *, head_ptr,
 		size_t __user *, len_ptr)
 {
-	struct robust_list_head __user *head = futex_get_robust_list_common(pid, false);
+	struct robust_list_head __user *head = futex_get_robust_list_common(pid, false, -1);
 
 	if (IS_ERR(head))
 		return PTR_ERR(head);
@@ -150,6 +158,51 @@ SYSCALL_DEFINE4(set_robust_list2, struct robust_list_head *, head, unsigned int,
 	return -EINVAL;
 }
 
+SYSCALL_DEFINE4(get_robust_list2, int, pid,
+		void __user * __user *, head_ptr,
+		unsigned int, index, unsigned int, flags)
+{
+	void __user *entry_ptr;
+	uintptr_t entry;
+
+	if (index >= FUTEX_ROBUST_LISTS_PER_USER)
+		return -EINVAL;
+
+	if (flags)
+		return -EINVAL;
+
+	/*
+	 * The first two indexes are reserved for the kernel to be used with the
+	 * legacy syscall, so we hide them from userspace.
+	 *
+	 * We map [0, FUTEX_ROBUST_LISTS_PER_USER) to
+	 *  [FUTEX_ROBUST_LIST2_IDX, FUTEX_ROBUST_LIST2_MAX_IDX)
+	 */
+	index += FUTEX_ROBUST_LIST2_IDX;
+
+	entry_ptr = futex_get_robust_list_common(pid, false, index);
+	if (IS_ERR(entry_ptr))
+		return PTR_ERR(entry_ptr);
+
+	entry = (uintptr_t) entry_ptr;
+
+	if (entry & FUTEX_ROBUST_LIST_ENTRY_32BIT) {
+		entry &= FUTEX_ROBUST_LIST_ENTRY_MASK;
+
+		if (copy_to_user(head_ptr, &entry, sizeof(u32)))
+			return -EFAULT;
+
+		return 0;
+	} else {
+		struct robust_list_head *head;
+
+		entry &= FUTEX_ROBUST_LIST_ENTRY_MASK;
+		head = (__force struct robust_list_head __user *)entry;
+
+		return put_user(head, head_ptr);
+	}
+}
+
 long do_futex(u32 __user *uaddr, int op, u32 val, ktime_t *timeout,
 		u32 __user *uaddr2, u32 val2, u32 val3)
 {
@@ -524,7 +577,7 @@ COMPAT_SYSCALL_DEFINE3(get_robust_list, int, pid,
 			compat_uptr_t __user *, head_ptr,
 			compat_size_t __user *, len_ptr)
 {
-	struct robust_list_head32 __user *head = futex_get_robust_list_common(pid, true);
+	struct robust_list_head32 __user *head = futex_get_robust_list_common(pid, true, -1);
 
 	if (IS_ERR(head))
 		return PTR_ERR(head);

-- 
2.52.0


^ permalink raw reply related

* [PATCH v6 3/9] futex: Create set_robust_list2() syscall
From: André Almeida @ 2025-11-22  5:50 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Peter Zijlstra, Darren Hart,
	Davidlohr Bueso, Arnd Bergmann, Sebastian Andrzej Siewior,
	Waiman Long, Ryan Houdek
  Cc: linux-kernel, linux-kselftest, linux-api, kernel-dev,
	André Almeida
In-Reply-To: <20251122-tonyk-robust_futex-v6-0-05fea005a0fd@igalia.com>

Emulators (like FEX-Emu, to run x86 apps on top of Aarch64) have two
special needs about robust lists: to be able to register more than one
robust list, one of the app being emulated and one list for the emulator
itself; and to be able to walk on 32-bit robusts lists on a 64-bit
platform without compat entry points.

The current syscall allows for one robust list per task (on x86-64, it
can have two if compat is enabled) and on Aarch64 there's no way to
parse a 32-bit robust list. The current syscall cannot be expanded to
solve both needs, so create a new syscall, set_robust_list2() with the
following signature:

  sys_set_robust_list2(struct robust_list_head *head, unsigned int index,
		       unsigned int cmd, unsigned int flags)

The new syscall allows to set multiple lists per task, of 64-bit or
32-bit types.

 - `*head` is the same structure used in the current syscall.
 - `index` is the index of the list to be set with `head`.
 - `cmd` defines the operation to perform:
   - `FUTEX_ROBUST_LIST_CMD_SET_64` set a 64-bit robust list at `index`
   - `FUTEX_ROBUST_LIST_CMD_SET_32` set a 32-bit robust list at `index`
   - `FUTEX_ROBUST_LIST_CMD_LIST_LIMIT` get the limit of lists per task
 - `flag` is unused now but can be used to expand the interface

Setting an index twice overwrites the last instance.

The array of lists is dynamically allocated in the first use, but has a
fixed size determined by the kernel. 8 slots are more than enough to
cover the target use case and allows for more use cases. The command for
getting the list limit allows to userspace check if the kernel ever
expands this list. The first two slots are reserved for the kernel, to
store the original syscall robust_list_head's.

The array of lists is destroyed only during task exit.

The `FUTEX_ROBUST_LIST_CMD_SET_64` operation is only available for
64-bit kernels. In such kernels, lists created with
`FUTEX_ROBUST_LIST_CMD_SET_32` are marked with
`FUTEX_ROBUST_LIST_ENTRY_32BIT` and the kernel handles it with a special
function exit_robust_list32() to be able to walk in a list of 32-bit
pointers.

For 32-bit kernels, there's no special function available as every user
list and list handling functions will all have the same bitness.

Signed-off-by: André Almeida <andrealmeid@igalia.com>
---
 include/linux/futex.h      |  26 +++++++++++
 include/linux/sched.h      |   1 +
 include/uapi/linux/futex.h |  16 +++++++
 kernel/futex/core.c        | 111 ++++++++++++++++++++++++++++++++++++++++++---
 kernel/futex/syscalls.c    |  41 +++++++++++++++++
 5 files changed, 188 insertions(+), 7 deletions(-)

diff --git a/include/linux/futex.h b/include/linux/futex.h
index 322851e4a703..3dba249bcd32 100644
--- a/include/linux/futex.h
+++ b/include/linux/futex.h
@@ -5,6 +5,7 @@
 #include <linux/sched.h>
 #include <linux/ktime.h>
 #include <linux/mm_types.h>
+#include <linux/compat.h>
 
 #include <uapi/linux/futex.h>
 
@@ -62,12 +63,35 @@ enum {
 	FUTEX_STATE_DEAD,
 };
 
+#define FUTEX_ROBUST_LIST_NATIVE_IDX	0
+#define FUTEX_ROBUST_LIST_COMPAT_IDX	1
+#define FUTEX_ROBUST_LIST2_IDX		2
+#define FUTEX_ROBUST_LISTS_PER_USER	8
+#define FUTEX_ROBUST_LIST2_MAX_IDX	(FUTEX_ROBUST_LIST2_IDX + FUTEX_ROBUST_LISTS_PER_USER)
+
+/*
+ * List entries without _32BIT flag are using the native machine size
+ */
+#define FUTEX_ROBUST_LIST_ENTRY_INUSE	0x1UL
+#define FUTEX_ROBUST_LIST_ENTRY_32BIT	0x2UL
+#define FUTEX_ROBUST_LIST_ENTRY_MASK	(~0x3UL)
+
+static inline bool futex_in_32bit_syscall(void)
+{
+#ifdef CONFIG_X86
+	return !IS_ENABLED(CONFIG_64BIT) || in_32bit_syscall();
+#else
+	return !IS_ENABLED(CONFIG_64BIT);
+#endif
+}
+
 static inline void futex_init_task(struct task_struct *tsk)
 {
 	tsk->robust_list = NULL;
 #ifdef CONFIG_COMPAT
 	tsk->robust_list32 = NULL;
 #endif
+	tsk->futex_robust_lists = NULL;
 	INIT_LIST_HEAD(&tsk->pi_state_list);
 	tsk->pi_state_cache = NULL;
 	tsk->futex_state = FUTEX_STATE_OK;
@@ -82,6 +106,8 @@ long do_futex(u32 __user *uaddr, int op, u32 val, ktime_t *timeout,
 	      u32 __user *uaddr2, u32 val2, u32 val3);
 int futex_hash_prctl(unsigned long arg2, unsigned long arg3, unsigned long arg4);
 
+int futex_robust_list_set(uintptr_t head, enum robust_list2_cmd cmd, unsigned int index);
+
 #ifdef CONFIG_FUTEX_PRIVATE_HASH
 int futex_hash_allocate_default(void);
 void futex_hash_free(struct mm_struct *mm);
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 76cabfab5b73..de2f3cbe4953 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1331,6 +1331,7 @@ struct task_struct {
 #endif
 #ifdef CONFIG_FUTEX
 	struct robust_list_head __user	*robust_list;
+	uintptr_t			*futex_robust_lists;
 #ifdef CONFIG_COMPAT
 	struct robust_list_head32 __user *robust_list32;
 #endif
diff --git a/include/uapi/linux/futex.h b/include/uapi/linux/futex.h
index 86efb089893d..2ba5c0c3bb59 100644
--- a/include/uapi/linux/futex.h
+++ b/include/uapi/linux/futex.h
@@ -163,6 +163,22 @@ struct robust_list_head32 {
 	__u32			list_op_pending;
 };
 
+/*
+ * Commands for set_robust_list2 syscall
+ */
+enum robust_list2_cmd {
+	FUTEX_ROBUST_LIST_CMD_SET_64,
+	FUTEX_ROBUST_LIST_CMD_SET_32,
+	FUTEX_ROBUST_LIST_CMD_LIST_LIMIT,
+	FUTEX_ROBUST_LIST_CMD_USER_MAX,
+
+	/*
+	 * Kernel internal, rejected for user space
+	 */
+	FUTEX_ROBUST_LIST_SET_NATIVE = 128,
+	FUTEX_ROBUST_LIST_SET_COMPAT,
+};
+
 /*
  * Are there any waiters for this robust futex:
  */
diff --git a/kernel/futex/core.c b/kernel/futex/core.c
index 136639897ff9..14d8a7176367 100644
--- a/kernel/futex/core.c
+++ b/kernel/futex/core.c
@@ -71,6 +71,57 @@ struct futex_private_hash {
 	struct futex_hash_bucket queues[];
 };
 
+int futex_robust_list_set(uintptr_t head, enum robust_list2_cmd cmd,
+			  unsigned int index)
+{
+	uintptr_t entry = FUTEX_ROBUST_LIST_ENTRY_INUSE;
+	uintptr_t *rl = current->futex_robust_lists;
+
+	if (!rl) {
+		rl = kcalloc(FUTEX_ROBUST_LIST2_MAX_IDX, sizeof(*rl), GFP_KERNEL);
+		if (!rl)
+			return -ENOMEM;
+
+		scoped_guard(mutex, &current->futex_exit_mutex) {
+			/* check if another thread set the list before us */
+			if (current->futex_robust_lists) {
+				kfree(rl);
+				rl = current->futex_robust_lists;
+			} else {
+				current->futex_robust_lists = rl;
+			}
+		}
+
+	}
+
+	switch (cmd) {
+	case FUTEX_ROBUST_LIST_CMD_SET_64:
+		if (futex_in_32bit_syscall())
+			return -EINVAL;
+		break;
+	case FUTEX_ROBUST_LIST_CMD_SET_32:
+		entry |= FUTEX_ROBUST_LIST_ENTRY_32BIT;
+		break;
+	case FUTEX_ROBUST_LIST_SET_NATIVE:
+		index = FUTEX_ROBUST_LIST_NATIVE_IDX;
+		break;
+	case FUTEX_ROBUST_LIST_SET_COMPAT:
+		if (!IS_ENABLED(CONFIG_64BIT))
+			return -EINVAL;
+		index = FUTEX_ROBUST_LIST_COMPAT_IDX;
+		entry |= FUTEX_ROBUST_LIST_ENTRY_32BIT;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	entry |= head;
+	scoped_guard(mutex, &current->futex_exit_mutex)
+	rl[index] = entry;
+
+	return 0;
+}
+
 /*
  * Fault injections for futexes.
  */
@@ -1150,9 +1201,8 @@ static inline int fetch_robust_entry(struct robust_list __user **entry,
  *
  * We silently return on any sign of list-walking problem.
  */
-static void exit_robust_list(struct task_struct *curr)
+static void exit_robust_list(struct task_struct *curr, struct robust_list_head __user *head)
 {
-	struct robust_list_head __user *head = curr->robust_list;
 	struct robust_list __user *entry, *next_entry, *pending;
 	unsigned int limit = ROBUST_LIST_LIMIT, pi, pip;
 	unsigned int next_pi;
@@ -1244,9 +1294,8 @@ fetch_robust_entry32(u32 *uentry, struct robust_list __user **entry,
  *
  * We silently return on any sign of list-walking problem.
  */
-static void exit_robust_list32(struct task_struct *curr)
+static void exit_robust_list32(struct task_struct *curr, struct robust_list_head32 __user *head)
 {
-	struct robust_list_head32 __user *head = curr->robust_list32;
 	struct robust_list __user *entry, *next_entry, *pending;
 	unsigned int limit = ROBUST_LIST_LIMIT, pi, pip;
 	unsigned int next_pi;
@@ -1311,7 +1360,15 @@ static void exit_robust_list32(struct task_struct *curr)
 		handle_futex_death(uaddr, curr, pip, HANDLE_DEATH_PENDING);
 	}
 }
-#endif
+
+#else
+
+static void exit_robust_list32(struct task_struct *curr, struct robust_list_head32 __user *head)
+{
+	pr_crit("32-bit kernel should never call %s", __func__);
+}
+
+#endif /* CONFIG_64BIT */
 
 #ifdef CONFIG_FUTEX_PI
 
@@ -1404,20 +1461,60 @@ static void exit_pi_state_list(struct task_struct *curr)
 static inline void exit_pi_state_list(struct task_struct *curr) { }
 #endif
 
+static void exit_robust_lists(struct task_struct *tsk)
+{
+	uintptr_t *rl = tsk->futex_robust_lists;
+
+	tsk->futex_robust_lists = NULL;
+
+	for (unsigned int idx = 0; idx < FUTEX_ROBUST_LIST2_MAX_IDX; idx++) {
+		uintptr_t entry = rl[idx];
+
+		if (!(entry & FUTEX_ROBUST_LIST_ENTRY_MASK))
+			continue;
+
+		/*
+		 * If the list type is the same as the kernel bitness, always
+		 * calls exit_robust_list(). exit_robust_list32() is only for
+		 * 32-bit lists in a 64-bit kernel.
+		 */
+		if (IS_ENABLED(CONFIG_64BIT) && (entry & FUTEX_ROBUST_LIST_ENTRY_32BIT)) {
+			struct robust_list_head32 __user *head;
+
+			entry &= FUTEX_ROBUST_LIST_ENTRY_MASK;
+
+			head = (__force struct robust_list_head32 __user *)entry;
+			exit_robust_list32(tsk, head);
+		} else {
+			struct robust_list_head __user *head;
+
+			entry &= FUTEX_ROBUST_LIST_ENTRY_MASK;
+
+			head = (__force struct robust_list_head __user *)entry;
+			exit_robust_list(tsk, head);
+		}
+	}
+
+	kfree(rl);
+}
+
 static void futex_cleanup(struct task_struct *tsk)
 {
 	if (unlikely(tsk->robust_list)) {
-		exit_robust_list(tsk);
+		exit_robust_list(tsk, tsk->robust_list);
 		tsk->robust_list = NULL;
 	}
 
 #ifdef CONFIG_64BIT
 	if (unlikely(tsk->robust_list32)) {
-		exit_robust_list32(tsk);
+		exit_robust_list32(tsk, tsk->robust_list32);
 		tsk->robust_list32 = NULL;
 	}
 #endif
 
+	if (unlikely(tsk->futex_robust_lists))
+		exit_robust_lists(tsk);
+
 	if (unlikely(!list_empty(&tsk->pi_state_list)))
 		exit_pi_state_list(tsk);
 }
diff --git a/kernel/futex/syscalls.c b/kernel/futex/syscalls.c
index 1de8ff230d54..0b7fa88aa34c 100644
--- a/kernel/futex/syscalls.c
+++ b/kernel/futex/syscalls.c
@@ -109,6 +109,47 @@ SYSCALL_DEFINE3(get_robust_list, int, pid,
 	return put_user(head, head_ptr);
 }
 
+SYSCALL_DEFINE4(set_robust_list2, struct robust_list_head *, head, unsigned int,
+		index, unsigned int, cmd, unsigned int, flags)
+{
+	uintptr_t entry = (__force uintptr_t)head;
+	size_t align = sizeof(u32);
+
+	if (flags)
+		return -EINVAL;
+
+	if (cmd >= FUTEX_ROBUST_LIST_CMD_USER_MAX)
+		return -EINVAL;
+
+	if (index >= FUTEX_ROBUST_LISTS_PER_USER)
+		return -EINVAL;
+
+	/*
+	 * The first two indexes are reserved for the kernel to be used with the
+	 * legacy syscall, so we hide them from userspace.
+	 *
+	 * We map [0, FUTEX_ROBUST_LISTS_PER_USER) to
+	 *  [FUTEX_ROBUST_LIST2_IDX, FUTEX_ROBUST_LIST2_MAX_IDX)
+	 */
+	index += FUTEX_ROBUST_LIST2_IDX;
+
+	switch (cmd) {
+	case FUTEX_ROBUST_LIST_CMD_SET_64:
+		if (futex_in_32bit_syscall())
+			return -EOPNOTSUPP;
+		align = sizeof(u64);
+		fallthrough;
+	case FUTEX_ROBUST_LIST_CMD_SET_32:
+		if (entry % align)
+			return -EINVAL;
+		return futex_robust_list_set(entry, cmd, index);
+	case FUTEX_ROBUST_LIST_CMD_LIST_LIMIT:
+		return FUTEX_ROBUST_LISTS_PER_USER;
+	}
+
+	return -EINVAL;
+}
+
 long do_futex(u32 __user *uaddr, int op, u32 val, ktime_t *timeout,
 		u32 __user *uaddr2, u32 val2, u32 val3)
 {

-- 
2.52.0


^ permalink raw reply related

* [PATCH v6 2/9] futex: Make exit_robust_list32() unconditionally available for 64-bit kernels
From: André Almeida @ 2025-11-22  5:50 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Peter Zijlstra, Darren Hart,
	Davidlohr Bueso, Arnd Bergmann, Sebastian Andrzej Siewior,
	Waiman Long, Ryan Houdek
  Cc: linux-kernel, linux-kselftest, linux-api, kernel-dev,
	André Almeida
In-Reply-To: <20251122-tonyk-robust_futex-v6-0-05fea005a0fd@igalia.com>

The new syscall set_robust_list2() needs to handle both 64-bit and
32-bit robust lists, but not every 64-bit platform have compat entry
points. Make exit_robust_list32() unconditionally available for 64-bit
kernels regardless of having a compat configuration.

Signed-off-by: André Almeida <andrealmeid@igalia.com>
---
 kernel/futex/core.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/kernel/futex/core.c b/kernel/futex/core.c
index c99d7baab24e..136639897ff9 100644
--- a/kernel/futex/core.c
+++ b/kernel/futex/core.c
@@ -31,7 +31,6 @@
  *  "The futexes are also cursed."
  *  "But they come in a choice of three flavours!"
  */
-#include <linux/compat.h>
 #include <linux/jhash.h>
 #include <linux/pagemap.h>
 #include <linux/debugfs.h>
@@ -1213,12 +1212,12 @@ static void exit_robust_list(struct task_struct *curr)
 	}
 }
 
-#ifdef CONFIG_COMPAT
+#ifdef CONFIG_64BIT
 static void __user *futex_uaddr(struct robust_list __user *entry,
 				compat_long_t futex_offset)
 {
-	compat_uptr_t base = ptr_to_compat(entry);
-	void __user *uaddr = compat_ptr(base + futex_offset);
+	u32 base = (u32)(unsigned long)(entry);
+	void __user *uaddr = (void __user *)(unsigned long)(base + futex_offset);
 
 	return uaddr;
 }
@@ -1227,13 +1226,13 @@ static void __user *futex_uaddr(struct robust_list __user *entry,
  * Fetch a robust-list pointer. Bit 0 signals PI futexes:
  */
 static inline int
-fetch_robust_entry32(compat_uptr_t *uentry, struct robust_list __user **entry,
-		   compat_uptr_t __user *head, unsigned int *pi)
+fetch_robust_entry32(u32 *uentry, struct robust_list __user **entry,
+		     u32 __user *head, unsigned int *pi)
 {
 	if (get_user(*uentry, head))
 		return -EFAULT;
 
-	*entry = compat_ptr((*uentry) & ~1);
+	*entry = (void __user *)(unsigned long)((*uentry) & ~1);
 	*pi = (unsigned int)(*uentry) & 1;
 
 	return 0;
@@ -1251,8 +1250,8 @@ static void exit_robust_list32(struct task_struct *curr)
 	struct robust_list __user *entry, *next_entry, *pending;
 	unsigned int limit = ROBUST_LIST_LIMIT, pi, pip;
 	unsigned int next_pi;
-	compat_uptr_t uentry, next_uentry, upending;
-	compat_long_t futex_offset;
+	u32 uentry, next_uentry, upending;
+	s32 futex_offset;
 	int rc;
 
 	/*
@@ -1412,7 +1411,7 @@ static void futex_cleanup(struct task_struct *tsk)
 		tsk->robust_list = NULL;
 	}
 
-#ifdef CONFIG_COMPAT
+#ifdef CONFIG_64BIT
 	if (unlikely(tsk->robust_list32)) {
 		exit_robust_list32(tsk);
 		tsk->robust_list32 = NULL;

-- 
2.52.0


^ permalink raw reply related

* [PATCH v6 0/9] futex: Create {set,get}_robust_list2() syscalls
From: André Almeida @ 2025-11-22  5:50 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Peter Zijlstra, Darren Hart,
	Davidlohr Bueso, Arnd Bergmann, Sebastian Andrzej Siewior,
	Waiman Long, Ryan Houdek
  Cc: linux-kernel, linux-kselftest, linux-api, kernel-dev,
	André Almeida

Hello,

This version is a complete rewrite of the syscall (thanks Thomas for the
suggestions!). 

 * Use case

The use-case for the new syscalls is detailed in the last patch version:

  https://lore.kernel.org/lkml/20250626-tonyk-robust_futex-v5-0-179194dbde8f@igalia.com

 * The syscall interface

Documented at patches 3/9 "futex: Create set_robust_list2() syscall" and
4/9 "futex: Create get_robust_list2() syscall".

 * Testing

I expanded the current robust list selftest to use the new interface,
and also ported the original syscall to use the new syscall internals,
and everything survived the tests.

 * Changelog

Changes from v5:
 - Complete interface rewrite, there are so many changes but the main
   ones are the following points
 - Array of robust lists now has a static size, allocated once during the
   first usage of the list
 - Now that the list of robust lists have a fixed size, I removed the
   logic of having a command for creating a new index on the list. To
   simplify things for everyone, userspace just need to call
   set_robust_list2(head, 32-bit/64-bit type, index).
 - Created get_robust_list2()
 - The new code can be better integrated with the original interface
 - v5: https://lore.kernel.org/r/20250626-tonyk-robust_futex-v5-0-179194dbde8f@igalia.com

Feedback is very welcomed!

---
André Almeida (9):
      futex: Use explicit sizes for compat_robust_list structs
      futex: Make exit_robust_list32() unconditionally available for 64-bit kernels
      futex: Create set_robust_list2() syscall
      futex: Create get_robust_list2() syscall
      futex: Wire up set_robust_list2 syscall
      futex: Wire up get_robust_list2 syscall
      selftests/futex: Expand for set_robust_list2()
      selftests/futex: Expand for get_robust_list2()
      futex: Use new robust list API internally

 arch/alpha/kernel/syscalls/syscall.tbl             |   2 +
 arch/arm/tools/syscall.tbl                         |   2 +
 arch/m68k/kernel/syscalls/syscall.tbl              |   2 +
 arch/microblaze/kernel/syscalls/syscall.tbl        |   2 +
 arch/mips/kernel/syscalls/syscall_n32.tbl          |   2 +
 arch/mips/kernel/syscalls/syscall_n64.tbl          |   2 +
 arch/mips/kernel/syscalls/syscall_o32.tbl          |   2 +
 arch/parisc/kernel/syscalls/syscall.tbl            |   2 +
 arch/powerpc/kernel/syscalls/syscall.tbl           |   2 +
 arch/s390/kernel/syscalls/syscall.tbl              |   2 +
 arch/sh/kernel/syscalls/syscall.tbl                |   2 +
 arch/sparc/kernel/syscalls/syscall.tbl             |   2 +
 arch/x86/entry/syscalls/syscall_32.tbl             |   2 +
 arch/x86/entry/syscalls/syscall_64.tbl             |   2 +
 arch/xtensa/kernel/syscalls/syscall.tbl            |   2 +
 include/linux/compat.h                             |  13 +-
 include/linux/futex.h                              |  30 +-
 include/linux/sched.h                              |   6 +-
 include/uapi/asm-generic/unistd.h                  |   7 +-
 include/uapi/linux/futex.h                         |  26 ++
 kernel/futex/core.c                                | 140 ++++--
 kernel/futex/syscalls.c                            | 134 +++++-
 kernel/sys_ni.c                                    |   2 +
 scripts/syscall.tbl                                |   1 +
 .../selftests/futex/functional/robust_list.c       | 504 +++++++++++++++++++--
 25 files changed, 788 insertions(+), 105 deletions(-)
---
base-commit: c42ba5a87bdccbca11403b7ca8bad1a57b833732
change-id: 20250225-tonyk-robust_futex-60adeedac695

Best regards,
-- 
André Almeida <andrealmeid@igalia.com>


^ permalink raw reply

* [PATCH v6 1/9] futex: Use explicit sizes for compat_robust_list structs
From: André Almeida @ 2025-11-22  5:50 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Peter Zijlstra, Darren Hart,
	Davidlohr Bueso, Arnd Bergmann, Sebastian Andrzej Siewior,
	Waiman Long, Ryan Houdek
  Cc: linux-kernel, linux-kselftest, linux-api, kernel-dev,
	André Almeida
In-Reply-To: <20251122-tonyk-robust_futex-v6-0-05fea005a0fd@igalia.com>

There are two functions for handling robust lists during a task exit:
exit_robust_list() and compat_exit_robust_list(). The first one handles
either 64-bit or 32-bit lists, depending on the kernel bitness.
compat_exit_robust_list() exists only in 64-bit kernels that supports
32-bit syscalls entry points (also known as compat entry points).

The new syscall set_robust_list2() needs to handle both 64-bit and
32-bit robust lists, regardless of compat entry being enabled, so it
needs to have both functions always available.

In preparation for this, use explicit size for struct members of
compat_robust_list and compat_robust_list_head. Rename the structs and
compat_exit_robust_list() to make clear which bitness it handles.

Keep exit_robust_list() as it is: used to handle the native bit size of
the kernel.

Signed-off-by: André Almeida <andrealmeid@igalia.com>
---
 include/linux/compat.h     | 13 +++----------
 include/linux/futex.h      |  2 +-
 include/linux/sched.h      |  2 +-
 include/uapi/linux/futex.h | 10 ++++++++++
 kernel/futex/core.c        | 20 ++++++++++----------
 kernel/futex/syscalls.c    |  8 ++++----
 6 files changed, 29 insertions(+), 26 deletions(-)

diff --git a/include/linux/compat.h b/include/linux/compat.h
index 56cebaff0c91..2c5a7f980182 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -385,15 +385,8 @@ struct compat_ifconf {
 	compat_caddr_t  ifcbuf;
 };
 
-struct compat_robust_list {
-	compat_uptr_t			next;
-};
-
-struct compat_robust_list_head {
-	struct compat_robust_list	list;
-	compat_long_t			futex_offset;
-	compat_uptr_t			list_op_pending;
-};
+struct robust_list32;
+struct robust_list_head32;
 
 #ifdef CONFIG_COMPAT_OLD_SIGACTION
 struct compat_old_sigaction {
@@ -672,7 +665,7 @@ asmlinkage long compat_sys_waitid(int, compat_pid_t,
 		struct compat_siginfo __user *, int,
 		struct compat_rusage __user *);
 asmlinkage long
-compat_sys_set_robust_list(struct compat_robust_list_head __user *head,
+compat_sys_set_robust_list(struct robust_list_head32 __user *head,
 			   compat_size_t len);
 asmlinkage long
 compat_sys_get_robust_list(int pid, compat_uptr_t __user *head_ptr,
diff --git a/include/linux/futex.h b/include/linux/futex.h
index 9e9750f04980..322851e4a703 100644
--- a/include/linux/futex.h
+++ b/include/linux/futex.h
@@ -66,7 +66,7 @@ static inline void futex_init_task(struct task_struct *tsk)
 {
 	tsk->robust_list = NULL;
 #ifdef CONFIG_COMPAT
-	tsk->compat_robust_list = NULL;
+	tsk->robust_list32 = NULL;
 #endif
 	INIT_LIST_HEAD(&tsk->pi_state_list);
 	tsk->pi_state_cache = NULL;
diff --git a/include/linux/sched.h b/include/linux/sched.h
index cbb7340c5866..76cabfab5b73 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1332,7 +1332,7 @@ struct task_struct {
 #ifdef CONFIG_FUTEX
 	struct robust_list_head __user	*robust_list;
 #ifdef CONFIG_COMPAT
-	struct compat_robust_list_head __user *compat_robust_list;
+	struct robust_list_head32 __user *robust_list32;
 #endif
 	struct list_head		pi_state_list;
 	struct futex_pi_state		*pi_state_cache;
diff --git a/include/uapi/linux/futex.h b/include/uapi/linux/futex.h
index 7e2744ec8933..86efb089893d 100644
--- a/include/uapi/linux/futex.h
+++ b/include/uapi/linux/futex.h
@@ -153,6 +153,16 @@ struct robust_list_head {
 	struct robust_list __user *list_op_pending;
 };
 
+struct robust_list32 {
+	__u32 next;
+};
+
+struct robust_list_head32 {
+	struct robust_list32	list;
+	__s32			futex_offset;
+	__u32			list_op_pending;
+};
+
 /*
  * Are there any waiters for this robust futex:
  */
diff --git a/kernel/futex/core.c b/kernel/futex/core.c
index 125804fbb5cb..c99d7baab24e 100644
--- a/kernel/futex/core.c
+++ b/kernel/futex/core.c
@@ -1227,7 +1227,7 @@ static void __user *futex_uaddr(struct robust_list __user *entry,
  * Fetch a robust-list pointer. Bit 0 signals PI futexes:
  */
 static inline int
-compat_fetch_robust_entry(compat_uptr_t *uentry, struct robust_list __user **entry,
+fetch_robust_entry32(compat_uptr_t *uentry, struct robust_list __user **entry,
 		   compat_uptr_t __user *head, unsigned int *pi)
 {
 	if (get_user(*uentry, head))
@@ -1245,9 +1245,9 @@ compat_fetch_robust_entry(compat_uptr_t *uentry, struct robust_list __user **ent
  *
  * We silently return on any sign of list-walking problem.
  */
-static void compat_exit_robust_list(struct task_struct *curr)
+static void exit_robust_list32(struct task_struct *curr)
 {
-	struct compat_robust_list_head __user *head = curr->compat_robust_list;
+	struct robust_list_head32 __user *head = curr->robust_list32;
 	struct robust_list __user *entry, *next_entry, *pending;
 	unsigned int limit = ROBUST_LIST_LIMIT, pi, pip;
 	unsigned int next_pi;
@@ -1259,7 +1259,7 @@ static void compat_exit_robust_list(struct task_struct *curr)
 	 * Fetch the list head (which was registered earlier, via
 	 * sys_set_robust_list()):
 	 */
-	if (compat_fetch_robust_entry(&uentry, &entry, &head->list.next, &pi))
+	if (fetch_robust_entry32(&uentry, &entry, &head->list.next, &pi))
 		return;
 	/*
 	 * Fetch the relative futex offset:
@@ -1270,7 +1270,7 @@ static void compat_exit_robust_list(struct task_struct *curr)
 	 * Fetch any possibly pending lock-add first, and handle it
 	 * if it exists:
 	 */
-	if (compat_fetch_robust_entry(&upending, &pending,
+	if (fetch_robust_entry32(&upending, &pending,
 			       &head->list_op_pending, &pip))
 		return;
 
@@ -1280,8 +1280,8 @@ static void compat_exit_robust_list(struct task_struct *curr)
 		 * Fetch the next entry in the list before calling
 		 * handle_futex_death:
 		 */
-		rc = compat_fetch_robust_entry(&next_uentry, &next_entry,
-			(compat_uptr_t __user *)&entry->next, &next_pi);
+		rc = fetch_robust_entry32(&next_uentry, &next_entry,
+			(u32 __user *)&entry->next, &next_pi);
 		/*
 		 * A pending lock might already be on the list, so
 		 * dont process it twice:
@@ -1413,9 +1413,9 @@ static void futex_cleanup(struct task_struct *tsk)
 	}
 
 #ifdef CONFIG_COMPAT
-	if (unlikely(tsk->compat_robust_list)) {
-		compat_exit_robust_list(tsk);
-		tsk->compat_robust_list = NULL;
+	if (unlikely(tsk->robust_list32)) {
+		exit_robust_list32(tsk);
+		tsk->robust_list32 = NULL;
 	}
 #endif
 
diff --git a/kernel/futex/syscalls.c b/kernel/futex/syscalls.c
index 880c9bf2f315..1de8ff230d54 100644
--- a/kernel/futex/syscalls.c
+++ b/kernel/futex/syscalls.c
@@ -43,7 +43,7 @@ static inline void __user *futex_task_robust_list(struct task_struct *p, bool co
 {
 #ifdef CONFIG_COMPAT
 	if (compat)
-		return p->compat_robust_list;
+		return p->robust_list32;
 #endif
 	return p->robust_list;
 }
@@ -468,13 +468,13 @@ SYSCALL_DEFINE4(futex_requeue,
 
 #ifdef CONFIG_COMPAT
 COMPAT_SYSCALL_DEFINE2(set_robust_list,
-		struct compat_robust_list_head __user *, head,
+		struct robust_list_head32 __user *, head,
 		compat_size_t, len)
 {
 	if (unlikely(len != sizeof(*head)))
 		return -EINVAL;
 
-	current->compat_robust_list = head;
+	current->robust_list32 = head;
 
 	return 0;
 }
@@ -483,7 +483,7 @@ COMPAT_SYSCALL_DEFINE3(get_robust_list, int, pid,
 			compat_uptr_t __user *, head_ptr,
 			compat_size_t __user *, len_ptr)
 {
-	struct compat_robust_list_head __user *head = futex_get_robust_list_common(pid, true);
+	struct robust_list_head32 __user *head = futex_get_robust_list_common(pid, true);
 
 	if (IS_ERR(head))
 		return PTR_ERR(head);

-- 
2.52.0


^ permalink raw reply related

* Re: [PATCH v6 04/20] liveupdate: luo_session: add sessions support
From: Pasha Tatashin @ 2025-11-21 21:30 UTC (permalink / raw)
  To: Pratyush Yadav
  Cc: jasonmiu, graf, rppt, dmatlack, rientjes, corbet, rdunlap,
	ilpo.jarvinen, kanie, ojeda, aliceryhl, masahiroy, akpm, tj,
	yoann.congal, mmaurer, roman.gushchin, chenridong, axboe,
	mark.rutland, jannh, vincent.guittot, hannes, dan.j.williams,
	david, joel.granados, rostedt, anna.schumaker, song, linux,
	linux-kernel, linux-doc, linux-mm, gregkh, tglx, mingo, bp,
	dave.hansen, x86, hpa, rafael, dakr, bartosz.golaszewski,
	cw00.choi, myungjoo.ham, yesanishhere, Jonathan.Cameron,
	quic_zijuhu, aleksander.lobakin, ira.weiny, andriy.shevchenko,
	leon, lukas, bhelgaas, wagi, djeffery, stuart.w.hayes, lennart,
	brauner, linux-api, linux-fsdevel, saeedm, ajayachandra, jgg,
	parav, leonro, witu, hughd, skhawaja, chrisl
In-Reply-To: <mafs08qfz1h3c.fsf@kernel.org>

> >  /*
> >   * The LUO FDT hooks all LUO state for sessions, fds, etc.
> > - * In the root it allso carries "liveupdate-number" 64-bit property that
> > + * In the root it also carries "liveupdate-number" 64-bit property that
>
> Nit: This needs a bit of patch massaging. Patch 2 added the typo, and
> this patch fixes it. It would be better to just update patch 2.

Yeap, this is fixed.


> > + * This structure is located at the beginning of a contiguous block of
> > + * physical memory preserved across the kexec. It provides the necessary
> > + * metadata to interpret the array of session entries that follow.
> > + */
> > +struct luo_session_header_ser {
> > +     u64 pgcnt;
>
> Why do you need pgcnt here? Can't the size be inferred from count? And
> since you use contiguous memory block, the folio will know its page
> count anyway, right? The less we have in the ABI the better IMO.

Right, I had pgnct because my allocators were using size as an
argument, but we removed that, so pgcnt can also be removed.

> Same for other structures below.
>
> > +     u64 count;
> > +} __packed;
> > +
> > +/**
> > + * struct luo_session_ser - Represents the serialized metadata for a LUO session.
> > + * @name:    The unique name of the session, copied from the `luo_session`
> > + *           structure.
> > + * @files:   The physical address of a contiguous memory block that holds
> > + *           the serialized state of files.
> > + * @pgcnt:   The number of pages occupied by the `files` memory block.
> > + * @count:   The total number of files that were part of this session during
> > + *           serialization. Used for iteration and validation during
> > + *           restoration.
> > + *
> > + * This structure is used to package session-specific metadata for transfer
> > + * between kernels via Kexec Handover. An array of these structures (one per
> > + * session) is created and passed to the new kernel, allowing it to reconstruct
> > + * the session context.
> > + *
> > + * If this structure is modified, LUO_SESSION_COMPATIBLE must be updated.
> > + */
> > +struct luo_session_ser {
> > +     char name[LIVEUPDATE_SESSION_NAME_LENGTH];
> > +     u64 files;
> > +     u64 pgcnt;
> > +     u64 count;
> > +} __packed;
> > +
> >  #endif /* _LINUX_LIVEUPDATE_ABI_LUO_H */
> [...]
> > +/* Create a "struct file" for session */
> > +static int luo_session_getfile(struct luo_session *session, struct file **filep)
> > +{
> > +     char name_buf[128];
> > +     struct file *file;
> > +
> > +     guard(mutex)(&session->mutex);
> > +     snprintf(name_buf, sizeof(name_buf), "[luo_session] %s", session->name);
> > +     file = anon_inode_getfile(name_buf, &luo_session_fops, session, O_RDWR);
>
> Nit: You can return the file directly and get rid of filep.

I prefer returning error here.

>
> > +     if (IS_ERR(file))
> > +             return PTR_ERR(file);
> > +
> > +     *filep = file;
> > +
> > +     return 0;
> > +}
> [...]
> > +int __init luo_session_setup_outgoing(void *fdt_out)
> > +{
> > +     struct luo_session_header_ser *header_ser;
> > +     u64 header_ser_pa;
> > +     int err;
> > +
> > +     header_ser = kho_alloc_preserve(LUO_SESSION_PGCNT << PAGE_SHIFT);
>
> Nit: The naming is a bit confusing here. At first glance I thought this
> was just allocating the header, but it allocates the whole session
> serialization buffer.

I made it a little clearer by adding "outgoing_buffer" local variable,
and then assigning head_ser to this local variable.

> > +     if (IS_ERR(header_ser))
> > +             return PTR_ERR(header_ser);
> > +     header_ser_pa = virt_to_phys(header_ser);
> > +
> > +     err = fdt_begin_node(fdt_out, LUO_FDT_SESSION_NODE_NAME);
> > +     err |= fdt_property_string(fdt_out, "compatible",
> > +                                LUO_FDT_SESSION_COMPATIBLE);
> > +     err |= fdt_property(fdt_out, LUO_FDT_SESSION_HEADER, &header_ser_pa,
> > +                         sizeof(header_ser_pa));
> > +     err |= fdt_end_node(fdt_out);
> > +
> > +     if (err)
> > +             goto err_unpreserve;
> > +
> > +     header_ser->pgcnt = LUO_SESSION_PGCNT;
> > +     INIT_LIST_HEAD(&luo_session_global.outgoing.list);
> > +     init_rwsem(&luo_session_global.outgoing.rwsem);
> > +     luo_session_global.outgoing.header_ser = header_ser;
> > +     luo_session_global.outgoing.ser = (void *)(header_ser + 1);
> > +     luo_session_global.outgoing.active = true;
> > +
> > +     return 0;
> > +
> > +err_unpreserve:
> > +     kho_unpreserve_free(header_ser);
> > +     return err;
> > +}
> [...]
> > +int luo_session_deserialize(void)
> > +{
> > +     struct luo_session_header *sh = &luo_session_global.incoming;
> > +     int err;
> > +
> > +     if (luo_session_is_deserialized())
> > +             return 0;
> > +
> > +     luo_session_global.deserialized = true;
> > +     if (!sh->active) {
> > +             INIT_LIST_HEAD(&sh->list);
> > +             init_rwsem(&sh->rwsem);
>
> Nit: it would be a bit simpler if LUO init always initialized this. And
> then luo_session_setup_incoming() can fill the list if it has any data.
> Slight reduction in code duplication and mental load.

These are now statically initialized.

>
> > +             return 0;
> > +     }
> > +
> > +     for (int i = 0; i < sh->header_ser->count; i++) {
> > +             struct luo_session *session;
> > +
> > +             session = luo_session_alloc(sh->ser[i].name);
> > +             if (IS_ERR(session)) {
> > +                     pr_warn("Failed to allocate session [%s] during deserialization %pe\n",
> > +                             sh->ser[i].name, session);
> > +                     return PTR_ERR(session);
> > +             }
> > +
> > +             err = luo_session_insert(sh, session);
> > +             if (err) {
> > +                     luo_session_free(session);
> > +                     pr_warn("Failed to insert session [%s] %pe\n",
> > +                             session->name, ERR_PTR(err));
> > +                     return err;
> > +             }
> > +
> > +             session->count = sh->ser[i].count;
> > +             session->files = sh->ser[i].files ? phys_to_virt(sh->ser[i].files) : 0;
> > +             session->pgcnt = sh->ser[i].pgcnt;
> > +     }
> > +
> > +     kho_restore_free(sh->header_ser);
> > +     sh->header_ser = NULL;
> > +     sh->ser = NULL;
> > +
> > +     return 0;
> > +}
> [...]
>
> --
> Regards,
> Pratyush Yadav

Thanks!

Pasha

^ permalink raw reply

* Re: [PATCH v6 06/20] liveupdate: luo_file: implement file systems callbacks
From: Pratyush Yadav @ 2025-11-21 17:24 UTC (permalink / raw)
  To: Pasha Tatashin
  Cc: pratyush, jasonmiu, graf, rppt, dmatlack, rientjes, corbet,
	rdunlap, ilpo.jarvinen, kanie, ojeda, aliceryhl, masahiroy, akpm,
	tj, yoann.congal, mmaurer, roman.gushchin, chenridong, axboe,
	mark.rutland, jannh, vincent.guittot, hannes, dan.j.williams,
	david, joel.granados, rostedt, anna.schumaker, song, linux,
	linux-kernel, linux-doc, linux-mm, gregkh, tglx, mingo, bp,
	dave.hansen, x86, hpa, rafael, dakr, bartosz.golaszewski,
	cw00.choi, myungjoo.ham, yesanishhere, Jonathan.Cameron,
	quic_zijuhu, aleksander.lobakin, ira.weiny, andriy.shevchenko,
	leon, lukas, bhelgaas, wagi, djeffery, stuart.w.hayes, lennart,
	brauner, linux-api, linux-fsdevel, saeedm, ajayachandra, jgg,
	parav, leonro, witu, hughd, skhawaja, chrisl
In-Reply-To: <20251115233409.768044-7-pasha.tatashin@soleen.com>

On Sat, Nov 15 2025, Pasha Tatashin wrote:

> This patch implements the core mechanism for managing preserved
> files throughout the live update lifecycle. It provides the logic to
> invoke the file handler callbacks (preserve, unpreserve, freeze,
> unfreeze, retrieve, and finish) at the appropriate stages.
>
> During the reboot phase, luo_file_freeze() serializes the final
> metadata for each file (handler compatible string, token, and data
> handle) into a memory region preserved by KHO. In the new kernel,
> luo_file_deserialize() reconstructs the in-memory file list from this
> data, preparing the session for retrieval.
>
> Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
[...]
> +
> +static LIST_HEAD(luo_file_handler_list);
> +
> +/* 2 4K pages, give space for 128 files per session */
> +#define LUO_FILE_PGCNT		2ul
> +#define LUO_FILE_MAX							\
> +	((LUO_FILE_PGCNT << PAGE_SHIFT) / sizeof(struct luo_file_ser))
> +
> +/**
> + * struct luo_file - Represents a single preserved file instance.
> + * @fh:            Pointer to the &struct liveupdate_file_handler that manages
> + *                 this type of file.
> + * @file:          Pointer to the kernel's &struct file that is being preserved.
> + *                 This is NULL in the new kernel until the file is successfully
> + *                 retrieved.
> + * @serialized_data: The opaque u64 handle to the serialized state of the file.
> + *                 This handle is passed back to the handler's .freeze(),
> + *                 .retrieve(), and .finish() callbacks, allowing it to track
> + *                 and update its serialized state across phases.
> + * @retrieved:     A flag indicating whether a user/kernel in the new kernel has
> + *                 successfully called retrieve() on this file. This prevents
> + *                 multiple retrieval attempts.
> + * @mutex:         A mutex that protects the fields of this specific instance
> + *                 (e.g., @retrieved, @file), ensuring that operations like
> + *                 retrieving or finishing a file are atomic.
> + * @list:          The list_head linking this instance into its parent
> + *                 session's list of preserved files.
> + * @token:         The user-provided unique token used to identify this file.
> + *
> + * This structure is the core in-kernel representation of a single file being
> + * managed through a live update. An instance is created by luo_preserve_file()
> + * to link a 'struct file' to its corresponding handler, a user-provided token,
> + * and the serialized state handle returned by the handler's .preserve()
> + * operation.
> + *
> + * These instances are tracked in a per-session list. The @serialized_data
> + * field, which holds a handle to the file's serialized state, may be updated
> + * during the .freeze() callback before being serialized for the next kernel.
> + * After reboot, these structures are recreated by luo_file_deserialize() and
> + * are finally cleaned up by luo_file_finish().
> + */
> +struct luo_file {
> +	struct liveupdate_file_handler *fh;
> +	struct file *file;
> +	u64 serialized_data;
> +	bool retrieved;
> +	struct mutex mutex;
> +	struct list_head list;
> +	u64 token;
> +};
> +
> +static int luo_session_alloc_files_mem(struct luo_session *session)
> +{
> +	size_t size;
> +	void *mem;
> +
> +	if (session->files)
> +		return 0;
> +
> +	WARN_ON_ONCE(session->count);
> +
> +	size = LUO_FILE_PGCNT << PAGE_SHIFT;
> +	mem = kho_alloc_preserve(size);
> +	if (IS_ERR(mem))
> +		return PTR_ERR(mem);
> +
> +	session->files = mem;
> +	session->pgcnt = LUO_FILE_PGCNT;

I think this is a layering violation. luo_session should take care of
managing the session, including the memory it needs. luo_files should
take care of managing the file, including the memory it needs for _the
file_. I think proper layering will make the code a lot easier to grok
and modify later. When I want to see how sessions are handled, I can do
to luo_session.c. I won't have to poke into luo_files.c.

So I think luo_session_preserve_fd() should first make sure there is
memory available to store the file in the session, and only then call
luo_preserve_file().

> +
> +	return 0;
> +}
> +
> +static void luo_session_free_files_mem(struct luo_session *session)
> +{
> +	/* If session has files, no need to free preservation memory */
> +	if (session->count)
> +		return;
> +
> +	if (!session->files)
> +		return;
> +
> +	kho_unpreserve_free(session->files);
> +	session->files = NULL;
> +	session->pgcnt = 0;
> +}
> +
> +static bool luo_token_is_used(struct luo_session *session, u64 token)
> +{
> +	struct luo_file *iter;
> +
> +	list_for_each_entry(iter, &session->files_list, list) {
> +		if (iter->token == token)
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
> +/**
> + * luo_preserve_file - Initiate the preservation of a file descriptor.
> + * @session: The session to which the preserved file will be added.
> + * @token:   A unique, user-provided identifier for the file.
> + * @fd:      The file descriptor to be preserved.
> + *
> + * This function orchestrates the first phase of preserving a file. Upon entry,
> + * it takes a reference to the 'struct file' via fget(), effectively making LUO
> + * a co-owner of the file. This reference is held until the file is either
> + * unpreserved or successfully finished in the next kernel, preventing the file
> + * from being prematurely destroyed.
> + *
> + * This function orchestrates the first phase of preserving a file. It performs
> + * the following steps:
> + *
> + * 1. Validates that the @token is not already in use within the session.
> + * 2. Ensures the session's memory for files serialization is allocated
> + *    (allocates if needed).
> + * 3. Iterates through registered handlers, calling can_preserve() to find one
> + *    compatible with the given @fd.
> + * 4. Calls the handler's .preserve() operation, which saves the file's state
> + *    and returns an opaque private data handle.
> + * 5. Adds the new instance to the session's internal list.
> + *
> + * On success, LUO takes a reference to the 'struct file' and considers it
> + * under its management until it is unpreserved or finished.
> + *
> + * In case of any failure, all intermediate allocations (file reference, memory
> + * for the 'luo_file' struct, etc.) are cleaned up before returning an error.
> + *
> + * Context: Can be called from an ioctl handler during normal system operation.
> + * Return: 0 on success. Returns a negative errno on failure:
> + *         -EEXIST if the token is already used.
> + *         -EBADF if the file descriptor is invalid.
> + *         -ENOSPC if the session is full.
> + *         -ENOENT if no compatible handler is found.
> + *         -ENOMEM on memory allocation failure.
> + *         Other erros might be returned by .preserve().
> + */
> +int luo_preserve_file(struct luo_session *session, u64 token, int fd)
> +{
> +	struct liveupdate_file_op_args args = {0};
> +	struct liveupdate_file_handler *fh;
> +	struct luo_file *luo_file;
> +	struct file *file;
> +	int err;
> +
> +	lockdep_assert_held(&session->mutex);
> +
> +	if (luo_token_is_used(session, token))
> +		return -EEXIST;
> +
> +	file = fget(fd);
> +	if (!file)
> +		return -EBADF;
> +
> +	err = luo_session_alloc_files_mem(session);
> +	if (err)
> +		goto  exit_err;
> +
> +	if (session->count == LUO_FILE_MAX) {
> +		err = -ENOSPC;
> +		goto exit_err;
> +	}

Similarly, luo_file has no business knowing the size of a session.
Checking session->count should also be done in
luo_session_preserve_fd(). luo_preserve_file() should never be called if
there is no space _in the session_ to accommodate the file.

> +
> +	err = -ENOENT;
> +	list_for_each_entry(fh, &luo_file_handler_list, list) {
> +		if (fh->ops->can_preserve(fh, file)) {
> +			err = 0;
> +			break;
> +		}
> +	}
> +
> +	/* err is still -ENOENT if no handler was found */
> +	if (err)
> +		goto exit_err;
> +
> +	luo_file = kzalloc(sizeof(*luo_file), GFP_KERNEL);
> +	if (!luo_file) {
> +		err = -ENOMEM;
> +		goto exit_err;
> +	}
> +
> +	luo_file->file = file;
> +	luo_file->fh = fh;
> +	luo_file->token = token;
> +	luo_file->retrieved = false;
> +	mutex_init(&luo_file->mutex);
> +
> +	args.handler = fh;
> +	args.session = (struct liveupdate_session *)session;
> +	args.file = file;
> +	err = fh->ops->preserve(&args);
> +	if (err) {
> +		mutex_destroy(&luo_file->mutex);
> +		kfree(luo_file);
> +		goto exit_err;
> +	} else {
> +		luo_file->serialized_data = args.serialized_data;
> +		list_add_tail(&luo_file->list, &session->files_list);
> +		session->count++;
> +	}
> +
> +	return 0;
> +
> +exit_err:
> +	fput(file);
> +	luo_session_free_files_mem(session);
> +
> +	return err;
> +}
> +
> +/**
> + * luo_file_unpreserve_files - Unpreserves all files from a session.
> + * @session: The session to be cleaned up.
> + *
> + * This function serves as the primary cleanup path for a session. It is
> + * invoked when the userspace agent closes the session's file descriptor.
> + *
> + * For each file, it performs the following cleanup actions:
> + *   1. Calls the handler's .unpreserve() callback to allow the handler to
> + *      release any resources it allocated.
> + *   2. Removes the file from the session's internal tracking list.
> + *   3. Releases the reference to the 'struct file' that was taken by
> + *      luo_preserve_file() via fput(), returning ownership.
> + *   4. Frees the memory associated with the internal 'struct luo_file'.
> + *
> + * After all individual files are unpreserved, it frees the contiguous memory
> + * block that was allocated to hold their serialization data.
> + */
> +void luo_file_unpreserve_files(struct luo_session *session)
> +{
> +	struct luo_file *luo_file;
> +
> +	lockdep_assert_held(&session->mutex);
> +
> +	while (!list_empty(&session->files_list)) {

Continuing with the layering thing, the list belongs to the session, so
it should traverse it. luo_session_release() should traverse the list
and call luo_file_unpreserve() on each file in the list. The body of
this loop becomes luo_file_unpreserve().

> +		struct liveupdate_file_op_args args = {0};
> +
> +		luo_file = list_last_entry(&session->files_list,
> +					   struct luo_file, list);
> +
> +		args.handler = luo_file->fh;
> +		args.session = (struct liveupdate_session *)session;
> +		args.file = luo_file->file;
> +		args.serialized_data = luo_file->serialized_data;
> +		luo_file->fh->ops->unpreserve(&args);
> +
> +		list_del(&luo_file->list);
> +		session->count--;

... and these two go into luo_session_release().

> +
> +		fput(luo_file->file);
> +		mutex_destroy(&luo_file->mutex);
> +		kfree(luo_file);
> +	}
> +
> +	luo_session_free_files_mem(session);
> +}
> +
> +static int luo_file_freeze_one(struct luo_session *session,
> +			       struct luo_file *luo_file)
> +{
> +	int err = 0;
> +
> +	guard(mutex)(&luo_file->mutex);
> +
> +	if (luo_file->fh->ops->freeze) {

Nit: "if (!luo_file->fh->ops->freeze) return 0;" would make this tad bit
neater. You probably don't even need the mutex since ops are const.

> +		struct liveupdate_file_op_args args = {0};
> +
> +		args.handler = luo_file->fh;
> +		args.session = (struct liveupdate_session *)session;
> +		args.file = luo_file->file;
> +		args.serialized_data = luo_file->serialized_data;
> +
> +		err = luo_file->fh->ops->freeze(&args);
> +		if (!err)
> +			luo_file->serialized_data = args.serialized_data;

Then this can be:

	if (err)
		return err;

	luo_file->serialized_data = args.serialized_data;
	return 0;

> +	}
> +
> +	return err;
> +}
> +
> +static void luo_file_unfreeze_one(struct luo_session *session,
> +				  struct luo_file *luo_file)
> +{
> +	guard(mutex)(&luo_file->mutex);
> +
> +	if (luo_file->fh->ops->unfreeze) {

Same here.

> +		struct liveupdate_file_op_args args = {0};
> +
> +		args.handler = luo_file->fh;
> +		args.session = (struct liveupdate_session *)session;
> +		args.file = luo_file->file;
> +		args.serialized_data = luo_file->serialized_data;
> +
> +		luo_file->fh->ops->unfreeze(&args);
> +	}
> +
> +	luo_file->serialized_data = 0;

The file will also need to be unpreserved after unfreeze. Resetting the
data here is not the right thing, since unpreserve is responsible for
freeing things, and it won't have access to its data.

> +}
> +
> +static void __luo_file_unfreeze(struct luo_session *session,
> +				struct luo_file *failed_entry)
> +{
> +	struct list_head *files_list = &session->files_list;
> +	struct luo_file *luo_file;
> +
> +	list_for_each_entry(luo_file, files_list, list) {
> +		if (luo_file == failed_entry)
> +			break;
> +
> +		luo_file_unfreeze_one(session, luo_file);
> +	}
> +
> +	memset(session->files, 0, session->pgcnt << PAGE_SHIFT);
> +}
> +
> +/**
> + * luo_file_freeze - Freezes all preserved files and serializes their metadata.
> + * @session: The session whose files are to be frozen.
> + *
> + * This function is called from the reboot() syscall path, just before the
> + * kernel transitions to the new image via kexec. Its purpose is to perform the
> + * final preparation and serialization of all preserved files in the session.
> + *
> + * It iterates through each preserved file in FIFO order (the order of
> + * preservation) and performs two main actions:
> + *
> + * 1. Freezes the File: It calls the handler's .freeze() callback for each
> + *    file. This gives the handler a final opportunity to quiesce the device or
> + *    prepare its state for the upcoming reboot. The handler may update its
> + *    private data handle during this step.
> + *
> + * 2. Serializes Metadata: After a successful freeze, it copies the final file
> + *    metadata—the handler's compatible string, the user token, and the final
> + *    private data handle—into the pre-allocated contiguous memory buffer
> + *    (session->files) that will be handed over to the next kernel via KHO.
> + *
> + * Error Handling (Rollback):
> + * This function is atomic. If any handler's .freeze() operation fails, the
> + * entire live update is aborted. The __luo_file_unfreeze() helper is
> + * immediately called to invoke the .unfreeze() op on all files that were
> + * successfully frozen before the point of failure, rolling them back to a
> + * running state. The function then returns an error, causing the reboot()
> + * syscall to fail.
> + *
> + * Context: Called only from the liveupdate_reboot() path.
> + * Return: 0 on success, or a negative errno on failure.
> + */
> +int luo_file_freeze(struct luo_session *session)
> +{
> +	struct luo_file_ser *file_ser = session->files;
> +	struct luo_file *luo_file;
> +	int err;
> +	int i;
> +
> +	lockdep_assert_held(&session->mutex);
> +
> +	if (!session->count)
> +		return 0;

Same comment about layering here...

> +
> +	if (WARN_ON(!file_ser))
> +		return -EINVAL;
> +
> +	i = 0;
> +	list_for_each_entry(luo_file, &session->files_list, list) {
> +		err = luo_file_freeze_one(session, luo_file);
> +		if (err < 0) {
> +			pr_warn("Freeze failed for session[%s] token[%#0llx] handler[%s] err[%pe]\n",
> +				session->name, luo_file->token,
> +				luo_file->fh->compatible, ERR_PTR(err));
> +			goto exit_err;
> +		}
> +
> +		strscpy(file_ser[i].compatible, luo_file->fh->compatible,
> +			sizeof(file_ser[i].compatible));
> +		file_ser[i].data = luo_file->serialized_data;
> +		file_ser[i].token = luo_file->token;
> +		i++;
> +	}
> +
> +	return 0;
> +
> +exit_err:
> +	__luo_file_unfreeze(session, luo_file);
> +
> +	return err;
> +}
> +
> +/**
> + * luo_file_unfreeze - Unfreezes all files in a session.
> + * @session: The session whose files are to be unfrozen.
> + *
> + * This function rolls back the state of all files in a session after the freeze
> + * phase has begun but must be aborted. It is the counterpart to
> + * luo_file_freeze().
> + *
> + * It invokes the __luo_file_unfreeze() helper with a NULL argument, which
> + * signals the helper to iterate through all files in the session  and call
> + * their respective .unfreeze() handler callbacks.
> + *
> + * Context: This is called when the live update is aborted during
> + *          the reboot() syscall, after luo_file_freeze() has been called.
> + */
> +void luo_file_unfreeze(struct luo_session *session)
> +{
> +	lockdep_assert_held(&session->mutex);
> +
> +	if (!session->count)
> +		return;

... and here.

> +
> +	__luo_file_unfreeze(session, NULL);
> +}
> +
> +/**
> + * luo_retrieve_file - Restores a preserved file from a session by its token.
> + * @session: The session from which to retrieve the file.
> + * @token:   The unique token identifying the file to be restored.
> + * @filep:   Output parameter; on success, this is populated with a pointer
> + *           to the newly retrieved 'struct file'.
> + *
> + * This function is the primary mechanism for recreating a file in the new
> + * kernel after a live update. It searches the session's list of deserialized
> + * files for an entry matching the provided @token.
> + *
> + * The operation is idempotent: if a file has already been successfully
> + * retrieved, this function will simply return a pointer to the existing
> + * 'struct file' and report success without re-executing the retrieve
> + * operation. This is handled by checking the 'retrieved' flag under a lock.
> + *
> + * File retrieval can happen in any order; it is not bound by the order of
> + * preservation.
> + *
> + * Context: Can be called from an ioctl or other in-kernel code in the new
> + *          kernel.
> + * Return: 0 on success. Returns a negative errno on failure:
> + *         -ENOENT if no file with the matching token is found.
> + *         Any error code returned by the handler's .retrieve() op.
> + */
> +int luo_retrieve_file(struct luo_session *session, u64 token,
> +		      struct file **filep)
> +{
> +	struct liveupdate_file_op_args args = {0};
> +	struct luo_file *luo_file;
> +	int err;
> +
> +	lockdep_assert_held(&session->mutex);
> +
> +	if (list_empty(&session->files_list))
> +		return -ENOENT;

... and here.

> +
> +	list_for_each_entry(luo_file, &session->files_list, list) {
> +		if (luo_file->token == token)
> +			break;
> +	}
> +
> +	if (luo_file->token != token)
> +		return -ENOENT;
> +
> +	guard(mutex)(&luo_file->mutex);
> +	if (luo_file->retrieved) {
> +		/*
> +		 * Someone is asking for this file again, so get a reference
> +		 * for them.
> +		 */

Should we even allow this? Is there a use case?

> +		get_file(luo_file->file);
> +		*filep = luo_file->file;
> +		return 0;
> +	}
> +
> +	args.handler = luo_file->fh;
> +	args.session = (struct liveupdate_session *)session;
> +	args.serialized_data = luo_file->serialized_data;
> +	err = luo_file->fh->ops->retrieve(&args);
> +	if (!err) {
> +		luo_file->file = args.file;
> +
> +		/* Get reference so we can keep this file in LUO until finish */
> +		get_file(luo_file->file);
> +		*filep = luo_file->file;
> +		luo_file->retrieved = true;
> +	}
> +
> +	return err;
> +}
> +
> +static int luo_file_can_finish_one(struct luo_session *session,
> +				   struct luo_file *luo_file)
> +{
> +	bool can_finish = true;
> +
> +	guard(mutex)(&luo_file->mutex);
> +
> +	if (luo_file->fh->ops->can_finish) {

Same nitpick about doing "if (!luo_file->fh->ops->can_finish)".

> +		struct liveupdate_file_op_args args = {0};
> +
> +		args.handler = luo_file->fh;
> +		args.session = (struct liveupdate_session *)session;
> +		args.file = luo_file->file;
> +		args.serialized_data = luo_file->serialized_data;
> +		args.retrieved = luo_file->retrieved;
> +		can_finish = luo_file->fh->ops->can_finish(&args);
> +	}
> +
> +	return can_finish ? 0 : -EBUSY;
> +}
> +
> +static void luo_file_finish_one(struct luo_session *session,
> +				struct luo_file *luo_file)
> +{
> +	struct liveupdate_file_op_args args = {0};
> +
> +	guard(mutex)(&luo_file->mutex);
> +
> +	args.handler = luo_file->fh;
> +	args.session = (struct liveupdate_session *)session;
> +	args.file = luo_file->file;
> +	args.serialized_data = luo_file->serialized_data;
> +	args.retrieved = luo_file->retrieved;
> +
> +	luo_file->fh->ops->finish(&args);
> +}
> +
> +/**
> + * luo_file_finish - Completes the lifecycle for all files in a session.
> + * @session: The session to be finalized.
> + *
> + * This function orchestrates the final teardown of a live update session in the
> + * new kernel. It should be called after all necessary files have been
> + * retrieved and the userspace agent is ready to release the preserved state.
> + *
> + * The function iterates through all tracked files. For each file, it performs
> + * the following sequence of cleanup actions:
> + *
> + * 1. If file is not yet retrieved, retrieves it, and calls can_finish() on
> + *    every file in the session. If all can_finish return true, continue to
> + *    finish.
> + * 2. Calls the handler's .finish() callback (via luo_file_finish_one) to
> + *    allow for final resource cleanup within the handler.
> + * 3. Releases LUO's ownership reference on the 'struct file' via fput(). This
> + *    is the counterpart to the get_file() call in luo_retrieve_file().
> + * 4. Removes the 'struct luo_file' from the session's internal list.
> + * 5. Frees the memory for the 'struct luo_file' instance itself.
> + *
> + * After successfully finishing all individual files, it frees the
> + * contiguous memory block that was used to transfer the serialized metadata
> + * from the previous kernel.
> + *
> + * Error Handling (Atomic Failure):
> + * This operation is atomic. If any handler's .can_finish() op fails, the entire
> + * function aborts immediately and returns an error.
> + *
> + * Context: Can be called from an ioctl handler in the new kernel.
> + * Return: 0 on success, or a negative errno on failure.
> + */
> +int luo_file_finish(struct luo_session *session)
> +{
> +	struct list_head *files_list = &session->files_list;
> +	struct luo_file *luo_file;
> +	int err;
> +
> +	if (!session->count)
> +		return 0;

Layering comment again.

> +
> +	lockdep_assert_held(&session->mutex);
> +
> +	list_for_each_entry(luo_file, files_list, list) {
> +		err = luo_file_can_finish_one(session, luo_file);
> +		if (err)
> +			return err;
> +	}
> +
> +	while (!list_empty(&session->files_list)) {
> +		luo_file = list_last_entry(&session->files_list,
> +					   struct luo_file, list);
> +
> +		luo_file_finish_one(session, luo_file);
> +
> +		if (luo_file->file)
> +			fput(luo_file->file);
> +		list_del(&luo_file->list);
> +		session->count--;
> +		mutex_destroy(&luo_file->mutex);
> +		kfree(luo_file);
> +	}
> +
> +	if (session->files) {
> +		kho_restore_free(session->files);
> +		session->files = NULL;
> +		session->pgcnt = 0;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * luo_file_deserialize - Reconstructs the list of preserved files in the new kernel.
> + * @session: The incoming session containing the serialized file data from KHO.
> + *
> + * This function is called during the early boot process of the new kernel. It
> + * takes the raw, contiguous memory block of 'struct luo_file_ser' entries,
> + * provided by the previous kernel, and transforms it back into a live,
> + * in-memory linked list of 'struct luo_file' instances.
> + *
> + * For each serialized entry, it performs the following steps:
> + *   1. Reads the 'compatible' string.
> + *   2. Searches the global list of registered file handlers for one that
> + *      matches the compatible string.
> + *   3. Allocates a new 'struct luo_file'.
> + *   4. Populates the new structure with the deserialized data (token, private
> + *      data handle) and links it to the found handler. The 'file' pointer is
> + *      initialized to NULL, as the file has not been retrieved yet.
> + *   5. Adds the new 'struct luo_file' to the session's files_list.
> + *
> + * This prepares the session for userspace, which can later call
> + * luo_retrieve_file() to restore the actual file descriptors.
> + *
> + * Context: Called from session deserialization.
> + */
> +int luo_file_deserialize(struct luo_session *session)
> +{
> +	struct luo_file_ser *file_ser;
> +	u64 i;
> +
> +	lockdep_assert_held(&session->mutex);
> +
> +	if (!session->files)
> +		return 0;

Layering again.

> +
> +	file_ser = session->files;
> +	for (i = 0; i < session->count; i++) {
> +		struct liveupdate_file_handler *fh;
> +		bool handler_found = false;
> +		struct luo_file *luo_file;
> +
> +		list_for_each_entry(fh, &luo_file_handler_list, list) {
> +			if (!strcmp(fh->compatible, file_ser[i].compatible)) {
> +				handler_found = true;
> +				break;
> +			}
> +		}
> +
> +		if (!handler_found) {
> +			pr_warn("No registered handler for compatible '%s'\n",
> +				file_ser[i].compatible);
> +			return -ENOENT;
> +		}
> +
> +		luo_file = kzalloc(sizeof(*luo_file), GFP_KERNEL);
> +		if (!luo_file)
> +			return -ENOMEM;
> +
> +		luo_file->fh = fh;
> +		luo_file->file = NULL;
> +		luo_file->serialized_data = file_ser[i].data;
> +		luo_file->token = file_ser[i].token;
> +		luo_file->retrieved = false;
> +		mutex_init(&luo_file->mutex);
> +		list_add_tail(&luo_file->list, &session->files_list);
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * liveupdate_register_file_handler - Register a file handler with LUO.
> + * @fh: Pointer to a caller-allocated &struct liveupdate_file_handler.
> + * The caller must initialize this structure, including a unique
> + * 'compatible' string and a valid 'fh' callbacks. This function adds the
> + * handler to the global list of supported file handlers.
> + *
> + * Context: Typically called during module initialization for file types that
> + * support live update preservation.
> + *
> + * Return: 0 on success. Negative errno on failure.
> + */
> +int liveupdate_register_file_handler(struct liveupdate_file_handler *fh)
> +{
> +	static DEFINE_MUTEX(register_file_handler_lock);
> +	struct liveupdate_file_handler *fh_iter;
> +
> +	if (!liveupdate_enabled())
> +		return -EOPNOTSUPP;
> +
> +	/*
> +	 * Once sessions have been deserialized, file handlers cannot be
> +	 * registered, it is too late.
> +	 */
> +	if (WARN_ON(luo_session_is_deserialized()))
> +		return -EBUSY;
> +
> +	/* Sanity check that all required callbacks are set */
> +	if (!fh->ops->preserve || !fh->ops->unpreserve ||
> +	    !fh->ops->retrieve || !fh->ops->finish) {

Should check can_preserve here, right?

> +		return -EINVAL;
> +	}
> +
> +	guard(mutex)(&register_file_handler_lock);
> +	list_for_each_entry(fh_iter, &luo_file_handler_list, list) {
> +		if (!strcmp(fh_iter->compatible, fh->compatible)) {
> +			pr_err("File handler registration failed: Compatible string '%s' already registered.\n",
> +			       fh->compatible);
> +			return -EEXIST;
> +		}
> +	}
> +
> +	if (!try_module_get(fh->ops->owner))
> +		return -EAGAIN;
> +
> +	INIT_LIST_HEAD(&fh->list);
> +	list_add_tail(&fh->list, &luo_file_handler_list);
> +
> +	return 0;
> +}
> +
> +/**
> + * liveupdate_get_token_outgoing - Get the token for a preserved file.
> + * @s:      The outgoing liveupdate session.
> + * @file:   The file object to search for.
> + * @tokenp: Output parameter for the found token.
> + *
> + * Searches the list of preserved files in an outgoing session for a matching
> + * file object. If found, the corresponding user-provided token is returned.
> + *
> + * This function is intended for in-kernel callers that need to correlate a
> + * file with its liveupdate token.
> + *
> + * Context: Can be called from any context that can acquire the session mutex.
> + * Return: 0 on success, -ENOENT if the file is not preserved in this session.
> + */
> +int liveupdate_get_token_outgoing(struct liveupdate_session *s,
> +				  struct file *file, u64 *tokenp)
> +{
> +	struct luo_session *session = (struct luo_session *)s;
> +	struct luo_file *luo_file;
> +	int err = -ENOENT;
> +
> +	list_for_each_entry(luo_file, &session->files_list, list) {
> +		if (luo_file->file == file) {
> +			if (tokenp)
> +				*tokenp = luo_file->token;
> +			err = 0;
> +			break;
> +		}
> +	}
> +
> +	return err;
> +}
> +
> +/**
> + * liveupdate_get_file_incoming - Retrieves a preserved file for in-kernel use.
> + * @s:      The incoming liveupdate session (restored from the previous kernel).
> + * @token:  The unique token identifying the file to retrieve.
> + * @filep:  On success, this will be populated with a pointer to the retrieved
> + *          'struct file'.
> + *
> + * Provides a kernel-internal API for other subsystems to retrieve their
> + * preserved files after a live update. This function is a simple wrapper
> + * around luo_retrieve_file(), allowing callers to find a file by its token.
> + *
> + * The operation is idempotent; subsequent calls for the same token will return
> + * a pointer to the same 'struct file' object.
> + *
> + * The caller receives a pointer to the file with a reference incremented. The
> + * file's lifetime is managed by LUO and any userspace file
> + * descriptors. If the caller needs to hold a reference to the file beyond the
> + * immediate scope, it must call get_file() itself.
> + *
> + * Context: Can be called from any context in the new kernel that has a handle
> + *          to a restored session.
> + * Return: 0 on success. Returns -ENOENT if no file with the matching token is
> + *         found, or any other negative errno on failure.
> + */
> +int liveupdate_get_file_incoming(struct liveupdate_session *s, u64 token,
> +				 struct file **filep)
> +{
> +	struct luo_session *session = (struct luo_session *)s;
> +
> +	return luo_retrieve_file(session, token, filep);
> +}
> diff --git a/kernel/liveupdate/luo_internal.h b/kernel/liveupdate/luo_internal.h
> index 5185ad37a8c1..1a36f2383123 100644
> --- a/kernel/liveupdate/luo_internal.h
> +++ b/kernel/liveupdate/luo_internal.h
> @@ -70,4 +70,13 @@ int luo_session_serialize(void);
>  int luo_session_deserialize(void);
>  bool luo_session_is_deserialized(void);
>  
> +int luo_preserve_file(struct luo_session *session, u64 token, int fd);
> +void luo_file_unpreserve_files(struct luo_session *session);
> +int luo_file_freeze(struct luo_session *session);
> +void luo_file_unfreeze(struct luo_session *session);
> +int luo_retrieve_file(struct luo_session *session, u64 token,
> +		      struct file **filep);
> +int luo_file_finish(struct luo_session *session);
> +int luo_file_deserialize(struct luo_session *session);
> +
>  #endif /* _LINUX_LUO_INTERNAL_H */

-- 
Regards,
Pratyush Yadav

^ permalink raw reply

* Re: [PATCH v6 05/20] liveupdate: luo_ioctl: add user interface
From: Pratyush Yadav @ 2025-11-21 16:45 UTC (permalink / raw)
  To: Pasha Tatashin
  Cc: pratyush, jasonmiu, graf, rppt, dmatlack, rientjes, corbet,
	rdunlap, ilpo.jarvinen, kanie, ojeda, aliceryhl, masahiroy, akpm,
	tj, yoann.congal, mmaurer, roman.gushchin, chenridong, axboe,
	mark.rutland, jannh, vincent.guittot, hannes, dan.j.williams,
	david, joel.granados, rostedt, anna.schumaker, song, linux,
	linux-kernel, linux-doc, linux-mm, gregkh, tglx, mingo, bp,
	dave.hansen, x86, hpa, rafael, dakr, bartosz.golaszewski,
	cw00.choi, myungjoo.ham, yesanishhere, Jonathan.Cameron,
	quic_zijuhu, aleksander.lobakin, ira.weiny, andriy.shevchenko,
	leon, lukas, bhelgaas, wagi, djeffery, stuart.w.hayes, lennart,
	brauner, linux-api, linux-fsdevel, saeedm, ajayachandra, jgg,
	parav, leonro, witu, hughd, skhawaja, chrisl
In-Reply-To: <20251115233409.768044-6-pasha.tatashin@soleen.com>

On Sat, Nov 15 2025, Pasha Tatashin wrote:

> Introduce the user-space interface for the Live Update Orchestrator
> via ioctl commands, enabling external control over the live update
> process and management of preserved resources.
>
> The idea is that there is going to be a single userspace agent driving
> the live update, therefore, only a single process can ever hold this
> device opened at a time.
>
> The following ioctl commands are introduced:
>
> LIVEUPDATE_IOCTL_CREATE_SESSION
> Provides a way for userspace to create a named session for grouping file
> descriptors that need to be preserved. It returns a new file descriptor
> representing the session.
>
> LIVEUPDATE_IOCTL_RETRIEVE_SESSION
> Allows the userspace agent in the new kernel to reclaim a preserved
> session by its name, receiving a new file descriptor to manage the
> restored resources.
>
> Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>

Reviewed-by: Pratyush Yadav <pratyush@kernel.org>

[...]

-- 
Regards,
Pratyush Yadav

^ permalink raw reply

* Re: [PATCH v6 04/20] liveupdate: luo_session: add sessions support
From: Pratyush Yadav @ 2025-11-21 16:32 UTC (permalink / raw)
  To: Pasha Tatashin
  Cc: pratyush, jasonmiu, graf, rppt, dmatlack, rientjes, corbet,
	rdunlap, ilpo.jarvinen, kanie, ojeda, aliceryhl, masahiroy, akpm,
	tj, yoann.congal, mmaurer, roman.gushchin, chenridong, axboe,
	mark.rutland, jannh, vincent.guittot, hannes, dan.j.williams,
	david, joel.granados, rostedt, anna.schumaker, song, linux,
	linux-kernel, linux-doc, linux-mm, gregkh, tglx, mingo, bp,
	dave.hansen, x86, hpa, rafael, dakr, bartosz.golaszewski,
	cw00.choi, myungjoo.ham, yesanishhere, Jonathan.Cameron,
	quic_zijuhu, aleksander.lobakin, ira.weiny, andriy.shevchenko,
	leon, lukas, bhelgaas, wagi, djeffery, stuart.w.hayes, lennart,
	brauner, linux-api, linux-fsdevel, saeedm, ajayachandra, jgg,
	parav, leonro, witu, hughd, skhawaja, chrisl
In-Reply-To: <20251115233409.768044-5-pasha.tatashin@soleen.com>

On Sat, Nov 15 2025, Pasha Tatashin wrote:

> Introduce concept of "Live Update Sessions" within the LUO framework.
> LUO sessions provide a mechanism to group and manage `struct file *`
> instances (representing file descriptors) that need to be preserved
> across a kexec-based live update.
>
> Each session is identified by a unique name and acts as a container
> for file objects whose state is critical to a userspace workload, such
> as a virtual machine or a high-performance database, aiming to maintain
> their functionality across a kernel transition.
>
> This groundwork establishes the framework for preserving file-backed
> state across kernel updates, with the actual file data preservation
> mechanisms to be implemented in subsequent patches.
>
> Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
[...]
>  
>  #ifndef _LINUX_LIVEUPDATE_ABI_LUO_H
>  #define _LINUX_LIVEUPDATE_ABI_LUO_H
>  
> +#include <uapi/linux/liveupdate.h>
> +
>  /*
>   * The LUO FDT hooks all LUO state for sessions, fds, etc.
> - * In the root it allso carries "liveupdate-number" 64-bit property that
> + * In the root it also carries "liveupdate-number" 64-bit property that

Nit: This needs a bit of patch massaging. Patch 2 added the typo, and
this patch fixes it. It would be better to just update patch 2.

>   * corresponds to the number of live-updates performed on this machine.
>   */
>  #define LUO_FDT_SIZE		PAGE_SIZE
> @@ -51,4 +82,54 @@
>  #define LUO_FDT_COMPATIBLE	"luo-v1"
>  #define LUO_FDT_LIVEUPDATE_NUM	"liveupdate-number"
>  
> +/*
> + * LUO FDT session node
> + * LUO_FDT_SESSION_HEADER:  is a u64 physical address of struct
> + *                          luo_session_header_ser
> + */
> +#define LUO_FDT_SESSION_NODE_NAME	"luo-session"
> +#define LUO_FDT_SESSION_COMPATIBLE	"luo-session-v1"
> +#define LUO_FDT_SESSION_HEADER		"luo-session-header"
> +
> +/**
> + * struct luo_session_header_ser - Header for the serialized session data block.
> + * @pgcnt: The total size, in pages, of the entire preserved memory block
> + *         that this header describes.
> + * @count: The number of 'struct luo_session_ser' entries that immediately
> + *         follow this header in the memory block.
> + *
> + * This structure is located at the beginning of a contiguous block of
> + * physical memory preserved across the kexec. It provides the necessary
> + * metadata to interpret the array of session entries that follow.
> + */
> +struct luo_session_header_ser {
> +	u64 pgcnt;

Why do you need pgcnt here? Can't the size be inferred from count? And
since you use contiguous memory block, the folio will know its page
count anyway, right? The less we have in the ABI the better IMO.

Same for other structures below.

> +	u64 count;
> +} __packed;
> +
> +/**
> + * struct luo_session_ser - Represents the serialized metadata for a LUO session.
> + * @name:    The unique name of the session, copied from the `luo_session`
> + *           structure.
> + * @files:   The physical address of a contiguous memory block that holds
> + *           the serialized state of files.
> + * @pgcnt:   The number of pages occupied by the `files` memory block.
> + * @count:   The total number of files that were part of this session during
> + *           serialization. Used for iteration and validation during
> + *           restoration.
> + *
> + * This structure is used to package session-specific metadata for transfer
> + * between kernels via Kexec Handover. An array of these structures (one per
> + * session) is created and passed to the new kernel, allowing it to reconstruct
> + * the session context.
> + *
> + * If this structure is modified, LUO_SESSION_COMPATIBLE must be updated.
> + */
> +struct luo_session_ser {
> +	char name[LIVEUPDATE_SESSION_NAME_LENGTH];
> +	u64 files;
> +	u64 pgcnt;
> +	u64 count;
> +} __packed;
> +
>  #endif /* _LINUX_LIVEUPDATE_ABI_LUO_H */
[...]
> +/* Create a "struct file" for session */
> +static int luo_session_getfile(struct luo_session *session, struct file **filep)
> +{
> +	char name_buf[128];
> +	struct file *file;
> +
> +	guard(mutex)(&session->mutex);
> +	snprintf(name_buf, sizeof(name_buf), "[luo_session] %s", session->name);
> +	file = anon_inode_getfile(name_buf, &luo_session_fops, session, O_RDWR);

Nit: You can return the file directly and get rid of filep.

> +	if (IS_ERR(file))
> +		return PTR_ERR(file);
> +
> +	*filep = file;
> +
> +	return 0;
> +}
[...]
> +int __init luo_session_setup_outgoing(void *fdt_out)
> +{
> +	struct luo_session_header_ser *header_ser;
> +	u64 header_ser_pa;
> +	int err;
> +
> +	header_ser = kho_alloc_preserve(LUO_SESSION_PGCNT << PAGE_SHIFT);

Nit: The naming is a bit confusing here. At first glance I thought this
was just allocating the header, but it allocates the whole session
serialization buffer.

> +	if (IS_ERR(header_ser))
> +		return PTR_ERR(header_ser);
> +	header_ser_pa = virt_to_phys(header_ser);
> +
> +	err = fdt_begin_node(fdt_out, LUO_FDT_SESSION_NODE_NAME);
> +	err |= fdt_property_string(fdt_out, "compatible",
> +				   LUO_FDT_SESSION_COMPATIBLE);
> +	err |= fdt_property(fdt_out, LUO_FDT_SESSION_HEADER, &header_ser_pa,
> +			    sizeof(header_ser_pa));
> +	err |= fdt_end_node(fdt_out);
> +
> +	if (err)
> +		goto err_unpreserve;
> +
> +	header_ser->pgcnt = LUO_SESSION_PGCNT;
> +	INIT_LIST_HEAD(&luo_session_global.outgoing.list);
> +	init_rwsem(&luo_session_global.outgoing.rwsem);
> +	luo_session_global.outgoing.header_ser = header_ser;
> +	luo_session_global.outgoing.ser = (void *)(header_ser + 1);
> +	luo_session_global.outgoing.active = true;
> +
> +	return 0;
> +
> +err_unpreserve:
> +	kho_unpreserve_free(header_ser);
> +	return err;
> +}
[...]
> +int luo_session_deserialize(void)
> +{
> +	struct luo_session_header *sh = &luo_session_global.incoming;
> +	int err;
> +
> +	if (luo_session_is_deserialized())
> +		return 0;
> +
> +	luo_session_global.deserialized = true;
> +	if (!sh->active) {
> +		INIT_LIST_HEAD(&sh->list);
> +		init_rwsem(&sh->rwsem);

Nit: it would be a bit simpler if LUO init always initialized this. And
then luo_session_setup_incoming() can fill the list if it has any data.
Slight reduction in code duplication and mental load.

> +		return 0;
> +	}
> +
> +	for (int i = 0; i < sh->header_ser->count; i++) {
> +		struct luo_session *session;
> +
> +		session = luo_session_alloc(sh->ser[i].name);
> +		if (IS_ERR(session)) {
> +			pr_warn("Failed to allocate session [%s] during deserialization %pe\n",
> +				sh->ser[i].name, session);
> +			return PTR_ERR(session);
> +		}
> +
> +		err = luo_session_insert(sh, session); 
> +		if (err) {
> +			luo_session_free(session);
> +			pr_warn("Failed to insert session [%s] %pe\n",
> +				session->name, ERR_PTR(err));
> +			return err;
> +		}
> +
> +		session->count = sh->ser[i].count;
> +		session->files = sh->ser[i].files ? phys_to_virt(sh->ser[i].files) : 0;
> +		session->pgcnt = sh->ser[i].pgcnt;
> +	}
> +
> +	kho_restore_free(sh->header_ser);
> +	sh->header_ser = NULL;
> +	sh->ser = NULL;
> +
> +	return 0;
> +}
[...]

-- 
Regards,
Pratyush Yadav

^ permalink raw reply

* Re: [PATCH v6 03/20] kexec: call liveupdate_reboot() before kexec
From: Pratyush Yadav @ 2025-11-21 15:55 UTC (permalink / raw)
  To: Pasha Tatashin
  Cc: pratyush, jasonmiu, graf, rppt, dmatlack, rientjes, corbet,
	rdunlap, ilpo.jarvinen, kanie, ojeda, aliceryhl, masahiroy, akpm,
	tj, yoann.congal, mmaurer, roman.gushchin, chenridong, axboe,
	mark.rutland, jannh, vincent.guittot, hannes, dan.j.williams,
	david, joel.granados, rostedt, anna.schumaker, song, linux,
	linux-kernel, linux-doc, linux-mm, gregkh, tglx, mingo, bp,
	dave.hansen, x86, hpa, rafael, dakr, bartosz.golaszewski,
	cw00.choi, myungjoo.ham, yesanishhere, Jonathan.Cameron,
	quic_zijuhu, aleksander.lobakin, ira.weiny, andriy.shevchenko,
	leon, lukas, bhelgaas, wagi, djeffery, stuart.w.hayes, lennart,
	brauner, linux-api, linux-fsdevel, saeedm, ajayachandra, jgg,
	parav, leonro, witu, hughd, skhawaja, chrisl
In-Reply-To: <20251115233409.768044-4-pasha.tatashin@soleen.com>

On Sat, Nov 15 2025, Pasha Tatashin wrote:

> Modify the kernel_kexec() to call liveupdate_reboot().
>
> This ensures that the Live Update Orchestrator is notified just
> before the kernel executes the kexec jump. The liveupdate_reboot()
> function triggers the final freeze event, allowing participating
> FDs perform last-minute check or state saving within the blackout
> window.
>
> If liveupdate_reboot() returns an error (indicating a failure during
> LUO finalization), the kexec operation is aborted to prevent proceeding
> with an inconsistent state. An error is returned to user.
>
> Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>

Reviewed-by: Pratyush Yadav <pratyush@kernel.org>

[...]

-- 
Regards,
Pratyush Yadav

^ permalink raw reply

* Re: [PATCH v4 1/3] init: remove deprecated "load_ramdisk" and "prompt_ramdisk" command line parameters
From: Randy Dunlap @ 2025-11-20 23:44 UTC (permalink / raw)
  To: Askar Safin, linux-fsdevel, linux-kernel
  Cc: Linus Torvalds, Greg Kroah-Hartman, Christian Brauner, Al Viro,
	Jan Kara, Christoph Hellwig, Jens Axboe, Andy Shevchenko,
	Aleksa Sarai, Thomas Weißschuh, Julian Stecklina, Gao Xiang,
	Art Nikpal, Andrew Morton, Alexander Graf, Rob Landley,
	Lennart Poettering, linux-arch, linux-block, initramfs, linux-api,
	linux-doc, Michal Simek, Luis Chamberlain, Kees Cook,
	Thorsten Blum, Heiko Carstens, Arnd Bergmann, Dave Young,
	Christophe Leroy, Krzysztof Kozlowski, Borislav Petkov,
	Jessica Clarke, Nicolas Schichan, David Disseldorp, patches
In-Reply-To: <20251119222407.3333257-2-safinaskar@gmail.com>



On 11/19/25 2:24 PM, Askar Safin wrote:
> ...which do nothing. They were deprecated (in documentation) in
> 6b99e6e6aa62 ("Documentation/admin-guide: blockdev/ramdisk: remove use of
> "rdev"") in 2020 and in kernel messages in c8376994c86c ("initrd: remove
> support for multiple floppies") in 2020.
> 
> Signed-off-by: Askar Safin <safinaskar@gmail.com>

Reviewed-by: Randy Dunlap <rdunlap@infradead.org>

Thanks.

> ---
>  Documentation/admin-guide/kernel-parameters.txt | 4 ----
>  arch/arm/configs/neponset_defconfig             | 2 +-
>  init/do_mounts.c                                | 7 -------
>  init/do_mounts_rd.c                             | 7 -------
>  4 files changed, 1 insertion(+), 19 deletions(-)


-- 
~Randy

^ permalink raw reply

* Re: [PATCH v6 06/20] liveupdate: luo_file: implement file systems callbacks
From: Pasha Tatashin @ 2025-11-20 20:25 UTC (permalink / raw)
  To: Mike Rapoport
  Cc: pratyush, jasonmiu, graf, dmatlack, rientjes, corbet, rdunlap,
	ilpo.jarvinen, kanie, ojeda, aliceryhl, masahiroy, akpm, tj,
	yoann.congal, mmaurer, roman.gushchin, chenridong, axboe,
	mark.rutland, jannh, vincent.guittot, hannes, dan.j.williams,
	david, joel.granados, rostedt, anna.schumaker, song, linux,
	linux-kernel, linux-doc, linux-mm, gregkh, tglx, mingo, bp,
	dave.hansen, x86, hpa, rafael, dakr, bartosz.golaszewski,
	cw00.choi, myungjoo.ham, yesanishhere, Jonathan.Cameron,
	quic_zijuhu, aleksander.lobakin, ira.weiny, andriy.shevchenko,
	leon, lukas, bhelgaas, wagi, djeffery, stuart.w.hayes, ptyadav,
	lennart, brauner, linux-api, linux-fsdevel, saeedm, ajayachandra,
	jgg, parav, leonro, witu, hughd, skhawaja, chrisl
In-Reply-To: <aR9N14KWaz6SdFcw@kernel.org>

On Thu, Nov 20, 2025 at 12:20 PM Mike Rapoport <rppt@kernel.org> wrote:
>
> On Mon, Nov 17, 2025 at 12:50:56PM -0500, Pasha Tatashin wrote:
> > > > +struct liveupdate_file_handler;
> > > > +struct liveupdate_session;
> > >
> > > Why struct liveupdate_session is a part of public LUO API?
> >
> > It is an obscure version of private "struct luo_session", in order to
> > give subsystem access to:
> > liveupdate_get_file_incoming(s, token, filep)
> > liveupdate_get_token_outgoing(s, file, tokenp)
> >
> > For example, if your FD depends on another FD within a session, you
> > can check if another FD is already preserved via
> > liveupdate_get_token_outgoing(), and during retrieval time you can
> > retrieve the "struct file" for your dependency.
>
> And it's essentially unused right now.

I am going to move this API to the end of the series, next to FLB :-)

>
> > > > +     }
> > > > +
> > > > +     return 0;
> > > > +
> > > > +exit_err:
> > > > +     fput(file);
> > > > +     luo_session_free_files_mem(session);
> > >
> > > The error handling in this function is a mess. Pasha, please, please, use
> > > goto consistently.
> >
> > How is this a mess? There is a single exit_err destination, no
> > exception, no early returns except at the very top of the function
> > where we do early returns before fget() which makes total sense.
> >
> > Do you want to add a separate destination for
> > luo_session_free_files_mem() ? But that is not necessary, in many
> > places it is considered totally reasonable for free(NULL) to work
> > correctly...
>
> You have a mix of releasing resources with goto or inside if (err).
> And while basic free() primitives like kfree() and vfree() work correctly
> with NULL as a parameter, luo_session_free_files_mem() is already not a
> basic primitive and it may grow with a time. It already has two conditions
> that essentially prevent anything from freeing and this will grow with the
> time.
>
> So yes, I want a separate goto destination for freeing each resource and a
> goto for
>
>         err = fh->ops->preserve(&args);
>         if (err)

Thanks, I made the change.

>
> case.
>
> > > > +             luo_file = kzalloc(sizeof(*luo_file), GFP_KERNEL);
> > > > +             if (!luo_file)
> > > > +                     return -ENOMEM;
> > >
> > > Shouldn't we free files allocated on the previous iterations?
> >
> > No, for the same reason explained in luo_session.c :-)
>
> A comment here as well please :)

Done

>
> > > > +int liveupdate_get_file_incoming(struct liveupdate_session *s, u64 token,
> > > > +                              struct file **filep)
> > > > +{
> > >
> > > Ditto.
> >
> > These two functions are part of the public API allowing dependency
> > tracking for vfio->iommu->memfd during preservation.
>
> So like with FLB, until we get actual users for them they are dead code.
> And until it's clear how exactly dependency tracking for vfio->iommu->memfd
> will work, we won't know if this API is useful at all or we'll need
> something else in the end.

SGTM

>
> --
> Sincerely yours,
> Mike.

^ permalink raw reply

* Re: [PATCH v6 05/20] liveupdate: luo_ioctl: add user interface
From: Pasha Tatashin @ 2025-11-20 20:13 UTC (permalink / raw)
  To: David Matlack
  Cc: pratyush, jasonmiu, graf, rppt, rientjes, corbet, rdunlap,
	ilpo.jarvinen, kanie, ojeda, aliceryhl, masahiroy, akpm, tj,
	yoann.congal, mmaurer, roman.gushchin, chenridong, axboe,
	mark.rutland, jannh, vincent.guittot, hannes, dan.j.williams,
	david, joel.granados, rostedt, anna.schumaker, song, linux,
	linux-kernel, linux-doc, linux-mm, gregkh, tglx, mingo, bp,
	dave.hansen, x86, hpa, rafael, dakr, bartosz.golaszewski,
	cw00.choi, myungjoo.ham, yesanishhere, Jonathan.Cameron,
	quic_zijuhu, aleksander.lobakin, ira.weiny, andriy.shevchenko,
	leon, lukas, bhelgaas, wagi, djeffery, stuart.w.hayes, ptyadav,
	lennart, brauner, linux-api, linux-fsdevel, saeedm, ajayachandra,
	jgg, parav, leonro, witu, hughd, skhawaja, chrisl
In-Reply-To: <CALzav=dmFQr+BrqzRDgio0q68MPRVnZPK4-wUXVj47o1FObgNg@mail.gmail.com>

On Thu, Nov 20, 2025 at 2:43 PM David Matlack <dmatlack@google.com> wrote:
>
> On Thu, Nov 20, 2025 at 11:23 AM Pasha Tatashin
> <pasha.tatashin@soleen.com> wrote:
> >
> > On Thu, Nov 20, 2025 at 1:38 PM David Matlack <dmatlack@google.com> wrote:
> > >
> > > On Sat, Nov 15, 2025 at 3:34 PM Pasha Tatashin
> > > <pasha.tatashin@soleen.com> wrote:
> > > > The idea is that there is going to be a single userspace agent driving
> > > > the live update, therefore, only a single process can ever hold this
> > > > device opened at a time.
> > > ...
> > > > +static int luo_open(struct inode *inodep, struct file *filep)
> > > > +{
> > > > +       struct luo_device_state *ldev = container_of(filep->private_data,
> > > > +                                                    struct luo_device_state,
> > > > +                                                    miscdev);
> > > > +
> > > > +       if (atomic_cmpxchg(&ldev->in_use, 0, 1))
> > > > +               return -EBUSY;
> > >
> > > Can you remind me why the kernel needs to enforce this? What would be
> > > wrong or unsafe from the kernel perspective if there were multiple
> > > userspace agents holding open files for /dev/liveupdate, each with
> > > their own sessions?
> >
> > By enforcing a singleton, we will ensure a consistent view for tooling
> > like luoadm (which will track incoming/outgoing sessions, UUIDs, etc.)
> > and prevent conflicting commands regarding the transition state.
> >
> > This is not a bottleneck because the vast majority of the work
> > (preserving devicse/memory) is handled via the individual Session FDs.
> > Also, since sessions persist even if /dev/liveupdate is closed, we
> > allow the agent upgrade, or crashing without requiring concurrent
> > access.
>
> Yeah, I'm not concerned about bottlenecking. It just seems like an
> artificial constraint to impose on userspace at this point. The only
> ioctls on /dev/liveupdate are to create a session and retreive a
> session. Neither of those will conflict with having multiple open
> files for /dev/liveupdate.

Enforcing tooling consistency, and improving security for global
state. Otherwise, it can be relaxed.

Pasha

^ permalink raw reply

* Re: [PATCH v6 05/20] liveupdate: luo_ioctl: add user interface
From: David Matlack @ 2025-11-20 19:42 UTC (permalink / raw)
  To: Pasha Tatashin
  Cc: pratyush, jasonmiu, graf, rppt, rientjes, corbet, rdunlap,
	ilpo.jarvinen, kanie, ojeda, aliceryhl, masahiroy, akpm, tj,
	yoann.congal, mmaurer, roman.gushchin, chenridong, axboe,
	mark.rutland, jannh, vincent.guittot, hannes, dan.j.williams,
	david, joel.granados, rostedt, anna.schumaker, song, linux,
	linux-kernel, linux-doc, linux-mm, gregkh, tglx, mingo, bp,
	dave.hansen, x86, hpa, rafael, dakr, bartosz.golaszewski,
	cw00.choi, myungjoo.ham, yesanishhere, Jonathan.Cameron,
	quic_zijuhu, aleksander.lobakin, ira.weiny, andriy.shevchenko,
	leon, lukas, bhelgaas, wagi, djeffery, stuart.w.hayes, ptyadav,
	lennart, brauner, linux-api, linux-fsdevel, saeedm, ajayachandra,
	jgg, parav, leonro, witu, hughd, skhawaja, chrisl
In-Reply-To: <CA+CK2bD4Y3CMHcTGKradmv-hAbdtA7zsw2CYeh7-8LNianYMZw@mail.gmail.com>

On Thu, Nov 20, 2025 at 11:23 AM Pasha Tatashin
<pasha.tatashin@soleen.com> wrote:
>
> On Thu, Nov 20, 2025 at 1:38 PM David Matlack <dmatlack@google.com> wrote:
> >
> > On Sat, Nov 15, 2025 at 3:34 PM Pasha Tatashin
> > <pasha.tatashin@soleen.com> wrote:
> > > The idea is that there is going to be a single userspace agent driving
> > > the live update, therefore, only a single process can ever hold this
> > > device opened at a time.
> > ...
> > > +static int luo_open(struct inode *inodep, struct file *filep)
> > > +{
> > > +       struct luo_device_state *ldev = container_of(filep->private_data,
> > > +                                                    struct luo_device_state,
> > > +                                                    miscdev);
> > > +
> > > +       if (atomic_cmpxchg(&ldev->in_use, 0, 1))
> > > +               return -EBUSY;
> >
> > Can you remind me why the kernel needs to enforce this? What would be
> > wrong or unsafe from the kernel perspective if there were multiple
> > userspace agents holding open files for /dev/liveupdate, each with
> > their own sessions?
>
> By enforcing a singleton, we will ensure a consistent view for tooling
> like luoadm (which will track incoming/outgoing sessions, UUIDs, etc.)
> and prevent conflicting commands regarding the transition state.
>
> This is not a bottleneck because the vast majority of the work
> (preserving devicse/memory) is handled via the individual Session FDs.
> Also, since sessions persist even if /dev/liveupdate is closed, we
> allow the agent upgrade, or crashing without requiring concurrent
> access.

Yeah, I'm not concerned about bottlenecking. It just seems like an
artificial constraint to impose on userspace at this point. The only
ioctls on /dev/liveupdate are to create a session and retreive a
session. Neither of those will conflict with having multiple open
files for /dev/liveupdate.

^ permalink raw reply

* Re: [PATCH v6 05/20] liveupdate: luo_ioctl: add user interface
From: Pasha Tatashin @ 2025-11-20 19:22 UTC (permalink / raw)
  To: David Matlack
  Cc: pratyush, jasonmiu, graf, rppt, rientjes, corbet, rdunlap,
	ilpo.jarvinen, kanie, ojeda, aliceryhl, masahiroy, akpm, tj,
	yoann.congal, mmaurer, roman.gushchin, chenridong, axboe,
	mark.rutland, jannh, vincent.guittot, hannes, dan.j.williams,
	david, joel.granados, rostedt, anna.schumaker, song, linux,
	linux-kernel, linux-doc, linux-mm, gregkh, tglx, mingo, bp,
	dave.hansen, x86, hpa, rafael, dakr, bartosz.golaszewski,
	cw00.choi, myungjoo.ham, yesanishhere, Jonathan.Cameron,
	quic_zijuhu, aleksander.lobakin, ira.weiny, andriy.shevchenko,
	leon, lukas, bhelgaas, wagi, djeffery, stuart.w.hayes, ptyadav,
	lennart, brauner, linux-api, linux-fsdevel, saeedm, ajayachandra,
	jgg, parav, leonro, witu, hughd, skhawaja, chrisl
In-Reply-To: <CALzav=c-KJg8q8-4EaDC1M+GErTCiRKtn5qRbh1wa08zJ0N4ng@mail.gmail.com>

On Thu, Nov 20, 2025 at 1:38 PM David Matlack <dmatlack@google.com> wrote:
>
> On Sat, Nov 15, 2025 at 3:34 PM Pasha Tatashin
> <pasha.tatashin@soleen.com> wrote:
> > The idea is that there is going to be a single userspace agent driving
> > the live update, therefore, only a single process can ever hold this
> > device opened at a time.
> ...
> > +static int luo_open(struct inode *inodep, struct file *filep)
> > +{
> > +       struct luo_device_state *ldev = container_of(filep->private_data,
> > +                                                    struct luo_device_state,
> > +                                                    miscdev);
> > +
> > +       if (atomic_cmpxchg(&ldev->in_use, 0, 1))
> > +               return -EBUSY;
>
> Can you remind me why the kernel needs to enforce this? What would be
> wrong or unsafe from the kernel perspective if there were multiple
> userspace agents holding open files for /dev/liveupdate, each with
> their own sessions?

By enforcing a singleton, we will ensure a consistent view for tooling
like luoadm (which will track incoming/outgoing sessions, UUIDs, etc.)
and prevent conflicting commands regarding the transition state.

This is not a bottleneck because the vast majority of the work
(preserving devicse/memory) is handled via the individual Session FDs.
Also, since sessions persist even if /dev/liveupdate is closed, we
allow the agent upgrade, or crashing without requiring concurrent
access.

Pasha

^ permalink raw reply

* Re: [PATCH v6 08/20] liveupdate: luo_flb: Introduce File-Lifecycle-Bound global state
From: Pasha Tatashin @ 2025-11-20 19:10 UTC (permalink / raw)
  To: Mike Rapoport
  Cc: pratyush, jasonmiu, graf, dmatlack, rientjes, corbet, rdunlap,
	ilpo.jarvinen, kanie, ojeda, aliceryhl, masahiroy, akpm, tj,
	yoann.congal, mmaurer, roman.gushchin, chenridong, axboe,
	mark.rutland, jannh, vincent.guittot, hannes, dan.j.williams,
	david, joel.granados, rostedt, anna.schumaker, song, linux,
	linux-kernel, linux-doc, linux-mm, gregkh, tglx, mingo, bp,
	dave.hansen, x86, hpa, rafael, dakr, bartosz.golaszewski,
	cw00.choi, myungjoo.ham, yesanishhere, Jonathan.Cameron,
	quic_zijuhu, aleksander.lobakin, ira.weiny, andriy.shevchenko,
	leon, lukas, bhelgaas, wagi, djeffery, stuart.w.hayes, ptyadav,
	lennart, brauner, linux-api, linux-fsdevel, saeedm, ajayachandra,
	jgg, parav, leonro, witu, hughd, skhawaja, chrisl
In-Reply-To: <aR9i9SXGDQ6bi1mi@kernel.org>

On Thu, Nov 20, 2025 at 1:50 PM Mike Rapoport <rppt@kernel.org> wrote:
>
> On Tue, Nov 18, 2025 at 10:37:30AM -0500, Pasha Tatashin wrote:
> > On Tue, Nov 18, 2025 at 6:28 AM Mike Rapoport <rppt@kernel.org> wrote:
> > >
> > > On Mon, Nov 17, 2025 at 10:54:29PM -0500, Pasha Tatashin wrote:
> > > > >
> > > > > The concept makes sense to me, but it's hard to review the implementation
> > > > > without an actual user.
> > > >
> > > > There are three users: we will have HugeTLB support that is going to
> > > > be posted as RFC in a few weeks. Also, in two weeks we are going to
> > > > have an updated VFIO and IOMMU series posted both using FLBs. In the
> > > > mean time, this series provides an FLB in-kernel test that verifies
> > > > that multiple FLBs can be attached to File-Handlers, and the basic
> > > > interfaces are working.
> > >
> > > Which means that essentially there won't be a real kernel user for FLB for
> > > a while.
> > > We usually don't merge dead code because some future patchset depends on
> > > it.
> >
> > I understand the concern. I would prefer to merge FLB with the rest of
> > the LUO series; I don't view it as completely dead code since I have
> > added the in-kernel test that specifically exercises and validates
> > this API.
>
> The test exercises a simple happy flow, but it still does not validate that
> this API is what we'll be using in the end.
> It's quite probable that the first upstream user of FLB will use this exact
> API, but chances are that it will require adjustments to "the real life".
>
> It does look sane, but without an actual user (sorry, but the test does not
> count) it's hard to anticipate the potential required changes and potential
> corner cases.
>
> Let's hold FLB until it can be actually consumed by HugeTLB or VFIO or
> IOMMU.

Ok

> > > I think it should stay in mm-nonmm-unstable if Andrew does not mind keeping
> > > it there until the first user is going to land and then FLB will move
> > > upstream along with that user.
> >
> > My reasoning for pushing for inclusion now is that there are many
> > developers who currently depend on the FLB functionality. Having it in
> > a public tree, preferably upstream, or at least linux-next, would be
> > highly beneficial for their development and testing.
> >
> > However, to avoid blocking the entire series, I am going to move the
> > FLB patch and the in-kernel test patch to be the last two patches in
> > LUOv7.
> >
> > This way, the rest of the LUO series can be merged without them if
> > they are blocked, however, in this case it would be best if the two
> > FLB patches stayed in mm tree to allow VFIO/IOMMU/PCI/HugeTLB
> > preservation developers to use them, as they all depend on functional
> > FLB.
>
> That's pretty much what I'm suggesting just without "if they are blocked" :)

SGTM

>
> > Pasha
>
> --
> Sincerely yours,
> Mike.
>

^ permalink raw reply

* Re: [PATCH v6 08/20] liveupdate: luo_flb: Introduce File-Lifecycle-Bound global state
From: Mike Rapoport @ 2025-11-20 18:50 UTC (permalink / raw)
  To: Pasha Tatashin
  Cc: pratyush, jasonmiu, graf, dmatlack, rientjes, corbet, rdunlap,
	ilpo.jarvinen, kanie, ojeda, aliceryhl, masahiroy, akpm, tj,
	yoann.congal, mmaurer, roman.gushchin, chenridong, axboe,
	mark.rutland, jannh, vincent.guittot, hannes, dan.j.williams,
	david, joel.granados, rostedt, anna.schumaker, song, linux,
	linux-kernel, linux-doc, linux-mm, gregkh, tglx, mingo, bp,
	dave.hansen, x86, hpa, rafael, dakr, bartosz.golaszewski,
	cw00.choi, myungjoo.ham, yesanishhere, Jonathan.Cameron,
	quic_zijuhu, aleksander.lobakin, ira.weiny, andriy.shevchenko,
	leon, lukas, bhelgaas, wagi, djeffery, stuart.w.hayes, ptyadav,
	lennart, brauner, linux-api, linux-fsdevel, saeedm, ajayachandra,
	jgg, parav, leonro, witu, hughd, skhawaja, chrisl
In-Reply-To: <CA+CK2bASYtBndN24HZhkndDpsrU1rwjCokE=9eLZUq2Jhj6bag@mail.gmail.com>

On Tue, Nov 18, 2025 at 10:37:30AM -0500, Pasha Tatashin wrote:
> On Tue, Nov 18, 2025 at 6:28 AM Mike Rapoport <rppt@kernel.org> wrote:
> >
> > On Mon, Nov 17, 2025 at 10:54:29PM -0500, Pasha Tatashin wrote:
> > > >
> > > > The concept makes sense to me, but it's hard to review the implementation
> > > > without an actual user.
> > >
> > > There are three users: we will have HugeTLB support that is going to
> > > be posted as RFC in a few weeks. Also, in two weeks we are going to
> > > have an updated VFIO and IOMMU series posted both using FLBs. In the
> > > mean time, this series provides an FLB in-kernel test that verifies
> > > that multiple FLBs can be attached to File-Handlers, and the basic
> > > interfaces are working.
> >
> > Which means that essentially there won't be a real kernel user for FLB for
> > a while.
> > We usually don't merge dead code because some future patchset depends on
> > it.
> 
> I understand the concern. I would prefer to merge FLB with the rest of
> the LUO series; I don't view it as completely dead code since I have
> added the in-kernel test that specifically exercises and validates
> this API.

The test exercises a simple happy flow, but it still does not validate that
this API is what we'll be using in the end.
It's quite probable that the first upstream user of FLB will use this exact
API, but chances are that it will require adjustments to "the real life".

It does look sane, but without an actual user (sorry, but the test does not
count) it's hard to anticipate the potential required changes and potential
corner cases.

Let's hold FLB until it can be actually consumed by HugeTLB or VFIO or
IOMMU.
 
> > I think it should stay in mm-nonmm-unstable if Andrew does not mind keeping
> > it there until the first user is going to land and then FLB will move
> > upstream along with that user.
> 
> My reasoning for pushing for inclusion now is that there are many
> developers who currently depend on the FLB functionality. Having it in
> a public tree, preferably upstream, or at least linux-next, would be
> highly beneficial for their development and testing.
> 
> However, to avoid blocking the entire series, I am going to move the
> FLB patch and the in-kernel test patch to be the last two patches in
> LUOv7.
> 
> This way, the rest of the LUO series can be merged without them if
> they are blocked, however, in this case it would be best if the two
> FLB patches stayed in mm tree to allow VFIO/IOMMU/PCI/HugeTLB
> preservation developers to use them, as they all depend on functional
> FLB.

That's pretty much what I'm suggesting just without "if they are blocked" :) 
 
> Pasha

-- 
Sincerely yours,
Mike.

^ permalink raw reply

* Re: [PATCH v6 05/20] liveupdate: luo_ioctl: add user interface
From: David Matlack @ 2025-11-20 18:37 UTC (permalink / raw)
  To: Pasha Tatashin
  Cc: pratyush, jasonmiu, graf, rppt, rientjes, corbet, rdunlap,
	ilpo.jarvinen, kanie, ojeda, aliceryhl, masahiroy, akpm, tj,
	yoann.congal, mmaurer, roman.gushchin, chenridong, axboe,
	mark.rutland, jannh, vincent.guittot, hannes, dan.j.williams,
	david, joel.granados, rostedt, anna.schumaker, song, linux,
	linux-kernel, linux-doc, linux-mm, gregkh, tglx, mingo, bp,
	dave.hansen, x86, hpa, rafael, dakr, bartosz.golaszewski,
	cw00.choi, myungjoo.ham, yesanishhere, Jonathan.Cameron,
	quic_zijuhu, aleksander.lobakin, ira.weiny, andriy.shevchenko,
	leon, lukas, bhelgaas, wagi, djeffery, stuart.w.hayes, ptyadav,
	lennart, brauner, linux-api, linux-fsdevel, saeedm, ajayachandra,
	jgg, parav, leonro, witu, hughd, skhawaja, chrisl
In-Reply-To: <20251115233409.768044-6-pasha.tatashin@soleen.com>

On Sat, Nov 15, 2025 at 3:34 PM Pasha Tatashin
<pasha.tatashin@soleen.com> wrote:
> The idea is that there is going to be a single userspace agent driving
> the live update, therefore, only a single process can ever hold this
> device opened at a time.
...
> +static int luo_open(struct inode *inodep, struct file *filep)
> +{
> +       struct luo_device_state *ldev = container_of(filep->private_data,
> +                                                    struct luo_device_state,
> +                                                    miscdev);
> +
> +       if (atomic_cmpxchg(&ldev->in_use, 0, 1))
> +               return -EBUSY;

Can you remind me why the kernel needs to enforce this? What would be
wrong or unsafe from the kernel perspective if there were multiple
userspace agents holding open files for /dev/liveupdate, each with
their own sessions?

^ permalink raw reply

* Re: [PATCH v6 06/20] liveupdate: luo_file: implement file systems callbacks
From: Mike Rapoport @ 2025-11-20 17:20 UTC (permalink / raw)
  To: Pasha Tatashin
  Cc: pratyush, jasonmiu, graf, dmatlack, rientjes, corbet, rdunlap,
	ilpo.jarvinen, kanie, ojeda, aliceryhl, masahiroy, akpm, tj,
	yoann.congal, mmaurer, roman.gushchin, chenridong, axboe,
	mark.rutland, jannh, vincent.guittot, hannes, dan.j.williams,
	david, joel.granados, rostedt, anna.schumaker, song, linux,
	linux-kernel, linux-doc, linux-mm, gregkh, tglx, mingo, bp,
	dave.hansen, x86, hpa, rafael, dakr, bartosz.golaszewski,
	cw00.choi, myungjoo.ham, yesanishhere, Jonathan.Cameron,
	quic_zijuhu, aleksander.lobakin, ira.weiny, andriy.shevchenko,
	leon, lukas, bhelgaas, wagi, djeffery, stuart.w.hayes, ptyadav,
	lennart, brauner, linux-api, linux-fsdevel, saeedm, ajayachandra,
	jgg, parav, leonro, witu, hughd, skhawaja, chrisl
In-Reply-To: <CA+CK2bBFS754hdPfNAkMp_PqNpOB2nY02OkWbhRdoUiZ+ah=jw@mail.gmail.com>

On Mon, Nov 17, 2025 at 12:50:56PM -0500, Pasha Tatashin wrote:
> > > +struct liveupdate_file_handler;
> > > +struct liveupdate_session;
> >
> > Why struct liveupdate_session is a part of public LUO API?
> 
> It is an obscure version of private "struct luo_session", in order to
> give subsystem access to:
> liveupdate_get_file_incoming(s, token, filep)
> liveupdate_get_token_outgoing(s, file, tokenp)
> 
> For example, if your FD depends on another FD within a session, you
> can check if another FD is already preserved via
> liveupdate_get_token_outgoing(), and during retrieval time you can
> retrieve the "struct file" for your dependency.
 
And it's essentially unused right now.

> > > +     }
> > > +
> > > +     return 0;
> > > +
> > > +exit_err:
> > > +     fput(file);
> > > +     luo_session_free_files_mem(session);
> >
> > The error handling in this function is a mess. Pasha, please, please, use
> > goto consistently.
> 
> How is this a mess? There is a single exit_err destination, no
> exception, no early returns except at the very top of the function
> where we do early returns before fget() which makes total sense.
> 
> Do you want to add a separate destination for
> luo_session_free_files_mem() ? But that is not necessary, in many
> places it is considered totally reasonable for free(NULL) to work
> correctly...

You have a mix of releasing resources with goto or inside if (err).
And while basic free() primitives like kfree() and vfree() work correctly
with NULL as a parameter, luo_session_free_files_mem() is already not a
basic primitive and it may grow with a time. It already has two conditions
that essentially prevent anything from freeing and this will grow with the
time.

So yes, I want a separate goto destination for freeing each resource and a
goto for 

	err = fh->ops->preserve(&args);
	if (err)

case.

> > > +             luo_file = kzalloc(sizeof(*luo_file), GFP_KERNEL);
> > > +             if (!luo_file)
> > > +                     return -ENOMEM;
> >
> > Shouldn't we free files allocated on the previous iterations?
> 
> No, for the same reason explained in luo_session.c :-)

A comment here as well please :)

> > > +int liveupdate_get_file_incoming(struct liveupdate_session *s, u64 token,
> > > +                              struct file **filep)
> > > +{
> >
> > Ditto.
> 
> These two functions are part of the public API allowing dependency
> tracking for vfio->iommu->memfd during preservation.

So like with FLB, until we get actual users for them they are dead code. 
And until it's clear how exactly dependency tracking for vfio->iommu->memfd
will work, we won't know if this API is useful at all or we'll need
something else in the end.

-- 
Sincerely yours,
Mike.

^ permalink raw reply

* Re: [PATCH v6 15/20] mm: memfd_luo: allow preserving memfd
From: Pratyush Yadav @ 2025-11-20 15:34 UTC (permalink / raw)
  To: Pasha Tatashin
  Cc: Mike Rapoport, pratyush, jasonmiu, graf, dmatlack, rientjes,
	corbet, rdunlap, ilpo.jarvinen, kanie, ojeda, aliceryhl,
	masahiroy, akpm, tj, yoann.congal, mmaurer, roman.gushchin,
	chenridong, axboe, mark.rutland, jannh, vincent.guittot, hannes,
	dan.j.williams, david, joel.granados, rostedt, anna.schumaker,
	song, linux, linux-kernel, linux-doc, linux-mm, gregkh, tglx,
	mingo, bp, dave.hansen, x86, hpa, rafael, dakr,
	bartosz.golaszewski, cw00.choi, myungjoo.ham, yesanishhere,
	Jonathan.Cameron, quic_zijuhu, aleksander.lobakin, ira.weiny,
	andriy.shevchenko, leon, lukas, bhelgaas, wagi, djeffery,
	stuart.w.hayes, lennart, brauner, linux-api, linux-fsdevel,
	saeedm, ajayachandra, jgg, parav, leonro, witu, hughd, skhawaja,
	chrisl
In-Reply-To: <CA+CK2bADcVsRnovkwWftPCbubXoaFrPzSavMU+G9f3XAz3YMLQ@mail.gmail.com>

On Wed, Nov 19 2025, Pasha Tatashin wrote:

> On Mon, Nov 17, 2025 at 6:04 AM Mike Rapoport <rppt@kernel.org> wrote:
>>
>> On Sat, Nov 15, 2025 at 06:34:01PM -0500, Pasha Tatashin wrote:
>> > From: Pratyush Yadav <ptyadav@amazon.de>
>> >
>> > The ability to preserve a memfd allows userspace to use KHO and LUO to
>> > transfer its memory contents to the next kernel. This is useful in many
>> > ways. For one, it can be used with IOMMUFD as the backing store for
>> > IOMMU page tables. Preserving IOMMUFD is essential for performing a
>> > hypervisor live update with passthrough devices. memfd support provides
>> > the first building block for making that possible.
>> >
>> > For another, applications with a large amount of memory that takes time
>> > to reconstruct, reboots to consume kernel upgrades can be very
>> > expensive. memfd with LUO gives those applications reboot-persistent
>> > memory that they can use to quickly save and reconstruct that state.
>> >
>> > While memfd is backed by either hugetlbfs or shmem, currently only
>> > support on shmem is added. To be more precise, support for anonymous
>> > shmem files is added.
>> >
>> > The handover to the next kernel is not transparent. All the properties
>> > of the file are not preserved; only its memory contents, position, and
>> > size. The recreated file gets the UID and GID of the task doing the
>> > restore, and the task's cgroup gets charged with the memory.
>> >
>> > Once preserved, the file cannot grow or shrink, and all its pages are
>> > pinned to avoid migrations and swapping. The file can still be read from
>> > or written to.
>> >
>> > Use vmalloc to get the buffer to hold the folios, and preserve
>> > it using kho_preserve_vmalloc(). This doesn't have the size limit.
>> >
>> > Co-developed-by: Pasha Tatashin <pasha.tatashin@soleen.com>
>> > Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
>> > Signed-off-by: Pratyush Yadav <ptyadav@amazon.de>
[...]
>> > +     struct inode *inode = file_inode(file);
>> > +     struct memfd_luo_folio_ser *pfolios;
>> > +     struct kho_vmalloc *kho_vmalloc;
>> > +     unsigned int max_folios;
>> > +     long i, size, nr_pinned;
>> > +     struct folio **folios;
>>
>> pfolios and folios read like the former is a pointer to latter.
>> I'd s/pfolios/folios_ser/

folios_ser is a tricky name, it is very close to folio_ser (which is
what you might use for one member of the array).

I was bit by this when hacking on some hugetlb preservation code. I
wrote folios_ser instead of folio_ser in a loop, and then had to spend
half an hour trying to figure out why the code wasn't working. It is
kinda hard to differentiate between the two visually.

Not that I have a better name off the top of my head. Just saying that
this naming causes weird readability problems.

>
> Done
>
[...]

-- 
Regards,
Pratyush Yadav

^ permalink raw reply

* Re: RFC: Serial port DTR/RTS - O_<something>
From: Ned Ulbricht @ 2025-11-20 13:31 UTC (permalink / raw)
  To: H. Peter Anvin, Maciej W. Rozycki
  Cc: Greg KH, Theodore Ts'o, Maarten Brock,
	linux-serial@vger.kernel.org, linux-api@vger.kernel.org, LKML
In-Reply-To: <f643f1f6-7e69-4be6-ac8a-7b1a3a9c402d@zytor.com>

On 11/18/25 10:05, H. Peter Anvin wrote:

>> "(O_EXCL|O_NOFOLLOW)" provokes a thought...
>>
>> As essential context, fs/open.c build_open_flags() has:
>>
>> if (flags & O_CREAT) {
>>      op->intent |= LOOKUP_CREATE;
>>      if (flags & O_EXCL) {
>>          op->intent |= LOOKUP_EXCL;
>>          flags |= O_NOFOLLOW;
>>      }
>> }

[snip]

> I had missed the bit in the spec that says that O_CREAT|O_EXCL is required to
> imply O_NOFOLLOW (as Linux indeed does as seen above.)

Fwiw, earlier today I had an ultimately unsuccessful series of searches
using the Austin Group Issue tracker at:

https://austingroupbugs.net/view_all_bug_page.php

Searched (serially): "O_EXCL", "O_NOFOLLOW", "EEXIST", "ELOOP"; all with
no other filter refinements.  Then searched filtering by 'Section'
(multiple adjacent selections): 'open' .. 'openat'. In all results,
simply eyeball-scanned 'Summary' (w/o opening most).

Apparent upshot, unless I'm mistaken, is that the exact error return is
a trivial conflict with no appreciable impact on higher levels.


In roughly same vein, FreeBSD open(2) man page, specifically at
"[EMLINK]" and "STANDARDS", might possibly be stretched to read as
implicitly encouraging that assessment.

https://man.freebsd.org/cgi/man.cgi?query=open&manpath=FreeBSD+16.0-CURRENT

Alhough I don't have a FreeBSD box available to actually test
(O_CREAT|O_EXCL|O_NOFOLLOW) symlink behavior on that platform.  (Maybe
that combo's wired to detonate tnt nasal daemons? Dunno;-)


Unfortunately, this does prompt a close re-scrutinization of linux's
open(2) man page. Notwithstanding the damn spec, the linux man page
should precisely and accurately reflect the observed error return?


Ned

^ permalink raw reply

* Re: Safety of resolving untrusted paths with detached mount dirfd
From: Aleksa Sarai @ 2025-11-20  9:24 UTC (permalink / raw)
  To: Demi Marie Obenour
  Cc: Alyssa Ross, linux-fsdevel, Jann Horn, Eric W. Biederman, jlayton,
	Bruce Fields, Al Viro, Arnd Bergmann, shuah, David Howells,
	Andy Lutomirski, Christian Brauner, Tycho Andersen, linux-kernel,
	linux-api
In-Reply-To: <cdf9deb2-7a09-48c5-97e2-2ea6d5901882@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2982 bytes --]

On 2025-11-19, Demi Marie Obenour <demiobenour@gmail.com> wrote:
> On 11/19/25 21:18, Aleksa Sarai wrote:
> > On 2025-11-19, Alyssa Ross <hi@alyssa.is> wrote:
> >> Hello,
> >>
> >> As we know, it's not safe to use chroot() for resolving untrusted paths
> >> within some root, as a subdirectory could be moved outside of the
> >> process root while walking the path[1].  On the other hand,
> >> LOOKUP_BENEATH is supposed to be robust against this, and going by [2],
> >> it sounds like resolving with the mount namespace root as dirfd should
> >> also be.
> >>
> >> My question is: would resolving an untrusted path against a detached
> >> mount root dirfd opened with OPEN_TREE_CLONE (not necessarily a
> >> filesystem root) also be expected to be robust against traversal issues?
> >> i.e. can I rely on an untrusted path never resolving to a path that
> >> isn't under the mount root?
> > 
> > No, if you hit an absolute symlink or use an absolute path it will
> > resolve to your current->fs->root (mount namespace root or chroot).
> > However, OPEN_TREE_CLONE will stop ".." from naively stepping out of the
> > detached bind-mount. If you are dealing with procfs then magic-links can
> > also jump out.
> 
> Is using open_tree_attr() with MOUNT_ATTR_NOSYMFOLLOW enough to prevent
> these?  Will it still provide protection even if someone concurrently
> renames one of the files out from under the root?  I know that can
> escape a chroot, but I wonder if this provides more guarantees.

That will block symlinks (in a similar manner to RESOLVE_NO_SYMLINKS),
so those particular problems would not be an issue. Of course, a lot of
symlink usages are valid and so this will block those as well (back when
I wrote openat2 I did a cursory scan and something like 15% of system
paths contained symlinks on my system).

I think that ".." will not be a problem even with renames because the
detached mount is associated with the directory (just like how moving a
bind-mount source doesn't suddenly expose more information).

It also goes without saying that you need to make sure an absolute path
*never* gets passed to any of the helper functions you write to do this
-- in my view this is usually going to be quite a fragile setup. Who is
providing the paths to your program?

> https://github.com/QubesOS/qubes-secpack/blob/main/QSBs/qsb-014-2015.txt
> was the chroot breakout.
> 
> > You can always use RESOLVE_BENEATH or RESOLVE_IN_ROOT in combination
> > with OPEN_TREE_CLONE.
>
> Unfortunately not everything supports that.  For instance, mkdirat()
> doesn't.

You can openat2(RESOLVE_BENEATH) the parent directory and then mkdirat()
the final component (because mkdirat does not follow trailing symlinks).
This is what libpathrs[1] does, and it works for most *at() syscalls
(those that support AT_EMPTY_PATH are even easier).

[1]: https://github.com/cyphar/libpathrs

-- 
Aleksa Sarai
https://www.cyphar.com/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 265 bytes --]

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox