[PATCH] efi_loader: delete handle from events when a protocol is uninstalled

[Ilias Apalodimas]

Hi Henrich


> >
> > +/**
> > + * efi_purge_handle() - Clean the deleted handle from the various lists
> > + * @handle: handle to remove
> > + *
> > + * Return: status code
> > + */
> > +static efi_status_t efi_purge_handle(efi_handle_t handle)
> > +{
> > +     struct efi_register_notify_event *item, *next;
> > +
> > +     if (!list_empty(&handle->protocols))
> > +             return EFI_ACCESS_DENIED;
> > +     /* The handle is about to be freed. Remove it from events */
> > +     list_for_each_entry_safe(item, next, &efi_register_notify_events, link) {
>
> You are not deleting from efi_register_notify_events.

I am not sure I understand this.  I am not trying to delete the event
here, but the handle on which the notified protocol was installed.

>  Why do you use the safe version of the iterator?

No particular reason, I just tend to prefer it over the unsafe one.
The latter is perfectly fine here.

[...]

> >
> > -     /* If the last protocol has been removed, delete the handle. */
> > -     if (list_empty(&handle->protocols)) {
>
> >       size_t i = 0;
> > @@ -2795,25 +2827,25 @@ efi_uninstall_multiple_protocol_interfaces_int(efi_handle_t handle,
> >               return EFI_INVALID_PARAMETER;
> >
> >       efi_va_copy(argptr_copy, argptr);
> > +     protocol = efi_va_arg(argptr, efi_guid_t*);
> >       for (;;) {
> > -             protocol = efi_va_arg(argptr, efi_guid_t*);
> > +             bool preserve = true;
> > +
> >               if (!protocol)
> >                       break;
> >               protocol_interface = efi_va_arg(argptr, void*);
> > +             next_protocol = efi_va_arg(argptr, efi_guid_t*);
> > +             if (!next_protocol)
> > +                     preserve = false;
>
> Is your idea that if 5 protocols are to be removed but only the first 3
> are installed your want to reinstall the first 3?

Not only that. Image you install 2 protocols and then someone tries to
uninstall 3. If the invalid guid/protocol is at the beginning or the
middle of the list you'll be fine.  The current code will reinstall
the previously removed protocols.
However, if the invalid protocol is last, you will have removed 2
valid protocols and the handle will be empty.  In that case, we will
delete the handle before trying to reinstall the uninstalled
protocols.

>
> Can we have some code comment describing what the function should do? In
> 5 years nobody will remember why the code was implemented this way.
>

Of course

[...]

Cheers
/Ilias