Menu

#1240 Index allocate/deallocate of INTEGER values over 255 fails

backport-needed
accepted
nobody
None
5
2013-01-08
2012-11-15
fhew
No

While testing my app, I think I've discovered an issue...

My app uses AgentX to interact with net-snmp and during the course of
events it typically performs a large number of the following sequence:

newIndex = IndexAllocate(ANY_INDEX) (of an INTEGER value)
Register(OID.newIndex);
...

Unregister(OID.index)
IndexDeallocate(OID, index)
...

============ time passes ============

I have investigate this further, I don't know which end has the problem,
but I have located the issue. Its not an issue of the number of allocations,
but 'selective index usage'. I believe if you will see the problem with this
simplified test:

IndexAllocate(1);
IndexAllocate(256);

IndexDeallocate(256);

The Background:
Net-SNMP records its index allocations as an ordered
linked list. When you deallocate an index, it scans
the list until it either finds your entry, or
(in theory) finds an entry beyond your request.

The trouble is, that (at least in the case of INTEGER indexes on
a little-endian machine) is that the test for 'beyond your entry'
isn't correct. ie. 'did I match the entry I'm looking for, or have
I passed it.'

I don't know if this is a generic routine (for all types of indexes)
or if its a little/big endian issue, but the routine unregister_index()
in agent/agent_index.c uses memcmp() as its check technique.

On a typical scenario (looking for index '216'), the comparisons
look like this:

...
d8 00 00 00 d6 00 00 00 d8 is > d6, so keep scanning
d8 00 00 00 d7 00 00 00 d8 is > d7, so keep scanning
d8 00 00 00 d8 00 00 00 d8 is <= d8, so exit the scan

Given my simplified two entry test (1, 256) above, the linked
list to traverse looks like this:

01 00 00 00
00 01 00 00

so the comparison (when deallocating index '256') goes like this:

memcmp(00 01 00 00 01 00 00 00)
^^ ^^
memcmp will test the first two bytes of the sequences
to find that 00 is <= 01 so it exits the scan prematurely!

It should be scanning the most significant octet first!

So... is the problem:

  • the list (with INTEGERS) was built in little-endian mode?
    (I happen to be running on Intel)
  • the list (with INTEGERS) is not endian-ness independent?
  • the list scanner assumed little-endian-ess (INTEGERS)?
  • the list scanner assumed a STRING index and didn't account correctly
    for INTEGER indexes (in a known or unknown) endianess storage format?

Since I don't know what the original intent was
(for the allocator, and for the various index types)
I'm reluctant to develop and submit a fix for this problem.

============ more time passes ============

After reviewing the existing code, I see in the file 'agent/agent_index.c'
the register_index() routine performs three different actions (for integers,
strings and OIDs) to determine prior existence of an index. The attached
patch duplicates that relevant bit of code into the unregister_index()
routine also.

I have tested the patch (for INTEGER style indexes only) against my
examples above and it resolves the problem.

1 Attachments

Discussion

  • Dave Shield

    Dave Shield - 2013-01-08
    • status: open --> accepted
     
  • Dave Shield

    Dave Shield - 2013-01-08

    Applied to V5-4-patches and above

     

Log in to post a comment.