On Mon, Jan 23, 2023 at 5:09 PM Ivan Khoronzhuk ivan.khoronzhuk@gmail.com wrote:
On Mon, Jan 23, 2023 at 04:34:33PM +0100, Jens Wiklander wrote:
On Mon, Jan 23, 2023 at 04:51:29PM +0200, Ivan Khoronzhuk wrote:
The arg->session is not valid if arg->ret != NULL, so can't be assigned. Leave retry for just "ret" error to save same behaviour.
Signed-off-by: Ivan Khoronzhuk ivan.khoronzhuk@gmail.com
common/avb_verify.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/common/avb_verify.c b/common/avb_verify.c index 0520a71455..97451592f5 100644 --- a/common/avb_verify.c +++ b/common/avb_verify.c @@ -619,10 +619,14 @@ static int get_open_session(struct AvbOpsData *ops_data) memset(&arg, 0, sizeof(arg)); tee_optee_ta_uuid_to_octets(arg.uuid, &uuid); rc = tee_open_session(tee, &arg, 0, NULL);
if (!rc) {ops_data->tee = tee;ops_data->session = arg.session;}
if (rc)continue;if (arg.ret)return -EIO;ops_data->tee = tee;ops_data->session = arg.session;}
return 0;
It looks like this function is still slightly broken. The function should, if I understand it correctly, return usable tee and session pointers on success, else return an error code. The unconditional return 0 at the end doesn't seem right.
Thanks, Jens
It doesn't return, it loops infinitely... Yes, it looks so, but it's how it works. I don't see a reason why the function must loop trying to open the session that potentially never will be opened. But this is how it's implemented and I didn't wont to change this behaviour that can have some "sacral" roots, only add a fix I bother, I've
No worries, no need to be bug compatible here. :-)
mentioned it in the comment. Better would be drop this loop ofc and add the following:
if (ret || arg.ret)return -EIO;
A loop is needed to find the TEE device. Right now I guess there's only one and it will be found in the first round.
I can do this in v3 if you don't mind.
Yes, please fix the function to return an error if a session can't be found.
Thanks, Jens