af-packet: handle null bpf filter - v1

[jasonish]

Commit 587c326 made our YAML parser
more cmopliant with respect to what should be a NULL value and now when
a key contains no value it is NULL instead of an empty string, or
something else like a literal "null" or "~".

Check that the returned bpf_filter is null before checking strlen.

Bug: https://redmine.openinfosecfoundation.org/issues/6302

[jasonish]

I guess we should re-open the discussion on ConfGetChildValueWithDefault and when it should return failiure. While I don't like it, our usage patterns suggests that this should maybe return failure when the fields exists but contains a null value. There are other ways to check if a value exists and is none if needed. Thoughts?

[codecov]

Codecov Report

Merging #9468 (ddd5262) into master (2b57179) will decrease coverage by 0.02%.
The diff coverage is 0.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #9468      +/-   ##
==========================================
- Coverage   82.18%   82.17%   -0.02%     
==========================================
  Files         968      968              
  Lines      274199   274199              
==========================================
- Hits       225363   225329      -34     
- Misses      48836    48870      +34     

Flag	Coverage Δ
fuzzcorpus	64.09% <0.00%> (ø)
suricata-verify	60.86% <0.00%> (-0.05%)	⬇️
unittests	62.88% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

📢 Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OISF).

[suricata-qa]

Information: QA ran without warnings.

Pipeline 15875

[inashivb]

I guess we should re-open the discussion on ConfGetChildValueWithDefault and when it should return failiure. While I don't like it, our usage patterns suggests that this should maybe return failure when the fields exists but contains a null value. There are other ways to check if a value exists and is none if needed. Thoughts?

I was thinking perhaps we could have a cocci script check whether we're checking the NULL values returned in such fn calls and if not, replace the call to ConfGetChildValueWithDefault with the fn ConfGetChildValueString ref: 6761e0a
Even if the value is not supposed to be string in certain cases, this will at least bring out all the unhandled parts of the code that we should manually inspect once. Wdyt? I had created a ticket for this a while back: https://redmine.openinfosecfoundation.org/issues/6249

[victorjulien]

I guess we should re-open the discussion on ConfGetChildValueWithDefault and when it should return failiure. While I don't like it, our usage patterns suggests that this should maybe return failure when the fields exists but contains a null value. There are other ways to check if a value exists and is none if needed. Thoughts?

I was thinking perhaps we could have a cocci script check whether we're checking the NULL values returned in such fn calls and if not, replace the call to ConfGetChildValueWithDefault with the fn ConfGetChildValueString ref: 6761e0a Even if the value is not supposed to be string in certain cases, this will at least bring out all the unhandled parts of the code that we should manually inspect once. Wdyt? I had created a ticket for this a while back: https://redmine.openinfosecfoundation.org/issues/6249

The number of call sites seems limited, so a manual review/update pass is more appropriate I think.

[jasonish]

A more general solution that matches our usage: #9476