Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

apparmor: fix incorrect usage of sizeof on char ptr #2235

Merged
merged 1 commit into from
Aug 3, 2023

Conversation

ancientmodern
Copy link
Contributor

@ancientmodern ancientmodern commented Aug 2, 2023

This makes the apparmor replace file path on riscv64 (#2234) zdtm/static/apparmor_stacking appear as /sys/kernel/security/apparmor/policy/namespaces/criu_stacking_test/.replac, where the final e is missing and leads to an error on open(). Although here sizeof(path) - offset - my_offset should be automatically converted to a quite large unsigned int, and the return value is still 9. Maybe it's due to different glibc implementation? 🤔

As paths are always declared as char path[PATH_MAX] when calling this function, I think it's okay to directly use PATH_MAX rather than introducing a new argument like size?

@ancientmodern ancientmodern mentioned this pull request Aug 2, 2023
In criu/apparmor.c: write_aa_policy(), the arg path is passed as a char
pointer. The original code used sizeof(path) to get the size of it,
which is incorrect as it always return the size of the char pointer
(typically 8 or 4), not the actual capacity of the char array.

Given that this function is only invoked with path declared as `char
path[PATH_MAX]`, replacing sizeof(path) with PATH_MAX should correctly
represent the maximum size of it.

Fixes: 8723e3f ("check: add a feature test for apparmor_stacking")

Signed-off-by: Haorong Lu <[email protected]>
@codecov-commenter
Copy link

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.26% 🎉

Comparison is base (88249fe) 70.38% compared to head (6ec8aa4) 70.65%.

❗ Current head 6ec8aa4 differs from pull request most recent head 1c7af5e. Consider uploading reports for the commit 1c7af5e to get more accurate results

Additional details and impacted files
@@             Coverage Diff              @@
##           criu-dev    #2235      +/-   ##
============================================
+ Coverage     70.38%   70.65%   +0.26%     
============================================
  Files           134      133       -1     
  Lines         34044    33321     -723     
============================================
- Hits          23963    23544     -419     
+ Misses        10081     9777     -304     
Files Changed Coverage Δ
criu/apparmor.c 54.01% <100.00%> (ø)

... and 14 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@avagin avagin merged commit 9118601 into checkpoint-restore:criu-dev Aug 3, 2023
34 of 37 checks passed
@ancientmodern ancientmodern deleted the apparmor-sizeof branch August 4, 2023 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants