Fix: Replace libc rand() with DPDK PRNG in RX loss simulators - #1694
Open
awilczyns wants to merge 1 commit into
Open
Fix: Replace libc rand() with DPDK PRNG in RX loss simulators#1694awilczyns wants to merge 1 commit into
awilczyns wants to merge 1 commit into
Conversation
Coverity CID 561265 (DC.WEAK_CRYPTO / CWE-676) flags rand() in ra_simulate_pkt_loss(). The security premise does not apply: the value only decides whether to drop a packet in a fault injector armed by ST30_RX_FLAG_SIMULATE_PKT_LOSS, and feeds no key, nonce, token or authorization decision. rte_drand() is likewise not cryptographically secure, so this is not a security improvement. The real defect at those lines is a two-world-rule violation: glibc rand() takes a process-global lock on every call, so with the flag armed each received packet serializes the RX polling tasklet against every other rand() caller in the process, including RX tasklets of other sessions on other lcores. rte_drand()/rte_rand_max() use per-lcore LFSR258 state with no lock, and rte_rand_max() is unbiased unlike `% max`. Value range is unchanged: burst_loss_max is always >= 1 (attach defaults 0 to 1), so both forms yield [1, burst_loss_max]. No new test accompanies this change because it has no observable behavioral delta; the existing St30p.rx_simulate_pkt_loss integration case covers the path. Applied to the identical video sibling rv_simulate_pkt_loss() so the same CID cannot reappear there. Signed-off-by: Wilczynski, Andrzej <andrzej.wilczynski@intel.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Coverity CID 561265 (DC.WEAK_CRYPTO / CWE-676) flags rand() in ra_simulate_pkt_loss(). The security premise does not apply: the value only decides whether to drop a packet in a fault injector armed by ST30_RX_FLAG_SIMULATE_PKT_LOSS, and feeds no key, nonce, token or authorization decision. rte_drand() is likewise not cryptographically secure, so this is not a security improvement.
The real defect at those lines is a two-world-rule violation: glibc rand() takes a process-global lock on every call, so with the flag armed each received packet serializes the RX polling tasklet against every other rand() caller in the process, including RX tasklets of other sessions on other lcores. rte_drand()/rte_rand_max() use per-lcore LFSR258 state with no lock, and rte_rand_max() is unbiased unlike
% max.Value range is unchanged: burst_loss_max is always >= 1 (attach defaults 0 to 1), so both forms yield [1, burst_loss_max]. No new test accompanies this change because it has no observable behavioral delta; the existing St30p.rx_simulate_pkt_loss integration case covers the path.
Applied to the identical video sibling rv_simulate_pkt_loss() so the same CID cannot reappear there.