Re: [PATCH] [binutils/testsuite] debuginfod.exp: Improve port selection

[Aaron Merey <amerey at redhat dot com>]

On Thu, Feb 13, 2020 at 3:42 AM Andreas Schwab <schwab@suse.de> wrote:
> > +# Find an unused port
> > +set port 7999
> > +set found 0
> > +while { ! $found } {
> > +  incr port
> > +  if { $port == 65536 } {
> > +    fail "$test (no available ports)"
>
> That should be unsupported or untested, not fail.

Thanks Andreas, I've updated the patch.

Aaron

From 70266b489c43c3f7c64c477be1c997eb331d2922 Mon Sep 17 00:00:00 2001
From: Aaron Merey <amerey@redhat.com>
Date: Thu, 13 Feb 2020 10:21:50 -0500
Subject: [PATCH] [binutils/testsuite] debuginfod.exp: Improve port selection

The procedure to find an unused port is susceptible to a TOCTOU failure.
Change port finding in order to avoid this. Also use 'expect' to interact
with the server process since we now use the server's output to determine
whether a port is in use.

* binutils/testsuite/binutils-all/debuginfod.exp: Improve port selection.
---
 .../testsuite/binutils-all/debuginfod.exp     | 52 +++++++++----------
 1 file changed, 25 insertions(+), 27 deletions(-)

diff --git a/binutils/testsuite/binutils-all/debuginfod.exp b/binutils/testsuite/binutils-all/debuginfod.exp
index 7e1c6380a5..f057ad0155 100644
--- a/binutils/testsuite/binutils-all/debuginfod.exp
+++ b/binutils/testsuite/binutils-all/debuginfod.exp
@@ -72,9 +72,6 @@ if { ![binutils_assemble $srcdir/$subdir/linkdebug.s tmpdir/linkdebug.debug] } {
     fail "$test (assemble linkdebug)"
 }
 
-# Find an unused port
-set port [exec sh -c "while true; do PORT=`expr '(' \$RANDOM % 1000 ')' + 9000`; ss -atn | fgrep \":\$PORT\" || break; done; echo \$PORT"]
-
 set cache [file join [pwd] "tmpdir/.debuginfod_cache"]
 set db [file join [pwd] "tmpdir/.debuginfod.db"]
 
@@ -99,34 +96,36 @@ file delete -force $db
 set conf_objdump [binutils_run $OBJDUMP "-WK tmpdir/testprog"]
 set conf_readelf [binutils_run $READELF "-wK tmpdir/testprog"]
 
-set debuginfod_pid 0
-
-# Kill the server if we abort early
-proc sigint_handler {} {
-    global debuginfod_pid
+# Find an unused port
+set port 7999
+set found 0
+while { ! $found } {
+  incr port
+  if { $port == 65536 } {
+    untested "$test (no available ports)"
+    return
+  }
 
-    if { $debuginfod_pid != 0 } {
-        catch {exec kill -INT $debuginfod_pid}
+  spawn debuginfod -vvvv -d $db -p $port -F tmpdir/dbg
+  expect {
+    "started http server on IPv4 IPv6 port=$port" {
+      set found 1
     }
-
-    exit
-}
-
-trap sigint_handler INT
-
-# Start a debuginfod server.
-set debuginfod_pid [exec debuginfod -d $db -p $port -F tmpdir/dbg 2>/dev/null &]
-
-if { !$debuginfod_pid } {
-    fail "$test (server init)"
-    return
+    "Failed to bind to port" {
+      exec kill -INT -[exp_pid]
+      catch {close}; catch {wait -i $spawn_id}
+    }
+    timeout {
+      fail "$test (find port timeout)"
+      return
+    }
+  }
 }
 
 set metrics [list "ready 1" \
              "thread_work_total{role=\"traverse\"} 1" \
              "thread_work_pending{role=\"scan\"} 0" \
-             "thread_busy{role=\"scan\"} 0" \
-             "groom{statistic=\"buildids\"} 2"]
+             "thread_busy{role=\"scan\"} 0"]
 
 # Check server metrics to confirm init has completed.
 foreach m $metrics {
@@ -145,7 +144,8 @@ foreach m $metrics {
 
   if { $timelim == 0 } {
     fail "$test (server init timeout)"
-    catch {exec kill -INT $debuginfod_pid}
+    exec kill -INT -[exp_pid]
+    catch {close}; catch {wait -i $spawn_id}
     return
   }
 }
@@ -196,5 +196,3 @@ if { [regexp ".*DEBUGINFOD.*" $conf_readelf] } {
 } else {
     untested "$test (readelf not configured with debuginfod)"
 }
-
-catch {exec kill -INT $debuginfod_pid}
-- 
2.24.1