On Sat, 20 Apr 2024 at 17:31, Michael Brown <mc...@ipxe.org> wrote: > > On 19/04/2024 11:02, Mike Beaton wrote: > > Dear Michael, > > > > I don't know if you had time to answer one follow-up question. > > > > Obviously one thing that someone might want to do is to notify on > > protocol installs and trap installs of this protocol - e.g. so that > > something other than UefiBootManagerLib can manage and monitor HTTP > > boot, but still allowing the original callback to occur, by hooking it. > > Not sure if this counts as 'supported' or not (possibly not...) though I > > think it may count as 'quite likely to happen'. However, one could hook > > in such a way that the uninstall would succeed anyway, assuming that the > > function pointer within the original installed protocol is writeable. > > > > My question is: was the above is roughly what you were thinking of, that > > might cause the assert to fail, or, if not, if you had the time to give > > a very brief sketch of what else it might be (just a plausible, very > > rough example)? Certainly not saying you're wrong, just that it would be > > helpful (to me!) to understand what sort of thing you were thinking of! > > I don't have a specific use case in mind for why someone might want to > have opened this particular protocol in a way that would subsequently > cause UninstallMultipleProtocolInterfaces() to fail (e.g. opening with > BY_CHILD_CONTROLLER attributes). Just that, as a general rule, there > exists a design flaw in the UEFI specification that means that > operations that should have been chosen at the design stage to be > conceptually impossible to fail (such as freeing memory or uninstalling > protocols) are instead allowed to return a failure status. > > This design issue manifests itself as extremely unreliable behaviour on > the removal or shutdown paths of many UEFI drivers. For example: many > drivers will simply deadlock the system if disconnected from their > underlying controllers (e.g. via the UEFI shell "disconnect" command). > > In the case of UninstallMultipleProtocolInterfaces(), the failure mode > is particularly problematic since the specification dictates that the > firmware must do the absolutely worst thing possible by *reinstalling* > any protocol instances that it had managed to uninstall, and > consequently retriggering driver Start() method calls. This generally > leads to chaos and confusion (and use-after-free bugs that could > probably be fairly easily extended to obtain a Secure Boot exploit). > > There's nothing that you really need to do specifically in HttpBootDxe > to work around this design flaw. But it's definitely worth removing the > unjustified ASSERT(), since that ASSERT() may cause a crash in a system > that could otherwise continue to operate successfully. > > Hope that helps, > > Michael >
It does help. Thank you for a useful and clear explanation - I was already aware of some (but certainly not all) of it. I have already posted a revised patch with the ASSERT removed - but am now more clear that I really had better stick with that, not try to argue against it. ;) Thanks again, Mike -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#118052): https://edk2.groups.io/g/devel/message/118052 Mute This Topic: https://groups.io/mt/105368366/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
