diff --git a/README.md b/README.md index 47839dc..884ce0e 100644 --- a/README.md +++ b/README.md @@ -37,135 +37,120 @@ The example goes into detail around how to use, or look at `src/SNMP_Agent.h` fo If you're coming from v1, most, but not all APIs are drop-in replaceable. Some of the API's, especially around strings have changed. Look in `SNMP_Agent.h` for details. -If you're upgrading from any prior release (including the original Arduino_SNMP `v2.1.x`, or this fork's `v2.2.0` / `v3.0.0` / `v3.1.0` / `v3.1.1` / `v3.1.2` / `v3.1.3` / `v3.1.4`), read **"What's New in v3.1.5"** immediately below. v3.1.5 is a single cumulative release that folds: (1) the v2.2.0 C-style string-model refactor, (2) the v3.0.0 critical BER TLV bug fixes (PR #60), (3) the 4-phase zero-heap deterministic-memory refactor, (4) v3.1.1 sketch-overridable tuning + SNMP_Sensor bug fixes, (5) v3.1.2 snmpTrapOID.0 RFC-3416 fix, (6) v3.1.3 ESP8266 auto-tune profile, (7) v3.1.4 startup-heap ASNPool + narrowed SortableOID + universal OCTET=256 defaults, and (8) v3.1.5 arduino-lint LD003 extras/demos compliance. +If you're upgrading from v2.0/v2.1, note the **2.2.0 string model change** below. ---- +If you're upgrading from v2.2, v3.0.0 is **source-compatible** (no API changes) but fixes several critical BER TLV encoding/decoding bugs. Mandatory upgrade if you use GetBulk, large responses (length ≥ 128 bytes, especially exactly 256 bytes), or snmpbulkwalk. + +If you're upgrading from v3.0.0 / v3.0.6 to v3.1.0: **100% source + wire compatible, zero API changes, zero breaking changes.** v3.1.0 closes out the 4-phase zero-heap refactor (eliminates all remaining `std::vector` / `std::deque` / `std::list` from library source; replaces last `make_shared` temp-allocations with pool-allocated raw BER objects; drops 3 dead standard-container includes + 1 dead inline method that was pulling shared_ptr machinery per-TU). Flash is slightly smaller on every target (−0.71% average vs v3.0.0 baseline; largest win PlatformIO esp32dev −1.3% = −9.6 KB), BSS is deterministic +48.9 KB (linker-reported, no mid-packet fragmentation, tuneable down via `SNMP_POOL_ASN_OBJECTS` if you're on esp01_1m). Mandatory upgrade if you've ever seen ESP-01 heap-fragmentation panics after 30+ days of SNMP polling. -## What's New in v3.1.5 (Cumulative: ALL changes since v2.1.0) +--- -v3.1.5 is a single release combining every earlier in-tree milestone category plus the latest patch-level housekeeping fixes. If you are upgrading from the historical `Arduino_SNMP` v2.1.x (or from any prior v2.2.0 / v3.0.0 / v3.1.x fork release) you are getting everything at once in this tag. Eight broad buckets of change rolled into v3.1.5: +## 3.1.0 Zero-Heap Refactor (Deterministic Memory) -1. **v2.2.0 (embedded string model):** all `std::string` replaced with fixed C-style `char[]` / `const char*`. No heap fragmentation from string reallocs; 3–8 KB Flash saved on ESP8266. -2. **v3.0.0 (BER TLV hardening, upstream PR #60):** 3 critical real on-the-wire BER bugs fixed (long-form header off-by-one, `length == 256` silently encoded as 0 which broke `snmpbulkwalk`, and a double-store undefined-behavior sign-extend). 101/101 tests green. -3. **Zero-Heap Deterministic-Memory Refactor (4-phase):** hot-path packet processing (`agent.loop()`, GET/GETNEXT/GETBULK/SET decode + build, TRAP/INFORM send) now performs **zero** `malloc`/`new`/`calloc`/`realloc`. All ASN.1 BER objects come from a compile-time-sized global placement pool; all VarBind/PDU/agent callback lists use fixed C-arrays with explicit `constexpr` capacity caps. No mid-packet heap-fragmentation panics after 30+ days of polling. -4. **v3.1.1 (sketch-overridable tuning + SNMP_Sensor bug fixes):** every size/pool/buffer constant wrapped with `#ifndef … #endif` so sketch-side defines or build-flags win, no patching library sources required; SNMP_Sensor `char*` → `const char*` OID const-correctness; critical SNMP_Sensor `addReadWriteStringHandler(&sysContact, 25, true)` hardcoded 25-byte SET cap fixed → `sizeof(sysContactValue)`. -5. **v3.1.2 (RFC-3416 snmpTrapOID.0 fix, issue #64):** SNMPv2 Trap/Inform VarBind #2 name `sysObjectID.0` → correct `snmpTrapOID.0` (`.1.3.6.1.6.3.1.1.4.1.0`) per RFC 3416 §3.1, so net-snmp `snmptrapd` can look up the NOTIFICATION-TYPE. -6. **v3.1.3 (ESP8266 auto-tune profile):** on ESP8266, unless `SNMP_SKIP_ESP8266_AUTOTUNE=1` is set, automatically shrinks all pool/buffer constants (ASNPool 64→24, callbacks 64→24, VarBinds 16→6, OCTET max 500→256, packet 1400→1024, slot size 768→640, etc.) saving ~28 KB BSS vs v3.1.2 so WiFi + LittleFS + ArduinoJson + SNMPAgent fit on the 80 KB-DRAM D1 mini / ESP-01. -7. **v3.1.4 (startup-heap ASNPool + narrowed SortableOID + universal OCTET 256):** the single biggest static BSS sink — `ASNPool slots[N]` — moves out of `.bss` into a **one-shot startup-time `new Slot[N]()` allocation** done exactly once on the first `asn_new()` call; never deallocated, never reallocated, never grows, count fixed at compile-time (opt-out back to static with `SNMP_POOLS_IN_BSS 1`). Effect: ESP8266 tiny gains a further ~15.5 KB BSS, ESP32 gains ~24.8 KB BSS free, and the ASNPool no longer occupies linker-reported global/static RAM on any target. Additionally: `SortableOIDType::sortingMap` narrows `unsigned long[32]` → `uint32_t[32]` (SMIv2 sub-IDs fit in 32 bits; saves 128 B/instantiation on 64-bit hosts + width matches encoder math); `OCTET_TYPE_MAX_LENGTH` universal default 500 → 256, still sketch-overridable. -8. **v3.1.5 (arduino-lint compliance, patch-level housekeeping):** - (a) Rule LD003 fix: `demos/` folder contained 3 `.ino` files → moved whole tree to `extras/demos/` (Arduino library spec allows sketches only under `examples/` or `extras/`). 3 `.ino` files are pure 100% renames; `platformio.ini` `lib_extra_dirs` updated `../../..` → `../../../..` (+1 nesting level). Fixes the `arduino/arduino-lint-action@v1.0.0` CI failure: `ERROR: Sketch(es) found outside examples and extras folders`. - (b) README absolute-path sanitization sweep: maintainer-local deep-home-folder `file:///...` references across README files stripped to GitHub-native repo-root-relative links. No code or API change; 100% wire and consumer compatible on top of v3.1.4. +v3.1.0 is the final release of the 4-phase deterministic-memory refactor. Steady-state packet processing (`agent.loop()`, `sendTrapTo`, GET/GETNEXT/GETBULK/SET decode + response build) now performs **zero** `malloc`/`new`/`calloc`/`realloc`. All ASN.1 BER objects come from a compile-time-sized global placement pool (`SNMP_POOL_ASN_OBJECTS = 64` slots × 768 B = 48 KB BSS, tunable). All VarBind/PDU/agent callback lists use fixed C-arrays with explicit compile-time capacity caps. -### User-visible API changes since v2.1.0 (only 2, both from v2.2.0) -Everything else — `addXxxHandler` / `sortHandlers` / `sendTrapTo` / `setUDP` / `begin` / `stop` / `loop` — is **100% source + wire compatible** back to v2.2.0. +### Why this change +- **Zero heap fragmentation under load** — field units that previously crashed after 30+ days of 1 Hz SNMP polling (heap fragmented so a 512-byte UDP packet buffer couldn't allocate) now run indefinitely with linker-reported deterministic memory. +- **Smaller flash** (−0.71% geometric mean across all 4 DoD targets) — removing dead ``/`` includes, deleting a zero-call-site inline method that pulled shared_ptr machinery per-TU, and collapsing 3 separate `remove_if` lambda-template instantiations into one C-style predicate dispatcher collectively saved ~4.6 KB. +- **Compile-time sizing, fail-safe** — every queue/array/buffer has a `constexpr` maximum (see table below). Overflows return well-defined error codes instead of undefined-behavior heap exhaustion panics. + +### Compile-Time Sizing Constants (tune in [defs.h](src/include/defs.h#L59-L75) before including) + +| Constant | Default | Purpose | +|------------------------------|---------|---------| +| `SNMP_MAX_OID_SUBIDENTIFIERS`| 32 | OID sub-IDs (realistic max ~17) | +| `SNMP_MAX_COMPLEX_CHILDREN` | 16 | Children per decoded PDU/VarBind-list ComplexType | +| `SNMP_MAX_VARBINDS` | 16 | VarBinds per request/response (snmpbulkwalk default is 10) | +| `SNMP_MAX_CALLBACKS_PER_AGENT` | 64 | Registered OID handlers per SNMPAgent instance | +| `SNMP_MAX_AGENTS` | 2 | Concurrent SNMPAgent instances (usually just 1) | +| `SNMP_MAX_UDP_PER_AGENT` | 2 | UDP transport interfaces per agent (WiFi + ETH fallback) | +| `SNMP_MAX_TRAPS_INFLIGHT` | 8 | INFORM retry queue + pending TRAP depth | +| `SNMP_MAX_CALLBACKS_PER_TRAP`| 16 | OID pointers embedded in a single SNMPTrap object | +| `SNMP_POOL_ASN_OBJECTS` | 64 | ASNPool slots — global BER_CONTAINER placement pool (decode + build + trap + clone) | +| `SNMP_POOL_VARBIND_OBJECTS` | 32 | VarBind placement pool (packet build/trap path) | + +**ESP-01 / esp01_1m tuning recommendation** (BSS clawback, drops RAM usage from 97.8% → ~68%): +```c +#define SNMP_MAX_COMPLEX_CHILDREN 8 +#define SNMP_MAX_VARBINDS 4 +#define SNMP_MAX_CALLBACKS_PER_AGENT 16 +#define SNMP_MAX_TRAPS_INFLIGHT 4 +#define SNMP_POOL_ASN_OBJECTS 32 +#define SNMP_POOL_VARBIND_OBJECTS 12 +/* include SNMP_Agent.h AFTER the #defines above */ +#include +``` +Saves ≈ `(64−32) × 768 B` = 24,576 B BSS immediately plus additional savings from smaller arrays in callbacks/varbinds. -| Symbol | Change since v2.1.0 | +### Verification (all green, Definition of Done) +| Check | Result | |---|---| -| `GETSTRING_FUNC` typedef | `const std::string (*)()` → **`const char* (*)()`** | -| `OIDType::string()` return type | `const std::string&` → **`const char*`** (zero-copy, returns a `const char*` into a fixed backing buffer) | +| Host catch2 (native clang 14 / g++) | ✅ 101/101 assertions in 10 test cases | +| AddressSanitizer (memory + leak) | ✅ 0 errors / 0 leaks (ASNPool path + heap-fallback both clean) | +| Arduino-CLI `esp8266:esp8266:generic` | ✅ Flash 254,424 B (24%), RAM 100% | +| Arduino-CLI `esp32:esp32:esp32` | ✅ Flash 904,223 B (68%), RAM 29% | +| PlatformIO env esp8266 (board esp01_1m, dout) | ✅ Flash 283,351 B (37.2%), RAM 97.8% | +| PlatformIO env esp32 (board esp32dev) | ✅ Flash 740,693 B (56.5%), RAM 29.1% | +| Build flags all targets | ✅ `-Wall -Wextra -Werror` across all 4 | +| `src/` header standard-container audit | ✅ 0 `` / 0 `` / 0 `` / 0 ``. Only 2 `` retained for public shared_ptr compat (OIDType::cloneOID + VarBind legacy ctors) | + +### Detailed Changelog (3.0.6 → 3.1.0) +1. **Phase 1.5 (last deque):** Changed 3 PDU-handler out-parameter signatures from `std::deque&` → `VarBind out[SNMP_MAX_VARBINDS] + int& outCount`. All 15 `emplace_back` → placement-construct via `appendResponseVarBind` helper. Deleted the last `std::deque` instance in `SNMPParser.cpp`. 15 internal `make_shared` / `make_shared` temp-allocations → direct `asn_new` pool raw-pointers. +2. **Phase 1.1–1.4 structural audit (no rework):** Verified all 4 Phase 1 conversions were already complete and operational from prior tags: + - OpaqueType `uint8_t* _value` calloc → `_value[OCTET_TYPE_MAX_LENGTH]` fixed array. + - OIDType `std::vector data` → `uint8_t data[SNMP_MAX_OID_SUBIDENTIFIERS+1] + int dataLen`. + - SortableOIDType `std::vector` → `unsigned long sortingMap[SNMP_MAX_OID_SUBIDENTIFIERS] + int sortingMapLen`. + - ComplexType `std::vector> values` → `BER_CONTAINER* values[SNMP_MAX_COMPLEX_CHILDREN] + int valuesLen + bool _ownsChildren` (uniform recursive ownership via asn_delete in dtor). +3. **Phase 3 (init-only callback new):** *Accepted as-is, no code change.* All `addXxxHandler` invocations happen exactly once inside `setup()`; this is explicitly allowed by the zero-heap blueprint (the ban applies to packet-hot-path / ISR / loop). Users who want 100% static BSS can still use `static IntegerCallback cb(…)` + `addHandler(&cb)` overload (already supported). +4. **Phase 4 (dead header + dead method sweep):** Dropped stale `` includes from [BER.h](src/include/BER.h) and [SNMPResponse.h](src/include/SNMPResponse.h); dropped last `` from [SNMPParser.h](src/include/SNMPParser.h) coincident with sig change; deleted dead inline `ComplexType::addValueToList(const shared_ptr&)` method (zero call sites anywhere in library/tests, was pulling shared_ptr refcount machinery into every TU that included BER.h). + +**Footprint delta vs v3.0.0 (baseline):** +- Arduino-CLI esp8266: Flash −2,648 B (−1.0%); BSS +48,901 B (+49.0 KB, deterministic ASNPool). +- Arduino-CLI esp32: Flash −4,976 B (−0.55%); BSS +48,901 B (+49.0 KB). +- PlatformIO esp01_1m: Flash −3,160 B (−1.1%); BSS +49,824 B (+49.8 KB). +- PlatformIO esp32dev: Flash −14,112 B (−1.87%); BSS +49,896 B (+49.9 KB). +- **Geometric mean flash reduction: −0.71% (−4,649 B avg).** -### String model change (v2.2.0 → present) — copy-paste migration +--- -**Static string handlers** (no `std::string` anymore; use a static `char[]` literal): -```cpp -// OLD, v2.1: -std::string sysDescr = "ESP32 SNMP Agent"; -snmp.addReadOnlyStaticStringHandler(".1.3.6.1.2.1.1.1.0", sysDescr); +## 3.0.0 Critical Bug Fixes + BER Hardening (Major Release) -// NEW, v3.1: -char sysDescr[] = "ESP32 SNMP Agent"; // or const char* PROGMEM literal -snmp.addReadOnlyStaticStringHandler(".1.3.6.1.2.1.1.1.0", sysDescr); -``` +v3.0.0 is a major hardening release focused on the BER TLV engine — fixes real-world interoperability failures with `snmpbulkwalk` and `snmpset` for responses ≥ 128 bytes, corrects 3-byte BER length header handling, removes undefined behavior in signed 3-byte integer decode, adds defensive overflow bounds checks, and removes a stack buffer overread from the test harness. -**Read-write string buffers** (static storage, no `malloc`): -```cpp -char _sysContactBuf[255]; -char* sysContact = _sysContactBuf; -snprintf(sysContact, sizeof(_sysContactBuf), "admin@example.com"); -snmp.addReadWriteStringHandler(".1.3.6.1.2.1.1.4.0", &sysContact, sizeof(_sysContactBuf), true); -``` +### Summary +- **Breaking for build/test tooling? No.** No API changes. +- **Breaking for binary flash footprint? Slightly smaller.** Removed dead memset, removed redundant `.reserve()` before `.assign()`, inlined compound-operator, hoisted temp array. +- **Breaking for network behavior? Fixes silent corruption.** Packets that used to be truncated/zero-length on the wire are now correctly formatted. -**Dynamic-string callbacks** (`GETSTRING_FUNC` now returns `const char*`): -```cpp -const char* getFirmwareVersion(void) { return LIBRARY_VERSION; } // "3.1.5" from defs.h -snmp.addReadOnlyStringHandler(".1.3.6.1.4.1.99.0", getFirmwareVersion); -``` +### Upstreamed from 0neblock/Arduino_SNMP PR #60 +All 5 commits of PR #60 (Aidan Cyr + N1IOX, 2024-08 to 2026-01) are integrated: -### Critical BER TLV bug fixes (v3.0.0 → present) -These are real on-the-wire failures. Upgrade if you use `snmpbulkwalk`, responses ≥ 128 bytes, or SNMP Set with 3-byte signed integer payloads. All five are integrated from upstream PR #60 plus defensive boundary hardening grown out of the audit. - -1. **Hardcoded `_length + 2` return bug (OIDType / Counter64 / ComplexType / BER_CONTAINER fromBuffer).** BER length fields ≥ 128 bytes use long-form headers (3+ bytes instead of 2). Before: returned a hardcoded `+ 2` regardless, walked off-structure. After: returns the actual TLV header bytes consumed. -2. **`length == 256` encoded as 0 (catastrophic).** `encode_ber_length_integer` used `if(integer > 256)` (off-by-one). Exactly 256-byte response PDUs serialized as `0x81 0x00` (= length 0 per ASN.1 BER), which net-snmp/pysnmp silently dropped. Fixed in both `encode_ber_length_integer` and its paired byte-counter `encode_ber_length_integer_count`. -3. **Undefined behavior `tempVal = tempVal |= 0xFF000000`** in IntegerType 3-byte signed decode (double-store, `-Wsequence-point` error). Reduced to `tempVal |= 0xFF000000;`. -4. **Stack buffer overread in test harness** `memcpy(&buffer[i], &randomLong, 10)` → `memcpy(…, sizeof(randomLong))` (2–6 bytes past stack end on 64-bit hosts). -5. **Defensive overflow pre-checks** added at BER_CONTAINER / OIDType / Counter64 / ComplexType `fromBuffer` entry. `ComplexType::fromBuffer` child-walk replaced buggy dual-condition loop with a descending `remaining` counter. - -### Deterministic zero-heap (4-phase) — what this means for your firmware -Before the zero-heap refactor, a single decoded SNMP PDU did ≥24 `new`/`delete` pairs (one per ASN.1 field, two per shared_ptr refcount block). Under sustained 1 Hz polling this fragmented the ESP-01 heap so badly that after ≈30 days, the next incoming 512-byte UDP packet could not be allocated contiguously → **panic reboot**. - -v3.1.0 replaces every hot-path allocation, and v3.1.4 extends the model with optional startup-heap pool allocation, using one of two storage strategies: -- **Compile-time-sized global placement pool** for all BER_CONTAINER subclass objects (`IntegerType`, `OctetType`, `OIDType`, `ComplexType`, …). Generic default: `SNMP_POOL_ASN_OBJECTS = 32` slots × `SNMP_POOL_SLOT_SIZE = 768 B` = ~24,576 B; ESP8266 tiny auto-profile drops that to 24 × 640 B = 15,360 B. v3.1.4 allocates the slot storage once at startup via `new Slot[N]()` (opt-out `SNMP_POOLS_IN_BSS 1` returns the old static `.bss` layout). If all N slots are ever simultaneously occupied (pathological trap storm), the code gracefully falls back to a regular `::new T` — defensive, never triggers in steady state (decode tree + serialise + free all return to pool before the next packet). `ASAN` is clean on both paths. -- **Fixed C-arrays with explicit count member** for every library list/queue: VarBinds per packet, OID handlers per agent, UDPs per agent, concurrent SNMPAgent instances, INFORM retry queue, callback list per SNMPTrap object, child-values inside ComplexType. Every such buffer has a compile-time `constexpr` maximum. Overflow returns a well-defined error code — no OOM panic. - -#### 14 Compile-Time Sizing Constants (tune before `#include `) -Declared in [defs.h](src/include/defs.h): - -| Constant | Default (non-ESP8266) | `_SNMP_ESP8266_TINY` auto (ESP8266, on by default) | Purpose | -|------------------------------|------------------------|------------------------------------------------------|---------| -| `MAX_SNMP_PACKET_LENGTH` | 1400 | 1024 | Incoming/outgoing UDP packet scratch buffer | -| `OCTET_TYPE_MAX_LENGTH` | 256 | 256 | OctetType / OpaqueType internal fixed buffer | -| `SNMP_MAX_COMMUNITY_LEN` | 64 | 64 | Community-string buffer | -| `SNMP_MAX_OID_STR_LEN` | 256 | 192 | Dotted-decimal OID storage (e.g. "1.3.6.1.4.1.…") | -| `SNMP_MAX_STRING_LEN` | = `OCTET_TYPE_MAX_LENGTH` | same | String column SET upper bound (alias) | -| `SNMP_MAX_OID_SUBIDENTIFIERS`| 32 | 32 | BER-encoded OID sub-ID count (realistic max ~17) | -| `SNMP_MAX_COMPLEX_CHILDREN` | 16 | 8 | Children per decoded PDU/VarBind-list ComplexType | -| `SNMP_MAX_VARBINDS` | 16 | 6 | VarBinds per request/response (snmpbulkwalk default = 10) | -| `SNMP_MAX_CALLBACKS_PER_AGENT` | 64 | 24 | Registered OID handlers per SNMPAgent instance | -| `SNMP_MAX_AGENTS` | 2 | 2 | Concurrent SNMPAgent instances (usually just 1) | -| `SNMP_MAX_UDP_PER_AGENT` | 2 | 2 | UDP transport interfaces per agent (WiFi + ETH fallback) | -| `SNMP_MAX_TRAPS_INFLIGHT` | 8 | 4 | INFORM retry queue + pending trap depth | -| `SNMP_MAX_CALLBACKS_PER_TRAP`| 16 | 8 | OID pointers embedded in a single SNMPTrap object | -| `SNMP_POOL_ASN_OBJECTS` | 32 | 24 | ASNPool slots — global BER_CONTAINER placement pool (decode + build + trap + clone) | -| `SNMP_POOL_VARBIND_OBJECTS` | 12 | 8 | VarBind placement pool (packet build/trap path) | -| `SNMP_POOL_SLOT_SIZE` | 768 | 640 | Byte payload size per ASNPool slot (must fit `SortableOIDType` largest subclass) | - -**Two opt-out `#define` switches** (place BEFORE `#include `): -- `#define SNMP_SKIP_ESP8266_AUTOTUNE 1` — disable the ESP8266 `_SNMP_ESP8266_TINY` shrink profile on ESP8266, use generic defaults above. -- `#define SNMP_POOLS_IN_BSS 1` — force the ASNPool back into static `.bss` arrays (v3.1.3 behavior). Without it (default, v3.1.4+ → still current in v3.1.5), ASNPool storage is allocated once at startup via `new Slot[N]()` so it does not count against linker-reported globals — ~20–50 KB more headroom for your code. - -#### ESP-01 1 MB / 80 KB-DRAM tuning (headroom: ~50 KB globals FREE / 80 KB) -Since v3.1.4 (still current in v3.1.5), on ESP8266 the `_SNMP_ESP8266_TINY` profile is **automatic** (no user defines needed) and ASNPool storage lives by default in the startup heap, not BSS. So most small sketches compile + link at **38–41% globals**, well under the 80 KB limit. Only re-tune if you need to serve >24 OIDs, >6 VarBinds/bulkwalk, or >4 in-flight traps. - -**Important ordering: put overrides BEFORE `#include ` in your sketch.ino.** +1. **BER `_length + 2` hardcoded-return bug** — BERDecode functions `OIDType`, `Counter64`, `ComplexType` all returned a hardcoded `_length + 2` instead of actual consumed bytes. When the BER length field encoded into long-form (≥ 128 byte payload = 3+ byte header instead of 2), the return value under-reported bytes consumed → packet parser walked off-structure into garbage. + * Now use `j + _length` / `i + _length` where j/i = actual TLV header bytes consumed. -```c -/* Optional: increase capacity on D1 mini / bigger ESP8266 modules. */ -#define SNMP_MAX_CALLBACKS_PER_AGENT 64 -#define SNMP_POOL_ASN_OBJECTS 32 -#define SNMP_MAX_VARBINDS 16 -#define SNMP_MAX_COMPLEX_CHILDREN 16 -#include -``` -→ Saves ≈ `(default v3.1.2 pool of 64 × 768 B) − (tiny 24 × 640 B)` = **~33,792 B BSS + pool-on-heap = an additional ~15,360 B / slot** versus v3.1.2 on ESP8266, measured in real builds. +2. **`length == 256` encoded as `0`** (catastrophic) — `encode_ber_length_integer()` had `if(integer > 256)` off-by-one. A response exactly 256 bytes long was encoded as `0x81 0x00` (= length 0 per ASN.1 BER). Net-snmp `snmpbulkwalk` fails immediately because the PDU is zero-length. Same bug existed in the paired byte-counter `encode_ber_length_integer_count()` used by `ComplexType::serialise()` shift-array logic → internal length/bookkeeping mismatch for any 256 ≤ len < 512 packet. + * Changed **both functions** to `if(integer >= 256)`. -### Release verification matrix (all green) +3. **Undefined behavior: `tempVal = tempVal |= 0xFF000000`** — 3-byte signed IntegerType sign-extend code used double-store (`= result of |=`) which is undefined order-of-operations → treated as error by `-Werror=sequence-point`. + * Simplified to `tempVal |= 0xFF000000;` -| Check | Result | -|---|---| -| Host catch2 (native clang 14 / g++) | ✅ 101/101 assertions in 10 test cases | -| AddressSanitizer (memory + leak) | ✅ 0 errors / 0 leaks (ASNPool path + heap-fallback both clean) | -| Arduino-CLI `esp8266:esp8266:d1_mini` + `examples/ESP32_SNMP` | ✅ Flash 261,340 B (24%), **globals 30,668 / 80,192 B (38%)** — 49,524 B FREE | -| Arduino-CLI `esp8266:esp8266:d1_mini` + `examples/SNMP_Sensor` | ✅ Flash 296,392 B (28%), **globals 33,128 / 80,192 B (41%)** — 47,064 B FREE | -| Arduino-CLI `esp32:esp32:esp32` + `examples/ESP32_SNMP` | ✅ Flash 918,531 B (70%), **globals 49,856 / 327,680 B (15%)** | -| Arduino-CLI `esp32:esp32:esp32` + `examples/SNMP_Sensor` | ✅ Flash 964,843 B (73%), **globals 50,984 / 327,680 B (15%)** | -| Build flags across all 4 targets | ✅ `-Wall -Wextra -Werror` clean | -| `src/` standard-container header audit | ✅ 0 `` / 0 `` / 0 `` / 0 ``. Only 2 `` retained purely for backwards-compat public shared_ptr ctor signatures (OIDType::cloneOID + legacy VarBind ctors). | - -### Net footprint vs v3.1.2 pre-optimization -Deterministic zero-hot-path heap unchanged (same deterministic pool sizing). Global BSS shrinks **per build**: -- ESP8266 d1_mini + `ESP32_SNMP.ino`: from **101% overflow (linker OOM)** (v3.1.2) → **30,668 B / 80,192 B (38%)** = **−~49 KB globals**. -- ESP32 DevKit + `ESP32_SNMP.ino`: from 74,680 B / 327,680 B (22%) (v3.1.3) → **49,856 B / 327,680 B (15%)** = **−24,824 B globals**, almost exactly the ASNPool slots[32] moved out of .bss into the startup heap. -- Flash: ≈ ±0.1% vs v3.1.3 (essentially unchanged; the startup `new Slot[]` code path is a handful of instructions). +4. **Stack buffer overread in tests** — `memcpy(&buffer[i], &randomLong, 10)` copied 10 bytes from a `long` (4 bytes on 32-bit, 8 bytes on 64-bit). Read 2–6 bytes past stack end. Also disabled flaky random-data "corrupt buffer" test section (random noise → sometimes valid BER → false CI failures) and exact-133-byte reparse false-negative (both temporarily `/* */` disabled per PR #60 guidance). + +### Additional Optimizations (not in PR #60) +Applied per the workspace [optimizations.md](.trae/rules/optimizations.md) Blueprint rules: + +* **[BER_CONTAINER::fromBuffer](src/BERDecode.cpp#L35-L51)** — boundary check used `_length + 2` (hardcoded header assumption). Now uses `_length + actual_header_len` computed via `ptr - buf`. Length-decoder's max iteration bound corrected: we've already consumed the type byte so decoder may iterate at most `max_len - 1` bytes, not `max_len`. +* **[OIDType::fromBuffer](src/BERDecode.cpp#L129-L142)** — Added overflow pre-check before dereferencing `*dataPtr`. Removed redundant `.reserve(_length)` immediately before `.assign(...)` (assign already sizes-to-fit → one allocator pass, not two). +* **[Counter64::fromBuffer](src/BERDecode.cpp#L216-L232)** — overflow pre-check added before value decode loop. +* **[ComplexType::fromBuffer](src/BERDecode.cpp#L286-L317)** — overflow pre-check + correct `max_len - outer_used` child-bounds propagation. Replaced buggy dual-condition `while(i < _length && i <= max_len)` with clean descending `remaining > 0` counter. +* **[SortableOIDType::generateSortingMap](src/BERDecode.cpp#L190-L208)** — removed dead `*1` multiplier in `.reserve(size * 1)` call. +* **[OIDType::generateInternalData](src/BEREncode.cpp#L145-L176)** — added `.reserve(SNMP_MAX_OID_STR_LEN)` to avoid mid-parse reallocations on each push_back; hoisted `uint8_t temp[10]` out of loop (was declared + zeroed every iteration); inverted the OID-component-valid guard (removes one nesting level + dead else); used `sizeof(temp)` instead of magic `10`. + +### Test Status +``` +All tests passed (97 assertions in 10 test cases) +``` +Built with `-std=c++11 -Wall -Wpedantic -Wextra -Werror`. --- @@ -181,13 +166,43 @@ As of v2.2.0, the entire library uses **fixed-size C-style strings** (`char[]` + ### Buffer Sizing Fixed maximum sizes are declared in [defs.h](src/include/defs.h): -| Constant | Default | ESP8266 tiny auto | Purpose | -|-----------------------------|---------|-------------------|----------------------------------| -| `SNMP_MAX_COMMUNITY_LEN` | 64 | 64 | Community string (RO/RW) | -| `SNMP_MAX_OID_STR_LEN` | 256 | 192 | OID dotted-decimal representation| -| `OCTET_TYPE_MAX_LENGTH` / `SNMP_MAX_STRING_LEN` | 256 | 256 | OctetString (OID value payload) | +| Constant | Value | Purpose | +|-----------------------------|-------|----------------------------------| +| `SNMP_MAX_COMMUNITY_LEN` | 64 | Community string (RO/RW) | +| `SNMP_MAX_OID_STR_LEN` | 256 | OID dotted-decimal representation| +| `SNMP_MAX_STRING_LEN` | 500 | OctetString (OID value payload) | + +These are conservative defaults; tune them in `defs.h` if you need smaller RAM footprint on the ESP-01. -These are sensible defaults; tune them from your sketch with `#define` BEFORE `#include ` if you need a smaller RAM footprint on the ESP-01. +### User-Code String Migration (v2.1 → v2.2) +Replace any `std::string` variables used with `addReadOnlyStaticStringHandler`, `addReadWriteStringHandler`, or `GETSTRING_FUNC` callbacks: + +Before (v2.1): +```cpp +#include +std::string sysDescr = "ESP32 SNMP Agent"; +snmp.addReadOnlyStaticStringHandler(".1.3.6.1.2.1.1.1.0", sysDescr); +``` + +After (v2.2): +```cpp +char sysDescr[] = "ESP32 SNMP Agent"; // or const char* to a PROGMEM literal +snmp.addReadOnlyStaticStringHandler(".1.3.6.1.2.1.1.1.0", sysDescr); +``` + +Read-write string: keep the `char**` pattern but use a static buffer (no `malloc`): +```cpp +char _sysContactBuf[255]; // static storage, NOT malloc'd +char* sysContact = _sysContactBuf; +snprintf(sysContact, sizeof(_sysContactBuf), "admin@example.com"); +snmp.addReadWriteStringHandler(".1.3.6.1.2.1.1.4.0", &sysContact, sizeof(_sysContactBuf), true); +``` + +`GETSTRING_FUNC` return type changed: now returns `const char*` (was `const std::string`): +```cpp +const char* getFirmwareVersion(void) { return LIBRARY_VERSION; } // "3.0.0" from defs.h +snmp.addReadOnlyStringHandler(".1.3.6.1.4.1.99.0", getFirmwareVersion); +``` --- @@ -332,13 +347,16 @@ I am working on adding the functionality to act as an SNMP Server or Manager. In | Version | Changes | |---------|----------------------------------------------------------------------| -| **3.1.5** | **Patch: arduino-lint LD003 compliance + README absolute-path (zero code/API/wire changes, on top of v3.1.4).** (1) Arduino library spec Rule LD003 fix: the `demos/` folder contained 3 `.ino` files (`arduino_cli_esp32`, `arduino_cli_esp8266`, `platformio_minimal/src/main`) that triggered `arduino/arduino-lint-action@v1.0.0` CI error `Sketch(es) found outside examples and extras folders`. Relocated entire `demos/` tree to `extras/demos/` (Arduino spec allows sketches under `extras/` or `examples/` only). 3 `.ino` files pure renames (git 100% match); `platformio.ini` `lib_extra_dirs` bumped `../../..` → `../../../..` plus `cd demos/platformio_minimal` → `cd extras/demos/platformio_minimal` comment banner. (2) README local-path sanitization: maintainer-local absolute paths stripped to bare repo-root-relative links, which GitHub renders natively as correct jump-to-line links. Version bump 3.1.4 → 3.1.5 in `library.properties` and `src/include/defs.h`. Host Catch2 101/101 green. No functional, API, or on-the-wire changes: 100% consumer compatible. | -| **3.1.4** | **Public milestone — v3.1.2 + v3.1.3 + v3.1.4 collapsed into one release (100% source & wire compatible, zero API breaks).** Delivered on top of v3.1.1, folding every change from the zero-heap baseline. (A) v3.1.2 RFC-3416 trap fix: SNMPv2c Trap/Inform VarBind #2 name `sysObjectID.0` → correct `snmpTrapOID.0` (`.1.3.6.1.6.3.1.1.4.1.0`), new named constant `SNMPv2_SNMPTRAP_OID_0` so net-snmp `snmptrapd` can look up NOTIFICATION-TYPE definitions (was logging "Cannot find TrapOID in TRAP2 PDU"). (B) v3.1.3 ESP8266 auto-tune + smaller generic defaults: on ESP8266, `_SNMP_ESP8266_TINY` profile activates automatically (opt-out `SNMP_SKIP_ESP8266_AUTOTUNE 1`) shrinking ASNPool/packet/VarBind/OCTET pools to safe small-sensor sizes; generic defaults also reduce ASNPool 64→32, VarBindPool 32→12; all constants now sketch-overridable via `#ifndef…#endif`; removes need for per-sketch shrink blocks. (C) v3.1.4 startup-heap ASNPool + narrower types: the single-biggest static BSS sink `ASNPool slots[N]` moves out of `.bss` into one-shot startup `new Slot[N]()` done exactly once on first `asn_new()` (default; opt-out back to static `.bss` via `SNMP_POOLS_IN_BSS 1`); `SortableOIDType::sortingMap unsigned long[32]` → `uint32_t[32]` (saves 128 B/instantiation on 64-bit hosts; SMIv2 sub-IDs fit in 32 bits exactly); `OCTET_TYPE_MAX_LENGTH` universal default 500→256 (sketch-overridable); `SNMP_Sensor.ino` portability fixes (LittleFS header, ESP8266 `FS_BEGIN()` + `os_random()` rng, timestamp `uint32_t`, removal of the 25-byte sysContact sysName sysLocation SET length cap). Net measured footprint: ESP8266:d1_mini + SNMP_Sensor globals dropped from v3.1.2 **101% OVERFLOW (linker OOM)** → v3.1.4 **33,128 / 80,192 B (41%)** with 47,064 B FREE; ESP32 + ESP32_SNMP globals dropped 74,680→49,856 B = **−24,824 B**. Full 4-target Arduino CLI matrix (2 sketches × esp8266+esp32) links clean. Host Catch2 101/101 green; `src/` header audit confirms zero `///`. | -| **3.1.1** | **Minor patch release — user tuning + example corrections.** Four targeted, zero-API-breakage changes on top of v3.1.0. (1) `defs.h` user-overridable sizing: every tuneable size/pool/buffer constant wrapped with `#ifndef … #endif` (15 total: `MAX_SNMP_PACKET_LENGTH`, `OCTET_TYPE_MAX_LENGTH`, 3 string `SNMP_MAX_*_LEN`, 10 `SNMP_MAX_*`/`SNMP_POOL_*`, `DEBUG`). Sketch-side `#define` placed BEFORE `#include ` or compiler `-D` flags now win over library defaults, no patch to headers needed. Large banner comment in defs.h documents the override order + ESP-01 clawback recipe. (2) Examples teach tuning: both `ESP32_SNMP.ino` and `SNMP_Sensor.ino` gain a top-of-sketch `COMPILE-TIME TUNING` banner showing commented-out 6-constant halved-ASNPool recipe (~24,576 B BSS saved). SNMP_Sensor banner additionally reminds ESP8266 users to swap `LITTLEFS` → `LittleFS` + install ESP8266LittleFS/ArduinoJson libraries. (3) `SNMP_Sensor.ino` const-correct OIDs: ~33 `char* oidFoo = ".1.3.6.1…"` variables → `const char* oidFoo`, matching the const string literals they bind. Eliminates deprecated `-Wwrite-strings` warnings on modern ESP32/ESP8266 toolchains. (4) CRITICAL `SNMP_Sensor.ino` SET length fix: `addReadWriteStringHandler(&sysContact, 25, true)` → `sizeof(sysContactValue)` (actual buffer size 255). Previously a 100-byte `snmpset` of sysContact/sysName/sysLocation was incorrectly rejected by the 25-byte artificial cap even though `loadSNMPValues()` used `strlcpy(…, sizeof(buf)=255)`; now the three paths (SNMP SET cap, declared C buffer, flash load) use a single consistent 255-byte maximum. Verification: host catch2 101/101 green; `examples/ESP32_SNMP` compiles under Arduino-CLI `esp32:esp32:esp32` with 0 warnings/errors. Net footprint delta vs v3.1.0: 0. | -| **3.1.0** | **Public release — cumulative of ALL changes since v2.1.0 (string model v2.2.0 + BER v3.0.0 + zero-heap refactor).** Delivered as one tested, backwards-compatible tag for upstream/public repos. 2 API signatures changed only (both from v2.2.0 string model, see above); `addXxxHandler/sendTrapTo/begin/loop/setUDP` all 100% unchanged since v2.2.0. (a) String model: all `std::string` removed library/examples/test-wide → fixed `char[]` + `const char*` + explicit length; `GETSTRING_FUNC` typedef → `const char*(*)()`; `OIDType::string()` → `const char*`; example sketch 3 `malloc`s → static buffers; 3 size constants: `SNMP_MAX_COMMUNITY_LEN=64`, `SNMP_MAX_OID_STR_LEN=256`, `SNMP_MAX_STRING_LEN=500`. (b) BER TLV critical fixes (PR #60 upstream + defensive): (1) 4× `fromBuffer` return `_length+2` hardcode → actual consumed bytes (fixes long-form header ≥128 B). (2) `encode_ber_length_integer*` off-by-one `>256` → `>=256` (fixes `length==256` → `0x8100` zero-length; broke snmpbulkwalk). (3) IntegerType 3-byte signed extend `tempVal = tempVal|=mask` UB → `tempVal|=mask`; fixed test-harness `memcpy(randomLong,10)` overread → `sizeof(randomLong)`; defensive max_len pre-checks on 4× fromBuffer; ComplexType child loop dual-condition → remaining counter. (c) Zero-heap 4-phase refactor: hot-path (`loop()`, GET/SET/BULK decode+build, TRAP/INFORM) 100% `malloc/new/calloc`-free. BER objects served from global placement pool (`SNMP_POOL_ASN_OBJECTS=64`, 768 B slot, heap-fallback defensive, ASAN clean on both). All library lists/queues → fixed `T[N] + int count` with compile-time caps; 10 sizing constants added: `SNMP_MAX_OID_SUBIDENTIFIERS=32, SNMP_MAX_COMPLEX_CHILDREN=16, SNMP_MAX_VARBINDS=16, SNMP_MAX_CALLBACKS_PER_AGENT=64, SNMP_MAX_AGENTS=2, SNMP_MAX_UDP_PER_AGENT=2, SNMP_MAX_TRAPS_INFLIGHT=8, SNMP_MAX_CALLBACKS_PER_TRAP=16, SNMP_POOL_ASN_OBJECTS=64, SNMP_POOL_VARBIND_OBJECTS=32`. 3 PDU-handler out-sigs `deque&` → `VarBind out[16]+int&outCount`. Dead ``/`` includes + zero-call-site ComplexType shared_ptr-overload removed. Final `src/` audit: 0 `///` (2 `` kept purely for backwards-compat shared_ptr public ctors). Verification: host catch2 101/101 green; ASAN clean; 4/4 strict DoD builds green under `-Wall -Wextra -Werror` (Arduino-CLI esp8266+esp32, PlatformIO esp01_1m+dout+esp32dev). Footprint: Flash −0.71% geometric mean vs pre-refactor (= −4.6 KB avg; esp32dev largest single win −14.1 KB = −1.87%). BSS +48.9 KB deterministic (linker-reported ASNPool; clawback via `#define SNMP_POOL_ASN_OBJECTS=32` before include cuts it in half for esp01_1m → drops from 97.8% to ~68% RAM). | -| 2.2.0 | (Internal precursor — absorbed into the cumulative v3.1.0 entry above for public release) | -| 2.1.0 | (Previous / original Arduino_SNMP) API cleanups, ESP-01 / ESP8266 support. | +| **3.1.5** | **Patch: arduino-lint LD003 compliance + README absolute-path PII sanitization.** Two targeted fixes, zero code/API/wire changes, fully on top of v3.1.4. (1) **Arduino library spec Rule LD003:** the `demos/` folder contained 3 `.ino` files (`arduino_cli_esp32`, `arduino_cli_esp8266`, `platformio_minimal/src/main`) which triggered `ERROR: Sketch(es) found outside examples and extras folders` under `arduino/arduino-lint-action@v1.0.0` on GitHub Actions. Relocated the entire `demos/` tree to `extras/demos/` (Arduino spec allows sketches under `extras/` or `examples/` only); 3 pure `.ino` moves recorded by git with 100% rename detection. Updated [platformio.ini](extras/demos/platformio_minimal/platformio.ini) `lib_extra_dirs` from `../../..` → `../../../..` plus the `cd` comment banner to account for the one extra nesting level. (2) **README local-path PII sanitization sweep:** all maintainer-local filesystem absolute paths (deep home-folder `file:///...` links) across `README.md`, `README2.md`, and `github-pull-request-post-v2.md` stripped down to bare repo-root-relative links (`src/include/defs.h#L59-L75` etc.), which GitHub renders natively as correct jump-to-line links. Same sanitization applied to the `tozip/README.md` release-zip mirror copy. Final repo-wide audit returns zero matches for local-home absolute paths anywhere in tracked files or release artifacts. Version bumped 3.1.4 → 3.1.5 in [library.properties](library.properties#L2) and [defs.h](src/include/defs.h#L34-L37). Host Catch2 101/101 green. No functional, API, or on-the-wire behavior changes — 100% consumer compatible. | +| **3.1.4** | **Major footprint pass — one-shot startup-heap pools + SortableOID shrink + universal OCTET 256 default.** All per-call / hot-path allocation remains 100% placement-new into fixed-capacity buffers (zero `new`/`malloc`/realloc after first `asn_new`). Changes: (1) **ASNPool startup-heap allocation (default on, opt-out via `#define SNMP_POOLS_IN_BSS 1`):** `ASNPool::Slot slots[N]` static BSS array replaced by one-shot `new Slot[N]()` called on first `asn_new()` from `_ensurePools()`. Count + layout still fixed at compile-time; never deallocated, never reallocated, never grows. Effect: moves the single biggest static BSS sink out of `.bss` onto the heap, clawing back **15–49 KB** of DRAM (depending on `SNMP_POOL_ASN_OBJECTS` + `SNMP_POOL_SLOT_SIZE`) on 80 KB-DRAM ESP8266 parts, which is the exact headroom needed to run WiFi + LittleFS + ArduinoJson alongside SNMP. API for `asn_new/asn_delete/isInPool/release` is byte-identical, public sigs unchanged. Guard: `SNMP_POOLS_IN_BSS 1` restores v3.1.3 behavior for codebases that prefer static pools. (2) **SortableOIDType `sortingMap` narrowed:** `unsigned long sortingMap[32]` (256 B on 64-bit / 128 B on 32-bit) → `uint32_t sortingMap[32]` (128 B / 128 B exactly on both word widths). Drops per-instance SortableOID footprint by 128 B on 64-bit; on 32-bit ESP8266/ESP32 the change is semantically cleaner because SNMP sub-IDs fit in 32 bits per SMIv2 rules, and the storage width now matches the encoder math with zero truncation risk. Updated [BERDecode.cpp generateSortingMap](src/BERDecode.cpp#L245) and [ValueCallbacks.cpp sort_oids](src/ValueCallbacks.cpp#L207) signatures accordingly. (3) **Universal `OCTET_TYPE_MAX_LENGTH` default tightened 500 → 256**, no longer ESP8266-tiny-exclusive. 256 B still covers 99% of real MIB strings (ifDescr, sysContact, sysName, sysLocation, EntityMIB strings all routinely < 128 B). Sketch-side override preserved: `#define OCTET_TYPE_MAX_LENGTH 500` before library include restores previous max. Effect on 32-bit targets: every OctetType/OpaqueType shrinks by −244 B, which in turn keeps SortableOIDType / ComplexType / VarBind layouts fitting into the smaller 640 B ESP8266-tiny slot. (4) Slot-size static assertion guards retained at the bottom of [BER.h](src/include/BER.h#L455) to loudly break any build where a concrete subclass grows larger than `SNMP_POOL_SLOT_SIZE`. (5) `_SNMP_ESP8266_TINY` auto-tune profile + `SNMP_SKIP_ESP8266_AUTOTUNE` opt-out still work as in v3.1.3. (6) Example sketches: BSS headroom further improved on ESP8266; ESP32 RAM also drops ~8 KB because the generic non-ESP8266 `ASNPool slots[32]` no longer occupies .bss (one-shot heap instead). Verification: host Catch2 101/101 green, 4x arduino-cli matrix (2 sketches × esp8266:d1_mini + esp32:esp32:esp32) all compile + link with measured BSS deltas documented above. Zero API changes, 100% wire-identical output (no BER code paths modified). | +| **3.1.3** | **Zero-Heap Footprint + ESP8266 Auto-Tune patch.** Library is safe for ESP-01 / D1 mini sketches that combine WiFi + LittleFS + ArduinoJson + SNMPAgent. Changes: (1) **defs.h ESP8266 auto-tune profile `_SNMP_ESP8266_TINY`:** when targeting ESP8266 (and user hasn't set `SNMP_SKIP_ESP8266_AUTOTUNE=1`), automatically reduces pool/buffer constants so static BSS shrinks by ≈ 28 KB vs v3.1.2 defaults. Tunes: `MAX_SNMP_PACKET_LENGTH 1400→1024`, `OCTET_TYPE_MAX_LENGTH 500→256`, `SNMP_MAX_OID_STR_LEN 256→192`, `SNMP_MAX_COMPLEX_CHILDREN 16→8`, `SNMP_MAX_VARBINDS 16→6`, `SNMP_MAX_CALLBACKS_PER_AGENT 64→24`, `SNMP_MAX_TRAPS_INFLIGHT 8→4`, `SNMP_MAX_CALLBACKS_PER_TRAP 16→8`, `SNMP_POOL_ASN_OBJECTS 64→24`, `SNMP_POOL_VARBIND_OBJECTS 32→8`, and new tunable `SNMP_POOL_SLOT_SIZE 768→640` (ESP8266 tiny only). (2) **Generic defaults (ESP32 / non-ESP8266) also reduced 30–50% where safe:** `SNMP_POOL_ASN_OBJECTS 64→32`, `SNMP_POOL_VARBIND_OBJECTS 32→12`, `SNMP_MAX_CALLBACKS_PER_AGENT 64→64` kept, but all other pool counters halved, yielding ≈ 25 KB RAM clawback on every non-ESP8266 target too. (3) **All constants remain sketch-overridable via `#ifndef` guards;** opt-out of ESP8266 auto-tune by `#define SNMP_SKIP_ESP8266_AUTOTUNE 1` before including `SNMP_Agent.h`. (4) `SNMP_Sensor.ino` no longer needs a sketch-side shrink-block (centralized), banner simplified. (5) Example sketches — cross-platform build matrix: `ESP32_SNMP.ino` + `SNMP_Sensor.ino` both link on esp8266:d1_mini (80 KB DRAM) at v3.1.3, with ≈ 10–14 KB globals headroom vs 101% BSS overflow on v3.1.2. Zero API changes, 100% wire-identical. Host Catch2 101/101 green. | +| **3.1.2** | **Patch: SNMPv2 Trap/Inform snmpTrapOID.0 RFC 3416 fix (issue #64 upstream).** Fixes a generic (non-hardware-specific) bug on every target: SNMPv2c Trap/Inform varbind #2 used the wrong **name** OID — `sysObjectID.0` (`.1.3.6.1.2.1.1.2.0`) instead of RFC-3416-required `snmpTrapOID.0` (`.1.3.6.1.6.3.1.1.4.1.0`). The VB #2 value (user-supplied notification-type OID set via `SNMPTrap::setTrapOID()`) was always correct; only the VB's *name* was mismatched. Effect on real receivers: net-snmp `snmptrapd` logged `Cannot find TrapOID in TRAP2 PDU` and was unable to look up the trap's NOTIFICATION-TYPE definition. Fix is two-part: (1) added new compile-time constant `SNMPv2_SNMPTRAP_OID_0` (".1.3.6.1.6.3.1.1.4.1.0") in [defs.h](src/include/defs.h#L174-L175) next to existing RFC1213 sysObjectID/sysUpTime constants; (2) [SNMPTrap.cpp](src/SNMPTrap.cpp#L5-L6) static OID initializers now use the named constants instead of duplicated magic literals — `SNMPTrap::s_timestampOID(RFC1213_OID_sysUpTime)` + `SNMPTrap::s_snmpTrapOID(SNMPv2_SNMPTRAP_OID_0)` — prevents re-typoing the 24-digit OID literals in future edits. 100% wire-compatible fix, zero API changes, zero footprint impact (constant folding produces identical binary). Verification: host catch2 101/101 green. | +| **3.1.1** | **Minor release — user-coder-friendly tuning + example hardening (patch).** Four targeted changes, zero source-API changes, zero footprint delta vs v3.1.0. (1) **defs.h user-overridable sizing:** every tuneable size/pool/buffer constant wrapped with `#ifndef … #endif` (15 in total: MAX_SNMP_PACKET_LENGTH, OCTET_TYPE_MAX_LENGTH, 3 × SNMP_MAX_*_LEN, 10 × SNMP_MAX_* / SNMP_POOL_*, DEBUG). Users can now write `#define SNMP_MAX_COMPLEX_CHILDREN 8` BEFORE `#include ` in their .ino, or pass `-DSNMP_POOL_ASN_OBJECTS=32` via build_flags, and their value wins over the library default without needing to patch defs.h. Added a large banner comment in defs.h documenting override ordering + ESP-01 clawback recipe. (2) **Examples teach the new tuning:** added identical COMPILE-TIME TUNING banners to both sketches (ESP32_SNMP.ino, SNMP_Sensor.ino) showing the 6-constant ESP-01 tune-down recipe + exact byte savings estimate (24,576 B BSS from halving ASNPool). SNMP_Sensor banner additionally warns ESP8266 users to swap LITTLEFS→LittleFS and install ESP8266LittleFS/ArduinoJson libraries. (3) **SNMP_Sensor const-correct OIDs:** 33 variables declared `char* oidFoo = ".1.3.6.1…"` → `const char* oidFoo`, matching the const string literal they point to. Eliminates deprecated `-Wwrite-strings` warnings on modern ESP32/ESP8266 toolchains. (4) **CRITICAL SNMP_Sensor SET length fix:** three `addReadWriteStringHandler(&sysContact, 25, true)` length 25 → `sizeof(sysContactValue)` (actual buffer size 255). Previously any SNMP SET of sysContact/sysName/sysLocation > 25 bytes was incorrectly rejected even though the declared storage was [255] and the load-from-flash path used strlcpy(…, 255); now SET max length, declared buffer size, and persistent-storage copy max length all agree, eliminating the inconsistent truncation. Verification: host catch2 101/101 green; examples/ESP32_SNMP compiled clean 0 warnings 0 errors (Arduino-CLI esp32:esp32:esp32, 72% Flash / 14% RAM). | +| **3.1.0** | **4-Phase Zero-Heap Refactor, deterministic-memory final release.** (a) Phase 1.5 last deque removed from hot path: 3 PDU handler out-param sigs changed `std::deque& → VarBind out[SNMP_MAX_VARBINDS] + int& outCount`; added placement-construct `appendResponseVarBind` helper; 15 emplace_back → helper; 15 internal `make_shared` temp allocations → direct raw `asn_new` pool pointers. (b) Phase 1.1–1.4 structural audit confirmed all 4 fixed-array conversions complete (OpaqueType `_value[]`, OIDType `data[]`, SortableOIDType `sortingMap[]`, ComplexType `values[]` + `_ownsChildren`). (c) Phase 3 init-only callback new documented compliant with Rules02 §1 (setup-only, exempt); no code change. (d) Phase 4 dead header/method sweep: dropped stale `` from BER.h + SNMPResponse.h; dropped last `` include from SNMPParser.h coincident with sig change; deleted dead inline ComplexType::addValueToList(shared_ptr<>) method (zero call sites, pulled shared_ptr machinery per-TU). Final `src/` audit: 0 ``/``/``/`` includes anywhere in library (only 2 `` retained for public shared_ptr API compat). 10 new compile-time sizing constants added to defs.h (SNMP_MAX_OID_SUBIDENTIFIERS, SNMP_MAX_COMPLEX_CHILDREN, SNMP_MAX_VARBINDS, SNMP_MAX_CALLBACKS_PER_AGENT, SNMP_MAX_AGENTS, SNMP_MAX_UDP_PER_AGENT, SNMP_MAX_TRAPS_INFLIGHT, SNMP_MAX_CALLBACKS_PER_TRAP, SNMP_POOL_ASN_OBJECTS, SNMP_POOL_VARBIND_OBJECTS); compile tuning guidance added for esp01_1m (SNMP_POOL_ASN_OBJECTS 64→32 = −24.5 KB BSS). Verification: host catch2 101/101 green, ASAN clean, 4/4 strict DoD cross builds green (-Wall -Wextra -Werror on Arduino-CLI esp8266/esp32 + PlatformIO esp01_1m/esp32dev). Flash −0.71% geometric mean vs v3.0.0 baseline (−4.6 KB avg; esp32dev biggest win −14.1 KB = −1.87%). BSS deterministic +48.9 KB (ASNPool 64×768 B, linker-reported at build time, tuneable). 100% source + wire compatible with v3.0.0; zero API changes; zero breaking changes. | +| **3.0.0** | **BER TLV hardening + PR #60 upstream integration.** Fixed 3 critical BER length bugs: (1) OIDType/Counter64/ComplexType returned hardcoded `_length + 2` regardless of actual TLV header bytes consumed → wrong for payloads ≥128B with 3+ byte long-form headers. (2) `encode_ber_length_integer` off-by-one used `integer > 256` instead of `>=` → length=256 encoded as `0x81 0x00` (=0) breaking snmpbulkwalk. (3) Double-store UB in IntegerType 3-byte sign-extend: `tempVal = tempVal |= 0xFF000000` → `tempVal |= 0xFF000000`. Added defensive boundary pre-checks at BER_CONTAINER/OIDType/Counter64/ComplexType fromBuffer entry. Removed redundant `.reserve()` before `.assign()` in OIDType decode. Hoisted `temp[10]` + added `.reserve(SNMP_MAX_OID_STR_LEN)` in OIDType encode. Replaced ComplexType dual-condition loop with descending `remaining` counter. Stack-buffer-overread fixed in tests memcpy(randomLong, 10) → sizeof(randomLong). Version bumped: defs.h LIBRARY_VERSION_{3,0,0} and library.properties 3.0.0. All tests pass: 97 assertions / 10 cases / 0 failures. | +| **2.2.0** | **Embedded refactor.** Removed all `std::string` usage across the entire library, examples, and test suite. All strings use compile-time fixed `char[]` buffers with explicit length tracking for binary OctetTypes. Added `LIBRARY_VERSION_*` defines in `defs.h`. Eliminated three `malloc` calls from example sketches. Estimated ~3–8 KB Flash savings on ESP8266 and zero heap fragmentation from string handling. All callbacks that previously took/returned `std::string` now take/return `const char*` or bounded `char**`. | +| 2.1.0 | (Previous) API cleanups, ESP-01 / ESP8266 support. | | 2.0.x | Rewrite from Arduino_SNMP v1. RFC-compliant SNMPv2c engine. | | 1.x | Original Arduino_SNMP project. | -Pull requests/comments are welcome +Pull requests/comments are welcome \ No newline at end of file diff --git a/examples/ESP32_SNMP/ESP32_SNMP.ino b/examples/ESP32_SNMP/ESP32_SNMP.ino index 563226f..2dbc107 100644 --- a/examples/ESP32_SNMP/ESP32_SNMP.ino +++ b/examples/ESP32_SNMP/ESP32_SNMP.ino @@ -49,6 +49,11 @@ TimestampCallback* timestampCallbackOID; char staticString[] = "This value will never change"; +/* Versioned sysDescr served on .1.3.6.1.2.1.1.1.0 — the handler keeps the pointer, + so this must be a static buffer, not a local. Lets `snmpget` confirm the exact + library build running on the chip during hardware testing. */ +static char sysDescrBuf[64]; + // Setup an SNMPTrap for later use SNMPTrap* settableNumberTrap = new SNMPTrap("public", SNMP_VERSION_2C); char _changingStringBuf[25]; @@ -56,6 +61,10 @@ char* changingString = _changingStringBuf; void setup(){ Serial.begin(115200); + + // Hardware-test banner: confirm the flashed library version in the serial monitor + Serial.printf("SNMP_Agent v%s\n", snmp.getVersion()); + WiFi.begin(ssid, password); // WiFi.begin(ssid); Serial.println(""); @@ -81,6 +90,11 @@ void setup(){ stuff[2] = 24; stuff[3] = 67; + // RFC1213 sysDescr: serves the library version, queryable from the SNMP terminal: + // snmpget -v 2c -c public .1.3.6.1.2.1.1.1.0 + snprintf(sysDescrBuf, sizeof(sysDescrBuf), "ESP32_SNMP demo (SNMP_Agent v%s)", snmp.getVersion()); + snmp.addReadOnlyStaticStringHandler(".1.3.6.1.2.1.1.1.0", sysDescrBuf); + // add 'callback' for an OID - pointer to an integer changingNumberOID = snmp.addIntegerHandler(".1.3.6.1.4.1.5.0", &changingNumber); diff --git a/examples/SNMP_Sensor/SNMP_Sensor.ino b/examples/SNMP_Sensor/SNMP_Sensor.ino index 4b360bf..42467c8 100644 --- a/examples/SNMP_Sensor/SNMP_Sensor.ino +++ b/examples/SNMP_Sensor/SNMP_Sensor.ino @@ -85,7 +85,10 @@ const char* oidSysName = ".1.3.6.1.2.1.1.5.0"; // OctetString SysName const char* oidSysLocation = ".1.3.6.1.2.1.1.6.0"; // OctetString SysLocation const char* oidSysServices = ".1.3.6.1.2.1.1.7.0"; // Integer sysServices -char sysDescr[] = "SNMP Agent"; +/* Versioned sysDescr served on .1.3.6.1.2.1.1.1.0 — filled in setup() with the + running library version so `snmpget` confirms the exact build during hardware + testing. 64 B leaves room for the version string; handler keeps the pointer. */ +static char sysDescr[64] = "SNMP Agent"; char sysObjectID[] = ""; uint32_t sysUptime = 0; char sysContactValue[255]; @@ -194,6 +197,10 @@ void printFile(const char* filename); void setup() { Serial.begin(115200); + + // Hardware-test banner: confirm the flashed library version in the serial monitor + Serial.printf("SNMP_Agent v%s\n", snmp.getVersion()); + if (!FS_BEGIN()) { Serial.println("LittleFS Mount Failed"); @@ -217,6 +224,10 @@ void setup() snmp.setUDP(&udp); snmp.begin(); + // Fill sysDescr (.1.3.6.1.2.1.1.1.0) with the version string, queryable from the + // SNMP terminal: snmpget -v 2c -c public .1.3.6.1.2.1.1.1.0 + snprintf(sysDescr, sizeof(sysDescr), "SNMP_Sensor demo (SNMP_Agent v%s)", snmp.getVersion()); + addRFC1213MIBHandler(); // RFC1213-MIB (System) addENTITYMIBHandler(); // ENTITY-MIB addENTITYSENSORMIBHandler(); // ENTITY-SENSOR-MIB diff --git a/library.properties b/library.properties index 73f264d..8fc78f1 100644 --- a/library.properties +++ b/library.properties @@ -1,5 +1,5 @@ name=SNMP_Agent -version=3.1.5 +version=3.1.23 author=Aidan Cyr maintainer=Aidan Cyr sentence=SNMP Agent: An fully compliant SNMPv2c Agent for esp32 for acting as an SNMP client device. diff --git a/src/BERDecode.cpp b/src/BERDecode.cpp index dd0944b..0183ded 100644 --- a/src/BERDecode.cpp +++ b/src/BERDecode.cpp @@ -21,17 +21,34 @@ ASNPool::Slot ASNPool::slots[SNMP_POOL_ASN_OBJECTS] = {}; #endif int ASNPool::usedCount = 0; +int ASNPool::permCount = 0; +int ASNPool::usedCountPeak = 0; void ASNPool::release(BER_CONTAINER* p){ if(!p) return; #ifndef SNMP_POOLS_IN_BSS if(!_poolsReady) { delete p; return; } #endif - p->~BER_CONTAINER(); + /* Double-release guard: without it, a double-destroyed object re-runs its + * destructor AND decrements usedCount twice. The counter then under-reports + * occupancy, rawAlloc() reuses a slot that still holds a live object, and + * live OIDs get corrupted — the root cause of degraded GetBulk responses + * and the "agent goes deaf" incidents (HARDWARE_TEST_REPORT.md §2/§9). */ for(int i = 0; i < SNMP_POOL_ASN_OBJECTS; i++){ if(static_cast(slots[i].storage) == static_cast(p)){ + if(!slots[i].occupied){ + /* Slot already free: second release of the same object. + * Do NOT run the destructor again, do NOT decrement. + * One-shot alarm when DEBUG>0: a caller is destroying twice. */ + if(!slots[i].doubleReleaseWarned){ + slots[i].doubleReleaseWarned = true; + SNMP_LOGE("ASNPool: DOUBLE RELEASE of slot %d detected (caller destroying an object twice)\n", i); + } + return; + } + p->~BER_CONTAINER(); slots[i].occupied = false; - if(usedCount > 0) usedCount--; + usedCount--; return; } } @@ -242,24 +259,6 @@ const char* OIDType::string() { return _valueStr; } -void SortableOIDType::generateSortingMap(uint32_t outMap[SNMP_MAX_OID_SUBIDENTIFIERS], int* outLen) const { - int count = 0; - - const uint8_t* ptr = this->data; - - ptr += 1; - int i = this->dataLen - 1; - - while(i > 0 && count < SNMP_MAX_OID_SUBIDENTIFIERS){ - long item; - size_t len = decode_ber_longform_integer(ptr, &item, i); - ptr += len; i -= len; - outMap[count++] = (uint32_t)item; - } - - *outLen = count; -} - int NullType::fromBuffer(const uint8_t *, size_t){ _length = 0; return 2; diff --git a/src/BEREncode.cpp b/src/BEREncode.cpp index b86d619..e8171a0 100644 --- a/src/BEREncode.cpp +++ b/src/BEREncode.cpp @@ -183,6 +183,26 @@ bool OIDType::generateInternalData() { return true; } +void OIDType::_init_from_cstr(const char* value, size_t len) noexcept { + memcpy(this->_valueStr, value, len); + this->_valueStr[len] = '\0'; + this->dataLen = 0; + this->valid = this->generateInternalData(); +} + +void OIDType::_init_from_cstr_with_data(const char* value, size_t len, const uint8_t* srcData, int srcLen, bool valid_) noexcept { + memcpy(this->_valueStr, value, len); + this->_valueStr[len] = '\0'; + if(srcLen > 0 && srcData) { + if(srcLen > (int)sizeof(this->data)) srcLen = (int)sizeof(this->data); + this->dataLen = srcLen; + memcpy(this->data, srcData, (size_t)srcLen); + } else { + this->dataLen = 0; + } + this->valid = valid_; +} + static inline void shift_arr_right(uint8_t* ptr, int num_length_bytes, size_t length){ for(int l = length+num_length_bytes-1; l-num_length_bytes >= 0; l--){ ptr[l] = ptr[l-num_length_bytes]; diff --git a/src/SNMPPDUHandler.cpp b/src/SNMPPDUHandler.cpp index a7968d7..210acbf 100644 --- a/src/SNMPPDUHandler.cpp +++ b/src/SNMPPDUHandler.cpp @@ -3,6 +3,32 @@ #include "include/BER.h" #include "include/ValueCallbacks.h" +/* Every `asn_new()` returns a RAW POINTER into the static ASNPool + * (placement-new slots). When such a raw pointer is bound to a function + * parameter of type `const std::shared_ptr&`, C++ + * implicitly constructs a TEMPORARY shared_ptr using the DEFAULT + * `delete T` deleter — which immediately calls `delete` on a pool slot + * address at scope exit → Undefined Behavior. On ESP-01 this corrupted + * pool metadata silently (no exception triggered since the double-free + * happened on a slot not currently in the free-list), causing + * SNMPResponse encode path to fail to build even a single VarBind → + * ZERO UDP TX bytes sent, agent DEAF despite UDP RX confirmed. + * + * FIX: wrap every `asn_new()` passed into a `shared_ptr` context + * with `pool_asn_sp(...)` below. It constructs a shared_ptr whose + * custom deleter calls `asn_delete` instead of `operator delete`. */ +namespace { + struct pool_asn_deleter { + void operator()(BER_CONTAINER* p) const noexcept { asn_delete(p); } + }; +} +template +static inline std::shared_ptr pool_asn_sp(T* raw_pool_ptr) noexcept { + static_assert(std::is_base_of::value, + "pool_asn_sp only accepts BER_CONTAINER-derived pointers"); + return std::shared_ptr(static_cast(raw_pool_ptr), pool_asn_deleter()); +} + template static inline bool appendResponseVarBind(VarBind out[], int &outCount, Args&&... args){ if(outCount >= SNMP_MAX_VARBINDS) return false; @@ -22,9 +48,9 @@ bool handleGetRequestPDU(ValueCallback* const *callbacks, int callbacksCount, co SNMP_LOGD("Couldn't find callback\n"); #if 1 if(isGetNextRequest){ - appendResponseVarBind(outResponseList, outResponseCount, requestVarBind, asn_new(ENDOFMIBVIEW)); + appendResponseVarBind(outResponseList, outResponseCount, requestVarBind, pool_asn_sp(asn_new(ENDOFMIBVIEW))); } else { - appendResponseVarBind(outResponseList, outResponseCount, requestVarBind, asn_new(NOSUCHOBJECT)); + appendResponseVarBind(outResponseList, outResponseCount, requestVarBind, pool_asn_sp(asn_new(NOSUCHOBJECT))); } #else @@ -104,7 +130,7 @@ bool handleGetBulkRequestPDU(ValueCallback* const *callbacks, int callbacksCount const VarBind& requestVarBind = varbindList[i]; ValueCallback* callback = ValueCallback::findCallback(callbacks, callbacksCount, requestVarBind.oid, true); if(!callback){ - appendResponseVarBind(outResponseList, outResponseCount, requestVarBind, asn_new(ENDOFMIBVIEW)); + appendResponseVarBind(outResponseList, outResponseCount, requestVarBind, pool_asn_sp(asn_new(ENDOFMIBVIEW))); continue; } @@ -130,7 +156,7 @@ bool handleGetBulkRequestPDU(ValueCallback* const *callbacks, int callbacksCount SNMP_LOGD("finding next callback for OID: %s\n", oid->string()); ValueCallback* callback = ValueCallback::findCallback(callbacks, callbacksCount, oid, true, foundAt, &foundAt); if(!callback){ - appendResponseVarBind(outResponseList, outResponseCount, oid, asn_new(ENDOFMIBVIEW)); + appendResponseVarBind(outResponseList, outResponseCount, oid, pool_asn_sp(asn_new(ENDOFMIBVIEW))); oid = nullptr; break; } diff --git a/src/SNMPPacket.cpp b/src/SNMPPacket.cpp index 00970ec..2ffa4d5 100644 --- a/src/SNMPPacket.cpp +++ b/src/SNMPPacket.cpp @@ -2,6 +2,35 @@ #define SNMP_PARSE_ERROR_AT_STATE(STATE) ((int)STATE * -1) - 10 + SNMP_PACKET_PARSE_ERROR_OFFSET +/* ASNPool objects are NOT allocated via `new`/`malloc`. They live in a + * statically-allocated placement-new pool (ASNPool::Slot[N]). When a + * `std::shared_ptr` takes ownership of a pool-allocated pointer via + * the default `delete T` deleter, it calls `delete` on a pool address → + * Undefined Behavior. In v3.1.14 this manifested as a silent SNMP agent + * death on ESP-01: UDP packets arrived (confirmed in T1 loop diagnostic) + * but ZERO response packets were ever built/sent. Pool metadata was + * corrupted by the bogus `delete`, causing ASNPool allocations for the + * response encode path to return nullptr → silently dropped. + * + * FIX: use an `asn_delete` deleter for every shared_ptr that wraps + * `asn_new(...)`. `asn_delete` correctly returns the slot to the + * pool (or no-ops if the object was actually on heap via COMPILING_TESTS + * fallback malloc). The pool also auto-resets on every loop() tick, so + * even a no-op deleter is safe. But using `asn_delete` is proper and + * also works correctly in host-based unit tests that malloc. */ +template +static void pool_asn_deleter(T* p) noexcept { + asn_delete(static_cast(p)); +} + +/* Convenience helper: wraps a pool-allocated pointer in shared_ptr with + * an deleter that calls asn_delete() instead of operator delete. This + * MUST be used for every `shared_ptr = asn_new(...)` assignment. */ +template +static inline std::shared_ptr pool_shared(T* p) noexcept { + return std::shared_ptr(p, pool_asn_deleter); +} + #define ASN_TYPE_FOR_STATE_SNMPVERSION INTEGER #define ASN_TYPE_FOR_STATE_COMMUNITY STRING #define ASN_TYPE_FOR_STATE_REQUESTID INTEGER @@ -43,7 +72,7 @@ SNMP_PACKET_PARSE_ERROR SNMPPacket::parsePacket(ComplexType *structure, enum SNM case SNMPVERSION: ASSERT_ASN_STATE_TYPE(value, SNMPVERSION); - this->snmpVersionPtr = std::shared_ptr(asn_new(static_cast(value)->_value)); + this->snmpVersionPtr = pool_shared(asn_new(static_cast(value)->_value)); this->snmpVersion = (SNMP_VERSION) this->snmpVersionPtr.get()->_value; if (this->snmpVersion >= SNMP_VERSION_MAX) { SNMP_LOGW("Invalid SNMP Version: %d\n", this->snmpVersion); @@ -56,13 +85,18 @@ SNMP_PACKET_PARSE_ERROR SNMPPacket::parsePacket(ComplexType *structure, enum SNM ASSERT_ASN_STATE_TYPE(value, COMMUNITY); { OctetType* src = static_cast(value); - this->communityStringPtr = std::shared_ptr(asn_new(src->_value, src->_valueLen)); + this->communityStringPtr = pool_shared(asn_new(src->_value, src->_valueLen)); } { size_t len = this->communityStringPtr.get()->_valueLen; - if(len > SNMP_MAX_COMMUNITY_LEN) len = SNMP_MAX_COMMUNITY_LEN; - memcpy(this->communityString, this->communityStringPtr.get()->_value, len); - this->communityString[len] = 0; + if(len > SNMP_MAX_COMMUNITY_LEN){ + SNMP_LOGW("parse COMMUNITY: incoming length %zu > SNMP_MAX_COMMUNITY_LEN=%d → reject (no truncation). Raise cap via #define SNMP_MAX_COMMUNITY_LEN N BEFORE including SNMP_Agent.h if needed.\n", + len, (int)SNMP_MAX_COMMUNITY_LEN); + this->communityString[0] = 0; + } else { + memcpy(this->communityString, this->communityStringPtr.get()->_value, len); + this->communityString[len] = 0; + } } state = PDU; break; @@ -74,7 +108,7 @@ SNMP_PACKET_PARSE_ERROR SNMPPacket::parsePacket(ComplexType *structure, enum SNM case REQUESTID: ASSERT_ASN_STATE_TYPE(value, REQUESTID); - this->requestIDPtr = std::shared_ptr(asn_new(static_cast(value)->_value)); + this->requestIDPtr = pool_shared(asn_new(static_cast(value)->_value)); this->requestID = this->requestIDPtr.get()->_value; state = ERRORSTATUS; break; @@ -141,6 +175,10 @@ SNMP_PACKET_PARSE_ERROR SNMPPacket::parseFrom(unsigned char* buf, size_t max_len } packet = asn_new(STRUCTURE); + if(!packet){ + SNMP_LOGE("SNMPPacket::parseFrom: pool exhausted (root ComplexType). Raise SNMP_POOL_ASN_OBJECTS.\n"); + return SNMP_BUFFER_ERROR_MAX_LEN_EXCEEDED; + } SNMP_BUFFER_PARSE_ERROR decodePacket = packet->fromBuffer(buf, max_len); if(decodePacket <= 0){ @@ -159,10 +197,15 @@ int SNMPPacket::serialiseInto(uint8_t* buf, size_t max_len){ return 0; } -bool SNMPPacket::build(){ +bool SNMPPacket::_build_pdu_envelope(BuildPDUHeaderFn fill_pdu, void* userdata){ asn_delete(this->packet); ComplexType* root = asn_new(STRUCTURE); + if(!root){ + SNMP_LOGE("_build_pdu_envelope: pool exhausted (root). Raise SNMP_POOL_ASN_OBJECTS.\n"); + this->packet = nullptr; + return false; + } root->_ownsChildren = true; this->packet = root; @@ -177,27 +220,39 @@ bool SNMPPacket::build(){ root->addValueToListRaw(asn_new(this->communityString)); ComplexType* snmpPDU = asn_new(this->packetPDUType); + if(!snmpPDU){ + SNMP_LOGE("_build_pdu_envelope: pool exhausted (snmpPDU). Raise SNMP_POOL_ASN_OBJECTS.\n"); + return false; + } snmpPDU->_ownsChildren = true; - if(this->requestIDPtr) - snmpPDU->addValueToListRaw(asn_new(this->requestIDPtr->_value)); - else - snmpPDU->addValueToListRaw(asn_new(this->requestID)); - - - snmpPDU->addValueToListRaw(asn_new(this->errorStatus.errorStatus)); - snmpPDU->addValueToListRaw(asn_new(this->errorIndex.errorIndex)); + if(!fill_pdu(snmpPDU, userdata)) return false; ComplexType* varBindList = this->generateVarBindListRaw(); if(!varBindList) return false; snmpPDU->addValueToListRaw(varBindList); - root->addValueToListRaw(snmpPDU); return true; } +static bool _packet_build_fill_pdu(ComplexType* snmpPDU, void* userdata){ + SNMPPacket* self = static_cast(userdata); + if(self->requestIDPtr) + snmpPDU->addValueToListRaw(asn_new(self->requestIDPtr->_value)); + else + snmpPDU->addValueToListRaw(asn_new(self->requestID)); + + snmpPDU->addValueToListRaw(asn_new(self->errorStatus.errorStatus)); + snmpPDU->addValueToListRaw(asn_new(self->errorIndex.errorIndex)); + return true; +} + +bool SNMPPacket::build(){ + return _build_pdu_envelope(_packet_build_fill_pdu, this); +} + void SNMPPacket::setCommunityString(const char *CommunityString){ this->communityStringPtr = nullptr; size_t len = strlen(CommunityString); @@ -228,49 +283,33 @@ void SNMPPacket::setVersion(SNMP_VERSION SnmpVersion){ ComplexType* SNMPPacket::generateVarBindListRaw(){ SNMP_LOGD("generateVarBindListRaw from SNMPPacket"); ComplexType* list = asn_new(STRUCTURE); + if(!list){ + SNMP_LOGE("SNMPPacket::generateVarBindListRaw: pool exhausted (list). Raise SNMP_POOL_ASN_OBJECTS.\n"); + return nullptr; + } list->_ownsChildren = true; for(int vbIdx = 0; vbIdx < this->varbindCount; vbIdx++){ const VarBind& varBindItem = this->varbindList[vbIdx]; ComplexType* varBind = asn_new(STRUCTURE); - varBind->_ownsChildren = true; + if(!varBind){ + SNMP_LOGE("SNMPPacket::generateVarBindListRaw: pool exhausted (vb %d/%d).\n", vbIdx, this->varbindCount); + asn_delete(list); + return nullptr; + } + varBind->_ownsChildren = false; - varBind->addValueToListRaw(varBindItem.oid->cloneRaw()); + if(varBindItem.oid){ + varBind->addValueToListRaw(varBindItem.oid); + } else { + varBind->addValueToListRaw(asn_new()); + } - BER_CONTAINER* src = varBindItem.value; - BER_CONTAINER* clonedValue = nullptr; - if(!src){ - clonedValue = asn_new(); - } else switch(src->_type){ - case INTEGER: clonedValue = asn_new(static_cast(src)->_value); break; - case STRING: - { - OctetType* so = static_cast(src); - clonedValue = asn_new(so->_value, so->_valueLen); - } break; - case OID: clonedValue = static_cast(src)->cloneRaw(); break; - case NULLTYPE: clonedValue = asn_new(); break; - case NOSUCHOBJECT: clonedValue = asn_new(NOSUCHOBJECT); break; - case NOSUCHINSTANCE: clonedValue = asn_new(NOSUCHINSTANCE); break; - case ENDOFMIBVIEW: clonedValue = asn_new(ENDOFMIBVIEW); break; - case NETWORK_ADDRESS: - { - NetworkAddress* so = static_cast(src); - clonedValue = asn_new(so->_value); - } break; - case TIMESTAMP: clonedValue = asn_new(static_cast(src)->_value); break; - case COUNTER32: clonedValue = asn_new(static_cast(src)->_value); break; - case GAUGE32: clonedValue = asn_new(static_cast(src)->_value); break; - case COUNTER64: clonedValue = asn_new(static_cast(src)->_value); break; - case OPAQUE: - { - OpaqueType* so = static_cast(src); - clonedValue = asn_new(so->_value, so->_dataLength); - } break; - default: - clonedValue = asn_new(); break; + if(varBindItem.value){ + varBind->addValueToListRaw(varBindItem.value); + } else { + varBind->addValueToListRaw(asn_new()); } - varBind->addValueToListRaw(clonedValue); list->addValueToListRaw(varBind); } @@ -279,7 +318,7 @@ ComplexType* SNMPPacket::generateVarBindListRaw(){ } std::shared_ptr SNMPPacket::generateVarBindList(){ - return std::shared_ptr(generateVarBindListRaw()); + return pool_shared(generateVarBindListRaw()); } snmp_request_id_t SNMPPacket::generate_request_id(){ diff --git a/src/SNMPParser.cpp b/src/SNMPParser.cpp index 88b7ae1..437f228 100644 --- a/src/SNMPParser.cpp +++ b/src/SNMPParser.cpp @@ -83,6 +83,14 @@ SNMP_ERROR_RESPONSE handlePacket(uint8_t* buffer, int packetLength, int* respons break; } + /* NOTE: no explicit request.~SNMPPacket() here. The automatic destructor at + * scope exit destroys `request` exactly once, AFTER response.serialiseInto(). + * An earlier explicit call double-destroyed every pool-backed parse object + * (refcount hit 0 twice) and ASNPool::release() decremented usedCount twice + * per object — corrupting the pool counter until slots could be handed to + * two live objects. Symptom on ESP-01: degraded/short GetBulk responses + * under trap concurrency, with no exhaustion logs. */ + if(pass){ for(int idx = 0; idx < outResponseCount; idx++){ const VarBind& item = outResponseList[idx]; diff --git a/src/SNMPTrap.cpp b/src/SNMPTrap.cpp index 5694bd2..2fc4d48 100644 --- a/src/SNMPTrap.cpp +++ b/src/SNMPTrap.cpp @@ -2,52 +2,66 @@ #include "include/SNMPParser.h" #include "include/defs.h" +/* Pool object deleter helper — must match the deleter used in SNMPPacket.cpp. + * asn_delete() correctly returns a pool-allocated BER_CONTAINER slot back to + * the ASNPool free-list instead of calling operator delete (which would UB + * on a placement-new'd slot in the static pool, corrupting the pool metadata + * and silently dropping all responses on ESP-01). */ +namespace { + struct trap_pool_deleter { + template + void operator()(T* p) const noexcept { + asn_delete(static_cast(p)); + } + }; +} + +std::shared_ptr SNMPTrap::generateVarBindList(){ + return std::shared_ptr(generateVarBindListRaw(), trap_pool_deleter()); +} + OIDType SNMPTrap::s_timestampOID(RFC1213_OID_sysUpTime); OIDType SNMPTrap::s_snmpTrapOID(SNMPv2_SNMPTRAP_OID_0); SNMPTrap::~SNMPTrap(){ asn_delete(packet); + if(_trapOIDOwned) asn_delete(trapOID); + trapOID = nullptr; + _trapOIDOwned = false; } -bool SNMPTrap::build(){ - asn_delete(packet); - - if(!this->trapOID) return false; - - ComplexType* root = asn_new(STRUCTURE); - root->_ownsChildren = true; - packet = root; - - root->addValueToListRaw(asn_new((int)this->snmpVersion)); - root->addValueToListRaw(asn_new(this->communityString)); - - ComplexType* trapPDU = asn_new(TrapPDU); - trapPDU->_ownsChildren = true; - - trapPDU->addValueToListRaw(trapOID->cloneRaw()); - trapPDU->addValueToListRaw(asn_new(agentIP)); - trapPDU->addValueToListRaw(asn_new(genericTrap)); - trapPDU->addValueToListRaw(asn_new(specificTrap)); +static bool _trap_build_fill_pdu(ComplexType* trapPDU, void* userdata){ + SNMPTrap* self = static_cast(userdata); + trapPDU->addValueToListRaw(self->trapOID->cloneRaw()); + trapPDU->addValueToListRaw(asn_new(self->agentIP)); + trapPDU->addValueToListRaw(asn_new(self->genericTrap)); + trapPDU->addValueToListRaw(asn_new(self->specificTrap)); - if(uptimeCallback){ - auto sp = std::static_pointer_cast(ValueCallback::getValueForCallback(uptimeCallback)); + if(self->uptimeCallback){ + auto sp = std::static_pointer_cast(ValueCallback::getValueForCallback(self->uptimeCallback)); if(sp) trapPDU->addValueToListRaw(asn_new(sp->_value)); else trapPDU->addValueToListRaw(asn_new(0)); } else { trapPDU->addValueToListRaw(asn_new(0)); } + return true; +} - ComplexType* ourVBList = this->generateVarBindListRaw(); - if(!ourVBList) return false; +bool SNMPTrap::build(){ + if(!this->trapOID) return false; - trapPDU->addValueToListRaw(ourVBList); - root->addValueToListRaw(trapPDU); - return true; + this->packetPDUType = TrapPDU; + + return _build_pdu_envelope(_trap_build_fill_pdu, this); } ComplexType* SNMPTrap::generateVarBindListRaw(){ SNMP_LOGD("generateVarBindListRaw from SNMPTrap"); ComplexType* ourVBList = asn_new(STRUCTURE); + if(!ourVBList){ + SNMP_LOGE("SNMPTrap::generateVarBindListRaw: pool exhausted (ourVBList). Raise SNMP_POOL_ASN_OBJECTS.\n"); + return nullptr; + } ourVBList->_ownsChildren = true; if(this->snmpVersion == SNMP_VERSION_2C){ @@ -56,6 +70,11 @@ ComplexType* SNMPTrap::generateVarBindListRaw(){ return nullptr; } ComplexType* timestampVarBind = asn_new(STRUCTURE); + if(!timestampVarBind){ + SNMP_LOGE("SNMPTrap::generateVarBindListRaw: pool exhausted (timestampVarBind).\n"); + asn_delete(ourVBList); + return nullptr; + } timestampVarBind->_ownsChildren = true; timestampVarBind->addValueToListRaw(timestampOID->cloneRaw()); @@ -69,6 +88,11 @@ ComplexType* SNMPTrap::generateVarBindListRaw(){ ourVBList->addValueToListRaw(timestampVarBind); ComplexType* oidVarBind = asn_new(STRUCTURE); + if(!oidVarBind){ + SNMP_LOGE("SNMPTrap::generateVarBindListRaw: pool exhausted (oidVarBind).\n"); + asn_delete(ourVBList); + return nullptr; + } oidVarBind->_ownsChildren = true; oidVarBind->addValueToListRaw(snmpTrapOID->cloneRaw()); oidVarBind->addValueToListRaw(trapOID->cloneRaw()); @@ -79,6 +103,11 @@ ComplexType* SNMPTrap::generateVarBindListRaw(){ ValueCallback* value = callbacks[i]; if(!value) continue; ComplexType* varBind = asn_new(STRUCTURE); + if(!varBind){ + SNMP_LOGE("SNMPTrap::generateVarBindListRaw: pool exhausted (callback %d/%d).\n", i, callbacksCount); + asn_delete(ourVBList); + return nullptr; + } varBind->_ownsChildren = true; varBind->addValueToListRaw(value->OID->cloneRaw()); @@ -125,11 +154,13 @@ ComplexType* SNMPTrap::generateVarBindListRaw(){ return ourVBList; } -std::shared_ptr SNMPTrap::generateVarBindList(){ - return std::shared_ptr(generateVarBindListRaw()); -} - -void SNMPTrap::addOIDPointer(ValueCallback* callback){ - if(callbacksCount >= SNMP_MAX_CALLBACKS_PER_TRAP) return; +bool SNMPTrap::addOIDPointer(ValueCallback* callback){ + if(!callback) return false; + if(callbacksCount >= SNMP_MAX_CALLBACKS_PER_TRAP){ + SNMP_LOGE("SNMPTrap::addOIDPointer: callbacks[] full (%d slots). Raise SNMP_MAX_CALLBACKS_PER_TRAP.\n", + SNMP_MAX_CALLBACKS_PER_TRAP); + return false; + } callbacks[callbacksCount++] = callback; + return true; } diff --git a/src/SNMPTrap.h b/src/SNMPTrap.h index e54e721..d2dac63 100644 --- a/src/SNMPTrap.h +++ b/src/SNMPTrap.h @@ -27,7 +27,7 @@ class SNMPTrap : public SNMPPacket { this->setVersion(version); this->setPDUType(Trapv2PDU); this->setCommunityString(community); - }; + } virtual ~SNMPTrap(); IPAddress agentIP; @@ -59,7 +59,21 @@ class SNMPTrap : public SNMPPacket { } void setTrapOID(OIDType* oid){ + if(_trapOIDOwned) asn_delete(trapOID); trapOID = oid; + _trapOIDOwned = false; + } + + void setTrapOID(const char* oid_str){ + if(_trapOIDOwned) asn_delete(trapOID); + trapOID = asn_new(oid_str); + _trapOIDOwned = true; + } + + void setTrapOID(const OIDType& oid_ref){ + if(_trapOIDOwned) asn_delete(trapOID); + trapOID = oid_ref.cloneRaw(); + _trapOIDOwned = true; } void setSpecificTrap(short num){ @@ -86,7 +100,7 @@ class SNMPTrap : public SNMPPacket { uptimeCallback = uptime; } - void addOIDPointer(ValueCallback* callback); + bool addOIDPointer(ValueCallback* callback); UDP* _udp = nullptr; @@ -101,6 +115,8 @@ class SNMPTrap : public SNMPPacket { } bool sendTo(const IPAddress& ip, bool skipBuild = false){ + ASNPool::resetAll(); + bool buildStatus = true; if(!skipBuild) { buildStatus = this->buildForSending(); @@ -132,6 +148,7 @@ class SNMPTrap : public SNMPPacket { protected: ValueCallback* callbacks[SNMP_MAX_CALLBACKS_PER_TRAP] = {nullptr}; int callbacksCount = 0; + bool _trapOIDOwned = false; std::shared_ptr generateVarBindList() override; ComplexType* generateVarBindListRaw() override; diff --git a/src/SNMP_Agent.cpp b/src/SNMP_Agent.cpp index bc93cb1..22f4cff 100644 --- a/src/SNMP_Agent.cpp +++ b/src/SNMP_Agent.cpp @@ -5,12 +5,12 @@ SNMPAgent* SNMPAgent::agents[SNMP_MAX_AGENTS] = {nullptr}; int SNMPAgent::agentsCount = 0; void SNMPAgent::setUDP(UDP* udp){ - if(this->udpCount < SNMP_MAX_UDP_PER_AGENT){ - this->_udp[this->udpCount++] = udp; - - + if(this->udpCount >= SNMP_MAX_UDP_PER_AGENT){ + SNMP_LOGE("setUDP: _udp[] full (%d slots). Raise SNMP_MAX_UDP_PER_AGENT.\n", SNMP_MAX_UDP_PER_AGENT); + this->begin(); + return; } - + this->_udp[this->udpCount++] = udp; this->begin(); } @@ -33,14 +33,14 @@ void SNMPAgent::stop(){ } SNMP_ERROR_RESPONSE SNMPAgent::loop(){ - + ASNPool::resetAll(); for(int i = 0; i < udpCount; i++){ UDP* udp = _udp[i]; int packetLength = udp->parsePacket(); if(packetLength > 0){ - SNMP_LOGD("Received packet from: %s, of size: %d", udp->remoteIP().toString().c_str(), packetLength); - + SNMP_LOGI("loop: UDP[%d] parsePacket=%d bytes remote=%s:%d\n", + i, packetLength, udp->remoteIP().toString().c_str(), udp->remotePort()); if(packetLength < 0 || packetLength > MAX_SNMP_PACKET_LENGTH){ SNMP_LOGW("Incoming packet too large: %d\n", packetLength); @@ -54,22 +54,20 @@ SNMP_ERROR_RESPONSE SNMPAgent::loop(){ SNMP_LOGW("Packet length mismatch: expected: %d, actual: %d\n", packetLength, readBytes); return SNMP_REQUEST_INVALID; } - + SNMP_LOGI("loop: UDP[%d] read OK. Calling handlePacket(len=%d)...\n", i, packetLength); int responseLength = 0; SNMP_ERROR_RESPONSE response = handlePacket(_packetBuffer, packetLength, &responseLength, MAX_SNMP_PACKET_LENGTH, callbacks, callbacksCount, _community, _readOnlyCommunity, informCallback, (void*)this); - + SNMP_LOGI("loop: handlePacket -> ret=%d, responseLength=%d\n", (int)response, responseLength); if(response > 0 && response != SNMP_INFORM_RESPONSE_OCCURRED){ - SNMP_LOGD("Built packet, sending back response to: %s, %d\n", udp->remoteIP().toString().c_str(), udp->remotePort()); - + SNMP_LOGI("loop: UDP TX beginPacket(remote=%s:%d) write=%d B ...", + udp->remoteIP().toString().c_str(), udp->remotePort(), responseLength); udp->beginPacket(udp->remoteIP(), udp->remotePort()); udp->write(_packetBuffer, responseLength); + bool ep = udp->endPacket(); + SNMP_LOGI(" done. endPacket=%d\n", (int)ep); - - - - - if(!udp->endPacket()){ + if(!ep){ SNMP_LOGW("Failed to send response packet\n"); } } @@ -217,15 +215,15 @@ ValueCallback* SNMPAgent::addGaugeHandler(const char *oid, uint32_t* value, bool } ValueCallback * SNMPAgent::addHandler(ValueCallback *callback, bool isSettable) { - + if(!callback) return nullptr; callback->isSettable = isSettable; - if(this->callbacksCount < SNMP_MAX_CALLBACKS_PER_AGENT){ - this->callbacks[this->callbacksCount++] = callback; - - - + if(this->callbacksCount >= SNMP_MAX_CALLBACKS_PER_AGENT){ + SNMP_LOGE("addHandler: callbacks[] full (%d slots), OID %s NOT registered. Raise SNMP_MAX_CALLBACKS_PER_AGENT.\n", + SNMP_MAX_CALLBACKS_PER_AGENT, callback->OID ? callback->OID->string() : "(null)"); + delete callback; + return nullptr; } - + this->callbacks[this->callbacksCount++] = callback; return callback; } @@ -238,19 +236,19 @@ bool SNMPAgent::sortHandlers(){ return true; } - - - - - - - - - - - - - +void SNMPAgent::printAllOIDsTo(Print& out) const { + char line[SNMP_MAX_OID_STR_LEN + 32]; + for(int i = 0; i < this->callbacksCount; i++){ + const ValueCallback* cb = this->callbacks[i]; + if(!cb || !cb->OID) continue; + const char* oidStr = cb->OID->string(); + const char* typeStr = ValueCallback::getTypeName(cb->type); + const char* tagStr = cb->getAccessTag(); + snprintf(line, sizeof(line), "[%2d] %s %-10s %s\r\n", + i, oidStr, typeStr, tagStr); + out.print(line); + } +} snmp_request_id_t SNMPAgent::sendTrapTo(SNMPTrap* trap, const IPAddress& ip, bool replaceQueuedRequests, int retries, int delay_ms){ return queue_and_send_trap(this->informList, this->informCount, trap, ip, replaceQueuedRequests, retries, delay_ms); @@ -276,15 +274,15 @@ void SNMPAgent::markTrapDeleted(SNMPTrap* trap){ } bool SNMPAgent::restartUDP() { - + bool all_ok = true; for(int i = 0; i < udpCount; i++){ _udp[i]->stop(); - _udp[i]->begin(AgentUDPport); - - - - - + uint8_t ok = _udp[i]->begin(AgentUDPport); + if(!ok){ + SNMP_LOGE("restartUDP: UDP[%d]->begin(port=%d) FAILED (returned 0). WiFi down? port already bound? check port permissions.\n", + i, (int)AgentUDPport); + all_ok = false; + } } - return true; + return all_ok; } diff --git a/src/SNMP_Agent.h b/src/SNMP_Agent.h index d2d36d8..f363131 100644 --- a/src/SNMP_Agent.h +++ b/src/SNMP_Agent.h @@ -5,6 +5,7 @@ #include "tests/required/millis.h" #include "tests/required/IPAddress.h" #include "tests/required/UDP.h" + #include "tests/required/Print.h" #else #include #include "IPAddress.h" @@ -29,16 +30,20 @@ class SNMPAgent { public: SNMPAgent(){ - SNMPAgent::agents[SNMPAgent::agentsCount++] = this; - }; + if(SNMPAgent::agentsCount < SNMP_MAX_AGENTS){ + SNMPAgent::agents[SNMPAgent::agentsCount++] = this; + } + } SNMPAgent(const char* community){ size_t len = strlen(community); if(len > SNMP_MAX_COMMUNITY_LEN) len = SNMP_MAX_COMMUNITY_LEN; memcpy(_community, community, len); _community[len] = 0; - SNMPAgent::agents[SNMPAgent::agentsCount++] = this; - }; + if(SNMPAgent::agentsCount < SNMP_MAX_AGENTS){ + SNMPAgent::agents[SNMPAgent::agentsCount++] = this; + } + } SNMPAgent(const char* readOnlyCommunity, const char* readWriteCommunity){ size_t len = strlen(readWriteCommunity); @@ -48,11 +53,17 @@ class SNMPAgent { len = strlen(readOnlyCommunity); if(len > SNMP_MAX_COMMUNITY_LEN) len = SNMP_MAX_COMMUNITY_LEN; memcpy(_readOnlyCommunity, readOnlyCommunity, len); - _readOnlyCommunity[len] = 0; - SNMPAgent::agents[SNMPAgent::agentsCount++] = this; + _readOnlyCommunity[len] = 0; if(SNMPAgent::agentsCount < SNMP_MAX_AGENTS){ + SNMPAgent::agents[SNMPAgent::agentsCount++] = this; + } } - void setReadOnlyCommunity(const char* community){ + /* Runtime library version (LIBRARY_VERSION from defs.h). Use in sketches to + print/serve the exact build under test, e.g. hardware-test banners. */ + static const char* getVersion(){ return LIBRARY_VERSION; } + + void + setReadOnlyCommunity(const char* community){ size_t len = strlen(community); if(len > SNMP_MAX_COMMUNITY_LEN) len = SNMP_MAX_COMMUNITY_LEN; memcpy(this->_readOnlyCommunity, community, len); @@ -111,6 +122,8 @@ class SNMPAgent { bool removeHandler(ValueCallback* callback); bool sortHandlers(); + void printAllOIDsTo(Print& out) const; + snmp_request_id_t sendTrapTo(SNMPTrap* trap, const IPAddress& ip, bool replaceQueuedRequests = true, int retries = 0, int delay_ms = 30000); static void markTrapDeleted(SNMPTrap* trap); diff --git a/src/ValueCallbacks.cpp b/src/ValueCallbacks.cpp index c518aa9..20c9ab6 100644 --- a/src/ValueCallbacks.cpp +++ b/src/ValueCallbacks.cpp @@ -3,6 +3,17 @@ #include +template +static void pool_asn_deleter(T* p) noexcept { + asn_delete(static_cast(p)); +} + +template +static inline std::shared_ptr pool_shared(T* p) noexcept { + if(!p) return nullptr; + return std::shared_ptr(p, pool_asn_deleter); +} + #define ASSERT_VALID_VALUE(value) if(!value) return nullptr; #define SETTING_NON_SETTABLE_ERROR READ_ONLY @@ -14,6 +25,21 @@ // #define ASSERT_CALLBACK_SETTABLE if(!(static_cast(this)->isSettable)) return SETTING_NON_SETTABLE_ERROR; #define ASSERT_CALLBACK_SETTABLE() +const char* ValueCallback::getTypeName(ASN_TYPE t) noexcept { + switch(t){ + case INTEGER: return "Integer"; + case STRING: return "String"; + case NULLTYPE: return "Null"; + case ASN_TYPE::OID: return "OID"; + case COUNTER32: return "Counter32"; + case GAUGE32: return "Gauge32"; + case TIMESTAMP: return "Timestamp"; + case OPAQUE: return "Opaque"; + case COUNTER64: return "Counter64"; + default: return "Unknown"; + } +} + ValueCallback* ValueCallback::findCallback(ValueCallback* const *callbacks, int callbacksCount, const OIDType* const oid, bool walk, int startAt, int *foundAt){ bool useNext = false; @@ -61,10 +87,10 @@ SNMP_ERROR_STATUS ValueCallback::setValueForCallback(ValueCallback* callback, co return SETTING_NON_SETTABLE_ERROR; } + callback->setOccurred = true; SNMP_ERROR_STATUS valid = callback->setTypeWithValue(value.get()); - - if(valid == NO_ERROR){ - callback->setOccurred = true; + if(valid != NO_ERROR){ + callback->setOccurred = false; } return valid; @@ -73,9 +99,9 @@ SNMP_ERROR_STATUS ValueCallback::setValueForCallback(ValueCallback* callback, co std::shared_ptr IntegerCallback::buildTypeWithValue(){ ASSERT_VALID_VALUE(this->value); - auto val = std::make_shared(*this->value); + auto val = pool_shared(asn_new(*this->value)); + if(!val) return nullptr; if(this->modifier != 0){ - // Apple local division if callback was asked to val->_value /= this->modifier; } return val; @@ -98,7 +124,7 @@ SNMP_ERROR_STATUS IntegerCallback::setTypeWithValue(BER_CONTAINER* rawValue){ std::shared_ptr TimestampCallback::buildTypeWithValue(){ ASSERT_VALID_VALUE(this->value); - return std::make_shared(*this->value); + return pool_shared(asn_new(*this->value)); } SNMP_ERROR_STATUS TimestampCallback::setTypeWithValue(BER_CONTAINER* rawValue){ @@ -114,7 +140,7 @@ SNMP_ERROR_STATUS TimestampCallback::setTypeWithValue(BER_CONTAINER* rawValue){ std::shared_ptr StringCallback::buildTypeWithValue(){ ASSERT_VALID_VALUE(this->value); - return std::make_shared(*this->value); + return pool_shared(asn_new(*this->value)); } SNMP_ERROR_STATUS StringCallback::setTypeWithValue(BER_CONTAINER* rawValue){ @@ -129,14 +155,14 @@ SNMP_ERROR_STATUS StringCallback::setTypeWithValue(BER_CONTAINER* rawValue){ } std::shared_ptr ReadOnlyStringCallback::buildTypeWithValue(){ - return std::make_shared(this->value); + return pool_shared(asn_new(this->value)); } std::shared_ptr OpaqueCallback::buildTypeWithValue(){ ASSERT_VALID_VALUE(this->value); - return std::make_shared(this->value, this->data_len); + return pool_shared(asn_new(this->value, this->data_len)); } SNMP_ERROR_STATUS OpaqueCallback::setTypeWithValue(BER_CONTAINER* rawValue){ @@ -152,15 +178,15 @@ SNMP_ERROR_STATUS OpaqueCallback::setTypeWithValue(BER_CONTAINER* rawValue){ } std::shared_ptr OIDCallback::buildTypeWithValue(){ - auto oid = std::make_shared(this->value); - if(!oid->valid) return nullptr; + auto oid = pool_shared(asn_new(this->value)); + if(!oid || !oid->valid) return nullptr; return oid; } std::shared_ptr Counter32Callback::buildTypeWithValue(){ ASSERT_VALID_VALUE(this->value); - return std::make_shared(*this->value); + return pool_shared(asn_new(*this->value)); } SNMP_ERROR_STATUS Counter32Callback::setTypeWithValue(BER_CONTAINER* rawValue){ @@ -168,14 +194,19 @@ SNMP_ERROR_STATUS Counter32Callback::setTypeWithValue(BER_CONTAINER* rawValue){ ASSERT_VALID_SETTABLE_VALUE(this->value); Counter32* val = static_cast(rawValue); - *this->value = val->_value; + uint32_t incoming = (uint32_t)val->_value; + if(*this->value != incoming){ + *this->value = incoming; + } else { + this->setOccurred = false; + } return NO_ERROR; } std::shared_ptr Gauge32Callback::buildTypeWithValue(){ ASSERT_VALID_VALUE(this->value); - return std::make_shared(*this->value); + return pool_shared(asn_new(*this->value)); } SNMP_ERROR_STATUS Gauge32Callback::setTypeWithValue(BER_CONTAINER* rawValue){ @@ -191,7 +222,7 @@ SNMP_ERROR_STATUS Gauge32Callback::setTypeWithValue(BER_CONTAINER* rawValue){ std::shared_ptr Counter64Callback::buildTypeWithValue(){ ASSERT_VALID_VALUE(this->value); - return std::make_shared(*this->value); + return pool_shared(asn_new(*this->value)); } SNMP_ERROR_STATUS Counter64Callback::setTypeWithValue(BER_CONTAINER* rawValue){ @@ -199,29 +230,43 @@ SNMP_ERROR_STATUS Counter64Callback::setTypeWithValue(BER_CONTAINER* rawValue){ ASSERT_VALID_SETTABLE_VALUE(this->value); Counter64* val = static_cast(rawValue); - *this->value = val->_value; - + if(*this->value != val->_value){ + *this->value = val->_value; + } else { + this->setOccurred = false; + } return NO_ERROR; } -bool SortableOIDType::sort_oids(SortableOIDType* oid1, SortableOIDType* oid2){ - const uint32_t* map1 = oid1->sortingMap; - const uint32_t* map2 = oid2->sortingMap; - int len1 = oid1->sortingMapLen; - int len2 = oid2->sortingMapLen; - - if(len1 == 0) return false; - if(len2 == 0) return true; - - int i = (len1 < len2) ? len1 : len2; - - for(int j = 0; j < i; j++){ - if(map1[j] != map2[j]){ - return map1[j] < map2[j]; - } +bool SortableOIDType::sort_oids(const SortableOIDType* oid1, const SortableOIDType* oid2){ + if(!oid1 || !oid2) return false; + if(oid1->dataLen == 0) return false; + if(oid2->dataLen == 0) return true; + + const uint8_t* p1 = oid1->data; + const uint8_t* p2 = oid2->data; + int rem1 = oid1->dataLen; + int rem2 = oid2->dataLen; + + while(rem1 > 0 && rem2 > 0){ + long sub1 = 0; + long sub2 = 0; + int consumed1 = 0; + int consumed2 = 0; + do { + sub1 = (sub1 << 7) | (*p1 & 0x7F); + consumed1++; + } while(rem1-- > 0 && (*p1++ & 0x80) != 0); + do { + sub2 = (sub2 << 7) | (*p2 & 0x7F); + consumed2++; + } while(rem2-- > 0 && (*p2++ & 0x80) != 0); + (void)consumed1; (void)consumed2; + if(sub1 != sub2) return sub1 < sub2; } - - return len1 < len2; + if(rem1 > 0) return false; + if(rem2 > 0) return true; + return false; } bool compare_callbacks (const ValueCallback* first, const ValueCallback* second){ diff --git a/src/include/BER.h b/src/include/BER.h index b7767ae..d3a7865 100644 --- a/src/include/BER.h +++ b/src/include/BER.h @@ -28,12 +28,13 @@ class BER_CONTAINER; struct ASNPool { #ifndef SNMP_POOL_SLOT_SIZE -#define SNMP_POOL_SLOT_SIZE 768 +#define SNMP_POOL_SLOT_SIZE 640 #endif struct Slot { alignas(8) char storage[SNMP_POOL_SLOT_SIZE]; bool occupied; + bool doubleReleaseWarned; /* one-shot alarm latch for DEBUG>0 */ }; /* SNMP_POOLS_IN_BSS = 1 forces ASNPool storage back to static .bss. @@ -50,6 +51,17 @@ struct ASNPool { static Slot slots[SNMP_POOL_ASN_OBJECTS]; #endif static int usedCount; + static int permCount; + static int usedCountPeak; /* high-water mark since boot — diagnostics */ + + static inline void freezePermCount() noexcept { + int high = 0; + for(int i = 0; i < SNMP_POOL_ASN_OBJECTS; i++){ + if(slots[i].occupied) high = i + 1; + } + permCount = high; + usedCount = high < usedCount ? high : usedCount; + } static inline bool isInPool(const void* p){ #ifndef SNMP_POOLS_IN_BSS @@ -64,21 +76,42 @@ struct ASNPool { } static void* rawAlloc(size_t sz){ - if(sz > SNMP_POOL_SLOT_SIZE) return nullptr; + if(sz > SNMP_POOL_SLOT_SIZE) { + SNMP_LOGE("ASNPool::rawAlloc: request %zu B > slot %d B\n", sz, (int)SNMP_POOL_SLOT_SIZE); + return nullptr; + } #ifndef SNMP_POOLS_IN_BSS if(!_poolsReady) _ensurePools(); #endif + if(usedCount >= SNMP_POOL_ASN_OBJECTS){ + SNMP_LOGE("ASNPool EXHAUSTED: %d/%d slots in use (%d B/slot). Raise SNMP_POOL_ASN_OBJECTS. First NULL deref = Exception 28.\n", + usedCount, (int)SNMP_POOL_ASN_OBJECTS, (int)SNMP_POOL_SLOT_SIZE); + return nullptr; + } for(int i = 0; i < SNMP_POOL_ASN_OBJECTS; i++){ if(!slots[i].occupied){ slots[i].occupied = true; usedCount++; + if(usedCount > usedCountPeak) usedCountPeak = usedCount; return slots[i].storage; } } + SNMP_LOGE("ASNPool EXHAUSTED: %d/%d slots in use (%d B/slot). Raise SNMP_POOL_ASN_OBJECTS. First NULL deref = Exception 28.\n", + usedCount, (int)SNMP_POOL_ASN_OBJECTS, (int)SNMP_POOL_SLOT_SIZE); return nullptr; } static void release(BER_CONTAINER* p); + + static inline void resetAll() noexcept { +#ifndef SNMP_POOLS_IN_BSS + if(!_poolsReady) return; +#endif + for(int i = permCount; i < SNMP_POOL_ASN_OBJECTS; i++){ + slots[i].occupied = false; + } + usedCount = permCount; + } }; template @@ -88,7 +121,16 @@ static inline T* asn_new(Args&&... args){ T* obj = ::new (slot) T(std::forward(args)...); return obj; } +#ifdef COMPILING_TESTS + /* Native host tests: pool capacity is a logic stress-test vector, not a + hard safety bound. Host has GB of free RAM so falling back to operator + new allows 101/101 Catch2 assertions to exercise logic end-to-end without + needing an enormous static pool. On real MCU targets (COMPILING_TESTS + undefined) we strictly return nullptr = zero hot-path heap. */ return ::new T(std::forward(args)...); +#else + return nullptr; +#endif } void asn_delete(BER_CONTAINER* p); @@ -159,7 +201,7 @@ typedef int SNMP_BUFFER_ENCODE_ERROR; class BER_CONTAINER { public: - BER_CONTAINER(ASN_TYPE type) : _type(type){}; + BER_CONTAINER(ASN_TYPE type) : _type(type){} virtual ~BER_CONTAINER()= default; ASN_TYPE _type; @@ -179,10 +221,10 @@ class BER_CONTAINER { class NetworkAddress: public BER_CONTAINER { public: - NetworkAddress(): BER_CONTAINER(NETWORK_ADDRESS) {}; + NetworkAddress(): BER_CONTAINER(NETWORK_ADDRESS) {} explicit NetworkAddress(const IPAddress& ip): NetworkAddress(){ _value = ip; - }; + } IPAddress _value = INADDR_NONE; @@ -194,10 +236,10 @@ class NetworkAddress: public BER_CONTAINER { class IntegerType: public BER_CONTAINER { public: - IntegerType(): BER_CONTAINER(INTEGER) {}; + IntegerType(): BER_CONTAINER(INTEGER) {} explicit IntegerType(int value): IntegerType(){ _value = value; - }; + } int _value = 0; @@ -210,10 +252,10 @@ class TimestampType: public IntegerType { public: TimestampType(): IntegerType(){ _type = TIMESTAMP; - }; + } explicit TimestampType(unsigned long value): IntegerType(value){ _type = TIMESTAMP; - }; + } }; class OctetType: public BER_CONTAINER { @@ -224,7 +266,7 @@ class OctetType: public BER_CONTAINER { memcpy(_value, value, len); _value[len] = 0; _valueLen = len; - }; + } OctetType(const char* value, size_t len): BER_CONTAINER(STRING) { if(len > SNMP_MAX_STRING_LEN) len = SNMP_MAX_STRING_LEN; memcpy(_value, value, len); @@ -239,7 +281,7 @@ class OctetType: public BER_CONTAINER { int serialise(uint8_t* buf, size_t max_len) override; int fromBuffer(const uint8_t *buf, size_t max_len) override; - OctetType(): BER_CONTAINER(STRING) { _value[0] = 0; }; + OctetType(): BER_CONTAINER(STRING) { _value[0] = 0; } friend class ComplexType; template friend U* asn_new(Args&&... args); }; @@ -268,7 +310,7 @@ class OpaqueType: public BER_CONTAINER { OpaqueType(): BER_CONTAINER(OPAQUE) { this->_dataLength = 0; - }; + } friend class ComplexType; template friend U* asn_new(Args&&... args); }; @@ -279,19 +321,23 @@ class OIDType: public BER_CONTAINER { explicit OIDType(const char* value): BER_CONTAINER(OID) { size_t len = strlen(value); if(len > SNMP_MAX_OID_STR_LEN) len = SNMP_MAX_OID_STR_LEN; - memcpy(_valueStr, value, len); - _valueStr[len] = 0; - this->dataLen = 0; - this->valid = this->generateInternalData(); - }; + _init_from_cstr(value, len); + } + + template + OIDType(const char (&value)[N]): BER_CONTAINER(OID) { + constexpr size_t cap = ( (N-1) > SNMP_MAX_OID_STR_LEN ) ? SNMP_MAX_OID_STR_LEN : (N-1); + _init_from_cstr(value, cap); + } std::shared_ptr cloneOID() const { - return std::shared_ptr(asn_new(this->_valueStr, this->data, this->dataLen, this->valid)); - }; + return std::shared_ptr(asn_new(this->_valueStr, this->data, this->dataLen, this->valid), + [](OIDType* p){ asn_delete(static_cast(p)); }); + } OIDType* cloneRaw() const { return asn_new(this->_valueStr, this->data, this->dataLen, this->valid); - }; + } const char* string(); bool valid = false; @@ -308,15 +354,6 @@ class OIDType: public BER_CONTAINER { bool isSubTreeOf(const OIDType* const oid){ if(oid->dataLen >= this->dataLen) return false; - /* oid must be an exact prefix of this data (front of array). - Old code used reverse-equal (from the tail) which works only for - equal-sized trailing bytes, but semantically OID subtree is a - leading-prefix check, which is the same as: - compare first oid->dataLen bytes of this->data == oid->data - Reverse-equal worked because the `std::equal(rbegin, rend, rbegin + off)` - form verified that the suffix matched offset-tail; mathematically - equivalent to prefix match when off = this->size - oid->size. For a - zero-allocation version we just do the direct prefix check. */ if(oid->dataLen == 0) return true; return memcmp(this->data, oid->data, (size_t)oid->dataLen) == 0; } @@ -325,9 +362,12 @@ class OIDType: public BER_CONTAINER { int serialise(uint8_t* buf, size_t max_len) override; int fromBuffer(const uint8_t *buf, size_t max_len) override; + void _init_from_cstr(const char* value, size_t len) noexcept; + void _init_from_cstr_with_data(const char* value, size_t len, const uint8_t* srcData, int srcLen, bool valid) noexcept; + friend class ComplexType; template friend U* asn_new(Args&&... args); - OIDType(): BER_CONTAINER(OID) { _valueStr[0] = 0; dataLen = 0; }; + OIDType(): BER_CONTAINER(OID) { _valueStr[0] = 0; dataLen = 0; } char _valueStr[SNMP_MAX_OID_STR_LEN + 1]; uint8_t data[SNMP_MAX_OID_SUBIDENTIFIERS + 1]; @@ -337,42 +377,37 @@ class OIDType: public BER_CONTAINER { explicit OIDType(const char* value, const uint8_t* srcData, int srcLen, bool valid): BER_CONTAINER(OID), valid(valid), dataLen(srcLen) { size_t len = strlen(value); if(len > SNMP_MAX_OID_STR_LEN) len = SNMP_MAX_OID_STR_LEN; - memcpy(_valueStr, value, len); - _valueStr[len] = 0; - if(srcLen > 0 && srcData) { - if(srcLen > (int)sizeof(this->data)) srcLen = (int)sizeof(this->data); - this->dataLen = srcLen; - memcpy(this->data, srcData, (size_t)srcLen); - } else { - this->dataLen = 0; - } - }; + _init_from_cstr_with_data(value, len, srcData, srcLen, valid); + } + + template + explicit OIDType(const char (&value)[N], const uint8_t* srcData, int srcLen, bool valid): BER_CONTAINER(OID), valid(valid), dataLen(srcLen) { + constexpr size_t cap = ( (N-1) > SNMP_MAX_OID_STR_LEN ) ? SNMP_MAX_OID_STR_LEN : (N-1); + _init_from_cstr_with_data(value, cap, srcData, srcLen, valid); + } bool generateInternalData(); }; class SortableOIDType: public OIDType { public: - explicit SortableOIDType(const char* value): OIDType(value), sortingMapLen(0) { - generateSortingMap(this->sortingMap, &this->sortingMapLen); - } + explicit SortableOIDType(const char* value): OIDType(value) {} + + template + SortableOIDType(const char (&value)[N]): OIDType(value) {} - static bool sort_oids(SortableOIDType* oid1, SortableOIDType* oid2); + static bool sort_oids(const SortableOIDType* oid1, const SortableOIDType* oid2); bool operator < (SortableOIDType& other){ return SortableOIDType::sort_oids(this, &other); } - uint32_t sortingMap[SNMP_MAX_OID_SUBIDENTIFIERS]; - int sortingMapLen; - private: - void generateSortingMap(uint32_t outMap[SNMP_MAX_OID_SUBIDENTIFIERS], int* outLen) const; }; class NullType: public BER_CONTAINER { public: - NullType(): BER_CONTAINER(NULLTYPE) {}; + NullType(): BER_CONTAINER(NULLTYPE) {} protected: int serialise(uint8_t* buf, size_t max_len) override; @@ -384,15 +419,15 @@ class ImplicitNullType: public NullType { explicit ImplicitNullType(ASN_TYPE type): NullType(){ //TODO: check that we're one of the implicit null types _type = type; - }; + } }; class Counter64: public BER_CONTAINER { public: - Counter64(): BER_CONTAINER(COUNTER64) {}; + Counter64(): BER_CONTAINER(COUNTER64) {} explicit Counter64(uint64_t value): Counter64(){ _value = value; - }; + } uint64_t _value = 0; @@ -405,10 +440,10 @@ class Counter32: public IntegerType { public: Counter32(): IntegerType(){ _type = COUNTER32; - }; + } explicit Counter32(unsigned int value): IntegerType(value){ _type = COUNTER32; - }; + } }; @@ -416,16 +451,16 @@ class Gauge: public IntegerType { // Unsigned int public: Gauge(): IntegerType(){ _type = GAUGE32; - }; + } explicit Gauge(unsigned int value): IntegerType(value){ _type = GAUGE32; - }; + } }; class ComplexType: public BER_CONTAINER { public: - explicit ComplexType(ASN_TYPE type): BER_CONTAINER(type), valuesLen(0), _ownsChildren(false) {}; + explicit ComplexType(ASN_TYPE type): BER_CONTAINER(type), valuesLen(0), _ownsChildren(false) {} ~ComplexType(){ if(this->_ownsChildren){ for(int n = 0; n < this->valuesLen; n++){ @@ -444,7 +479,15 @@ class ComplexType: public BER_CONTAINER { int serialise(uint8_t* buf, size_t max_len) override; BER_CONTAINER* addValueToListRaw(BER_CONTAINER* newObj){ - if(this->valuesLen >= SNMP_MAX_COMPLEX_CHILDREN) return nullptr; + if(!newObj){ + SNMP_LOGE("ComplexType::addValueToListRaw: nullptr child rejected (ASNPool exhausted?)\n"); + return nullptr; + } + if(this->valuesLen >= SNMP_MAX_COMPLEX_CHILDREN){ + SNMP_LOGE("ComplexType::addValueToListRaw: values[] full (%d max). Raise SNMP_MAX_COMPLEX_CHILDREN.\n", + (int)SNMP_MAX_COMPLEX_CHILDREN); + return nullptr; + } this->values[this->valuesLen++] = newObj; return newObj; } diff --git a/src/include/SNMPPacket.h b/src/include/SNMPPacket.h index 8675d87..421c922 100644 --- a/src/include/SNMPPacket.h +++ b/src/include/SNMPPacket.h @@ -36,7 +36,7 @@ union ErrorIndex { class SNMPPacket { public: - SNMPPacket(){}; + SNMPPacket(){} explicit SNMPPacket(const SNMPPacket& packet){ this->setRequestID(packet.requestID); this->setVersion(packet.snmpVersion); @@ -60,7 +60,7 @@ class SNMPPacket { * initializers: varbindCount=0 + default-constructed VarBind * slots) because callers (e.g. SNMPResponse copy-from-request) * populate the response via addResponse() calls. */ - }; + } virtual ~SNMPPacket(); @@ -152,6 +152,10 @@ class SNMPPacket { protected: virtual bool build(); + typedef bool (*BuildPDUHeaderFn)(ComplexType* snmpPDU, void* userdata); + + bool _build_pdu_envelope(BuildPDUHeaderFn fill_pdu, void* userdata); + virtual std::shared_ptr generateVarBindList(); /* Build the varbind tree with uniform recursive ownership (raw new'd; @@ -164,4 +168,4 @@ class SNMPPacket { }; -#endif \ No newline at end of file +#endif diff --git a/src/include/SNMPResponse.h b/src/include/SNMPResponse.h index 1a2e75a..931ba8d 100644 --- a/src/include/SNMPResponse.h +++ b/src/include/SNMPResponse.h @@ -9,9 +9,9 @@ #if 0 class ResponseVarBind : public VarBind { public: - explicit ResponseVarBind(OIDType* oid, ASN_TYPE type): VarBind(oid, type, nullptr){}; - explicit ResponseVarBind(VarBind* vb): VarBind(vb->oid->clone(), vb->type, nullptr){}; - explicit ResponseVarBind(ValueCallback* cb): VarBind(cb->OID->clone(), cb->type, nullptr){}; + explicit ResponseVarBind(OIDType* oid, ASN_TYPE type): VarBind(oid, type, nullptr){} + explicit ResponseVarBind(VarBind* vb): VarBind(vb->oid->clone(), vb->type, nullptr){} + explicit ResponseVarBind(ValueCallback* cb): VarBind(cb->OID->clone(), cb->type, nullptr){} SNMP_ERROR_STATUS errorStatus = NO_ERROR; }; #endif @@ -20,7 +20,7 @@ class SNMPResponse : public SNMPPacket { public: explicit SNMPResponse(const SNMPPacket& request): SNMPPacket(request){ this->setPDUType(GetResponsePDU); - }; + } bool addResponse(const VarBind& response); bool addErrorResponse(const VarBind& response); @@ -28,4 +28,4 @@ class SNMPResponse : public SNMPPacket { bool setGlobalError(SNMP_ERROR_STATUS error, int index, int overwrite); // Overwrite existing varbindError? }; -#endif \ No newline at end of file +#endif diff --git a/src/include/ValueCallbacks.h b/src/include/ValueCallbacks.h index 3f0363f..4fa4058 100644 --- a/src/include/ValueCallbacks.h +++ b/src/include/ValueCallbacks.h @@ -4,14 +4,25 @@ #include "BER.h" #include +template +static inline void vcb_pool_asn_deleter(T* p) noexcept { + asn_delete(static_cast(p)); +} + +template +static inline std::shared_ptr vcb_pool_shared(T* p) noexcept { + if(!p) return nullptr; + return std::shared_ptr(p, vcb_pool_asn_deleter); +} + typedef int (*GETINT_FUNC)() ; typedef uint32_t (*GETUINT_FUNC)(); typedef const char* (*GETSTRING_FUNC)(); class ValueCallback { public: - ValueCallback(SortableOIDType* oid, ASN_TYPE type): OID(oid), type(type){}; - ~ValueCallback(){ + ValueCallback(SortableOIDType* oid, ASN_TYPE type): OID(oid), type(type){} + virtual ~ValueCallback(){ asn_delete(OID); } SortableOIDType * const OID; @@ -25,6 +36,9 @@ class ValueCallback { setOccurred = false; } + static const char* getTypeName(ASN_TYPE t) noexcept; + virtual const char* getAccessTag() const noexcept { return isSettable ? "RW" : "RO"; } + static ValueCallback* findCallback(ValueCallback* const *callbacks, int callbacksCount, const OIDType* const oid, bool walk, int startAt = 0, int *foundAt = nullptr); static std::shared_ptr getValueForCallback(ValueCallback* callback); static SNMP_ERROR_STATUS setValueForCallback(ValueCallback* callback, const std::shared_ptr &value); @@ -40,7 +54,7 @@ bool remove_handler(ValueCallback** callbacks, int& callbacksCount, ValueCallbac class IntegerCallback: public ValueCallback { public: - IntegerCallback(SortableOIDType* oid, int* value): ValueCallback(oid, INTEGER), value(value) {}; + IntegerCallback(SortableOIDType* oid, int* value): ValueCallback(oid, INTEGER), value(value) {} protected: int* const value; @@ -52,13 +66,13 @@ class IntegerCallback: public ValueCallback { class StaticIntegerCallback: public ValueCallback { public: - StaticIntegerCallback(SortableOIDType* oid, int value): ValueCallback(oid, INTEGER), val(value) {}; + StaticIntegerCallback(SortableOIDType* oid, int value): ValueCallback(oid, INTEGER), val(value) {} protected: const int val; std::shared_ptr buildTypeWithValue() override { - return std::make_shared(val); + return vcb_pool_shared(asn_new(val)); } SNMP_ERROR_STATUS setTypeWithValue(BER_CONTAINER*) override { @@ -69,13 +83,14 @@ class StaticIntegerCallback: public ValueCallback { class DynamicIntegerCallback: public ValueCallback { public: DynamicIntegerCallback(SortableOIDType* oid, GETINT_FUNC callback_func): - ValueCallback(oid, INTEGER), m_callback(callback_func) {}; + ValueCallback(oid, INTEGER), m_callback(callback_func) {} + const char* getAccessTag() const noexcept override { return "DYN"; } protected: GETINT_FUNC m_callback; std::shared_ptr buildTypeWithValue() override { - return std::make_shared(m_callback()); + return vcb_pool_shared(asn_new(m_callback())); } SNMP_ERROR_STATUS setTypeWithValue(BER_CONTAINER*) override { @@ -85,7 +100,7 @@ class DynamicIntegerCallback: public ValueCallback { class TimestampCallback: public ValueCallback { public: - TimestampCallback(SortableOIDType* oid, uint32_t* value): ValueCallback(oid, TIMESTAMP), value(value) {}; + TimestampCallback(SortableOIDType* oid, uint32_t* value): ValueCallback(oid, TIMESTAMP), value(value) {} protected: uint32_t* const value; @@ -97,13 +112,14 @@ class TimestampCallback: public ValueCallback { class DynamicTimestampCallback: public ValueCallback { public: DynamicTimestampCallback(SortableOIDType* oid, GETUINT_FUNC callback_func): - ValueCallback(oid, TIMESTAMP), m_callback(callback_func) {}; + ValueCallback(oid, TIMESTAMP), m_callback(callback_func) {} + const char* getAccessTag() const noexcept override { return "DYN"; } protected: GETUINT_FUNC m_callback; std::shared_ptr buildTypeWithValue() override { - return std::make_shared(m_callback()); + return vcb_pool_shared(asn_new(m_callback())); } SNMP_ERROR_STATUS setTypeWithValue(BER_CONTAINER*) override { @@ -118,7 +134,7 @@ class ReadOnlyStringCallback: public ValueCallback { if(len > SNMP_MAX_STRING_LEN) len = SNMP_MAX_STRING_LEN; memcpy(this->value, value, len); this->value[len] = 0; - }; + } protected: char value[SNMP_MAX_STRING_LEN + 1]; @@ -126,27 +142,28 @@ class ReadOnlyStringCallback: public ValueCallback { std::shared_ptr buildTypeWithValue() override; SNMP_ERROR_STATUS setTypeWithValue(BER_CONTAINER*) override { return NO_ACCESS; - }; + } }; class DynamicStringCallback: public ValueCallback { public: - DynamicStringCallback(SortableOIDType* oid, GETSTRING_FUNC callback): ValueCallback(oid, STRING), m_callback(callback) {}; + DynamicStringCallback(SortableOIDType* oid, GETSTRING_FUNC callback): ValueCallback(oid, STRING), m_callback(callback) {} + const char* getAccessTag() const noexcept override { return "DYN"; } protected: GETSTRING_FUNC m_callback; std::shared_ptr buildTypeWithValue() override { - return std::make_shared(m_callback()); + return vcb_pool_shared(asn_new(m_callback())); } SNMP_ERROR_STATUS setTypeWithValue(BER_CONTAINER*) override { return NO_ACCESS; - }; + } }; class StringCallback: public ValueCallback { public: - StringCallback(SortableOIDType* oid, char** value, size_t max_len): ValueCallback(oid, STRING), value(value), max_len(max_len) {}; + StringCallback(SortableOIDType* oid, char** value, size_t max_len): ValueCallback(oid, STRING), value(value), max_len(max_len) {} protected: char** const value; @@ -158,7 +175,7 @@ class StringCallback: public ValueCallback { class OpaqueCallback: public ValueCallback { public: - OpaqueCallback(SortableOIDType* oid, uint8_t* value, int data_len): ValueCallback(oid, OPAQUE), value(value), data_len(data_len) {}; + OpaqueCallback(SortableOIDType* oid, uint8_t* value, int data_len): ValueCallback(oid, OPAQUE), value(value), data_len(data_len) {} protected: uint8_t* const value; @@ -175,7 +192,7 @@ class OIDCallback: public ValueCallback { if(len > SNMP_MAX_OID_STR_LEN) len = SNMP_MAX_OID_STR_LEN; memcpy(this->value, value, len); this->value[len] = 0; - }; + } protected: char value[SNMP_MAX_OID_STR_LEN + 1]; @@ -183,12 +200,12 @@ class OIDCallback: public ValueCallback { std::shared_ptr buildTypeWithValue() override; SNMP_ERROR_STATUS setTypeWithValue (BER_CONTAINER*) override{ return NO_ACCESS; - }; + } }; class Counter32Callback: public ValueCallback { public: - Counter32Callback(SortableOIDType* oid, uint32_t* value): ValueCallback(oid, COUNTER32), value(value) {}; + Counter32Callback(SortableOIDType* oid, uint32_t* value): ValueCallback(oid, COUNTER32), value(value) {} protected: uint32_t* const value; @@ -200,7 +217,7 @@ class Counter32Callback: public ValueCallback { class Gauge32Callback: public ValueCallback { public: - Gauge32Callback(SortableOIDType* oid, uint32_t* value): ValueCallback(oid, GAUGE32), value(value) {}; + Gauge32Callback(SortableOIDType* oid, uint32_t* value): ValueCallback(oid, GAUGE32), value(value) {} protected: uint32_t* const value; @@ -211,22 +228,23 @@ class Gauge32Callback: public ValueCallback { class DynamicGauge32Callback: public ValueCallback { public: - DynamicGauge32Callback(SortableOIDType* oid, GETUINT_FUNC callback_func): ValueCallback(oid, GAUGE32), m_callback(callback_func) {}; + DynamicGauge32Callback(SortableOIDType* oid, GETUINT_FUNC callback_func): ValueCallback(oid, GAUGE32), m_callback(callback_func) {} + const char* getAccessTag() const noexcept override { return "DYN"; } protected: GETUINT_FUNC m_callback; std::shared_ptr buildTypeWithValue() override { - return std::make_shared(m_callback()); + return vcb_pool_shared(asn_new(m_callback())); } SNMP_ERROR_STATUS setTypeWithValue (BER_CONTAINER*) override{ return NO_ACCESS; - }; + } }; class Counter64Callback: public ValueCallback { public: - Counter64Callback(SortableOIDType* oid, uint64_t* value): ValueCallback(oid, COUNTER64), value(value) {}; + Counter64Callback(SortableOIDType* oid, uint64_t* value): ValueCallback(oid, COUNTER64), value(value) {} protected: uint64_t* const value; diff --git a/src/include/defs.h b/src/include/defs.h index b6e6bc8..bc2a17a 100644 --- a/src/include/defs.h +++ b/src/include/defs.h @@ -33,8 +33,8 @@ #define LIBRARY_VERSION_MAJOR 3 #define LIBRARY_VERSION_MINOR 1 -#define LIBRARY_VERSION_PATCH 5 -#define LIBRARY_VERSION "3.1.5" +#define LIBRARY_VERSION_PATCH 23 +#define LIBRARY_VERSION "3.1.23" typedef enum SNMP_ERROR_RESPONSE { SNMP_NO_UDP = -10, @@ -104,7 +104,11 @@ extern const char* SNMP_TAG; #endif #ifndef SNMP_MAX_COMMUNITY_LEN - #define SNMP_MAX_COMMUNITY_LEN 64 + /* RFC 3418 §2.6 SNMPv2c community strings are traditionally short tokens; + 32 B is the practical RFC ceiling. Users who need legacy long strings + (e.g. v3-style views over v2c) can #define SNMP_MAX_COMMUNITY_LEN 64 + BEFORE including SNMP_Agent.h — value is guarded so sketch-side wins. */ + #define SNMP_MAX_COMMUNITY_LEN 32 #endif #ifndef SNMP_MAX_OID_STR_LEN #ifdef _SNMP_ESP8266_TINY @@ -124,10 +128,11 @@ extern const char* SNMP_TAG; #endif #ifndef SNMP_MAX_COMPLEX_CHILDREN #ifdef _SNMP_ESP8266_TINY - #define SNMP_MAX_COMPLEX_CHILDREN 8 + #define SNMP_MAX_COMPLEX_CHILDREN 16 #else - #define SNMP_MAX_COMPLEX_CHILDREN 16 /* Maximum children inside a single BER ComplexType (STRUCTURE / PDU / VarBindList). + #define SNMP_MAX_COMPLEX_CHILDREN 24 /* Maximum children inside a single BER ComplexType (STRUCTURE / PDU / VarBindList). Real-world GetResponses contain < 8 VarBinds; 16 covers bulkwalk default=10 + some slack. + 24 (default) / 16 (ESP8266_TINY) safely accommodates GetBulk + walk response envelopes. Used to size ComplexType::values[] fixed array. */ #endif #endif @@ -169,35 +174,117 @@ extern const char* SNMP_TAG; #endif #ifndef SNMP_POOL_ASN_OBJECTS #ifdef _SNMP_ESP8266_TINY - #define SNMP_POOL_ASN_OBJECTS 24 + #define SNMP_POOL_ASN_OBJECTS 76 #else - #define SNMP_POOL_ASN_OBJECTS 32 /* Global placement-pool slot count for BER_CONTAINER-derived ASN objects. - Upper bound: 4 traps in flight × 8 VBs each + ~16 for request decode = 48; - 32 (default) or 24 (ESP8266 tiny) both allow concurrent GET + response safely. */ + #define SNMP_POOL_ASN_OBJECTS 80 /* Global placement-pool slot count for BER_CONTAINER-derived ASN objects. + Headroom budget (per-tick transient, reset by ASNPool::resetAll() at each loop()): + - 14 inbound BER decode tree (v2c PDU with 6 VBs) + - 10 response-copy duplicates (version/community/reqid/clones) + - 30 response encode tree (bulkwalk/10 VB responses + envelope) + - 10 trap-send scratch during auto-trap / coldStart + 76 (ESP8266_TINY, tradeoff with WiFi heap) / 80 (default) covers the + 23-handler hwtest sketch with GET/GETNEXT/GETBULK/SET/auto-traps all working. */ #endif #endif #ifndef SNMP_POOL_VARBIND_OBJECTS #ifdef _SNMP_ESP8266_TINY - #define SNMP_POOL_VARBIND_OBJECTS 8 + #define SNMP_POOL_VARBIND_OBJECTS 24 #else - #define SNMP_POOL_VARBIND_OBJECTS 12 /* Global placement-pool slot count for transient VarBind objects. */ + #define SNMP_POOL_VARBIND_OBJECTS 32 /* Global placement-pool slot count for transient VarBind objects. */ #endif #endif + /* SNMP_POOL_SLOT_SIZE is the raw payload size of each ASNPool::Slot. - * Because each slot must fit the LARGEST concrete BER subclass we - * instantiate (SortableOIDType 576B + vtable ptr + tail bytes) the - * default is 768, wasting ~192 B per slot. ESP8266 TINY sets the - * slot to 640 B (exactly SortableOIDType on 64-bit) which still - * accommodates SortableOIDType on 32-bit Xtensa (576 B). */ + * Each slot must fit the LARGEST concrete BER subclass we instantiate. + * v3.1.25: right-sized to MEASURED sizeof() of the largest container + * (host sizeof probe, Xtensa-compatible layout): + * TINY config: OctetType = 288 B (_value[256] + base) -> slot 288 + * default config: OIDType/SortableOIDType = 312 B -> slot 312 + * The previous 512/640 values predated the v3.1.9 sortingMap removal + * and padded every slot with 224/328 dead bytes (~44% of the arena — + * 17 KB of the ESP-01's 39.5 KB pool was padding). static_assert + * guards in BER.h now pin every container <= slot so this can never + * silently go stale again; a sketch raising OCTET_TYPE_MAX_LENGTH or + * SNMP_MAX_OID_STR_LEN gets a compile error instead of a runtime + * placement failure. */ #ifndef SNMP_POOL_SLOT_SIZE #ifdef _SNMP_ESP8266_TINY - #define SNMP_POOL_SLOT_SIZE 640 + #define SNMP_POOL_SLOT_SIZE 288 #else - #define SNMP_POOL_SLOT_SIZE 768 + #define SNMP_POOL_SLOT_SIZE 312 #endif #endif +/* ===================================================================== + * COMPILE-TIME POOL FLOOR GUARDS + * --------------------------------------------------------------------- + * Minimum-safe values empirically proven on ESP-01 (ESP8266EX, 80 MHz, + * 80 KB RAM, CH340 USB). Anything BELOW these caused a NULL-pointer + * dereference crash (Exception 28 on Xtensa) the INSTANT the first UDP + * GetRequest arrived on port 161. Root cause: ASNPool::alloc() or + * VarBind pool returned nullptr on exhaustion, or callbacks[] array + * overflow (23 handlers into 20-slot array) silently corrupted the + * adjacent UDP pointer/cbCount members → SNMP socket went deaf. + * + * Full incident report: _hwtest_esp01/HARDWARE_TEST_REPORT.md §2 §9 + * ===================================================================== + */ +#if defined(__cplusplus) + + #define SNMP_FLOOR_MSG_HEAD \ + "\n[SNMP_Agent compile guard] Pool cap below empirically-proven minimum." \ + "\n Smaller values cause NULL allocation or silent array overflow on first " \ + "\n inbound SNMP packet → ESP8266 Exception 28 / hard fault." \ + "\n Full analysis: _hwtest_esp01/HARDWARE_TEST_REPORT.md section 2." + + static_assert(SNMP_MAX_CALLBACKS_PER_AGENT >= 24, + SNMP_FLOOR_MSG_HEAD + "\n -> SNMP_MAX_CALLBACKS_PER_AGENT must be >= 24 (test sketch registered 23" + "\n = 7 RFC1213 + 14 test leaves + 2 trap regs; anything <23 overwrites udp[]." + "\n Suggested: 32 (or leave default, which is 24 for ESP8266_TINY)."); + + static_assert(SNMP_POOL_ASN_OBJECTS >= 24, + SNMP_FLOOR_MSG_HEAD + "\n -> SNMP_POOL_ASN_OBJECTS must be >= 24." + "\n With 16: GetRequest BER parse ran dry before GetResponse was encoded," + "\n crashing at excvaddr=0x2C (nullptr + 44 bytes) inside handlePacket()."); + + static_assert(SNMP_POOL_VARBIND_OBJECTS >= 8, + SNMP_FLOOR_MSG_HEAD + "\n -> SNMP_POOL_VARBIND_OBJECTS must be >= 8." + "\n With 4: request + response VB lifetimes overlap → NULL deref."); + + static_assert(SNMP_MAX_COMPLEX_CHILDREN >= 6, + SNMP_FLOOR_MSG_HEAD + "\n -> SNMP_MAX_COMPLEX_CHILDREN must be >= 6." + "\n Absolute minimum for a v2c PDU wrapper (4 children + vb_list + headroom)."); + + static_assert(SNMP_MAX_VARBINDS >= 4, + SNMP_FLOOR_MSG_HEAD + "\n -> SNMP_MAX_VARBINDS must be >= 4." + "\n (ESP8266_TINY default is 6; 4 is rock-bottom for a GET pair.)"); + + static_assert(MAX_SNMP_PACKET_LENGTH >= 512, + SNMP_FLOOR_MSG_HEAD + "\n -> MAX_SNMP_PACKET_LENGTH must be >= 512 bytes." + "\n RFC 3417 sets 484 as the historic floor; 512 safely covers snmpwalk" + "\n responses with 12 varbinds plus PDU headers. ESP8266 default 1024."); + + static_assert(OCTET_TYPE_MAX_LENGTH >= 64, + SNMP_FLOOR_MSG_HEAD + "\n -> OCTET_TYPE_MAX_LENGTH must be >= 64." + "\n sysContact/Name/Location in RFC1213 use 64-byte buffers minimum."); + + static_assert(SNMP_MAX_OID_SUBIDENTIFIERS >= 16, + SNMP_FLOOR_MSG_HEAD + "\n -> SNMP_MAX_OID_SUBIDENTIFIERS must be >= 16." + "\n enterprises.99999.1.14 = 12 sub-IDs. 16 is 33% headroom minimum."); + + #undef SNMP_FLOOR_MSG_HEAD + +#endif + #define SNMP_ERROR_OK 1 #define SNMP_PACKET_PARSE_ERROR_OFFSET -20 diff --git a/tests/required/IPAddress.h b/tests/required/IPAddress.h index 66b6c78..90d2b00 100644 --- a/tests/required/IPAddress.h +++ b/tests/required/IPAddress.h @@ -28,7 +28,7 @@ class TEMPSTR { public: - const char* c_str(){return "";}; + const char* c_str(){return "";} }; class IPAddress @@ -58,7 +58,7 @@ class IPAddress TEMPSTR toString(){ return TEMPSTR(); - }; + } // Overloaded cast operator to allow IPAddress objects to be used where a pointer // to a four-byte uint8_t array is expected @@ -91,4 +91,4 @@ class IPAddress const IPAddress INADDR_NONE(0, 0, 0, 0); #endif -#endif \ No newline at end of file +#endif diff --git a/tests/required/Print.h b/tests/required/Print.h new file mode 100644 index 0000000..2c50862 --- /dev/null +++ b/tests/required/Print.h @@ -0,0 +1,28 @@ +#ifndef Print_h +#define Print_h + +#ifdef COMPILING_TESTS +#include +#include + +class Print { + public: + virtual ~Print(){} + virtual size_t write(uint8_t){ return 1; } + size_t write(const char* str){ + if(!str) return 0; + size_t n = 0; + while(*str){ write((uint8_t)*str++); n++; } + return n; + } + size_t write(const uint8_t* buf, size_t n){ + size_t i = 0; + for(; i < n; i++) write(buf[i]); + return i; + } + size_t print(const char* s){ return write(s); } + size_t println(const char* s){ size_t n = write(s); write((uint8_t)'\r'); write((uint8_t)'\n'); return n+2; } +}; +#endif + +#endif diff --git a/tests/required/Serial.h b/tests/required/Serial.h index 49c614a..2b5e98b 100644 --- a/tests/required/Serial.h +++ b/tests/required/Serial.h @@ -2,16 +2,30 @@ #define Serial_h #include +#include #ifdef COMPILING_TESTS class HardwareSerial { public: int begin(int){return 0;}; - int printf(...){return 0;}; - int println(...){return 0;}; - int print(...){return 0;}; + /* Real formatting so demo sketch banners (e.g. "SNMP_Agent v%s") appear in + host test runs (`make example`), letting you verify the version banner + output before flashing to hardware. */ + int printf(const char* format, ...){ + va_list args; + va_start(args, format); + int n = vprintf(format, args); + va_end(args); + return n; + }; + /* Templated no-ops: passing non-POD objects (e.g. IPAddress) through a + variadic list is a hard error under -Wnon-pod-varargs, so print/println + must not be variadic. Templates accept any single-argument call. */ + template int print(const T&){ return 0; } + template int println(const T&){ return 0; } + int println(){ return 0; } }; HardwareSerial Serial; #endif -#endif \ No newline at end of file +#endif diff --git a/tests/required/UDP.h b/tests/required/UDP.h index 61ef221..7681013 100644 --- a/tests/required/UDP.h +++ b/tests/required/UDP.h @@ -8,12 +8,12 @@ class UDP { public: - void begin(int){}; + uint8_t begin(int){ return 1; } int parsePacket(){ return 0; } - void beginPacket(IPAddress, uint16_t){}; - int endPacket(){ return 1; }; - void write(uint8_t*, size_t){}; - void stop(){}; + void beginPacket(IPAddress, uint16_t){} + int endPacket(){ return 1; } + void write(uint8_t*, size_t){} + void stop(){} int read(uint8_t*, int){ return 0; } IPAddress remoteIP(){return IPAddress();} int remotePort(){return 0;} @@ -21,4 +21,4 @@ class UDP { }; #endif -#endif \ No newline at end of file +#endif