On Fri, Dec 14, 2018 at 18:47:42 +0000, Aaron Lindsay wrote:
> On Dec 14 12:50, Emilio G. Cota wrote:
> > On Fri, Dec 14, 2018 at 12:08:22 -0500, Emilio G. Cota wrote:
> > > On Fri, Dec 14, 2018 at 15:57:32 +0000, Aaron Lindsay wrote:
> > (snip)
> > > > I added a function to the user-facing plugin API in my own version of
> > > > Pavel's plugin patchset to clear all existing plugin instrumentation,
> > (snip)
> > > I think the following API call would do what you need:
> > >
> > > typedef int (*qemu_plugin_reset_cb_t)(qemu_plugin_id_t id);
> > > void qemu_plugin_reset(qemu_plugin_id_t id, qemu_plugin_reset_cb_t cb);
> > >
> > > (or maybe qemu_plugin_reinstall?)
> >
> > An alternative is to track the TBs that a plugin has inserted
> > instrumentation into, and only flush those. This would require
> > us to do an additional hash table insert when adding a
> > direct callback, but it allow us to avoid exporting tb_flush indirectly
> > to plugins, which could be abused by malicious plugins to perform
> > a DoS attack.
>
> I don't think I have a preference. Though now I'm curious - when/why do
> you expect users might run plugins that could be malicious in this way?
When this work started, others expressed concern about potentially
malicious plugins, although I think they were thinking of preventing
plugins from gaining access to data structures that could affect
the guest, e.g. CPUState. That's why I'm using a hash table for
the plugin context id, although that's probably overkill (do we
care about malicious plugins messing with the subscriptions of
other plugins? Not sure we should.)
Here I'm thinking more about giving plugin developers too much rope to
hang themselves with, e.g. calling repeatedly plugin_reset() out
of convenience, without realising the performance impact that
it entails. That's why I'd like to explore the alternative.
Emilio
