Fix dangling pointer issue #344
Conversation
|
Is it this that I have been seeing in my valgrind reports but for my life not been able to figure out what it was? |
|
Possibly! I do wonder if there are other cases like this, I'll have a skim through the commands. |
|
Ok, I've checked, seems to be the only place where shady stuff was going on (that I can tell). |
|
It was because we passed that pointer into a function in unsafe block that rust did not catch the error. |
|
That was it! I just ran valgrind on using code that temporary stored the converted variable inside the function scope. And all errors were gone. We should really have some kind of guideline saying that 'performing work inside of unsafe blocks should be kept to an absolute minimum'. I know I have done a lot of 'fancy' code inside those unsafe blocks. We should probably try to move as much code as possible out of the unsafe calls. I wonder if there is some way to do that without creating to much bloat... |
This fixes an issue with the handling of the nonce field when opening authentication sessions. The ESYS layer expects either a pointer to a valid nonce of at least 16 bytes, or NULL if no nonce is provided. Our handling, however, passed down an invalid pointer that was referencing a now-defunct structure. What happened, both before parallaxsecond#340 and after, was that the `Nonce` input was moved into a separate scope, either a `match` or a lambda function, then converted to `TPM2B_NONCE`. A reference to this `TPM2B_NONCE` was taken and converted to `*const TPM2B_NONCE`, which was passed outside of the scope. The pointer, therefore, ended up referencing a structure that was dropped at the end of that inner scope. To ensure memory safety, we need to keep ownership of the `TPM2B_NONCE` while the call is being made. Signed-off-by: Ionut Mihalcea <ionut.mihalcea@arm.com>
ionut-arm
force-pushed
the
dangling-pointer
branch
2 times, most recently
from
807505d
to
7e7aaac
Compare
Signed-off-by: Ionut Mihalcea <ionut.mihalcea@arm.com>
ionut-arm
force-pushed
the
dangling-pointer
branch
from
7e7aaac
to
a75043b
Compare
|
Ok, I've added a workflow running valgrind over the test suite, which seems to work fine on Ubuntu (was failing due to some leaks lower down the stack on Fedora, oops 😬 ). |
Port the security fix related to the nonce dangling pointer during auth session creation, and fix the other issues flagged by the compiler. Signed-off-by: Ionut Mihalcea <ionut.mihalcea@arm.com>
This fixes an issue with the handling of the nonce field when opening
authentication sessions. The ESYS layer expects either a pointer to a
valid nonce of at least 16 bytes, or NULL if no nonce is provided. Our
handling, however, passed down an invalid pointer that was referencing a
now-defunct structure.
What happened, both before #340 and after, was that the
Nonceinputwas moved into a separate scope, either a
matchor a lambda function,then converted to
TPM2B_NONCE. A reference to thisTPM2B_NONCEwastaken and converted to
*const TPM2B_NONCE, which was passed outside ofthe scope. The pointer, therefore, ended up referencing a structure that
was dropped at the end of that inner scope.
To ensure memory safety, we need to keep ownership of the
TPM2B_NONCEwhile the call is being made.