Bug #21226 FLUSH PRIVILEGES does not provided feedback when it fails.
Submitted: 22 Jul 2006 1:58 Modified: 12 Aug 2008 16:44
Reporter: Morgan Tocker Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server Severity:S3 (Non-critical)
Version:4.0,5.1, 5.0.54 OS:Any
Assigned to: Kristofer Pettersson CPU Architecture:Any
Tags: Contribution
Triage: D3 (Medium)

[22 Jul 2006 1:58] Morgan Tocker
Description:
Both 4.0.26 and 5.1.11 do not report errors to the user during 'flush privileges'. In 4.0.26, grant_reload() and acl_reload() do not return a value and no error is reported to the caller. In 5.1.11, they return a my_bool, but the error return values are ignored by the caller in reload_acl_and_cache().

One of the problems that we have encountered is that 'flush privileges' does not appear to load the new privileges but no error is reported back to the user via the SQL interface. This only occurs when servers are near out of memory, and I suspect that there are memory allocation errors that cause the failure.

How to repeat:
Insert a considerable number of user accounts in the privilege tables.  Issue a FLUSH PRIVILEGES command.

Suggested fix:
Alert the user when FLUSH PRIVILEGES failed.
[23 Jul 2006 9:22] Valeriy Kravchuk
Thank you for the bug report. Even in the latest 5.1.12-BK (ChangeSet@1.2262, 2006-07-22 13:37:14-04:00, mikael@dator5.(none) +2 -0), we have in sql/sql_parse.cc, line 6939 and down:

