qemu-commits
[Top][All Lists]
Advanced
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-commits] [qemu/qemu] 3a81a1: monitor: Plug memory leak on QMP erro

From: GitHub
Subject: [Qemu-commits] [qemu/qemu] 3a81a1: monitor: Plug memory leak on QMP error
Date: Thu, 26 Nov 2015 09:00:04 -0800 
  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: 3a81a10179b702e031d8f84438193d83a64b4122
      
https://github.com/qemu/qemu/commit/3a81a10179b702e031d8f84438193d83a64b4122
  Author: Markus Armbruster <address@hidden>
  Date:   2015-11-26 (Thu, 26 Nov 2015)

  Changed paths:
    M monitor.c

  Log Message:
  -----------
  monitor: Plug memory leak on QMP error

Leak introduced in commit 8a4f501..710aec9, v2.4.0.

Signed-off-by: Markus Armbruster <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Reviewed-by: Luiz Capitulino <address@hidden>


  Commit: 4f2d31fbc0bfdf41feea7d1be49f4f7ffa005534
      
https://github.com/qemu/qemu/commit/4f2d31fbc0bfdf41feea7d1be49f4f7ffa005534
  Author: Markus Armbruster <address@hidden>
  Date:   2015-11-26 (Thu, 26 Nov 2015)

  Changed paths:
    M qobject/json-streamer.c

  Log Message:
  -----------
  qjson: Apply nesting limit more sanely

The nesting limit from commit 29c75dd "json-streamer: limit the
maximum recursion depth and maximum token count" applies separately to
braces and brackets.  This makes no sense.  Apply it to their sum,
because that's actually a measure of recursion depth.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>


  Commit: 0753113a26bb8c77f951b1ea91fd4f36d099c37a
      
https://github.com/qemu/qemu/commit/0753113a26bb8c77f951b1ea91fd4f36d099c37a
  Author: Markus Armbruster <address@hidden>
  Date:   2015-11-26 (Thu, 26 Nov 2015)

  Changed paths:
    M qobject/json-streamer.c

  Log Message:
  -----------
  qjson: Don't crash when input exceeds nesting limit

We limit nesting depth and input size to defend against input
triggering excessive heap or stack memory use (commit 29c75dd
json-streamer: limit the maximum recursion depth and maximum token
count).  However, when the nesting limit is exceeded,
parser_context_peek_token()'s assertion fails.

Broken in commit 65c0f1e "json-parser: don't replicate tokens at each
level of recursion".

To reproduce stuff 1025 open braces or brackets into QMP.

Fix by taking the error exit instead of the normal one.

Reported-by: Eric Blake <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>


  Commit: f0ae0304c7a41a42b7d4a6cde450da938d3c2cc7
      
https://github.com/qemu/qemu/commit/f0ae0304c7a41a42b7d4a6cde450da938d3c2cc7
  Author: Markus Armbruster <address@hidden>
  Date:   2015-11-26 (Thu, 26 Nov 2015)

  Changed paths:
    M tests/check-qjson.c

  Log Message:
  -----------
  check-qjson: Add test for JSON nesting depth limit

This would have prevented the regression mentioned in the previous
commit.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>


  Commit: b8d3b1da3cdbb02e180618d6be346c564723015d
      
https://github.com/qemu/qemu/commit/b8d3b1da3cdbb02e180618d6be346c564723015d
  Author: Markus Armbruster <address@hidden>
  Date:   2015-11-26 (Thu, 26 Nov 2015)

  Changed paths:
    M include/qapi/qmp/json-lexer.h
    M qobject/json-lexer.c

  Log Message:
  -----------
  qjson: Spell out some silent assumptions

Signed-off-by: Markus Armbruster <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Eric Blake <address@hidden>


  Commit: c54616608af442edf4cfb7397a1909c2653efba0
      
https://github.com/qemu/qemu/commit/c54616608af442edf4cfb7397a1909c2653efba0
  Author: Markus Armbruster <address@hidden>
  Date:   2015-11-26 (Thu, 26 Nov 2015)

  Changed paths:
    M include/qapi/qmp/json-lexer.h
    M qobject/json-lexer.c
    M qobject/json-parser.c
    M qobject/json-streamer.c

  Log Message:
  -----------
  qjson: Give each of the six structural chars its own token type

