Re: [Qemu-devel] [PATCH] monitor: Add whitelist support for QMP commands

[Julia Suvorova]

On 01.02.2019 0:03, Eric Blake wrote:
> On 1/31/19 2:26 PM, Julia Suvorova via Qemu-devel wrote:
> > The whitelist option allows to run a reduced monitor with a subset of
> > QMP commands. This allows the monitor to run in secure mode, which is
> > convenient for sending commands via the WebSocket monitor using the
> > web UI. This is planned to be done on micro:bit board.
> >
> > The list of allowed commands should be written to a file, one per line.
> > The command line will look like this:
> >      -mon chardev_name,mode=control,whitelist=path_to_file
> >
> > Signed-off-by: Julia Suvorova <address@hidden>
> > ---
>
> >   
> > -void monitor_init(Chardev *chr, int flags)
> > +static void process_whitelist_file(Monitor *mon, const char *whitelist_file)
> > +{
> > +    char cmd_name[256];
> > +    FILE *fd = fopen(whitelist_file, "r");
>
> If you use qemu_open() here (followed by fdopen if you still prefer
> fscanf over read), then you can support "/dev/fdset/NNN" to
> auto-magically support someone passing in the whitelist via an inherited
> file descriptor, rather than having to be somewhere on disk that qemu
> can directly open().

Cool, thanks!

> > +
> > +    if (fd == NULL) {
> > +        error_report("Could not open whitelist file: %s", strerror(errno));
> > +        exit(1);
> > +    }
> > +
> > +    mon->whitelist = g_hash_table_new_full(g_str_hash,
> > +                                           g_str_equal,
> > +                                           g_free,
> > +                                           NULL);
> > +
> > +    g_hash_table_add(mon->whitelist, g_strdup("qmp_capabilities"));
> > +    g_hash_table_add(mon->whitelist, g_strdup("query-commands"));
> > +
> > +    while (fscanf(fd, "%255s", cmd_name) == 1) {
>
> %255s fits your cmd_name array declaration and stops consuming at either
> 255 bytes or at the first whitespace encountered, but where do you check
> for overflow from a file that passes more than 255 non-whitespace bytes
> without a newline?  Also, this is a bit sloppy in that it skips all
> leading whitespace, rather than ensuring that the user actually passed
> newline-separated command names.  Does glib provide any interfaces for
> more easily reading in an array of lines from a file?

With glib it would be something like that:

  g_mapped_file_new_from_fd()  : fd -> GMappedFile
  g_mapped_file_get_contents() : GMappedFile -> char *
  g_strsplit_set()             : char * -> char **

All this would need to be released afterwards.
Do you think it would be better?

Best regards, Julia Suvorova.