Hello,
tried to debug it in a VM.

# apt install moc-dbgsym
$ gdb -q --args mocp
Reading symbols from mocp...Reading symbols from 
/usr/lib/debug/.build-id/a2/ee23893fc19ab8924a10916a485b2a9c478e7c.debug...done.
done.
(gdb) set width 0
(gdb) set height 0
(gdb) run
Starting program: /usr/bin/mocp 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/aarch64-linux-gnu/libthread_db.so.1".

Program received signal SIGSEGV, Segmentation fault.
memcmp () at ../sysdeps/aarch64/memcmp.S:60
60      ../sysdeps/aarch64/memcmp.S: Datei oder Verzeichnis nicht gefunden.

(gdb) bt
#0  memcmp () at ../sysdeps/aarch64/memcmp.S:60
#1  0x0000aaaaaaac29b0 in free_popt_clone (opts=0xaaaaaabebf20) at main.c:759
#2  0x0000aaaaaaac29ec in free_popt_clone (opts=0xaaaaaabebbd0) at main.c:761
#3  0x0000aaaaaaac2d28 in render_popt_command_line () at main.c:894
#4  0x0000aaaaaaabbd68 in log_popt_command_line () at main.c:1168
#5  main (argc=<optimized out>, argv=<optimized out>) at main.c:1224
(gdb) up
#1  0x0000aaaaaaac29b0 in free_popt_clone (opts=0xaaaaaabebf20) at main.c:759
759             for (ix = 0; memcmp (&opts[ix], &table_end, sizeof 
(table_end)); ix += 1) {

(gdb) print opts[0]
$1 = {longName = 0xaaaaaaafa340 "debug", shortName = 68 'D', argInfo = 0, arg = 
0x0, val = 1, descrip = 0xaaaaaaafa348 "Turn on logging to a file", argDescrip 
= 0x0}
...
(gdb) print opts[11]
$12 = {longName = 0xaaaaaaafa5e0 "nosync", shortName = 110 'n', argInfo = 0, 
arg = 0x0, val = 12, descrip = 0xaaaaaaafa5e8 "Don't synchronize the playlist 
with other clients", argDescrip = 0x0}
(gdb) print opts[12]
$13 = {longName = 0x0, shortName = 0 '\000', argInfo = 0, arg = 0x0, val = 0, 
descrip = 0x0, argDescrip = 0x0}

(gdb) print table_end
$14 = {longName = 0x0, shortName = 0 '\000', argInfo = 0, arg = 0x0, val = 0, 
descrip = 0x0, argDescrip = 0x0}

(gdb) print sizeof(opts[12])
$15 = 48

(gdb) x/48xb &opts[12]
0xaaaaaabec160: 0x00    0x00    0x00    0x00    0x00    0x00    0x00    0x00
0xaaaaaabec168: 0x00    0x00    0x00    0x00    0x00    0x00    0x00    0x00
0xaaaaaabec170: 0x00    0x00    0x00    0x00    0x00    0x00    0x00    0x00
0xaaaaaabec178: 0x00    0x00    0x00    0x00    0x00    0x00    0x00    0x00
0xaaaaaabec180: 0x00    0x00    0x00    0x00    0x00    0x00    0x00    0x00
0xaaaaaabec188: 0x00    0x00    0x00    0x00    0x00    0x00    0x00    0x00
(gdb) x/48xb &table_end
0xfffffffff1d8: 0x00    0x00    0x00    0x00    0x00    0x00    0x00    0x00
0xfffffffff1e0: 0x00    0x31    0xbd    0xaa    0x00    0x00    0x00    0x00
0xfffffffff1e8: 0x00    0x00    0x00    0x00    0x00    0x00    0x00    0x00
0xfffffffff1f0: 0x00    0x00    0x00    0x00    0xaa    0xaa    0x00    0x00
0xfffffffff1f8: 0x00    0x00    0x00    0x00    0x00    0x00    0x00    0x00
0xfffffffff200: 0x00    0x00    0x00    0x00    0x00    0x00    0x00    0x00



This looks to me like padding bytes got initialized in the opts
array, specifically element at index 12.
But in the local variable table_end the padding bytes are not 
initialized to 0.
Therefore the memcmp cannot detect equality and the loop accesses opts
beyond its end.

Attached patch adds a small helper function poptcmp to do
explicit compares to each member of the struct.

With that applied it does not crash on arm64 anymore.


Kind regards,
Bernhard
From 56b8902f7456505563e8e910747b196a41872b85 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bernhard=20=C3=83=C2=9Cbelacker?= <bernha...@mailbox.org>
Date: Fri, 26 May 2017 17:13:38 +0200
Subject: Stop iterating beyond the last option element.

https://bugs.debian.org/855125
---
 main.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/main.c b/main.c
index e0b34ba..8d7afec 100644
--- a/main.c
+++ b/main.c
@@ -682,6 +682,20 @@ static void prepend_mocp_opts (poptContext ctx)
 	}
 }
 
