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.
_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel