Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-40848

Server crash on a compound statement with deeply nested BEGIN blocks

    XMLWordPrintable

Details

    • Not for Release Notes

    Description

      Problem and Use Case

      Any user who can run a statement can crash the whole server with a single
      statement of about 44 KB. The statement is an anonymous compound statement
      whose BEGIN ... END blocks are nested a few thousand levels deep. The server
      dies on SIGSEGV. Every connected client loses its session at the same time.

      No privilege beyond the ability to connect and run a statement is required. No
      table is touched, no stored data is read, and nothing is created.

      Rejecting the statement does not prevent the crash. A body that cannot compile
      crashes the server at the same nesting depth as a valid one, because the crash
      happens while the server cleans up after the rejection rather than while it
      runs the statement.

      A stored variant is likely and is not yet measured. A procedure nested just
      below the depth that crashes CREATE PROCEDURE is created successfully and
      written to mysql.proc. The same walk over the block structure runs again on
      every CALL, starting from a point where far more of the stack has already
      been consumed, so a procedure that was created without incident can crash the
      server every time anyone calls it. The routine survives a restart, and the
      crash is triggered by whoever calls the procedure rather than by whoever
      created it. Confirming this is the first thing to do on the ticket; the
      commands are under Reproduction.

      Affected: every maintained branch, 10.6 through main.
      Not a regression. The recursion has had this shape since at least 2013.

      Context

      Each BEGIN ... END block in a compound statement pushes a parse context.
      LEX::sp_block_init() at sql/sql_lex.cc:8196 calls
      sp_pcontext::push_context(), which links the new context into its parent
      through the parent's m_children array. The nesting depth of that tree is
      set entirely by the statement text, and nothing bounds it.
      sql/sp_pcontext.cc contains no check_stack_overrun() call on any
      branch, and it walks the tree recursively in two places.

      The first walk is the teardown. sp_pcontext::~sp_pcontext() at
      sql/sp_pcontext.cc:124 loops over m_children and deletes each entry at
      line 127. Deleting a child runs the child destructor, which deletes its own
      children, so the recursion depth equals the block nesting depth.
      sp_head::~sp_head() triggers the walk with delete m_pcont at
      sql/sp_head.cc:913. That destructor also runs when the parser rejects the
      body: lex_end_nops() calls sp_head::destroy() at
      sql/sql_lex.cc:1389 for every statement that built an sp_head,
      including one the parser abandoned. This confirms the report's central claim
      that a body which cannot compile crashes the server just the same.

      The second walk is sp_pcontext::retrieve_field_definitions() at
      sql/sp_pcontext.cc:688, which calls itself on each child at lines 700 and
      757. A block that declares no variables recurses immediately at line 700, so an
      empty nested block still costs a full frame. This walk runs from
      sp_head::rcontext_create() at sql/sp_head.cc:1721 and
      sql/sp_head.cc:1732, and from trigger execution at sql/sp_head.cc:1837.
      It is on the execution path, not the parse path.

      The report attributes both of its measured thresholds to the destructor and
      treats retrieve_field_definitions() as unreachable. That holds for proof of
      concept B and not for proof of concept A, and the difference explains why the
      two thresholds are so far apart. An anonymous BEGIN NOT ATOMIC block is
      executed. LEX::maybe_start_compound_statement() at sql/sql_lex.cc:8785
      builds an sp_head with the procedure handler, SQLCOM_COMPOUND at
      sql/sql_parse.cc:5658 hands it to do_execute_sp(), and
      sp_head::execute_procedure() at sql/sp_head.cc:2203 calls
      rcontext_create() at line 2247. A valid anonymous block therefore reaches
      retrieve_field_definitions() deep inside the execution stack, long before
      the teardown runs. A rejected CREATE PROCEDURE body never executes, so only
      the destructor runs, and it runs close to the top of the stack. Two recursions
      with different base offsets account for the reported depths of 3039 and 4588
      without requiring them to be the same frame.

      That reading is falsifiable without a debugger. A malformed anonymous
      BEGIN NOT ATOMIC body should survive well past depth 3039 and crash near
      the CREATE PROCEDURE threshold instead, because the rejected body never
      executes and only the teardown walk runs. The report did not test that case.

      The two downward walks are not the only unguarded recursions in the file.
      Eleven more call sites walk up the parent chain one frame per nesting level,
      across find_variable(), find_goto_label(), find_label(),
      find_label_current_loop_start(), find_condition(), find_type_def(),
      find_handler() and find_cursor(). Each is a tail call, so an optimizing
      compiler may fold it into a loop. They are listed for completeness rather than
      as confirmed crash sites.

      One note for whoever fixes it: dropping a check_stack_overrun() call at the
      recursion point does not work here, for two separate reasons.

      • The destructor cannot report an error and must not stop halfway, so the
        teardown walk has no way to fail cleanly. MDEV-39209 hit the same wall on
        cleanup_stranded_units() and was resolved by making the walk iterative.
      • A guard at parse time would never trip. The C stack does not grow with
        nesting depth while parsing, because the bison stack is heap allocated
        through my_yyoverflow() at sql/sql_parse.cc:7412. The report's own
        measurements show this: 4587 levels parse to a clean syntax error, and 6000
        nested IF ... END IF blocks parse without incident.

      That leaves two workable shapes: an explicit nesting depth limit enforced in
      LEX::sp_block_init(), which fixes every walk at once by never building the
      deep tree, or iterative rewrites of the destructor and of
      retrieve_field_definitions().

      The reachable nesting depth has a ceiling of its own. my_yyoverflow()
      refuses to grow the parser stack past MY_YACC_MAX, 32000 entries, and each
      nested block consumes several entries. On builds with a much larger thread
      stack (ASAN, UBSAN, and MSAN with debug all raise it) the parser ceiling may be
      reached before the crash depth, which is consistent with
      mysql-test/main/lotofstack.test already excluding those builds. On an
      ordinary build the crash depth is well inside the parser ceiling.

      Validation against the source. Confirmed: no check_stack_overrun() in
      sql/sp_pcontext.cc on any branch; the recursive destructor; the recursive
      retrieve_field_definitions(); the destructor running on the rejection path;
      the cost of 11 bytes per nesting level (BEGIN plus END and its terminator); and
      the 44061 byte statement size the generator produces at depth 4000. Corrected:
      the report cites lines 126 and 685/686, while on main the destructor is at 124
      with the delete at 127, and retrieve_field_definitions() is at 688 with
      recursive calls at both 700 and 757 rather than one; and proof of concept A
      crashes in the execution walk rather than the teardown walk. Unverified without
      a running server: the depths 3039 and 4588 themselves, the base offsets that
      separate them, and the stored procedure variant.

      Version mapping. Every branch from 10.6 through main carries the recursive
      destructor, the recursive retrieve_field_definitions(), zero
      check_stack_overrun() calls in sql/sp_pcontext.cc, and the
      BEGIN NOT ATOMIC grammar rule. The recursive teardown predates the
      maintained branches; commit dfcc502ab540 (2013) converted an equally recursive
      sp_pcontext::destroy() into the current destructor. The reported revision
      a217e1000f37a429a9c1d1c308952a959276b77e is on main.

      Reproduction

      Plain SQL, no build tuning

      Against any maintained branch. The anonymous form needs no DELIMITER
      handling, because a compound statement can be prepared:

      EXECUTE IMMEDIATE CONCAT('BEGIN NOT ATOMIC ', REPEAT('BEGIN ', 6000),
                               'SET @x= 1; ', REPEAT('END; ', 6000), 'END');
      

      The server dies on SIGSEGV. The boundary is sharp on a given build: the report
      measures depth 3038 surviving and depth 3039 crashing on a non-sanitizer Debug
      build with the default 292 KiB thread_stack. Depth 6000 is used above so
      the case still crashes on a release build, whose frames are smaller.

      The rejection path is reached by making the body invalid, which keeps the
      statement from ever executing:

      EXECUTE IMMEDIATE CONCAT('BEGIN NOT ATOMIC ', REPEAT('BEGIN ', 6000),
                               'THIS IS NOT VALID SQL; ', REPEAT('END; ', 6000),
                               'END');
      

      Confirming the stored procedure variant

      Raise the depth until CREATE PROCEDURE itself crashes, then back off one
      step and call the procedure that was accepted. The point to establish is
      whether a depth that CREATE PROCEDURE accepts is one that CALL cannot
      survive, which is what turns a transient crash into a stored one.

      DELIMITER //
      CREATE PROCEDURE p1() BEGIN BEGIN /* nested to the chosen depth */ END; END; //
      DELIMITER ;
      CALL p1();
      

      If the CALL crashes, restart and repeat the CALL to show that the
      routine persists, and check whether mariadb-dump --routines and
      SHOW CREATE PROCEDURE p1 are affected as well.

      MTR

      The case belongs in the existing mysql-test/main/lotofstack.test, which
      already collects the deeply nested stack overrun cases and already excludes the
      ASAN, UBSAN and MSAN-with-debug builds whose larger thread stacks make the
      depth unreachable. Insert the block before the # End of 10.6 tests line,
      since the fix targets 10.6 and later.

      # Depth 6000 is roughly twice the depth at which an anonymous compound
      # statement exhausts the stack on a non-sanitizer Debug build with the
      # default 292K thread_stack, so the case still reaches the crash on a
      # release build, whose frames are smaller. It also stays under the
      # ceiling imposed by MY_YACC_MAX, past which the parser rejects the
      # statement before the crash depth is reached.
      SET @depth= 6000;
       
      --disable_result_log
       
      # A valid body is executed, which walks the block tree once to build the
      # runtime context.
      --error 0,ER_STACK_OVERRUN_NEED_MORE
      EXECUTE IMMEDIATE CONCAT('BEGIN NOT ATOMIC ', REPEAT('BEGIN ', @depth),
                               'SET @x= 1; ', REPEAT('END; ', @depth), 'END');
       
      # A body that cannot compile is never executed, so this case reaches only
      # the teardown of the block tree, which runs on the rejection path too.
      --error ER_PARSE_ERROR,ER_STACK_OVERRUN_NEED_MORE
      EXECUTE IMMEDIATE CONCAT('BEGIN NOT ATOMIC ', REPEAT('BEGIN ', @depth),
                               'THIS IS NOT VALID SQL; ', REPEAT('END; ', @depth),
                               'END');
       
      --enable_result_log
       
      SET @depth= NULL;
      SET @x= NULL;
      

      Against unfixed code the server crashes on the first statement. That first case
      accepts success as well as the error, because the two candidate fixes differ in
      outcome: bounding the nesting depth reports an error, while making the two
      walks iterative lets the statement run to completion. Record the .result
      against whichever fix lands.

      Repoted by: daffainfo

      Attachments

        1. mdev_40848.test
          0.2 kB
        2. mdev_40848_teardown.test
          0.3 kB
        3. mdev_40848_sp.test
          0.5 kB

        Issue Links

          Activity

            People

              Khaled.amr Khaled Amr
              bnestere Brandon Nesterenko
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Git Integration

                  Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.