+static int poptcmp (const struct poptOption* a, const struct poptOption* b)
+{
+	int equal = (
+		a->longName == b->longName &&
+		a->shortName == b->shortName &&
+		a->argInfo == b->argInfo &&
+		a->arg == b->arg &&
+		a->val == b->val &&
+		a->descrip == b->descrip &&
+		a->argDescrip == b->argDescrip);
+
+	return !equal;
+}
+
 /* Return a copy of the POPT option table structure which is suitable
  * for rendering the POPT expansions of the command line. */
 #ifndef OPENWRT
@@ -696,7 +710,7 @@ struct poptOption *clone_popt_options (struct poptOption *opts)
 	assert (opts);
 
 	for (count = 1;
-	     memcmp (&opts[count - 1], &specials[2], sizeof (struct poptOption));
+	     poptcmp (&opts[count - 1], &specials[2]);
 	     count += 1);
 
 	result = xcalloc (count, sizeof (struct poptOption));
@@ -705,15 +719,15 @@ struct poptOption *clone_popt_options (struct poptOption *opts)
 		if (opts[ix].argInfo == POPT_ARG_CALLBACK)
 			continue;
 
-		if (!memcmp (&opts[ix], &specials[0], sizeof (struct poptOption)))
+		if (!poptcmp (&opts[ix], &specials[0]))
 			continue;
 
-		if (!memcmp (&opts[ix], &specials[1], sizeof (struct poptOption)))
+		if (!poptcmp (&opts[ix], &specials[1]))
 			continue;
 
 		memcpy (&result[iy], &opts[ix], sizeof (struct poptOption));
 
-		if (!memcmp (&opts[ix], &specials[2], sizeof (struct poptOption)))
+		if (!poptcmp (&opts[ix], &specials[2]))
 			continue;
 
 		if (opts[ix].argInfo == POPT_ARG_INCLUDE_TABLE) {
@@ -756,7 +770,7 @@ void free_popt_clone (struct poptOption *opts)
 
 	assert (opts);
 
-	for (ix = 0; memcmp (&opts[ix], &table_end, sizeof (table_end)); ix += 1) {
+	for (ix = 0; poptcmp (&opts[ix], &table_end); ix += 1) {
 		if (opts[ix].argInfo == POPT_ARG_INCLUDE_TABLE)
 			free_popt_clone (opts[ix].arg);
 	}
@@ -779,7 +793,7 @@ struct poptOption *find_popt_option (struct poptOption *opts, int wanted)
 	while (1) {
 		struct poptOption *result;
 
-		if (!memcmp (&opts[ix], &table_end, sizeof (table_end)))
+		if (!poptcmp (&opts[ix], &table_end))
 			break;
 
 		assert (opts[ix].argInfo != POPT_ARG_CALLBACK);
-- 
2.11.0

Reply via email to