Update eventlog.rs #770
Update eventlog.rs #770
Conversation
|
Can you add a bit more info in the commit message and do you have a link to this slack thread so I can check it out? Let's let the linter run, but looks like you may have added some extra spaces. |
gcoon151
force-pushed
the
patch-1
branch
2 times, most recently
from
72238f1 to
09cb71e
Compare
I think I fixed both the commit message and spacing. |
During testing of a TDX KBS -> Trustee conversation eventlog.rs panicked. This workaround simply discards the events that aren't the size we expect. Signed-off-by: Gerald Coon <[email protected]>
|
Thanks for the fix. Could you share some test cases or the ccel binary file you met the error? cc @mythi |
Frank put a lot of the details including the event entry that caused it to panic in the CNCF #confidential-containers-peerpod slack channel here https://cloud-native.slack.com/archives/C04A2EJ70BX/p1743708184751959?thread_ts=1743444129.613059&cid=C04A2EJ70BX (the relevant part starts "There are several "EV_EFI...") This was running the peer pod on an IBM cloud TDX VSI for purpose of testing/developing the cloud-api-adaptor kata-remote/peer pod talking to a compose version of the trustee. |
|
Relevant comments from Mikko that I think answer what you're looking for, "Might be a bug in my code that parses CCEL with OVMF" ... "az-vtpm-tdx does not include the CC eventlog" |
|
We recognize this is a partial fix also for the specific thing that IBM Cloud TDX VSIs do. I suspect a better fix would be to do more comprehensive input parsing to avoid DDoS. One thing at a time though. |
There was a problem hiding this comment.
Looks fine. Note that the next line is trying to index into the buffer, so this is really just a bounds check. Possibly would be better to move onto its own line and log an error if we have this type of event but it is short. It sounds like there is some root cause still to be understood, but this seems reasonable. I would like to hear from @mythi tho
|
I was working on a better fix for this. Let me get that PR asap |
Ignore invalid events that can cause problems (as discussed on slack)