[GH-ISSUE #546] Missing address validation on receiver side leads to core dump; unclear TransferRequest field naming in Rust SDK #184

New issue

Closed

opened 2026-10-29 21:10:37 +00:00 by bsakaguchi · 4 comments

bsakaguchi commented

2026-10-29 21:10:37 +00:00

Owner

Originally created by @ActivePeter on GitHub (Jun 24, 2025).
Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/546

The receiver side currently does not validate the target address before performing a write operation. If the sender mistakenly provides an invalid or unmapped address, the receiver will attempt to write to it directly, resulting in an immediate core dump.

In addition, the TransferRequest struct in the Rust SDK has the following issues:

pub struct TransferRequest {
    pub opcode: OpcodeEnum,
    pub source: *mut c_void,
    pub target_id: i32,
    pub target_offset: u64,
    pub length: u64,
}

The field name target_offset is misleading — it actually represents a full address rather than an offset. A more appropriate name would be target_address.
The struct lacks proper documentation or inline comments, making it difficult for users to understand the purpose, units, and constraints of each field.
There are no links or references to related documentation, making the interface harder to use correctly.

Originally created by @ActivePeter on GitHub (Jun 24, 2025). Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/546 The receiver side currently does not validate the target address before performing a write operation. If the sender mistakenly provides an invalid or unmapped address, the receiver will attempt to write to it directly, resulting in an immediate core dump. In addition, the TransferRequest struct in the Rust SDK has the following issues: ``` pub struct TransferRequest { pub opcode: OpcodeEnum, pub source: *mut c_void, pub target_id: i32, pub target_offset: u64, pub length: u64, } ``` - The field name target_offset is misleading — it actually represents a full address rather than an offset. A more appropriate name would be target_address. - The struct lacks proper documentation or inline comments, making it difficult for users to understand the purpose, units, and constraints of each field. - There are no links or references to related documentation, making the interface harder to use correctly.

bsakaguchi

2026-10-29 21:10:37 +00:00

closed this issue
added the
stale

auto-closed
labels

bsakaguchi commented

2026-10-29 21:10:38 +00:00

Author

Owner

@stmatengss commented on GitHub (Jun 24, 2025):

Address checking overhead:
Comprehensive address validation does introduce substantial overhead, particularly given that a single Transfer Engine may manage numerous large memory regions. However, we agree that an optional configuration to enable these checks would be a reasonable addition.
Rust API compatibility:
Since Rust APIs aren't widely adopted in state-of-the-art LLM engines (e.g., sglang/vLLM), we currently maintain these interfaces with lower priority. We'd welcome community contributions here – feel free to submit a PR if you'd like to improve this functionality.

@stmatengss commented on GitHub (Jun 24, 2025): 1. **Address checking overhead:** Comprehensive address validation does introduce substantial overhead, particularly given that a single Transfer Engine may manage numerous large memory regions. However, we agree that an **optional configuration to enable these checks** would be a reasonable addition. 2. **Rust API compatibility:** Since Rust APIs aren't widely adopted in state-of-the-art LLM engines (e.g., sglang/vLLM), we currently maintain these interfaces with lower priority. We'd welcome community contributions here – feel free to submit a PR if you'd like to improve this functionality.

bsakaguchi commented

2026-10-29 21:10:38 +00:00

Author

Owner

@ActivePeter commented on GitHub (Jun 24, 2025):

Address checking overhead:
Comprehensive address validation does introduce substantial overhead, particularly given that a single Transfer Engine may manage numerous large memory regions. However, we agree that an optional configuration to enable these checks would be a reasonable addition.

Rust API compatibility:
Since Rust APIs aren't widely adopted in state-of-the-art LLM engines (e.g., sglang/vLLM), we currently maintain these interfaces with lower priority. We'd welcome community contributions here – feel free to submit a PR if you'd like to improve this functionality.

In practice, I believe the overhead of address validation is relatively minimal—on the node holding the memory, it usually only involves comparing the requested address against local metadata structures. In contrast, in distributed environments, incorrect target addresses are more likely to occur—for example, due to inconsistent views of memory lifetimes across nodes, or missing consensus on allocation state. In such cases, the cost of a failed assumption can be severe (e.g., a segmentation fault or data corruption), while the cost of a check is relatively low. Therefore, enabling basic address checks by default could significantly improve system robustness with negligible performance impact in most scenarios.

@ActivePeter commented on GitHub (Jun 24, 2025): > 1. **Address checking overhead:** > Comprehensive address validation does introduce substantial overhead, particularly given that a single Transfer Engine may manage numerous large memory regions. However, we agree that an **optional configuration to enable these checks** would be a reasonable addition. > 2. **Rust API compatibility:** > Since Rust APIs aren't widely adopted in state-of-the-art LLM engines (e.g., sglang/vLLM), we currently maintain these interfaces with lower priority. We'd welcome community contributions here – feel free to submit a PR if you'd like to improve this functionality. In practice, I believe the overhead of address validation is relatively minimal—on the node holding the memory, it usually only involves comparing the requested address against local metadata structures. In contrast, in distributed environments, incorrect target addresses are more likely to occur—for example, due to inconsistent views of memory lifetimes across nodes, or missing consensus on allocation state. In such cases, the cost of a failed assumption can be severe (e.g., a segmentation fault or data corruption), while the cost of a check is relatively low. Therefore, enabling basic address checks by default could significantly improve system robustness with negligible performance impact in most scenarios.

bsakaguchi commented

2026-10-29 21:10:38 +00:00

Author

Owner

@github-actions[bot] commented on GitHub (Jul 4, 2026):

This issue has had no activity for 90 days and will be closed in 7 days if there is no further activity. Please comment or react if it should stay open.

@github-actions[bot] commented on GitHub (Jul 4, 2026): This issue has had no activity for 90 days and will be closed in 7 days if there is no further activity. Please comment or react if it should stay open.

bsakaguchi commented

2026-10-29 21:10:38 +00:00

Author

Owner

@github-actions[bot] commented on GitHub (Jul 13, 2026):

Closing due to 3 months of inactivity. If this is still relevant, please comment and we can reopen.

@github-actions[bot] commented on GitHub (Jul 13, 2026): Closing due to 3 months of inactivity. If this is still relevant, please comment and we can reopen.