Fencepost issue.

[GoogleCodeExporter]

As seen on the list (reported by Mat Hostetter <mjhostetter@gmail.com>):

http://groups.google.com/group/memcached/browse_thread/thread/45b70cced2c4bd32

In items.c in memcached-1.2.8, these arrays allow indices up to but
not including LARGEST_ID:

#define LARGEST_ID 255
...
static item *heads[LARGEST_ID];
static item *tails[LARGEST_ID];

So this comment (which appears twice) is confused:

   /* always true, warns: assert(it->slabs_clsid <= LARGEST_ID); */

The assert should be checking for clsid < LARGEST_ID, not <=.
And then the warning would go away.  Or was the intent always to have
256
entries in the arrays, not 255?

Similarly, this code in items.c should be checking >=, not >, as
heads[LARGEST_ID] would be out-of-bounds:

   if (slabs_clsid > LARGEST_ID) return NULL;
   it = heads[slabs_clsid];

IMHO the confusion stems from the fact that LARGEST_ID is not itself a
valid id, as the name seems to imply. 

Original issue reported on code.google.com by dsalli...@gmail.com on 4 Jun 2009 at 3:56

[GoogleCodeExporter]

I've a branch on github with a fix.  I do not it possible to exploit this 
issue, but
correctness is correctness.  :)

Original comment by dsalli...@gmail.com on 22 Jun 2009 at 7:52

• Changed state: Started
• Added labels: Priority-Low
• Removed labels: Priority-Medium

[GoogleCodeExporter]

Looks good, but could you also just nuke the item_init() function? It's just a 
waste of time (they are already set to 
0)

Original comment by trond.no...@gmail.com on 10 Sep 2009 at 7:34

[GoogleCodeExporter]

Original comment by trond.no...@gmail.com on 10 Sep 2009 at 10:10

• Changed state: Fixed