Bug#819287: Related Ubuntu bug updated

2016-03-26 Thread Imre Deak
This same issue was tracked already in Ubuntu Launchpad, I added a link
to this bug over there:
https://bugs.launchpad.net/ubuntu/+source/ifupdown/+bug/1416793?comments=all



Bug#819287: [ifupdown] ifquery crashes with segfault during boot on Ubuntu 15.10

2016-03-25 Thread Imre Deak

Package: ifupdown
Version: 0.7.54ubuntu1.3

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

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  0x00403381 in main (argc=, argv=) at 
main.c:630
(gdb) l
230 in ../sysdeps/x86_64/multiarch/strcmp-sse42.S
(gdb) f 1
#1  0x00403381 in main (argc=, argv=) 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