lib: unpack size_t values safely

[verma-divyanshu-git]

Summary

• add a typed size_t unpack helper while preserving the uint64_t wire format
• reject packed values above SIZE_MAX and update the affected unpack call sites
• add focused unit coverage for normal and overflowing values

Fixes #548

[meta-cla]

Hi @verma-divyanshu-git!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

[qdeslandes]

Hi, could you please disclose if/how much AI has been used for this work? Thank you.

[verma-divyanshu-git]

Hi @qdeslandes , I used AI to help with the initial understanding & investigation (searching the relevant unpack call sites etc) and used it to draft an initial version of the patch.

I reviewed and edited the diff, reproduced the incompatible-pointer-type issue from the reported 32-bit build failure, verified the size_t to uint64_t conversion and overflow behavior across the affected unpack sites, and I own the final submission.

[qdeslandes]

Hi @qdeslandes , I used AI to help with the initial understanding & investigation (searching the relevant unpack call sites etc) and used it to draft an initial version of the patch.

I reviewed and edited the diff, reproduced the incompatible-pointer-type issue from the reported 32-bit build failure, verified the size_t to uint64_t conversion and overflow behavior across the affected unpack sites, and I own the final submission.

Thank you for this answer.

Did you build and run the code? Did you run the tests? Did you sign the CLA?

[verma-divyanshu-git]

Hi @qdeslandes , I used AI to help with the initial understanding & investigation (searching the relevant unpack call sites etc) and used it to draft an initial version of the patch.
I reviewed and edited the diff, reproduced the incompatible-pointer-type issue from the reported 32-bit build failure, verified the size_t to uint64_t conversion and overflow behavior across the affected unpack sites, and I own the final submission.

Thank you for this answer.

Did you build and run the code? Did you run the tests? Did you sign the CLA?

Yes. I built and tested the updated branch in a fresh privileged Fedora 44 environment.

Results:

build completed successfully
unit tests: 23/23 passed
e2e tests: 93/93 passed
integration tests: 2/2 passed
formatting/static checks: 4/4 passed
coverage target completed successfully

The CLA is signed as well.

[verma-divyanshu-git]

@qdeslandes The CLA check is green and the branch is up to date with main.

When you have a chance, could you please review the patch?

@sudipm-mukherjee, if convenient, could you confirm that the current branch resolves the i386 package build reported in #548?

[verma-divyanshu-git]

hi @qdeslandes, could you please take another look when you have a chance? the cla is green, the branch is mergeable, and all local checks passed; if everything looks good, could you please merge it? thank you.

[sudipm-mukherjee]

@sudipm-mukherjee, if convenient, could you confirm that the current branch resolves the i386 package build reported in #548?

sorry was busy with $dayjob. I will try to check it tonight.

[qdeslandes]

LGTM, except for a couple of nits:

• This PR could be a single commit (72 chars wide)
• Remove the merge commit

[qdeslandes]

assert_err would be better, so we ensure the call fails, but we don't freeze a specific errno value.

[qdeslandes]

@claude review

[github-actions]

Claude review of PR #569 (b7740f0)

Suggestions

• Guard tautological SIZE_MAX comparison — src/libbpfilter/pack.c:546 — On 64-bit, _value > SIZE_MAX is always false and may trigger -Wtype-limits; wrap with #if SIZE_MAX < UINT64_MAX like the test already does

Nits

(None beyond what the maintainer already flagged: use assert_err in the overflow test, squash to single commit)

CLAUDE.md improvements

(None identified)

Workflow run

[github-actions]

Claude: suggestion: On 64-bit platforms where SIZE_MAX == UINT64_MAX, this comparison is tautologically false — a uint64_t can never exceed UINT64_MAX. With -Wall -Wextra (enabled in CMakeLists.txt:63), GCC may emit a -Wtype-limits warning.

The test file already uses #if SIZE_MAX < UINT64_MAX to guard the overflow test. Wrapping this check with the same guard keeps them consistent and eliminates dead code on 64-bit:

#if SIZE_MAX < UINT64_MAX
    if (_value > SIZE_MAX)
        return -EOVERFLOW;
#endif