Message ID | 20190405015258.6780-3-xypron.glpk@gmx.de |
---|---|
State | Accepted, archived |
Delegated to: | Heinrich Schuchardt |
Headers | show |
Series | efi_loader: parameter checks in StartImage and Exit() | expand |
On Fri, Apr 05, 2019 at 03:52:58AM +0200, Heinrich Schuchardt wrote: > Add parameter checks in the StartImage() and Exit() boottime services: > - check that the image handle is valid and has the loaded image protocol > installed > - in StartImage() record the current image > - in Exit() check that the image is the current image > > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > --- > v2 > avoid `parent_image` may be used uninitialized > --- > lib/efi_loader/efi_boottime.c | 27 +++++++++++++++++++++++++++ > 1 file changed, 27 insertions(+) > > diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c > index 6aac8391c5..970c01614e 100644 > --- a/lib/efi_loader/efi_boottime.c > +++ b/lib/efi_loader/efi_boottime.c > @@ -26,6 +26,9 @@ LIST_HEAD(efi_obj_list); > /* List of all events */ > LIST_HEAD(efi_events); > > +/* Handle of the currently executing image */ > +static efi_handle_t current_image; > + > /* > * If we're running on nasty systems (32bit ARM booting into non-EFI Linux) > * we need to do trickery with caches. Since we don't want to break the EFI > @@ -2631,9 +2634,18 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle, > struct efi_loaded_image_obj *image_obj = > (struct efi_loaded_image_obj *)image_handle; > efi_status_t ret; > + void *info; > + efi_handle_t parent_image = current_image; > > EFI_ENTRY("%p, %p, %p", image_handle, exit_data_size, exit_data); > > + /* Check parameters */ > + ret = EFI_CALL(efi_open_protocol(image_handle, &efi_guid_loaded_image, > + &info, NULL, NULL, > + EFI_OPEN_PROTOCOL_GET_PROTOCOL)); > + if (ret != EFI_SUCCESS) > + return EFI_EXIT(EFI_INVALID_PARAMETER); > + > efi_is_direct_boot = false; > > /* call the image! */ > @@ -2662,9 +2674,11 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle, > __efi_nesting_dec(), > (unsigned long)((uintptr_t)image_obj->exit_status & > ~EFI_ERROR_MASK)); > + current_image = parent_image; > return EFI_EXIT(image_obj->exit_status); > } > > + current_image = image_handle; > ret = EFI_CALL(image_obj->entry(image_handle, &systab)); > > /* > @@ -2699,12 +2713,23 @@ static efi_status_t EFIAPI efi_exit(efi_handle_t image_handle, > * TODO: We should call the unload procedure of the loaded > * image protocol. > */ > + efi_status_t ret; > + void *info; > struct efi_loaded_image_obj *image_obj = > (struct efi_loaded_image_obj *)image_handle; > > EFI_ENTRY("%p, %ld, %zu, %p", image_handle, exit_status, > exit_data_size, exit_data); > > + /* Check parameters */ > + if (image_handle != current_image) > + goto out; > + ret = EFI_CALL(efi_open_protocol(image_handle, &efi_guid_loaded_image, > + &info, NULL, NULL, > + EFI_OPEN_PROTOCOL_GET_PROTOCOL)); Why do we need this protocol check again? -Takahiro Akashi > + if (ret != EFI_SUCCESS) > + goto out; > + > /* Make sure entry/exit counts for EFI world cross-overs match */ > EFI_EXIT(exit_status); > > @@ -2718,6 +2743,8 @@ static efi_status_t EFIAPI efi_exit(efi_handle_t image_handle, > longjmp(&image_obj->exit_jmp, 1); > > panic("EFI application exited"); > +out: > + return EFI_EXIT(EFI_INVALID_PARAMETER); > } > > /** > -- > 2.20.1 >
On Fri, Apr 05, 2019 at 03:52:58AM +0200, Heinrich Schuchardt wrote: > Add parameter checks in the StartImage() and Exit() boottime services: > - check that the image handle is valid and has the loaded image protocol > installed > - in StartImage() record the current image > - in Exit() check that the image is the current image Does this check logic work for a case of nested calls of StartImage() at all? -Takahiro Akashi > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > --- > v2 > avoid `parent_image` may be used uninitialized > --- > lib/efi_loader/efi_boottime.c | 27 +++++++++++++++++++++++++++ > 1 file changed, 27 insertions(+) > > diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c > index 6aac8391c5..970c01614e 100644 > --- a/lib/efi_loader/efi_boottime.c > +++ b/lib/efi_loader/efi_boottime.c > @@ -26,6 +26,9 @@ LIST_HEAD(efi_obj_list); > /* List of all events */ > LIST_HEAD(efi_events); > > +/* Handle of the currently executing image */ > +static efi_handle_t current_image; > + > /* > * If we're running on nasty systems (32bit ARM booting into non-EFI Linux) > * we need to do trickery with caches. Since we don't want to break the EFI > @@ -2631,9 +2634,18 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle, > struct efi_loaded_image_obj *image_obj = > (struct efi_loaded_image_obj *)image_handle; > efi_status_t ret; > + void *info; > + efi_handle_t parent_image = current_image; > > EFI_ENTRY("%p, %p, %p", image_handle, exit_data_size, exit_data); > > + /* Check parameters */ > + ret = EFI_CALL(efi_open_protocol(image_handle, &efi_guid_loaded_image, > + &info, NULL, NULL, > + EFI_OPEN_PROTOCOL_GET_PROTOCOL)); > + if (ret != EFI_SUCCESS) > + return EFI_EXIT(EFI_INVALID_PARAMETER); > + > efi_is_direct_boot = false; > > /* call the image! */ > @@ -2662,9 +2674,11 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle, > __efi_nesting_dec(), > (unsigned long)((uintptr_t)image_obj->exit_status & > ~EFI_ERROR_MASK)); > + current_image = parent_image; > return EFI_EXIT(image_obj->exit_status); > } > > + current_image = image_handle; > ret = EFI_CALL(image_obj->entry(image_handle, &systab)); > > /* > @@ -2699,12 +2713,23 @@ static efi_status_t EFIAPI efi_exit(efi_handle_t image_handle, > * TODO: We should call the unload procedure of the loaded > * image protocol. > */ > + efi_status_t ret; > + void *info; > struct efi_loaded_image_obj *image_obj = > (struct efi_loaded_image_obj *)image_handle; > > EFI_ENTRY("%p, %ld, %zu, %p", image_handle, exit_status, > exit_data_size, exit_data); > > + /* Check parameters */ > + if (image_handle != current_image) > + goto out; > + ret = EFI_CALL(efi_open_protocol(image_handle, &efi_guid_loaded_image, > + &info, NULL, NULL, > + EFI_OPEN_PROTOCOL_GET_PROTOCOL)); > + if (ret != EFI_SUCCESS) > + goto out; > + > /* Make sure entry/exit counts for EFI world cross-overs match */ > EFI_EXIT(exit_status); > > @@ -2718,6 +2743,8 @@ static efi_status_t EFIAPI efi_exit(efi_handle_t image_handle, > longjmp(&image_obj->exit_jmp, 1); > > panic("EFI application exited"); > +out: > + return EFI_EXIT(EFI_INVALID_PARAMETER); > } > > /** > -- > 2.20.1 >
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 6aac8391c5..970c01614e 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -26,6 +26,9 @@ LIST_HEAD(efi_obj_list); /* List of all events */ LIST_HEAD(efi_events); +/* Handle of the currently executing image */ +static efi_handle_t current_image; + /* * If we're running on nasty systems (32bit ARM booting into non-EFI Linux) * we need to do trickery with caches. Since we don't want to break the EFI @@ -2631,9 +2634,18 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle, struct efi_loaded_image_obj *image_obj = (struct efi_loaded_image_obj *)image_handle; efi_status_t ret; + void *info; + efi_handle_t parent_image = current_image; EFI_ENTRY("%p, %p, %p", image_handle, exit_data_size, exit_data); + /* Check parameters */ + ret = EFI_CALL(efi_open_protocol(image_handle, &efi_guid_loaded_image, + &info, NULL, NULL, + EFI_OPEN_PROTOCOL_GET_PROTOCOL)); + if (ret != EFI_SUCCESS) + return EFI_EXIT(EFI_INVALID_PARAMETER); + efi_is_direct_boot = false; /* call the image! */ @@ -2662,9 +2674,11 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle, __efi_nesting_dec(), (unsigned long)((uintptr_t)image_obj->exit_status & ~EFI_ERROR_MASK)); + current_image = parent_image; return EFI_EXIT(image_obj->exit_status); } + current_image = image_handle; ret = EFI_CALL(image_obj->entry(image_handle, &systab)); /* @@ -2699,12 +2713,23 @@ static efi_status_t EFIAPI efi_exit(efi_handle_t image_handle, * TODO: We should call the unload procedure of the loaded * image protocol. */ + efi_status_t ret; + void *info; struct efi_loaded_image_obj *image_obj = (struct efi_loaded_image_obj *)image_handle; EFI_ENTRY("%p, %ld, %zu, %p", image_handle, exit_status, exit_data_size, exit_data); + /* Check parameters */ + if (image_handle != current_image) + goto out; + ret = EFI_CALL(efi_open_protocol(image_handle, &efi_guid_loaded_image, + &info, NULL, NULL, + EFI_OPEN_PROTOCOL_GET_PROTOCOL)); + if (ret != EFI_SUCCESS) + goto out; + /* Make sure entry/exit counts for EFI world cross-overs match */ EFI_EXIT(exit_status); @@ -2718,6 +2743,8 @@ static efi_status_t EFIAPI efi_exit(efi_handle_t image_handle, longjmp(&image_obj->exit_jmp, 1); panic("EFI application exited"); +out: + return EFI_EXIT(EFI_INVALID_PARAMETER); } /**
Add parameter checks in the StartImage() and Exit() boottime services: - check that the image handle is valid and has the loaded image protocol installed - in StartImage() record the current image - in Exit() check that the image is the current image Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> --- v2 avoid `parent_image` may be used uninitialized --- lib/efi_loader/efi_boottime.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) -- 2.20.1