app: validate repair tile link count limits#8498
app: validate repair tile link count limits#8498two-heart wants to merge 1 commit intofiredancer-io:mainfrom
Conversation
Reject firedancer configs with shred_tile_count>16 or sign_tile_count>17 to prevent repair init out-of-bounds writes. No really a likely real world config, but we should not oob in any case.
There was a problem hiding this comment.
Pull request overview
Adds defensive validation for Firedancer configs to prevent repair tile link-count misconfiguration that could lead to out-of-bounds writes during repair initialization.
Changes:
- Introduces max constants for repair-related shred/sign link limits.
- Validates
layout.shred_tile_count <= 16whenis_firedanceris set. - Validates
firedancer.layout.sign_tile_count <= 17whenis_firedanceris set.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if( FD_UNLIKELY( config->firedancer.layout.sign_tile_count > (FD_REPAIR_MAX_SIGN_LINK_CNT + 1U) ) ) { | ||
| FD_LOG_ERR(( "`firedancer.layout.sign_tile_count` must be <= %u", FD_REPAIR_MAX_SIGN_LINK_CNT + 1U )); | ||
| } |
There was a problem hiding this comment.
The sign tile limit is expressed as FD_REPAIR_MAX_SIGN_LINK_CNT + 1U, but the macro name/value (16) represents link count while the config field is a tile count. This is easy to misread as an off-by-one bug. Consider defining a dedicated constant for the config upper bound (e.g., FD_REPAIR_MAX_SIGN_TILE_CNT = 17) and/or add a short comment explaining why one extra sign tile is allowed (e.g., sign tile 0 is not wired to repair).
| #define FD_REPAIR_MAX_SHRED_LINK_CNT (16U) | ||
| #define FD_REPAIR_MAX_SIGN_LINK_CNT (16U) |
There was a problem hiding this comment.
These max values duplicate the repair tile’s fixed-size arrays (e.g., MAX_SHRED_TILE_CNT / MAX_SIGN_TILE_CNT in src/discof/repair/fd_repair_tile.c). Keeping separate constants here risks drift (config could allow values that later OOB in the tile, or reject valid ones) if the repair tile limits change. Consider moving the limits into a shared header (or exported fd_repair_* constant) and referencing that from both the repair tile and config validation.
| } \ | ||
| } while(0) | ||
|
|
||
| #define FD_REPAIR_MAX_SHRED_LINK_CNT (16U) |
There was a problem hiding this comment.
Not a good place for this. Just FD_TEST them in the repair tile where it assigns to the arrays
Reject firedancer configs with shred_tile_count>16 or sign_tile_count>17 to prevent repair init out-of-bounds writes. No really a likely real world config, but we should not oob in any case.