Package: ifupdown
Version: 0.7.54ubuntu1.3

The following crash is easily reproducible on Ubuntu/15.10:
"[    2.091111] ifquery[617]: segfault at 0 ip 00007f84bb722327 sp 
00007ffde43a0488 error 4 in libc-2.21.so[7f84bb5dd000+1c0000]"

Checking the corresponding core dump shows the problem:

$ gdb --core /var/crash/ifquery-617-11.core /sbin/ifquery
...
Core was generated by `ifquery --state eno1'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  __strncmp_sse42 () at ../sysdeps/x86_64/multiarch/strcmp-sse42.S:235
235     ../sysdeps/x86_64/multiarch/strcmp-sse42.S: No such file or directory.
(gdb) bt
#0  __strncmp_sse42 () at ../sysdeps/x86_64/multiarch/strcmp-sse42.S:235
#1  0x0000000000403381 in main (argc=<optimized out>, argv=<optimized out>) at 
main.c:630
(gdb) l
230     in ../sysdeps/x86_64/multiarch/strcmp-sse42.S
(gdb) f 1
#1  0x0000000000403381 in main (argc=<optimized out>, argv=<optimized out>) at 
main.c:630
warning: Source file is more recent than executable.
630                                             if (strncmp(target_iface[j], 
up_ifaces[i], l) == 0) {
(gdb) l
625                             for (int j = 0; j < n_target_ifaces; j++) {
626                                     size_t l = strlen(target_iface[j]);
627                                     bool found = false;
628     
629                                     for (int i = 0; i < n_up_ifaces; i++) {
630                                             if (strncmp(target_iface[j], 
up_ifaces[i], l) == 0) {
631                                                     if (up_ifaces[i][l] == 
'=') {
632                                                             
puts(up_ifaces[i]);
633                                                             found = true;
634                                                             break;
(gdb) p n_target_ifaces
$1 = 1
(gdb) p j
$2 = 0
(gdb) p target_iface[j]
$3 = 0x7ffde43a1f7a "eno1"
(gdb) p n_up_ifaces
$4 = 4
(gdb) p i
$5 = 0
(gdb) p up_ifaces[i]
$6 = 0x0
(gdb) p l
$7 = 4

So read_all_state() leaves uninitialized values in up_ifaces and
n_up_ifaces if the interface state file doesn't exist which leads to
strnmp segfaulting due to the invalid pointer passed to it. On my Ubuntu
15.10 system the state file doesn't exist yet when ifquery runs and so I
hit this problem easily. There doesn't seem to be any further issues
related to this though, the state file gets created eventually and the
network comes up fine.

I attached a patch that fixes this and gets rid of the boot time
segfault.

--Imre
>From 1fcf1cd2bf0a92ff87ad7a44b3c7dbb532be5669 Mon Sep 17 00:00:00 2001
From: Imre Deak <imre.d...@gmail.com>
Date: Sat, 26 Mar 2016 03:20:38 +0200
Subject: [PATCH] Fix read_all_state when no statefile exists

When no statefile exists read_all_state() may return with the interface
array and number of array entries being uninitialized. In case the
number of array entries happens to be non-zero the caller will access an
invalid pointer. This resulted in ifquery segfaulting during booting on
Ubuntu 15.10 while running ifquery at a time when the
/var/run/network/ifstate file didn't exist yet.

Fix this by making sure read_all_state() always initializes its return
values.

Signed-off-by: Imre Deak <imre.d...@gmail.com>
---
 debian/changelog | 6 ++++++
 main.c           | 6 +++---
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/debian/changelog b/debian/changelog
index a03d062..b30acf0 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+ifupdown (0.7.54ubuntu1.3ideak1) UNRELEASED; urgency=medium
+
+  * Fix read_all_state when no state file exists
+
+ -- Imre Deak <imre.d...@gmail.com>  Sat, 26 Mar 2016 03:21:19 +0200
+
 ifupdown (0.7.54ubuntu1.3) wily; urgency=medium
 
   * Fix false-positive recursion detection. (LP: #1545302)
diff --git a/main.c b/main.c
index 76a7ad8..8f2dbcc 100644
--- a/main.c
+++ b/main.c
@@ -272,6 +272,9 @@ static void read_all_state(const char *argv0, char ***ifaces, int *n_ifaces) {
 	FILE *lock_fp = lock_state(argv0);
 	FILE *state_fp = fopen(statefile, no_act ? "r" : "a+");
 
+	*n_ifaces = 0;
+	*ifaces = NULL;
+
 	if (state_fp == NULL) {
 		if (!no_act) {
 			fprintf(stderr, "%s: failed to open statefile %s: %s\n", argv0, statefile, strerror(errno));
@@ -290,9 +293,6 @@ static void read_all_state(const char *argv0, char ***ifaces, int *n_ifaces) {
 		}
 	}
 
-	*n_ifaces = 0;
-	*ifaces = NULL;
-
 	char buf[80];
 	char *p;
 
-- 
2.5.0

Reply via email to