* Re: [PATCH] conf: fix memory leak on the error path in parse_args()
@ 2021-03-17 15:44 Mark Hills
2021-03-17 16:03 ` Takashi Iwai
0 siblings, 1 reply; 3+ messages in thread
From: Mark Hills @ 2021-03-17 15:44 UTC (permalink / raw)
To: Jaroslav Kysela; +Cc: alsa-devel
Having a little trouble which bisected to this patch.
First noticed it's causing Chromium to crash out one of its subprocesses
(stack trace below)
Can actually be replicated with a simple "aplay -L":
aplay: conf.c:2207: snd_config_delete: Assertion `config' failed.
Aborted (core dumped)
Program received signal SIGABRT, Aborted.
0x00007ffff7984c7b in raise () from /lib64/libc.so.6
(gdb) bt
#0 0x00007ffff7984c7b in raise () from /lib64/libc.so.6
#1 0x00007ffff7965548 in abort () from /lib64/libc.so.6
#2 0x00007ffff796542f in __assert_fail_base.cold () from /lib64/libc.so.6
#3 0x00007ffff7975fc2 in __assert_fail () from /lib64/libc.so.6
#4 0x00007ffff7cce7e2 in snd_config_delete (config=<optimized out>) at conf.c:2207
#5 0x00007ffff7cd004d in parse_args (subs=0x6c66f0, str=<optimized out>, str@entry=0x641fa5 "CARD=Layla3G,DEV=0", defs=<optimized out>) at conf.c:5172
#6 0x00007ffff7cd0c31 in snd_config_expand (config=0x6594a0, root=root@entry=0x639330, args=args@entry=0x641fa5 "CARD=Layla3G,DEV=0", private_data=private_data@entry=0x0, result=result@entry=0x7fffffffdf80) at conf.c:5227
#7 0x00007ffff7cd13e2 in snd_config_search_definition (config=config@entry=0x639330, base=base@entry=0x40cf74 "pcm", name=name@entry=0x641fa0 "plug:CARD=Layla3G,DEV=0", result=result@entry=0x7fffffffdf80) at conf.c:5312
#8 0x00007ffff7cd8154 in try_config (config=config@entry=0x639330, list=list@entry=0x7fffffffe260, base=0x40cf74 "pcm", name=<optimized out>) at namehint.c:269
#9 0x00007ffff7cd8e8a in add_card (config=<optimized out>, rw_config=0x639330, list=list@entry=0x7fffffffe260, card=0) at namehint.c:488
#10 0x00007ffff7cd9410 in snd_device_name_hint (card=0, card@entry=-1, iface=<optimized out>, iface@entry=0x40cf74 "pcm", hints=hints@entry=0x7fffffffe308) at namehint.c:639
#11 0x000000000040442b in pcm_list () at aplay.c:339
#12 0x000000000040a638 in main (argc=2, argv=0x7fffffffe5c8) at aplay.c:824
I saw "namehint" in the stack; I'm using this fragment of asoundrc (it's
the only way to get chosen ALSA devices available in Chrome) but it seems
to be unrelated; commenting it out and the assertion still fails. I
haven't had time to look any deeper.
namehint.pcm [
"layla1|Layla: Microphone input 1"
"layla2|Layla: Microphone input 2"
"layla12|Layla: Microphone inputs 1,2"
"layla34|Layla: Line inputs 3,4 (DJ mixer)"
"layla|Layla: Analogue inputs 1,2,3,4"
"plug:shure|Shure X2U: Microphone input"
"plug:dj1|Audio 8 DJ: Input 1 (Left deck)"
"plug:dj2|Audio 8 DJ: Input 2 (Right deck)"
"plug:local|Local audio bus"
]
--
Mark
From ad5f255b4767d52f7aaafd3eb348c680e990f0b8 Mon Sep 17 00:00:00 2001
From: Jaroslav Kysela <perex@perex.cz>
Date: Wed, 10 Mar 2021 18:10:57 +0100
Subject: [PATCH] conf: fix memory leak on the error path in parse_args()
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
---
src/conf.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/conf.c b/src/conf.c
index 38eefbf8..14b14b59 100644
--- a/src/conf.c
+++ b/src/conf.c
@@ -5169,6 +5169,7 @@ static int parse_args(snd_config_t *subs, const char *str, snd_config_t *defs)
err = snd_config_add(subs, sub);
if (err < 0) {
_err:
+ snd_config_delete(sub);
free(val);
return err;
}
--
2.17.5
chrome: conf.c:2207: snd_config_delete: Assertion `config' failed.
Received signal 6
#0 0x55c2535f35e9 base::debug::CollectStackTrace()
#1 0x55c25355e723 base::debug::StackTrace::StackTrace()
#2 0x55c2535f3190 base::debug::(anonymous namespace)::StackDumpSignalHandler()
#3 0x7f35b16e6690 (/lib64/libpthread-2.30.so+0x1368f)
#4 0x7f35ac583c7b __GI_raise
#5 0x7f35ac564548 __GI_abort
#6 0x7f35ac56442f __assert_fail_base.cold
#7 0x7f35ac574fc2 __GI___assert_fail
#8 0x7f35acda27e2 snd_config_delete
#9 0x7f35acda404d parse_args
#10 0x7f35acda4c31 snd_config_expand
#11 0x7f35acda53e2 snd_config_search_definition
#12 0x7f35acdac154 try_config
#13 0x7f35acdace8a add_card
#14 0x7f35acdad13c snd_device_name_hint
#15 0x55c250eb3b13 media::AudioManagerAlsa::HasAnyAlsaAudioDevice()
#16 0x55c250ea9445 media::AudioSystemHelper::ComputeOutputParameters()
#17 0x55c250ea93e0 media::AudioSystemHelper::GetOutputStreamParameters()
#18 0x55c251d38db3 audio::SystemInfo::GetOutputStreamParameters()
#19 0x55c2511bd455 audio::mojom::SystemInfoStubDispatch::AcceptWithResponder()
#20 0x55c253d0bad3 mojo::InterfaceEndpointClient::HandleValidatedMessage()
#21 0x55c253d0e8bd mojo::MessageDispatcher::Accept()
#22 0x55c253d12380 mojo::internal::MultiplexRouter::ProcessIncomingMessage()
#23 0x55c253d11b09 mojo::internal::MultiplexRouter::Accept()
#24 0x55c253d0e8f9 mojo::MessageDispatcher::Accept()
#25 0x55c253d0a184 mojo::Connector::DispatchMessage()
#26 0x55c253d0a8b7 mojo::Connector::ReadAllAvailableMessages()
#27 0x55c25056d034 base::internal::Invoker<>::RunOnce()
#28 0x55c2535b52b2 base::TaskAnnotator::RunTask()
#29 0x55c2535c6c11 base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWorkImpl()
#30 0x55c2535c68ea base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWork()
#31 0x55c25357669a base::MessagePumpDefault::Run()
#32 0x55c2535c725b base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run()
#33 0x55c25359820b base::RunLoop::Run()
#34 0x55c2534d01da content::UtilityMain()
#35 0x55c2534fba9c content::RunZygote()
#36 0x55c2534fcd6b content::ContentMainRunnerImpl::Run()
#37 0x55c2534fa35d content::RunContentProcess()
#38 0x55c2534facfd content::ContentMain()
#39 0x55c25056a068 ChromeMain
#40 0x7f35ac565e6b __libc_start_main
#41 0x55c250569eaa _start
r8: 0000000000000000 r9: 00007ffd94a02da0 r10: 0000000000000008 r11: 0000000000000246
r12: 00007f35ace23e21 r13: 000000000000089f r14: 00007f35ace2af41 r15: 0000000000000004
di: 0000000000000002 si: 00007ffd94a02da0 bp: 00007f35ac6ebf38 bx: 00007f35a5d83f00
dx: 0000000000000000 ax: 0000000000000000 cx: 00007f35ac583c7b sp: 00007ffd94a02e18
ip: 00007f35ac583c7b efl: 0000000000000246 cgf: 002b000000000033 erf: 0000000000000000
trp: 0000000000000000 msk: 0000000000000000 cr2: 0000000000000000
[end of stack trace]
Calling _exit(1). Core file will not be generated.
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] conf: fix memory leak on the error path in parse_args()
2021-03-17 15:44 [PATCH] conf: fix memory leak on the error path in parse_args() Mark Hills
@ 2021-03-17 16:03 ` Takashi Iwai
2021-03-18 12:14 ` Mark Hills
0 siblings, 1 reply; 3+ messages in thread
From: Takashi Iwai @ 2021-03-17 16:03 UTC (permalink / raw)
To: Mark Hills; +Cc: alsa-devel
On Wed, 17 Mar 2021 16:44:20 +0100,
Mark Hills wrote:
>
> Having a little trouble which bisected to this patch.
>
> First noticed it's causing Chromium to crash out one of its subprocesses
> (stack trace below)
>
> Can actually be replicated with a simple "aplay -L":
>
> aplay: conf.c:2207: snd_config_delete: Assertion `config' failed.
> Aborted (core dumped)
That patch seems to have a few flaws.
Could you check the patch below covers it?
thanks,
Takashi
--- a/src/conf.c
+++ b/src/conf.c
@@ -5080,6 +5080,8 @@ static int parse_args(snd_config_t *subs, const char *str, snd_config_t *defs)
const char *new = str;
const char *tmp;
char *val = NULL;
+
+ sub = NULL;
err = parse_arg(&new, &varlen, &val);
if (err < 0)
goto _err;
@@ -5104,6 +5106,7 @@ static int parse_args(snd_config_t *subs, const char *str, snd_config_t *defs)
err = snd_config_search(subs, var, &sub);
if (err >= 0)
snd_config_delete(sub);
+ sub = NULL;
err = snd_config_search(def, "type", &typ);
if (err < 0) {
_invalid_type:
@@ -5169,7 +5172,8 @@ static int parse_args(snd_config_t *subs, const char *str, snd_config_t *defs)
err = snd_config_add(subs, sub);
if (err < 0) {
_err:
- snd_config_delete(sub);
+ if (sub)
+ snd_config_delete(sub);
free(val);
return err;
}
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] conf: fix memory leak on the error path in parse_args()
2021-03-17 16:03 ` Takashi Iwai
@ 2021-03-18 12:14 ` Mark Hills
0 siblings, 0 replies; 3+ messages in thread
From: Mark Hills @ 2021-03-18 12:14 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
On Wed, 17 Mar 2021, Takashi Iwai wrote:
> On Wed, 17 Mar 2021 16:44:20 +0100,
> Mark Hills wrote:
> >
> > Having a little trouble which bisected to this patch.
> >
> > First noticed it's causing Chromium to crash out one of its subprocesses
> > (stack trace below)
> >
> > Can actually be replicated with a simple "aplay -L":
> >
> > aplay: conf.c:2207: snd_config_delete: Assertion `config' failed.
> > Aborted (core dumped)
>
> That patch seems to have a few flaws.
> Could you check the patch below covers it?
Thanks. Yes, the patch builds ok and resolves my two test cases (aplay and
chromium). That's the only testing I've done so far.
> --- a/src/conf.c
> +++ b/src/conf.c
> @@ -5080,6 +5080,8 @@ static int parse_args(snd_config_t *subs, const char *str, snd_config_t *defs)
> const char *new = str;
> const char *tmp;
> char *val = NULL;
> +
> + sub = NULL;
> err = parse_arg(&new, &varlen, &val);
> if (err < 0)
> goto _err;
> @@ -5104,6 +5106,7 @@ static int parse_args(snd_config_t *subs, const char *str, snd_config_t *defs)
> err = snd_config_search(subs, var, &sub);
> if (err >= 0)
> snd_config_delete(sub);
> + sub = NULL;
> err = snd_config_search(def, "type", &typ);
> if (err < 0) {
> _invalid_type:
> @@ -5169,7 +5172,8 @@ static int parse_args(snd_config_t *subs, const char *str, snd_config_t *defs)
> err = snd_config_add(subs, sub);
> if (err < 0) {
> _err:
> - snd_config_delete(sub);
> + if (sub)
> + snd_config_delete(sub);
> free(val);
> return err;
> }
>
>
--
Mark
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-03-18 12:15 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-03-17 15:44 [PATCH] conf: fix memory leak on the error path in parse_args() Mark Hills
2021-03-17 16:03 ` Takashi Iwai
2021-03-18 12:14 ` Mark Hills
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).