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

composefs: Ensure buffer is suitably aligned for struct fsverity_digest #3340

Merged
merged 1 commit into from
Nov 14, 2024

Conversation

smcv
Copy link
Contributor

@smcv smcv commented Nov 14, 2024

struct fsverity_digest starts with a __u16, so it will normally require 16-bit alignment, which is not guaranteed for a char array.

Resolves: #3339


This is untested: sorry, I don't currently have any filesystems with fsverity enabled.

struct fsverity_digest starts with a __u16, so it will normally require
16-bit alignment, which is not guaranteed for a char array.

Resolves: ostreedev#3339
Signed-off-by: Simon McVittie <[email protected]>
@smcv
Copy link
Contributor Author

smcv commented Nov 14, 2024

On closer inspection, I don't have libcomposefs in my build environment either (we don't have it in Debian yet and I don't have enough mental bandwidth to be responsible for it) so I'm doing this completely blind. (Thanks, CI!)

@smcv smcv marked this pull request as draft November 14, 2024 13:57
@smcv smcv marked this pull request as ready for review November 14, 2024 18:38
@cgwalters
Copy link
Member

Thanks for this! What I'd hoped to do is land containers/composefs#394 and then we can switch to it.

Now the point that arises here is that the libcomposefs code is doing the same thing.

Of course fsverity-utils has a different copy of this code; it seems to heap allocate https://github.com/ebiggers/fsverity-utils/blob/2543e6e5037c0b1b448282c2793f8a6ce8b12d71/programs/cmd_measure.c#L47
which I think means it's always aligned properly.

Hmm anyways I think you're right. So I will also update the composefs code.

@cgwalters
Copy link
Member

This is untested: sorry, I don't currently have any filesystems with fsverity enabled.

It's pretty easy to make a temporary loopback, no? Not to mention temporary VMs.

@cgwalters cgwalters merged commit 111a45f into ostreedev:main Nov 14, 2024
26 checks passed
@cgwalters
Copy link
Member

@cgwalters
Copy link
Member

Yeah, that one is good because we're actually using a structure with a u16 at the start.

@smcv
Copy link
Contributor Author

smcv commented Nov 15, 2024

it seems to heap allocate

As per Standard C, that should be sufficiently well-aligned for any built-in type, so that's fine. It's stack and static allocations that are the usual problem with alignment.

It's pretty easy to make a temporary loopback, no? Not to mention temporary VMs.

I'm sure that's pretty easy if you have an immutable/image-based OS and a basic knowledge of how fsverity works; but, sorry, I have neither of those things, and my time is already spread more thinly than I can really cope with, so there's a pretty low limit to how much "can't you just" I am able to add here.

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.

FS_IOC_MEASURE_VERITY ioctl can read into an unaligned buffer
2 participants