Skip to content

Commit

Permalink
fix sequnce number compare and add codeql query for this
Browse files Browse the repository at this point in the history
  • Loading branch information
two-heart authored and ripatel-fd committed Dec 12, 2024
1 parent eef2f7e commit e0e325a
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 1 deletion.
37 changes: 37 additions & 0 deletions contrib/codeql/SeqCmp.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/**
* Finds relational comparisons of sequence numbers that are not using the fd_seq_* functions
* @id asymmetric-research/seq-cmp
* @kind problem
* @severity warning
* @precision low
*/

import cpp

predicate include(Location l) {
l.getFile().getRelativePath().matches("src/")
or not l.getFile().getBaseName().matches("fd_cstr%")
}

class SeqNum extends Variable {
SeqNum() {
this.getName().matches("%seq%") and
include(this.getLocation())
}
}

from SeqNum seqNum1, SeqNum seqNum2, Access a, Access b
where exists(
/* Using == and != is fine because they match the implementation of
fd_seq_eq and fd_seq_ne */
RelationalOperation cmp |
cmp.getAnOperand() = a and
cmp.getAnOperand() = b
) and
a = seqNum1.getAnAccess() and
b = seqNum2.getAnAccess() and
a != b and
include(a.getLocation()) and
include(b.getLocation()) and
a.getTarget().getName() < b.getTarget().getName() /* Avoid duplicate results */
select a, "Use fd_seq_lt, fd_seq_le, fd_seq_ge, fd_seq_gt or equivlanet implementations to compare sequence numbers"
2 changes: 1 addition & 1 deletion src/app/fddev/quic_trace/fd_quic_trace_rx_tile.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ before_frag( void * _ctx FD_FN_UNUSED,
ulong * tgt_fseq = fd_quic_trace_target_fseq[ in_idx ];
for(;;) {
ulong tgt_seq = fd_fseq_query( tgt_fseq );
if( FD_LIKELY( tgt_seq>=seq ) ) break;
if( FD_LIKELY( fd_seq_ge( tgt_seq, seq ) ) ) break;
FD_SPIN_PAUSE();
}

Expand Down

0 comments on commit e0e325a

Please sign in to comment.