Skip to content

Conversation

@Jakuje
Copy link
Collaborator

@Jakuje Jakuje commented Nov 20, 2025

As returned by SoftHSM: softhsm/SoftHSMv2#825

Fixes: #323

@Jakuje Jakuje force-pushed the allowed-mechanism-attribute branch from a2ac816 to 32243e0 Compare November 20, 2025 11:19
@Jakuje
Copy link
Collaborator Author

Jakuje commented Nov 20, 2025

OTOH this could be considered a softhsm bug (or at least oddness) and as a rust-cryptoki, we might want to shield a users from such pkcs11-module-specific issues. But then we would have to filter this out early in get_attributes, which does not sound great either.

@Jakuje Jakuje requested review from hug-dev and wiktor-k November 20, 2025 11:23
@nwalfield
Copy link
Contributor

nwalfield commented Nov 20, 2025

You already pointed out that at least two attributes can definitely be zero length. It thus seems like a bug in rust-cryptoki that it tries to allocate 0 bytes and expects to get a pointer that can be cast to a raw pointer and passed to std::slice::from_raw_parts:

Behavior is undefined if any of the following conditions are violated:

    data must be non-null, valid for reads for len * size_of::<T>() many bytes, and it must be properly aligned

@wiktor-k
Copy link
Collaborator

You already pointed out that at least two attributes can definitely be zero length. It thus seems like a bug in rust-cryptoki that it tries to allocate 0 bytes and expects to get a pointer that can be cast to a raw pointer and passed to std::slice::from_raw_parts:

Behavior is undefined if any of the following conditions are violated:

    data must be non-null, valid for reads for len * size_of::<T>() many bytes, and it must be properly aligned

Validity rules for reads explicitly point out that:

For memory accesses of size zero, every pointer is valid, including the null pointer.

I'm okay with the change here, with a bit of docs around it, since at first sight it looks like a premature optimization 🤔

@nwalfield
Copy link
Contributor

You already pointed out that at least two attributes can definitely be zero length. It thus seems like a bug in rust-cryptoki that it tries to allocate 0 bytes and expects to get a pointer that can be cast to a raw pointer and passed to std::slice::from_raw_parts:

Behavior is undefined if any of the following conditions are violated:

    data must be non-null, valid for reads for len * size_of::<T>() many bytes, and it must be properly aligned

Validity rules for reads explicitly point out that:

For memory accesses of size zero, every pointer is valid, including the null pointer.

I'm okay with the change here, with a bit of docs around it, since at first sight it looks like a premature optimization 🤔

That's not the only relevant thing. From slice::from_raw_parts:

data must be non-null and aligned even for zero-length slices or slices of ZSTs.

And that's the bug that we are seeing.

@Jakuje Jakuje force-pushed the allowed-mechanism-attribute branch from 32243e0 to 84339fb Compare November 20, 2025 14:13
Copy link

@neverpanic neverpanic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo, rest lg2m.

As returned by SoftHSM: softhsm/SoftHSMv2#825

Fixes: parallaxsecond#323

Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
@Jakuje Jakuje force-pushed the allowed-mechanism-attribute branch from 84339fb to 0204b08 Compare November 20, 2025 14:28
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.

Undefined behavior in CK_ATTRIBUTE::try_from or Session::get_attributes

4 participants