mlx5: Misc. fixes - #1791
Merged
Merged
mlx5: Misc. fixes#1791
Conversation
When mlx5_alloc_dbrec() falls back from the custom allocator to the default one, db is not cleared before jumping to default_alloc. If the default allocator also fails, the function returns db still holding a non-NULL garbage value instead of NULL. create_qp() checks the return value for NULL to detect failure. Since it is not NULL, create_qp() proceeds and crashes dereferencing qp->db[MLX5_RCV_DBR]. Clear db before entering the default_alloc path so any failure there returns NULL. Fixes: acee8f6 ("mlx5: Add custom allocation support for DBR") Signed-off-by: Maher Sanalla <msanalla@nvidia.com> Signed-off-by: Yishai Hadas <yishaih@nvidia.com>
The backward sweep in __mlx5_cq_clean() used:
while ((int) --prod_index - (int) cq->cons_index >= 0)
Both operands are uint32_t. Promoting them to int and then subtracting
is undefined behaviour when the result overflows (C11 §6.5p5). GCC and
Clang exploit that UB: they fold "(int)a - (int)b >= 0" into the plain
signed compare "(int)a >= (int)b", which has no exit when cons_index is
0x80000000 (INT_MIN), causing an infinite loop with the CQ spinlock held.
Every other thread polling that CQ blocks; the kernel-side QP is already
destroyed, so the process must be killed to recover. A long-lived process
sharing one CQ across many short-lived QPs hits this every 2^32
completions (~90 s at 24 M CQE/s, ~10 days at 2.4 k CQE/s).
Replace with a plain unsigned equality check:
while (prod_index != cq->cons_index) { --prod_index; ... }
prod_index starts at the value found by the forward scan, which begins
at cons_index and only increments, so prod_index >= cons_index always
holds. Decrementing prod_index each iteration reaches cons_index in
exactly (prod_index - cons_index) steps. No arithmetic on the loop
condition, no signed casts, no compiler-visible UB.
Fixes: 8c4791a ("libmlx5: First version of libmlx5")
Signed-off-by: Yishai Hadas <yishaih@nvidia.com>
Contributor
|
Isn't this fix also needed in the kernel? |
Member
Strictly speaking no, the kernel is compiled with -fno-signed-overflow flag which prevents this issue. Thanks |
Contributor
-fno-strict-overflow that is :-) |
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
4 participants
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.
This series includes two fixes in mlx5 provider as of below.
Extra details exist as part of the commit logs.