MySQL Bugs: #20498: Wrong usage of mysys/hash.c

Bug #20498	Wrong usage of mysys/hash.c
Submitted:	16 Jun 2006 10:25	Modified:	26 Jun 2006 13:24
Reporter:	Kristian Nielsen	Email Updates:
Status:	Closed	Impact on me:	None
Category:	MySQL Cluster: Cluster (NDB) storage engine	Severity:	S2 (Serious)
Version:	5.1.12	OS:	Linux (Linux/all)
Assigned to:	Kristian Nielsen	CPU Architecture:	Any

Description:
Found this problem with Valgrind, hence the P1/showstopper setting (feel free to change in wrong).

The problem is in this code, in ha_ndbcluster.cc:

THD_NDB_SHARE *
Thd_ndb::get_open_table(THD *thd, const void *key)
{
  DBUG_ENTER("Thd_ndb::get_open_table");
  HASH_SEARCH_STATE state;
  THD_NDB_SHARE *thd_ndb_share=
    (THD_NDB_SHARE*)hash_first(&open_tables, (byte *)key, sizeof(key), &state);

It passes 'key' for the hash key and 'sizeof(key)' for the key lenght. But that does not make any sense. The length should be the size of the buffer containing the key, not the size of the pointer to the buffer.

I *think* that the code was meant to use the 'const void *key' pointer value itself as the has key. For this, it should pass '(byte *)&key' to hash_first(). As it is, the code uses the first few bytes of an NdbDictionary::Table structure, which will probably lead to unpredictable results. I will commit a patch for this.

Alternatively, if the key was really meant to be the content of memory pointed to by 'key', the length passed to hash_first should be the size of the type pointed to, which I believe would be sizeof(NdbDictionary::Table).

How to repeat:
This gives complaints about uninitialized memory in hash_first():

    perl mysql-test-run.pl --valgrind-all ndb_autodiscover3

Also code inspection as described above.

Suggested fix:
I will commit a suggested fix shortly.

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/7752

Pushed to 5.1.12.

For documentation: This fixes a Valgrind report of access to uninitialized memory.

The bug was that the wrong pointer was used for hash lookup in the NDB code.

The bug could potentially cause wrong behaviour, but it has not been determined under what circumstances that would actually happen, and what the observed behaviour would be in those cases.

No feature change or fix of known misbehaviour to document for end users. Closed.