| Message ID | 20201113145525.85151-1-alex.chen@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] json: Fix a memleak in parse_pair() | expand |
> -----Original Message----- > From: Chenzhendong (alex) > Sent: Friday, November 13, 2020 10:55 PM > To: armbru@redhat.com > Cc: Chenzhendong (alex) <alex.chen@huawei.com>; qemu-devel@nongnu.org; > qemu-trivial@nongnu.org; Zhanghailiang <zhang.zhanghailiang@huawei.com>; > Chenqun (kuhn) <kuhn.chenqun@huawei.com> > Subject: [PATCH v2] json: Fix a memleak in parse_pair() > > In qobject_type(), NULL is returned when the 'QObject' returned from > parse_value() is not of QString type, and this 'QObject' memory will leaked. > So we need to first cache the 'QObject' returned from parse_value(), and finally > free 'QObject' memory at the end of the function. > Also, we add a testcast about invalid dict key. > > The memleak stack is as follows: > Direct leak of 32 byte(s) in 1 object(s) allocated from: > #0 0xfffe4b3c34fb in __interceptor_malloc (/lib64/libasan.so.4+0xd34fb) > #1 0xfffe4ae48aa3 in g_malloc (/lib64/libglib-2.0.so.0+0x58aa3) > #2 0xaaab3557d9f7 in qnum_from_int > /Images/source_org/qemu_master/qemu/qobject/qnum.c:25 > #3 0xaaab35584d23 in parse_literal > /Images/source_org/qemu_master/qemu/qobject/json-parser.c:511 > #4 0xaaab35584d23 in parse_value > /Images/source_org/qemu_master/qemu/qobject/json-parser.c:554 > #5 0xaaab35583d77 in parse_pair > /Images/source_org/qemu_master/qemu/qobject/json-parser.c:270 > #6 0xaaab355845db in parse_object > /Images/source_org/qemu_master/qemu/qobject/json-parser.c:327 > #7 0xaaab355845db in parse_value > /Images/source_org/qemu_master/qemu/qobject/json-parser.c:546 > #8 0xaaab35585b1b in json_parser_parse > /Images/source_org/qemu_master/qemu/qobject/json-parser.c:580 > #9 0xaaab35583703 in json_message_process_token > /Images/source_org/qemu_master/qemu/qobject/json-streamer.c:92 > #10 0xaaab355ddccf in json_lexer_feed_char > /Images/source_org/qemu_master/qemu/qobject/json-lexer.c:313 > #11 0xaaab355de0eb in json_lexer_feed > /Images/source_org/qemu_master/qemu/qobject/json-lexer.c:350 > #12 0xaaab354aff67 in tcp_chr_read > /Images/source_org/qemu_master/qemu/chardev/char-socket.c:525 > #13 0xfffe4ae429db in g_main_context_dispatch > (/lib64/libglib-2.0.so.0+0x529db) > #14 0xfffe4ae42d8f (/lib64/libglib-2.0.so.0+0x52d8f) > #15 0xfffe4ae430df in g_main_loop_run (/lib64/libglib-2.0.so.0+0x530df) > #16 0xaaab34d70bff in iothread_run > /Images/source_org/qemu_master/qemu/iothread.c:82 > #17 0xaaab3559d71b in qemu_thread_start > /Images/source_org/qemu_master/qemu/util/qemu-thread-posix.c:519 > > Fixes: 532fb5328473 ("qapi: Make more of qobject_to()") > Reported-by: Euler Robot <euler.robot@huawei.com> > Signed-off-by: Alex Chen <alex.chen@huawei.com> > Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com> > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > qobject/json-parser.c | 12 ++++++------ > tests/check-qjson.c | 9 +++++++++ > 2 files changed, 15 insertions(+), 6 deletions(-) > > diff --git a/qobject/json-parser.c b/qobject/json-parser.c index > d083810d37..c0f521b56b 100644 > --- a/qobject/json-parser.c > +++ b/qobject/json-parser.c > @@ -257,8 +257,9 @@ static JSONToken > *parser_context_peek_token(JSONParserContext *ctxt) > */ > static int parse_pair(JSONParserContext *ctxt, QDict *dict) { > + QObject *key_obj = NULL; > + QString *key; > QObject *value; > - QString *key = NULL; > JSONToken *peek, *token; > > peek = parser_context_peek_token(ctxt); @@ -267,7 +268,8 @@ static > int parse_pair(JSONParserContext *ctxt, QDict *dict) > goto out; > } > > - key = qobject_to(QString, parse_value(ctxt)); > + key_obj = parse_value(ctxt); > + key = qobject_to(QString, key_obj); > if (!key) { > parse_error(ctxt, peek, "key is not a string in object"); > goto out; > @@ -297,13 +299,11 @@ static int parse_pair(JSONParserContext *ctxt, > QDict *dict) > > qdict_put_obj(dict, qstring_get_str(key), value); > > - qobject_unref(key); > - > + qobject_unref(key_obj); > return 0; > > out: > - qobject_unref(key); > - > + qobject_unref(key_obj); > return -1; > } > Hi Alex, Perhaps it would be more appropriate to provide the testcase as a separate patch which from Markus. Thanks, Chen Qun > diff --git a/tests/check-qjson.c b/tests/check-qjson.c index > 07a773e653..9a02079099 100644 > --- a/tests/check-qjson.c > +++ b/tests/check-qjson.c > @@ -1415,6 +1415,14 @@ static void invalid_dict_comma(void) > g_assert(obj == NULL); > } > > +static void invalid_dict_key(void) > +{ > + Error *err = NULL; > + QObject *obj = qobject_from_json("{32:'abc'}", &err); > + error_free_or_abort(&err); > + g_assert(obj == NULL); > +} > + > static void unterminated_literal(void) > { > Error *err = NULL; > @@ -1500,6 +1508,7 @@ int main(int argc, char **argv) > g_test_add_func("/errors/unterminated/dict_comma", > unterminated_dict_comma); > g_test_add_func("/errors/invalid_array_comma", > invalid_array_comma); > g_test_add_func("/errors/invalid_dict_comma", invalid_dict_comma); > + g_test_add_func("/errors/invalid_dict_key", invalid_dict_key); > g_test_add_func("/errors/unterminated/literal", unterminated_literal); > g_test_add_func("/errors/limits/nesting", limits_nesting); > g_test_add_func("/errors/multiple_values", multiple_values); > -- > 2.19.1
"Chenqun (kuhn)" <kuhn.chenqun@huawei.com> writes: >> -----Original Message----- >> From: Chenzhendong (alex) >> Sent: Friday, November 13, 2020 10:55 PM >> To: armbru@redhat.com >> Cc: Chenzhendong (alex) <alex.chen@huawei.com>; qemu-devel@nongnu.org; >> qemu-trivial@nongnu.org; Zhanghailiang <zhang.zhanghailiang@huawei.com>; >> Chenqun (kuhn) <kuhn.chenqun@huawei.com> >> Subject: [PATCH v2] json: Fix a memleak in parse_pair() >> >> In qobject_type(), NULL is returned when the 'QObject' returned from >> parse_value() is not of QString type, and this 'QObject' memory will leaked. >> So we need to first cache the 'QObject' returned from parse_value(), and finally >> free 'QObject' memory at the end of the function. >> Also, we add a testcast about invalid dict key. >> >> The memleak stack is as follows: >> Direct leak of 32 byte(s) in 1 object(s) allocated from: >> #0 0xfffe4b3c34fb in __interceptor_malloc (/lib64/libasan.so.4+0xd34fb) >> #1 0xfffe4ae48aa3 in g_malloc (/lib64/libglib-2.0.so.0+0x58aa3) >> #2 0xaaab3557d9f7 in qnum_from_int >> /Images/source_org/qemu_master/qemu/qobject/qnum.c:25 >> #3 0xaaab35584d23 in parse_literal >> /Images/source_org/qemu_master/qemu/qobject/json-parser.c:511 >> #4 0xaaab35584d23 in parse_value >> /Images/source_org/qemu_master/qemu/qobject/json-parser.c:554 >> #5 0xaaab35583d77 in parse_pair >> /Images/source_org/qemu_master/qemu/qobject/json-parser.c:270 >> #6 0xaaab355845db in parse_object >> /Images/source_org/qemu_master/qemu/qobject/json-parser.c:327 >> #7 0xaaab355845db in parse_value >> /Images/source_org/qemu_master/qemu/qobject/json-parser.c:546 >> #8 0xaaab35585b1b in json_parser_parse >> /Images/source_org/qemu_master/qemu/qobject/json-parser.c:580 >> #9 0xaaab35583703 in json_message_process_token >> /Images/source_org/qemu_master/qemu/qobject/json-streamer.c:92 >> #10 0xaaab355ddccf in json_lexer_feed_char >> /Images/source_org/qemu_master/qemu/qobject/json-lexer.c:313 >> #11 0xaaab355de0eb in json_lexer_feed >> /Images/source_org/qemu_master/qemu/qobject/json-lexer.c:350 >> #12 0xaaab354aff67 in tcp_chr_read >> /Images/source_org/qemu_master/qemu/chardev/char-socket.c:525 >> #13 0xfffe4ae429db in g_main_context_dispatch >> (/lib64/libglib-2.0.so.0+0x529db) >> #14 0xfffe4ae42d8f (/lib64/libglib-2.0.so.0+0x52d8f) >> #15 0xfffe4ae430df in g_main_loop_run (/lib64/libglib-2.0.so.0+0x530df) >> #16 0xaaab34d70bff in iothread_run >> /Images/source_org/qemu_master/qemu/iothread.c:82 >> #17 0xaaab3559d71b in qemu_thread_start >> /Images/source_org/qemu_master/qemu/util/qemu-thread-posix.c:519 >> >> Fixes: 532fb5328473 ("qapi: Make more of qobject_to()") >> Reported-by: Euler Robot <euler.robot@huawei.com> >> Signed-off-by: Alex Chen <alex.chen@huawei.com> >> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> --- >> qobject/json-parser.c | 12 ++++++------ >> tests/check-qjson.c | 9 +++++++++ >> 2 files changed, 15 insertions(+), 6 deletions(-) >> >> diff --git a/qobject/json-parser.c b/qobject/json-parser.c index >> d083810d37..c0f521b56b 100644 >> --- a/qobject/json-parser.c >> +++ b/qobject/json-parser.c >> @@ -257,8 +257,9 @@ static JSONToken >> *parser_context_peek_token(JSONParserContext *ctxt) >> */ >> static int parse_pair(JSONParserContext *ctxt, QDict *dict) { >> + QObject *key_obj = NULL; >> + QString *key; >> QObject *value; >> - QString *key = NULL; >> JSONToken *peek, *token; >> >> peek = parser_context_peek_token(ctxt); @@ -267,7 +268,8 @@ static >> int parse_pair(JSONParserContext *ctxt, QDict *dict) >> goto out; >> } >> >> - key = qobject_to(QString, parse_value(ctxt)); >> + key_obj = parse_value(ctxt); >> + key = qobject_to(QString, key_obj); >> if (!key) { >> parse_error(ctxt, peek, "key is not a string in object"); >> goto out; >> @@ -297,13 +299,11 @@ static int parse_pair(JSONParserContext *ctxt, >> QDict *dict) >> >> qdict_put_obj(dict, qstring_get_str(key), value); >> >> - qobject_unref(key); >> - >> + qobject_unref(key_obj); >> return 0; >> >> out: >> - qobject_unref(key); >> - >> + qobject_unref(key_obj); >> return -1; >> } >> > > Hi Alex, > Perhaps it would be more appropriate to provide the testcase as a separate patch which from Markus. I appreciate your consideration. Since both fix and test case are simple, and we need to hurry to have a chance at getting this into 5.2, and the part of the work deserving credit the most is finding and analyzing the bug, I'd like to take the patch as is. Queued, thanks!
On 11/16/20 7:42 AM, Markus Armbruster wrote: > "Chenqun (kuhn)" <kuhn.chenqun@huawei.com> writes: > >>> -----Original Message----- >>> From: Chenzhendong (alex) >>> Sent: Friday, November 13, 2020 10:55 PM >>> To: armbru@redhat.com >>> Cc: Chenzhendong (alex) <alex.chen@huawei.com>; qemu-devel@nongnu.org; >>> qemu-trivial@nongnu.org; Zhanghailiang <zhang.zhanghailiang@huawei.com>; >>> Chenqun (kuhn) <kuhn.chenqun@huawei.com> >>> Subject: [PATCH v2] json: Fix a memleak in parse_pair() >>> >>> In qobject_type(), NULL is returned when the 'QObject' returned from >>> parse_value() is not of QString type, and this 'QObject' memory will leaked. >>> So we need to first cache the 'QObject' returned from parse_value(), and finally >>> free 'QObject' memory at the end of the function. >>> Also, we add a testcast about invalid dict key. >>> >>> The memleak stack is as follows: >>> Direct leak of 32 byte(s) in 1 object(s) allocated from: >>> #0 0xfffe4b3c34fb in __interceptor_malloc (/lib64/libasan.so.4+0xd34fb) >>> #1 0xfffe4ae48aa3 in g_malloc (/lib64/libglib-2.0.so.0+0x58aa3) >>> #2 0xaaab3557d9f7 in qnum_from_int >>> /Images/source_org/qemu_master/qemu/qobject/qnum.c:25 >>> #3 0xaaab35584d23 in parse_literal >>> /Images/source_org/qemu_master/qemu/qobject/json-parser.c:511 >>> #4 0xaaab35584d23 in parse_value >>> /Images/source_org/qemu_master/qemu/qobject/json-parser.c:554 >>> #5 0xaaab35583d77 in parse_pair >>> /Images/source_org/qemu_master/qemu/qobject/json-parser.c:270 >>> #6 0xaaab355845db in parse_object >>> /Images/source_org/qemu_master/qemu/qobject/json-parser.c:327 >>> #7 0xaaab355845db in parse_value >>> /Images/source_org/qemu_master/qemu/qobject/json-parser.c:546 >>> #8 0xaaab35585b1b in json_parser_parse >>> /Images/source_org/qemu_master/qemu/qobject/json-parser.c:580 >>> #9 0xaaab35583703 in json_message_process_token >>> /Images/source_org/qemu_master/qemu/qobject/json-streamer.c:92 >>> #10 0xaaab355ddccf in json_lexer_feed_char >>> /Images/source_org/qemu_master/qemu/qobject/json-lexer.c:313 >>> #11 0xaaab355de0eb in json_lexer_feed >>> /Images/source_org/qemu_master/qemu/qobject/json-lexer.c:350 >>> #12 0xaaab354aff67 in tcp_chr_read >>> /Images/source_org/qemu_master/qemu/chardev/char-socket.c:525 >>> #13 0xfffe4ae429db in g_main_context_dispatch >>> (/lib64/libglib-2.0.so.0+0x529db) >>> #14 0xfffe4ae42d8f (/lib64/libglib-2.0.so.0+0x52d8f) >>> #15 0xfffe4ae430df in g_main_loop_run (/lib64/libglib-2.0.so.0+0x530df) >>> #16 0xaaab34d70bff in iothread_run >>> /Images/source_org/qemu_master/qemu/iothread.c:82 >>> #17 0xaaab3559d71b in qemu_thread_start >>> /Images/source_org/qemu_master/qemu/util/qemu-thread-posix.c:519 >>> ... > > Queued, thanks! If possible can you s%/Images/source_org/qemu_master/qemu/%% to make description more readable...? Thanks, Phil.
On 2020/11/16 19:43, Philippe Mathieu-Daudé wrote: > On 11/16/20 7:42 AM, Markus Armbruster wrote: >> "Chenqun (kuhn)" <kuhn.chenqun@huawei.com> writes: >> >>>> -----Original Message----- >>>> From: Chenzhendong (alex) >>>> Sent: Friday, November 13, 2020 10:55 PM >>>> To: armbru@redhat.com >>>> Cc: Chenzhendong (alex) <alex.chen@huawei.com>; qemu-devel@nongnu.org; >>>> qemu-trivial@nongnu.org; Zhanghailiang <zhang.zhanghailiang@huawei.com>; >>>> Chenqun (kuhn) <kuhn.chenqun@huawei.com> >>>> Subject: [PATCH v2] json: Fix a memleak in parse_pair() >>>> >>>> In qobject_type(), NULL is returned when the 'QObject' returned from >>>> parse_value() is not of QString type, and this 'QObject' memory will leaked. >>>> So we need to first cache the 'QObject' returned from parse_value(), and finally >>>> free 'QObject' memory at the end of the function. >>>> Also, we add a testcast about invalid dict key. >>>> >>>> The memleak stack is as follows: >>>> Direct leak of 32 byte(s) in 1 object(s) allocated from: >>>> #0 0xfffe4b3c34fb in __interceptor_malloc (/lib64/libasan.so.4+0xd34fb) >>>> #1 0xfffe4ae48aa3 in g_malloc (/lib64/libglib-2.0.so.0+0x58aa3) >>>> #2 0xaaab3557d9f7 in qnum_from_int >>>> /Images/source_org/qemu_master/qemu/qobject/qnum.c:25 >>>> #3 0xaaab35584d23 in parse_literal >>>> /Images/source_org/qemu_master/qemu/qobject/json-parser.c:511 >>>> #4 0xaaab35584d23 in parse_value >>>> /Images/source_org/qemu_master/qemu/qobject/json-parser.c:554 >>>> #5 0xaaab35583d77 in parse_pair >>>> /Images/source_org/qemu_master/qemu/qobject/json-parser.c:270 >>>> #6 0xaaab355845db in parse_object >>>> /Images/source_org/qemu_master/qemu/qobject/json-parser.c:327 >>>> #7 0xaaab355845db in parse_value >>>> /Images/source_org/qemu_master/qemu/qobject/json-parser.c:546 >>>> #8 0xaaab35585b1b in json_parser_parse >>>> /Images/source_org/qemu_master/qemu/qobject/json-parser.c:580 >>>> #9 0xaaab35583703 in json_message_process_token >>>> /Images/source_org/qemu_master/qemu/qobject/json-streamer.c:92 >>>> #10 0xaaab355ddccf in json_lexer_feed_char >>>> /Images/source_org/qemu_master/qemu/qobject/json-lexer.c:313 >>>> #11 0xaaab355de0eb in json_lexer_feed >>>> /Images/source_org/qemu_master/qemu/qobject/json-lexer.c:350 >>>> #12 0xaaab354aff67 in tcp_chr_read >>>> /Images/source_org/qemu_master/qemu/chardev/char-socket.c:525 >>>> #13 0xfffe4ae429db in g_main_context_dispatch >>>> (/lib64/libglib-2.0.so.0+0x529db) >>>> #14 0xfffe4ae42d8f (/lib64/libglib-2.0.so.0+0x52d8f) >>>> #15 0xfffe4ae430df in g_main_loop_run (/lib64/libglib-2.0.so.0+0x530df) >>>> #16 0xaaab34d70bff in iothread_run >>>> /Images/source_org/qemu_master/qemu/iothread.c:82 >>>> #17 0xaaab3559d71b in qemu_thread_start >>>> /Images/source_org/qemu_master/qemu/util/qemu-thread-posix.c:519 >>>> > ... >> >> Queued, thanks! > > If possible can you s%/Images/source_org/qemu_master/qemu/%% to make > description more readable...? > Hi Philippe, I am sorry for that, considering that the patch has been queued, do I need to modify the commit message and send patch v3? Thanks, Alex
Alex Chen <alex.chen@huawei.com> writes: > On 2020/11/16 19:43, Philippe Mathieu-Daudé wrote: >> On 11/16/20 7:42 AM, Markus Armbruster wrote: >>> "Chenqun (kuhn)" <kuhn.chenqun@huawei.com> writes: >>> >>>>> -----Original Message----- >>>>> From: Chenzhendong (alex) >>>>> Sent: Friday, November 13, 2020 10:55 PM >>>>> To: armbru@redhat.com >>>>> Cc: Chenzhendong (alex) <alex.chen@huawei.com>; qemu-devel@nongnu.org; >>>>> qemu-trivial@nongnu.org; Zhanghailiang <zhang.zhanghailiang@huawei.com>; >>>>> Chenqun (kuhn) <kuhn.chenqun@huawei.com> >>>>> Subject: [PATCH v2] json: Fix a memleak in parse_pair() >>>>> >>>>> In qobject_type(), NULL is returned when the 'QObject' returned from >>>>> parse_value() is not of QString type, and this 'QObject' memory will leaked. >>>>> So we need to first cache the 'QObject' returned from parse_value(), and finally >>>>> free 'QObject' memory at the end of the function. >>>>> Also, we add a testcast about invalid dict key. >>>>> >>>>> The memleak stack is as follows: >>>>> Direct leak of 32 byte(s) in 1 object(s) allocated from: >>>>> #0 0xfffe4b3c34fb in __interceptor_malloc (/lib64/libasan.so.4+0xd34fb) >>>>> #1 0xfffe4ae48aa3 in g_malloc (/lib64/libglib-2.0.so.0+0x58aa3) >>>>> #2 0xaaab3557d9f7 in qnum_from_int >>>>> /Images/source_org/qemu_master/qemu/qobject/qnum.c:25 >>>>> #3 0xaaab35584d23 in parse_literal >>>>> /Images/source_org/qemu_master/qemu/qobject/json-parser.c:511 >>>>> #4 0xaaab35584d23 in parse_value >>>>> /Images/source_org/qemu_master/qemu/qobject/json-parser.c:554 >>>>> #5 0xaaab35583d77 in parse_pair >>>>> /Images/source_org/qemu_master/qemu/qobject/json-parser.c:270 >>>>> #6 0xaaab355845db in parse_object >>>>> /Images/source_org/qemu_master/qemu/qobject/json-parser.c:327 >>>>> #7 0xaaab355845db in parse_value >>>>> /Images/source_org/qemu_master/qemu/qobject/json-parser.c:546 >>>>> #8 0xaaab35585b1b in json_parser_parse >>>>> /Images/source_org/qemu_master/qemu/qobject/json-parser.c:580 >>>>> #9 0xaaab35583703 in json_message_process_token >>>>> /Images/source_org/qemu_master/qemu/qobject/json-streamer.c:92 >>>>> #10 0xaaab355ddccf in json_lexer_feed_char >>>>> /Images/source_org/qemu_master/qemu/qobject/json-lexer.c:313 >>>>> #11 0xaaab355de0eb in json_lexer_feed >>>>> /Images/source_org/qemu_master/qemu/qobject/json-lexer.c:350 >>>>> #12 0xaaab354aff67 in tcp_chr_read >>>>> /Images/source_org/qemu_master/qemu/chardev/char-socket.c:525 >>>>> #13 0xfffe4ae429db in g_main_context_dispatch >>>>> (/lib64/libglib-2.0.so.0+0x529db) >>>>> #14 0xfffe4ae42d8f (/lib64/libglib-2.0.so.0+0x52d8f) >>>>> #15 0xfffe4ae430df in g_main_loop_run (/lib64/libglib-2.0.so.0+0x530df) >>>>> #16 0xaaab34d70bff in iothread_run >>>>> /Images/source_org/qemu_master/qemu/iothread.c:82 >>>>> #17 0xaaab3559d71b in qemu_thread_start >>>>> /Images/source_org/qemu_master/qemu/util/qemu-thread-posix.c:519 >>>>> >> ... >>> >>> Queued, thanks! >> >> If possible can you s%/Images/source_org/qemu_master/qemu/%% to make >> description more readable...? >> > > Hi Philippe, > I am sorry for that, considering that the patch has been queued, > do I need to modify the commit message and send patch v3? I'll take care of it, no need to respin. Thanks!
On 11/16/20 3:03 PM, Markus Armbruster wrote: > Alex Chen <alex.chen@huawei.com> writes: > >> On 2020/11/16 19:43, Philippe Mathieu-Daudé wrote: >>> On 11/16/20 7:42 AM, Markus Armbruster wrote: >>>> "Chenqun (kuhn)" <kuhn.chenqun@huawei.com> writes: >>>> >>>>>> -----Original Message----- >>>>>> From: Chenzhendong (alex) >>>>>> Sent: Friday, November 13, 2020 10:55 PM >>>>>> To: armbru@redhat.com >>>>>> Cc: Chenzhendong (alex) <alex.chen@huawei.com>; qemu-devel@nongnu.org; >>>>>> qemu-trivial@nongnu.org; Zhanghailiang <zhang.zhanghailiang@huawei.com>; >>>>>> Chenqun (kuhn) <kuhn.chenqun@huawei.com> >>>>>> Subject: [PATCH v2] json: Fix a memleak in parse_pair() >>>>>> >>>>>> In qobject_type(), NULL is returned when the 'QObject' returned from >>>>>> parse_value() is not of QString type, and this 'QObject' memory will leaked. >>>>>> So we need to first cache the 'QObject' returned from parse_value(), and finally >>>>>> free 'QObject' memory at the end of the function. >>>>>> Also, we add a testcast about invalid dict key. >>>>>> >>>>>> The memleak stack is as follows: >>>>>> Direct leak of 32 byte(s) in 1 object(s) allocated from: >>>>>> #0 0xfffe4b3c34fb in __interceptor_malloc (/lib64/libasan.so.4+0xd34fb) >>>>>> #1 0xfffe4ae48aa3 in g_malloc (/lib64/libglib-2.0.so.0+0x58aa3) >>>>>> #2 0xaaab3557d9f7 in qnum_from_int >>>>>> /Images/source_org/qemu_master/qemu/qobject/qnum.c:25 >>>>>> #3 0xaaab35584d23 in parse_literal >>>>>> /Images/source_org/qemu_master/qemu/qobject/json-parser.c:511 >>>>>> #4 0xaaab35584d23 in parse_value >>>>>> /Images/source_org/qemu_master/qemu/qobject/json-parser.c:554 >>>>>> #5 0xaaab35583d77 in parse_pair >>>>>> /Images/source_org/qemu_master/qemu/qobject/json-parser.c:270 >>>>>> #6 0xaaab355845db in parse_object >>>>>> /Images/source_org/qemu_master/qemu/qobject/json-parser.c:327 >>>>>> #7 0xaaab355845db in parse_value >>>>>> /Images/source_org/qemu_master/qemu/qobject/json-parser.c:546 >>>>>> #8 0xaaab35585b1b in json_parser_parse >>>>>> /Images/source_org/qemu_master/qemu/qobject/json-parser.c:580 >>>>>> #9 0xaaab35583703 in json_message_process_token >>>>>> /Images/source_org/qemu_master/qemu/qobject/json-streamer.c:92 >>>>>> #10 0xaaab355ddccf in json_lexer_feed_char >>>>>> /Images/source_org/qemu_master/qemu/qobject/json-lexer.c:313 >>>>>> #11 0xaaab355de0eb in json_lexer_feed >>>>>> /Images/source_org/qemu_master/qemu/qobject/json-lexer.c:350 >>>>>> #12 0xaaab354aff67 in tcp_chr_read >>>>>> /Images/source_org/qemu_master/qemu/chardev/char-socket.c:525 >>>>>> #13 0xfffe4ae429db in g_main_context_dispatch >>>>>> (/lib64/libglib-2.0.so.0+0x529db) >>>>>> #14 0xfffe4ae42d8f (/lib64/libglib-2.0.so.0+0x52d8f) >>>>>> #15 0xfffe4ae430df in g_main_loop_run (/lib64/libglib-2.0.so.0+0x530df) >>>>>> #16 0xaaab34d70bff in iothread_run >>>>>> /Images/source_org/qemu_master/qemu/iothread.c:82 >>>>>> #17 0xaaab3559d71b in qemu_thread_start >>>>>> /Images/source_org/qemu_master/qemu/util/qemu-thread-posix.c:519 >>>>>> >>> ... >>>> >>>> Queued, thanks! >>> >>> If possible can you s%/Images/source_org/qemu_master/qemu/%% to make >>> description more readable...? >>> >> >> Hi Philippe, >> I am sorry for that, considering that the patch has been queued, >> do I need to modify the commit message and send patch v3? As the patch was already queued, this was a comment for the maintainer, sorry for not being clear enough. > > I'll take care of it, no need to respin. Thanks! Thanks!
diff --git a/qobject/json-parser.c b/qobject/json-parser.c index d083810d37..c0f521b56b 100644 --- a/qobject/json-parser.c +++ b/qobject/json-parser.c @@ -257,8 +257,9 @@ static JSONToken *parser_context_peek_token(JSONParserContext *ctxt) */ static int parse_pair(JSONParserContext *ctxt, QDict *dict) { + QObject *key_obj = NULL; + QString *key; QObject *value; - QString *key = NULL; JSONToken *peek, *token; peek = parser_context_peek_token(ctxt); @@ -267,7 +268,8 @@ static int parse_pair(JSONParserContext *ctxt, QDict *dict) goto out; } - key = qobject_to(QString, parse_value(ctxt)); + key_obj = parse_value(ctxt); + key = qobject_to(QString, key_obj); if (!key) { parse_error(ctxt, peek, "key is not a string in object"); goto out; @@ -297,13 +299,11 @@ static int parse_pair(JSONParserContext *ctxt, QDict *dict) qdict_put_obj(dict, qstring_get_str(key), value); - qobject_unref(key); - + qobject_unref(key_obj); return 0; out: - qobject_unref(key); - + qobject_unref(key_obj); return -1; } diff --git a/tests/check-qjson.c b/tests/check-qjson.c index 07a773e653..9a02079099 100644 --- a/tests/check-qjson.c +++ b/tests/check-qjson.c @@ -1415,6 +1415,14 @@ static void invalid_dict_comma(void) g_assert(obj == NULL); } +static void invalid_dict_key(void) +{ + Error *err = NULL; + QObject *obj = qobject_from_json("{32:'abc'}", &err); + error_free_or_abort(&err); + g_assert(obj == NULL); +} + static void unterminated_literal(void) { Error *err = NULL; @@ -1500,6 +1508,7 @@ int main(int argc, char **argv) g_test_add_func("/errors/unterminated/dict_comma", unterminated_dict_comma); g_test_add_func("/errors/invalid_array_comma", invalid_array_comma); g_test_add_func("/errors/invalid_dict_comma", invalid_dict_comma); + g_test_add_func("/errors/invalid_dict_key", invalid_dict_key); g_test_add_func("/errors/unterminated/literal", unterminated_literal); g_test_add_func("/errors/limits/nesting", limits_nesting); g_test_add_func("/errors/multiple_values", multiple_values);
In qobject_type(), NULL is returned when the 'QObject' returned from parse_value() is not of QString type, and this 'QObject' memory will leaked. So we need to first cache the 'QObject' returned from parse_value(), and finally free 'QObject' memory at the end of the function. Also, we add a testcast about invalid dict key. The memleak stack is as follows: Direct leak of 32 byte(s) in 1 object(s) allocated from: #0 0xfffe4b3c34fb in __interceptor_malloc (/lib64/libasan.so.4+0xd34fb) #1 0xfffe4ae48aa3 in g_malloc (/lib64/libglib-2.0.so.0+0x58aa3) #2 0xaaab3557d9f7 in qnum_from_int /Images/source_org/qemu_master/qemu/qobject/qnum.c:25 #3 0xaaab35584d23 in parse_literal /Images/source_org/qemu_master/qemu/qobject/json-parser.c:511 #4 0xaaab35584d23 in parse_value /Images/source_org/qemu_master/qemu/qobject/json-parser.c:554 #5 0xaaab35583d77 in parse_pair /Images/source_org/qemu_master/qemu/qobject/json-parser.c:270 #6 0xaaab355845db in parse_object /Images/source_org/qemu_master/qemu/qobject/json-parser.c:327 #7 0xaaab355845db in parse_value /Images/source_org/qemu_master/qemu/qobject/json-parser.c:546 #8 0xaaab35585b1b in json_parser_parse /Images/source_org/qemu_master/qemu/qobject/json-parser.c:580 #9 0xaaab35583703 in json_message_process_token /Images/source_org/qemu_master/qemu/qobject/json-streamer.c:92 #10 0xaaab355ddccf in json_lexer_feed_char /Images/source_org/qemu_master/qemu/qobject/json-lexer.c:313 #11 0xaaab355de0eb in json_lexer_feed /Images/source_org/qemu_master/qemu/qobject/json-lexer.c:350 #12 0xaaab354aff67 in tcp_chr_read /Images/source_org/qemu_master/qemu/chardev/char-socket.c:525 #13 0xfffe4ae429db in g_main_context_dispatch (/lib64/libglib-2.0.so.0+0x529db) #14 0xfffe4ae42d8f (/lib64/libglib-2.0.so.0+0x52d8f) #15 0xfffe4ae430df in g_main_loop_run (/lib64/libglib-2.0.so.0+0x530df) #16 0xaaab34d70bff in iothread_run /Images/source_org/qemu_master/qemu/iothread.c:82 #17 0xaaab3559d71b in qemu_thread_start /Images/source_org/qemu_master/qemu/util/qemu-thread-posix.c:519 Fixes: 532fb5328473 ("qapi: Make more of qobject_to()") Reported-by: Euler Robot <euler.robot@huawei.com> Signed-off-by: Alex Chen <alex.chen@huawei.com> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> --- qobject/json-parser.c | 12 ++++++------ tests/check-qjson.c | 9 +++++++++ 2 files changed, 15 insertions(+), 6 deletions(-)