-
Notifications
You must be signed in to change notification settings - Fork 44
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
Add certify example #521
base: main
Are you sure you want to change the base?
Add certify example #521
Conversation
Okay, I've updated this with many thanks to @ionut-arm for teaching me about how this works. It seems to be working BUT I think this is highlighting a bug in this library. I think we aren't cleaning up sessions properly. This example fails with:
Thing is, if we look at those lines: We clear sessions just before, then we run out of session memory. I have recently been noticing a lot of issues with x0901 errors in the kernel due to session memory, and so I am led to think we have a bug somewhere in how we flush and handle sessions. |
Indeed, you seem to be calling Given that |
You could give the AuthSession an inner Arc that has Drop instead? But yes alternately we need flush_sessions or similar. :) |
The problem with that is the cascade effects to the interface: since you're flushing the session whenever the object gets destoyed, you can't have it implement If you'd like to do this please go ahead! My suggestion was primarily based on expected change size. :) |
Well you can have Clone because you can use Arc/Rc. Probably Rc in this case, because I think these are not really thread safe objects at all.
Well, changing to have an inner that handles things with an Rc is fine, but the question here is do we actually have the apis exposed to flush a session so we can write the drop handler? |
Sorry, I should've been clearer! What I meant about
Of course, here's an example. |
To help explain I'm suggesting we have:
That way the outher AuthSession is still copy/clone, but then when it's dropped we can still auto-flush. |
Ah, I see! Yes, that'll work, though There might be some other issues that I'm not foreseeing, but hopefully implementable if you want to give it a go. |
I think the main issue I'd forsee is the whole API currently still relies on a fair bit of manual memory management anyway. So having some types that auto-free and some that don't could be confusing. Anyway, for now I updated the example with a manual drop of the session and it passes! But eventually if you run these in a loop, duplication, duplication secret and certify all end up leaking memory and causing the TPM RM to lockup. I'm not 100% sure why, I can't find what I'm "not freeing". These are the first examples that use Policy Sessions, so could it by that I need to free those also? |
@ionut-arm Anyway, separate to me fixing session handling, I think a pre-lim review of this would be great then I'll get it sorted for merge by squashing and fixing the commit messages. Is that okay/ |
Yes, apologies for the delay. Staying on top of things has been challenging as of late, will add this to my TODO stack, I'll hopefully get to it today or tomorrow! |
This shows how to make an AIK that has been endorsed and is used to certify other objects.