[Home]

Summary:ASTERISK-17927: [patch] Invalid read and null pointer deref on asterisk shutdown
Reporter:Mark Murawski (kobaz)Labels:
Date Opened:2011-05-26 00:45:03Date Closed:2011-07-18 07:29:26
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 20110526__issue19369.diff.txt
( 1) el_read.patch
Description:==10244== Thread 1:
==10244== Invalid read of size 4
==10244==    at 0x81962B0: el_gets (read.c:254)
==10244==    by 0x80859D1: main (asterisk.c:3861)
==10244==  Address 0x4515d54 is 68 bytes inside a block of size 776 free'd
==10244==    at 0x402421C: free (vg_replace_malloc.c:366)
==10244==    by 0x8194249: el_end (el.c:126)
==10244==    by 0x807EB77: quit_handler (asterisk.c:1659)
==10244==    by 0x80830CF: monitor_sig_flags (asterisk.c:3089)
==10244==    by 0x818B1B3: dummy_start (utils.c:973)
==10244==    by 0x4462C38: start_thread (pthread_create.c:304)
==10244==    by 0x42A28ED: clone (clone.S:130)
==10244==
==10244== Invalid read of size 4
==10244==    at 0x81962C5: el_gets (read.c:258)
==10244==    by 0x80859D1: main (asterisk.c:3861)
==10244==  Address 0x4515fb0 is 672 bytes inside a block of size 776 free'd
==10244==    at 0x402421C: free (vg_replace_malloc.c:366)
==10244==    by 0x8194249: el_end (el.c:126)
==10244==    by 0x807EB77: quit_handler (asterisk.c:1659)
==10244==    by 0x80830CF: monitor_sig_flags (asterisk.c:3089)
==10244==    by 0x818B1B3: dummy_start (utils.c:973)
==10244==    by 0x4462C38: start_thread (pthread_create.c:304)
==10244==    by 0x42A28ED: clone (clone.S:130)
==10244==
==10244== Invalid read of size 1
==10244==    at 0x81962CB: el_gets (read.c:258)
==10244==    by 0x80859D1: main (asterisk.c:3861)
==10244==  Address 0xa is not stack'd, malloc'd or (recently) free'd
==10244==
==10244==
==10244== Process terminating with default action of signal 11 (SIGSEGV): dumping core
==10244==  Access not within mapped region at address 0xA
==10244==    at 0x81962CB: el_gets (read.c:258)
==10244==    by 0x80859D1: main (asterisk.c:3861)
Comments:By: Mark Murawski (kobaz) 2011-05-26 01:26:41

Patch to make sure we don't read from editline if we've just destroyed the editline. Patch also moves the need_quit check to before the read when remotely attached.



By: Tilghman Lesher (tilghman) 2011-05-26 02:14:25

I don't believe you've actually fixed the race; you've just made it less likely.  You'll probably need to revert the patch from ASTERISK-16416 and apply a different fix that doesn't reintroduce the race.

By: Mark Murawski (kobaz) 2011-05-26 02:17:09

Yeah, I wasn't sure if it was truly fixed because there's no locking involved.  I'll check out that other patch.  Thanks.

By: Tilghman Lesher (tilghman) 2011-05-26 02:51:26

Probably what needs to happen is if it's that other thread, instead of interfering with "el", it should set a flag and send a SIGURG to consolethread (if it exists).  Consolethread should then check that flag.  If consolethread, though, is AST_PTHREADT_NULL, then "el" probably won't be anything but NULL anyway (but take care of that case, because sending a signal to AST_PTHREADT_NULL is also bad).

By: Tilghman Lesher (tilghman) 2011-05-26 03:17:18

Something like this patch...

By: Leif Madsen (lmadsen) 2011-05-26 19:20:02

Will this stop the "crash on shutdown" I keep seeing?

By: Tilghman Lesher (tilghman) 2011-05-26 21:38:16

lmadsen: very likely, yes

By: Kinsey Moore (kmoore) 2011-06-30 13:02:38.871-0500

I couldn't reproduce the bug on my system to test this properly, but Tilghman's patch looks good.

Acked-by: Kinsey Moore <kmoore@digium.com>

By: Kinsey Moore (kmoore) 2011-06-30 13:07:24.176-0500

Was there a reviewboard entry associated with this issue?  I couldn't find one.

By: Kinsey Moore (kmoore) 2011-07-08 15:12:31.574-0500

Performed as expected and looks like it's ready to go in.  Ship it!

By: Mark Murawski (kobaz) 2011-07-18 07:07:40.027-0500

r328593 | markm | 2011-07-18 08:06:50 -0400 (Mon, 18 Jul 2011) | 8 lines

Fixed invalid read and null pointer deref on asterisk shutdown.

In some cases when starting asterisk with -c and hitting control-c to shutdown, there will be an invalid read and null pointer deref causing a crash.

(closes issue ASTERISK-17927)
Reported by: Mark Murawski
Tested by: Mark Murawski, Kinsey Moore, Tilghman Lesher


By: Mark Murawski (kobaz) 2011-07-18 07:28:35.501-0500

Merged to 1.10 (r328609)
Merged to trunk (r328610)