On Mon, Jan 23, 2023 at 5:09 PM Ivan Khoronzhuk <ivan.khoronz...@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.khoronz...@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