nspawn: avoid NULL pointer dereference

mrc0mmand · mrc0mmand · commit 825210d4e5d5 · 2023-05-16T11:27:10.000+02:00
When merging the settings we take the pointer to the array of extra devices, but don't reset the array counter to zero. This later leads to a NULL pointer dereference, where device_node_array_free() attempts to loop over a NULL pointer: + systemd-nspawn --oci-bundle=/var/lib/machines/testsuite-13.oci-bundle.Npo ../src/nspawn/nspawn-settings.c:118:29: runtime error: member access within null pointer of type 'struct DeviceNode' #0 0x4b91ee in device_node_array_free ../src/nspawn/nspawn-settings.c:118 #1 0x4ba42a in settings_free ../src/nspawn/nspawn-settings.c:161 #2 0x410b79 in settings_freep ../src/nspawn/nspawn-settings.h:249 #3 0x446ce8 in load_oci_bundle ../src/nspawn/nspawn.c:4733 #4 0x44ff42 in run ../src/nspawn/nspawn.c:5476 #5 0x455296 in main ../src/nspawn/nspawn.c:5919 #6 0x7f0cb7a4a50f in __libc_start_call_main (/lib64/libc.so.6+0x2750f) #7 0x7f0cb7a4a5c8 in __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x275c8) #8 0x40d284 in _start (/usr/bin/systemd-nspawn+0x40d284) SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../src/nspawn/nspawn-settings.c:118:29 in Also, add an appropriate assert to catch such issues in the future.
diff --git a/src/nspawn/nspawn-settings.c b/src/nspawn/nspawn-settings.c
@@ -114,6 +114,8 @@ static void free_oci_hooks(OciHook *h, size_t n) {
 void device_node_array_free(DeviceNode *node, size_t n) {
         size_t i;
 
+        assert(node || n == 0);
+
         for (i = 0; i < n; i++)
                 free(node[i].path);
 
diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
@@ -4651,6 +4651,7 @@ static int merge_settings(Settings *settings, const char *path) {
         device_node_array_free(arg_extra_nodes, arg_n_extra_nodes);
         arg_extra_nodes = TAKE_PTR(settings->extra_nodes);
         arg_n_extra_nodes = settings->n_extra_nodes;
+        settings->n_extra_nodes = 0;
 
         return 0;
 }

Original file line number	Diff line number	Diff line change
`@@ -4651,6 +4651,7 @@ static int merge_settings(Settings settings, const char path) {`
`4651`	`4651`	`device_node_array_free(arg_extra_nodes, arg_n_extra_nodes);`
`4652`	`4652`	`arg_extra_nodes = TAKE_PTR(settings->extra_nodes);`
`4653`	`4653`	`arg_n_extra_nodes = settings->n_extra_nodes;`
	`4654`	`+ settings->n_extra_nodes = 0;`
`4654`	`4655`
`4655`	`4656`	`return 0;`
`4656`	`4657`	`}`