Simplifies things, because we always check for a specific one.

Signed-off-by: Markus Armbruster <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Eric Blake <address@hidden>


  Commit: 50e2a467f5315fa36c547fb6330659ba45f6bb83
      
https://github.com/qemu/qemu/commit/50e2a467f5315fa36c547fb6330659ba45f6bb83
  Author: Markus Armbruster <address@hidden>
  Date:   2015-11-26 (Thu, 26 Nov 2015)

  Changed paths:
    M qobject/json-parser.c

  Log Message:
  -----------
  qjson: Inline token_is_keyword() and simplify

Signed-off-by: Markus Armbruster <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Eric Blake <address@hidden>


  Commit: 6b9606f68ec589def27bd2a9cea97ec63cffd581
      
https://github.com/qemu/qemu/commit/6b9606f68ec589def27bd2a9cea97ec63cffd581
  Author: Markus Armbruster <address@hidden>
  Date:   2015-11-26 (Thu, 26 Nov 2015)

  Changed paths:
    M qobject/json-parser.c

  Log Message:
  -----------
  qjson: Inline token_is_escape() and simplify

Signed-off-by: Markus Armbruster <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Eric Blake <address@hidden>


  Commit: d2ca7c0b0d876cf0e219ae7a92252626b0913a28
      
https://github.com/qemu/qemu/commit/d2ca7c0b0d876cf0e219ae7a92252626b0913a28
  Author: Paolo Bonzini <address@hidden>
  Date:   2015-11-26 (Thu, 26 Nov 2015)

  Changed paths:
    M include/qapi/qmp/json-lexer.h
    M include/qapi/qmp/json-streamer.h
    M qobject/json-lexer.c
    M qobject/json-streamer.c

  Log Message:
  -----------
  qjson: replace QString in JSONLexer with GString

JSONLexer only needs a simple resizable buffer.  json-streamer.c
can allocate memory for each token instead of relying on reference
counting of QStrings.

Signed-off-by: Paolo Bonzini <address@hidden>
Message-Id: <address@hidden>
[Straightforwardly rebased on my patches, checkpatch made happy]
Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Eric Blake <address@hidden>


  Commit: d538b25543f4db026bb435066e2403a542522c40
      
https://github.com/qemu/qemu/commit/d538b25543f4db026bb435066e2403a542522c40
  Author: Markus Armbruster <address@hidden>
  Date:   2015-11-26 (Thu, 26 Nov 2015)

  Changed paths:
    M qobject/json-parser.c

  Log Message:
  -----------
  qjson: Convert to parser to recursive descent

We backtrack in parse_value(), even though JSON is LL(1) and thus can
be parsed by straightforward recursive descent.  Do exactly that.

Based on an almost-correct patch from Paolo Bonzini.

Signed-off-by: Paolo Bonzini <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Eric Blake <address@hidden>


  Commit: 95385fe9ace7db156b924da6b6f5c9082b68ba68
      
https://github.com/qemu/qemu/commit/95385fe9ace7db156b924da6b6f5c9082b68ba68
  Author: Paolo Bonzini <address@hidden>
  Date:   2015-11-26 (Thu, 26 Nov 2015)

  Changed paths:
    M include/qapi/qmp/json-parser.h
    M include/qapi/qmp/json-streamer.h
    M monitor.c
    M qga/main.c
    M qobject/json-parser.c
    M qobject/json-streamer.c
    M qobject/qjson.c
    M tests/libqtest.c

  Log Message:
  -----------
  qjson: store tokens in a GQueue

Even though we still have the "streamer" concept, the tokens can now
be deleted as they are read.  While doing so convert from QList to
GQueue, since the next step will make tokens not a QObject and we
will have to do the conversion anyway.

Signed-off-by: Paolo Bonzini <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Eric Blake <address@hidden>


  Commit: 9bada8971173345ceb37ed1a47b00a01a4dd48cf
      
