On Fri, Apr 16, 2021 at 05:58:15PM +0200, Christian Brauner wrote: > They could probably refactor this but I'm not sure why they'd bother. If > they fail processing any of those files they end up aborting the > whole transaction. > (And the original code didn't check the error code btw.) Wait a sec... What does aborting the transaction do to descriptor table? <rereads> Oh, lovely... binder_apply_fd_fixups() is deeply misguided. What it should do is * go through t->fd_fixups, reserving descriptor numbers and putting them into t->buffer (and I'd probably duplicate them into struct binder_txn_fd_fixup). Cleanup in case of failure: go through the list, releasing the descriptors we'd already reserved, doing fput() on fixup->file in all entries and freeing the entries as we go. * On success, go through the list, doing fd_install() and freeing the entries. That's it. No rereading from the buffer, no binder_deferred_fd_close() crap, etc. Again, YOU CAN NOT UNDO fd_install(). Ever. Kernel can not decide it shouldn't have put something in descriptor table and take it back. You can't unring that bell.