bool reload_acl_and_cache(THD *thd, ulong options, TABLE_LIST *tables,
                          bool *write_to_binlog)
{
  bool result=0;
  select_errors=0;                              /* Write if more errors */
  bool tmp_write_to_binlog= 1;

  if (thd && thd->in_sub_stmt)
  {
    my_error(ER_STMT_NOT_ALLOWED_IN_SF_OR_TRG, MYF(0), "FLUSH");
    return 1;
  }

#ifndef NO_EMBEDDED_ACCESS_CHECKS
  if (options & REFRESH_GRANT)
  {
    THD *tmp_thd= 0;
    /*
      If reload_acl_and_cache() is called from SIGHUP handler we have to
      allocate temporary THD for execution of acl_reload()/grant_reload().
    */
    if (!thd && (thd= (tmp_thd= new THD)))
    {
      thd->thread_stack= (char*) &tmp_thd;
      thd->store_globals();
    }
    if (thd)
    {
      (void)acl_reload(thd);
      (void)grant_reload(thd);
    }
...
That is, result from acl_reload() is ignored.

Even more clear it is in 4.0-BK sources:

bool reload_acl_and_cache(THD *thd, ulong options, TABLE_LIST *tables)
{
  bool result=0;
  bool error_already_sent=0;
  select_errors=0;                              /* Write if more errors */
  if (options & REFRESH_GRANT)
  {
    acl_reload(thd);
    grant_reload(thd);
    if (mqh_used)
      reset_mqh(thd,(LEX_USER *) NULL,true);
  }
...
[5 Jun 2007 15:06] Martin Friebe
see comment

Attachment: load_acl_mem.patch (text/x-patch), 2.67 KiB.

[5 Jun 2007 15:15] Martin Friebe
There were to location in grant_load, which is called inside grant_reload that allocate memory (by object creation) but do not set an error. So nothing will be reported.
The hash functions do already set an error.

acl_reload I couldn't find memory related isues, but there are errors that go to the server error log, but are not reported. Maybe that should also be done for some of the warnings.

and of course I forgot in the patch, in sql_parse, the part quoted in the original, the error need to be passed on 

    if (thd)
    {
      if (acl_reload(thd))
        result= 1;
      if (grant_reload(thd))
        result= 1;
    }

Sorry, no tests. Unless osomeone has an idea how to force the out of memory to happen at the correct instruction
[5 Jun 2007 15:19] Martin Friebe
same as above, just added changes to sql_parse.cc

Attachment: load_acl_mem.patch (text/x-patch), 3.17 KiB.

[10 Jun 2008 14:42] Mark Callaghan
5.0.54 is still broken for this as code in sql_parse.cc ignores the return values from acl_reload and grant_reload
    if (thd)
    {
      (void)acl_reload(thd);
      (void)grant_reload(thd);
    }
5.1.24 has been fixed

For our 5.0 build, I added the following to sql_parse after the calls to acl_reload and grant_reload.

      if (result)
      {
        /* When an error is returned, my_message may have not been called and the
         * client will hang waiting for a response. Code in sql_acl is very lax
         * about checking for and reporting errors. This guarantees that the
         * client gets an error. */
        my_message(ER_UNKNOWN_ERROR, "FLUSH PRIVILEGES failed", MYF(0));
      }
    }
[10 Jun 2008 14:42] Mark Callaghan
I disagree with the triage level. Not knowing that a permission push has failed can be a big security problem.
[18 Jun 2008 19:08] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/48121

2636 Kristofer Pettersson	2008-06-18
      Bug#21226 FLUSH PRIVILEGES does not provided feedback when it fails.
        
      If during a FLUSH PRIVILEGES the server fails to load the new privilege
      tables, the error message is lost. This patch is a back port from 5.1 which
      adresses this issue by setting the server in an error state if a failure
      occurrs.
        
      This patch also corrects an incorrect variable assignment which might
      cause an error state to be reverted by coincidence.
[18 Jun 2008 19:16] Chad MILLER
Revno 2636 looks good to me.
[7 Aug 2008 2:24] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/51059

2686 Davi Arnaut	2008-08-06
      Bug#21226 FLUSH PRIVILEGES does not provided feedback when it fails.
      
      Post-merge fix: remove spurious semicolon that caused the function
      to return failure regardless of the outcome.
[7 Aug 2008 2:26] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/51060

2686 Davi Arnaut	2008-08-06
      Bug#21226 FLUSH PRIVILEGES does not provided feedback when it fails.
      
      Post-merge fix: remove spurious semicolon that caused the function
      to return failure regardless of the outcome.
[7 Aug 2008 18:38] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/51138

2744 Marc Alff	2008-08-07 [merge]
      Merge mysql-6.0-bugteam -> local bugfix branch
[7 Aug 2008 22:10] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/51152

2687 Chad MILLER	2008-08-07 [merge]
      Merge from bugteam trunk.
[8 Aug 2008 13:31] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/51192

2751 Chad MILLER	2008-08-08 [merge]
      Merge from 5.1 bugteam local.

-- 
MySQL Code Commits Mailing List
For list archives: http://lists.mysql.com/commits
To unsubscribe:    http://lists.mysql.com/commits?unsub=commits@bugs.mysql.com
[12 Aug 2008 14:33] Bugs System
Pushed into 6.0.7-alpha  (revid:davi.arnaut@sun.com-20080807022358-rbwzc6yk0ztfagsc) (version source revid:davi.arnaut@sun.com-20080812141852-8e6knbqclpfd8irn) (pib:3)
[12 Aug 2008 15:08] Bugs System
Pushed into 5.1.28  (revid:davi.arnaut@sun.com-20080807022358-rbwzc6yk0ztfagsc) (version source revid:davi.arnaut@sun.com-20080812142843-he05ncsggstbn57z) (pib:3)
[12 Aug 2008 16:44] Paul Dubois
Noted in 5.1.28, 6.0.7 changelogs.

The FLUSH PRIVILEGES statement did not produce an error when it
failed.
[12 Aug 2008 19:13] Bugs System
Pushed into 5.0.68  (revid:kpettersson@mysql.com-20080618190930-xf3b3ys23j4z3dd7) (version source revid:davi.arnaut@sun.com-20080812185100-d47qb8mz2ye6pe6b) (pib:3)
[12 Aug 2008 19:44] Paul Dubois
Noted in 5.0.68 changelog.
[14 Aug 2008 7:32] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/51602

2684 He Zhenxing	2008-08-14 [merge]
      Merge 6.0 -> 6.0-rpl-testfixes
[14 Aug 2008 7:36] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/51603

2684 He Zhenxing	2008-08-14 [merge]
      Merge 6.0 -> 6.0-rpl-testfixes
[28 Aug 2008 20:14] Bugs System
Pushed into 6.0.7-alpha  (revid:cbell@mysql.com-20080822132131-uveo6wiuecy6m2b8) (version source revid:cbell@mysql.com-20080822132131-uveo6wiuecy6m2b8) (pib:3)
[13 Sep 2008 19:39] Bugs System
Pushed into 6.0.6-alpha  (revid:davi.arnaut@sun.com-20080807022358-rbwzc6yk0ztfagsc) (version source revid:sergefp@mysql.com-20080611231653-nmuqmw6dedjra79i) (pib:3)