Dns rust keywords 7529 v4 #12605
Dns rust keywords 7529 v4 #12605
Conversation
Ticket: 7529 Ticket: 3725 Adds url for dns.opcode on the way Move dns.query unit tests to SV on the way
Ticket: 6723
Ticket: 6723
So that its contents can be reused when translating unit tests to SV tests
catenacyber
requested review from
jasonish,
jufajardini and
victorjulien
as code owners
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #12605 +/- ##
========================================
Coverage 80.74% 80.75%
========================================
Files 931 925 -6
Lines 259144 258691 -453
========================================
- Hits 209242 208894 -348
+ Misses 49902 49797 -105
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
Information: QA ran without warnings. Pipeline 24762 |
|
Where is |
Please give me a better name :-) It comes from your comment #12535 (comment)
|
| name: static_cstring_from!("dns.answer.name"), | ||
| desc: static_cstring_from!("DNS answer name sticky buffer"), | ||
| url: static_cstring_from!("/rules/dns-keywords.html#dns-answer-name"), |
There was a problem hiding this comment.
I don't really think this is what I had in mind. The Rust developer still has to concern themselves that this is a C struct, which is what I'd like to abstract away from.
There was a problem hiding this comment.
Thanks for the feedback.
I did not manage to have a rust string "dns.answer.name", because then I need to allocate "dns.answer.name\0" and I leak memory cf fixup commit and red CI before that...
Do you see another way ?
I also tried Cstring::from(c"dns.answer.name") but it looks like a big project because it is breaking other tools like rustfmt which ignores Cargo.toml showing this is edition 2021
There was a problem hiding this comment.
The real problem is that our module system of registering keywords is "unsafe" (in the rust sense). We pass it a pointer to a C string and it requires that C string to remain rather than taking ownership, or a copy of it.
I don't have any examples doing this in Suricata, but I've created a "shadow" registry before. Basically a static mutable hasmap that maps Rust strings to C strings so they can live until the program dies. Not ideal but might be easier than modifying the keyword registration to take ownership of passed in strings?
There was a problem hiding this comment.
The real problem is that our module system of registering keywords is "unsafe" (in the rust sense). We pass it a pointer to a C string and it requires that C string to remain rather than taking ownership, or a copy of it.
Indeed, the C registering system expects only static strings that it does not free
How do you free your "shadow" registry ?
What do you think of c"dns.answer.name" ?
There was a problem hiding this comment.
How do you free your "shadow" registry ?
I don't. Because its held onto until program exit, it is not reported as a leak. It was never orphaned. Rust actually has Box::leak which is a common way to forget about pointer that is passed to C that will live til program exit, but ASAN reports that as a leak.
There was a problem hiding this comment.
What do you think of
c"dns.answer.name"?
Need Rust 1.77. But I like it in cases where you'd never want to pass a formatted string at least.
There was a problem hiding this comment.
Will try this shadow registry approach then...
Link to ticket: https://redmine.openinfosecfoundation.org/issues/
https://redmine.openinfosecfoundation.org/issues/7529
https://redmine.openinfosecfoundation.org/issues/3725
Describe changes:
UTHBuildPacketRealto produce more real packets#11932 next protocol
#12535 with new commits to
SV_BRANCH=OISF/suricata-verify#2297