[redis]github-529 Report

1. Symptom

Server crashed on “bitop” commond when a non-existing key is used.

1.1 Severity

severe

1.2 Was there exception thrown?

yes. crash

1.2.1 Were there multiple exceptions?

no

1.3 Scope of the failure

one node

2. How to reproduce this failure

2.0 Version

2.5.9

2.1 Configuration

standard

2.2 Reproduction procedure

1. set a "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"

2. bitop or x a b

  --- This is a bitwise “or” between key a and key b, storing value in key x.

  --- The key to reproduce this failure is that “b” is a non-existing key in the db.

2.2.1 Timing order

In this order

2.2.2 Events order externally controllable?

Yes. Deterministic.

2.3 Can the logs tell how to reproduce the failure?

Yes. Even the server log, if you read it carefully, contains.

2.4 How many machines needed?

1.

3. Diagnosis procedure

3.1 Detailed Symptom (where you start)

the crash dump.

3.2 Backward inference

From the log, we can notice that the command failed at was “bitop”:

[5095] 31 Aug 20:01:02.287 # argv[0]: 'bitop'

[5095] 31 Aug 20:01:02.287 # argv[1]: 'or'

[5095] 31 Aug 20:01:02.287 # argv[2]: 'x'

[5095] 31 Aug 20:01:02.287 # argv[3]: 'a'

[5095] 31 Aug 20:01:02.287 # argv[4]: 'b'

4. Root cause

When the key was non-existence, the bitopCommand handles it wrongly (not clearing minlen of the keys, resulting in a later crash).

/* BITOP op_name target_key src_key1 src_key2 src_key3 ... src_keyN */

void bitopCommand(redisClient *c) {

   .. .. ..

    for (j = 0; j < numkeys; j++) {

        o = lookupKeyRead(c->db,c->argv[j+3]);

        /* Handle non-existing keys as empty strings. */

        if (o == NULL) {

            objects[j] = NULL;

            src[j] = NULL;

            len[j] = 0;

  +        minlen = 0; ← the key doen’t exist, should set “minlen” to 0!

            continue;

        }

  }

 

.. ..

     /* Fast path: as far as we have data for all the input bitmaps we

         * can take a fast path that performs much better than the

         * vanilla algorithm. */

        j = 0;

        if (minlen && numkeys <= 16) {

         crashed in this block!

            unsigned long *lp[16];

            unsigned long *lres = (unsigned long*) res;

            /* Note: sds pointer is always aligned to 8 byte boundary. */

            memcpy(lp,src,sizeof(unsigned long*)*numkeys);

            memcpy(res,src[0],minlen);

    .. ..

    }

4.1 Category:

Incorrect exception handling