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

ostree_sepolicy_host_enabled is _OSTREE_PUBLIC but not listed in .sym file #3182

Closed
smcv opened this issue Feb 19, 2024 · 5 comments · Fixed by #3196
Closed

ostree_sepolicy_host_enabled is _OSTREE_PUBLIC but not listed in .sym file #3182

smcv opened this issue Feb 19, 2024 · 5 comments · Fixed by #3196

Comments

@smcv
Copy link
Contributor

smcv commented Feb 19, 2024

#3151 added ostree_sepolicy_host_enabled() as public API in 2024.3, but it wasn't added to src/libostree/libostree-released.sym by #3172 and therefore is not part of the ABI. I suspect this was not as intended?

cc @cgwalters @mvo5

@mvo5
Copy link
Contributor

mvo5 commented Feb 19, 2024

It looks like I overlooked this, sorry for this! I will wait for Colin to confirm though, my contribution was mostly a drive-by to make testing on non-selinux systems easier (and that worked out great!). But once he confirms I'm happy to do a followup with whatever is needed to make this right.

@smcv
Copy link
Contributor Author

smcv commented Feb 19, 2024

I don't think you necessarily did anything wrong - updating the list of symbols is more of a release-manager thing. I mainly cc'd you because this means the feature you added will not be 100% working in 2024.3, even though it superficially looks as though it should.

cgwalters added a commit to cgwalters/ostree that referenced this issue Feb 23, 2024
As this is only used by internal code, just drop the `_OSTREE_PUBLIC`
marker for now.  If we have a reason to export it we can do that
later.

Closes: ostreedev#3182
@cgwalters
Copy link
Member

@smcv Thanks for reporting this! Out of curiosity how/why did you find this? Is there some tool that noticed the mismatch, or you were just eyeballing commits, or something else?

#3196 will fix this, although if you had some reason to use it in external code, we can make it public.

cgwalters added a commit to cgwalters/ostree that referenced this issue Feb 23, 2024
As this is only used by internal code, just drop the `_OSTREE_PUBLIC`
marker for now.  If we have a reason to export it we can do that
later.

Closes: ostreedev#3182
@smcv
Copy link
Contributor Author

smcv commented Feb 23, 2024

Out of curiosity how/why did you find this?

Debian version-controls a file with a list of the symbols in the public ABI as part of the packaging. This is partly so that we can use them to generate appropriate dependencies (for example flatpak 1.14.5-1 was compiled against a vaguely contemporary libostree, but it turns out that all the symbols it uses were already available in 2020.8, so that's what the dependency says), and partly so that we can raise appropriate red flags if public symbols disappear.

The canonical way to control this file is to do a build, look at the list of new symbols that the build system spits out (conveniently, it's provided in diff format), update the list of symbols, and loop until there are no discrepancies; but that's tedious and involves building each new release at least twice. So, for libraries like libostree with well-maintained ABIs, I shortcut the process by trying to notice new ABI in the source diff (which I'm responsible for at least skim-reading anyway, to check for obvious copyright infringement, malicious changes or other badness) and adding the new symbols to the list ahead of time.

In this case, I noticed that there was a new _OSTREE_PUBLIC symbol but no new verdef in the .syms files.

@smcv
Copy link
Contributor Author

smcv commented Feb 23, 2024

if you had some reason to use it in external code, we can make it public

I have no use-case for this symbol. SELinux is technically something that you can use on Debian, but it's very rare, and I don't know of any particular reason for things outside the ostree package to want to call this function.

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 a pull request may close this issue.

3 participants