https://github.com/qemu/qemu/commit/9bada8971173345ceb37ed1a47b00a01a4dd48cf
  Author: Paolo Bonzini <address@hidden>
  Date:   2015-11-26 (Thu, 26 Nov 2015)

  Changed paths:
    M include/qapi/qmp/json-streamer.h
    M qobject/json-parser.c
    M qobject/json-streamer.c

  Log Message:
  -----------
  qjson: surprise, allocating 6 QObjects per token is expensive

Replace the contents of the tokens GQueue with a simple struct.  This cuts
the amount of memory allocated by tests/check-qjson from ~500MB to ~20MB,
and the execution time from 600ms to 80ms on my laptop.  Still a lot (some
could be saved by using an intrusive list, such as QSIMPLEQ, instead of
the GQueue), but the savings are already massive and the right thing to
do would probably be to get rid of json-streamer completely.

Signed-off-by: Paolo Bonzini <address@hidden>
Message-Id: <address@hidden>
[Straightforwardly rebased on my patches]
Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Eric Blake <address@hidden>


  Commit: df649835fe48f635a93316fdefe96ced7189316e
      
https://github.com/qemu/qemu/commit/df649835fe48f635a93316fdefe96ced7189316e
  Author: Markus Armbruster <address@hidden>
  Date:   2015-11-26 (Thu, 26 Nov 2015)

  Changed paths:
    M qobject/json-streamer.c

  Log Message:
  -----------
  qjson: Limit number of tokens in addition to total size

Commit 29c75dd "json-streamer: limit the maximum recursion depth and
maximum token count" attempts to guard against excessive heap usage by
limiting total token size (it says "token count", but that's a lie).

Total token size is a rather imprecise predictor of heap usage: many
small tokens use more space than few large tokens with the same input
size, because there's a constant per-token overhead: 37 bytes on my
system.

Tighten this up: limit the token count to 2Mi.  Chosen to roughly
match the 64MiB total token size limit.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>


  Commit: a5df35070a4c7fa8e2d9c6bd7175ee8e3e0f7641
      
https://github.com/qemu/qemu/commit/a5df35070a4c7fa8e2d9c6bd7175ee8e3e0f7641
  Author: Peter Maydell <address@hidden>
  Date:   2015-11-26 (Thu, 26 Nov 2015)

  Changed paths:
    M include/qapi/qmp/json-lexer.h
    M include/qapi/qmp/json-parser.h
    M include/qapi/qmp/json-streamer.h
    M monitor.c
    M qga/main.c
    M qobject/json-lexer.c
    M qobject/json-parser.c
    M qobject/json-streamer.c
    M qobject/qjson.c
    M tests/check-qjson.c
    M tests/libqtest.c

  Log Message:
  -----------
  Merge remote-tracking branch 'remotes/armbru/tags/pull-monitor-2015-11-26' 
into staging

QMP and QObject patches

# gpg: Signature made Thu 26 Nov 2015 09:07:18 GMT using RSA key ID EB918653
# gpg: Good signature from "Markus Armbruster <address@hidden>"
# gpg:                 aka "Markus Armbruster <address@hidden>"

* remotes/armbru/tags/pull-monitor-2015-11-26:
  qjson: Limit number of tokens in addition to total size
  qjson: surprise, allocating 6 QObjects per token is expensive
  qjson: store tokens in a GQueue
  qjson: Convert to parser to recursive descent
  qjson: replace QString in JSONLexer with GString
  qjson: Inline token_is_escape() and simplify
  qjson: Inline token_is_keyword() and simplify
  qjson: Give each of the six structural chars its own token type
  qjson: Spell out some silent assumptions
  check-qjson: Add test for JSON nesting depth limit
  qjson: Don't crash when input exceeds nesting limit
  qjson: Apply nesting limit more sanely
  monitor: Plug memory leak on QMP error

Signed-off-by: Peter Maydell <address@hidden>


Compare: https://github.com/qemu/qemu/compare/317e4db6e904...a5df35070a4c
reply via email to
[Prev in Thread] Current Thread [Next in Thread]