[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [virt-tools-list] [PATCH virt-viewer 15/19] Hook up handling of Monitors



On Tue, Jul 17, 2012 at 05:24:02PM +0200, Marc-André Lureau wrote:
> On Tue, Jul 17, 2012 at 5:09 PM, Christophe Fergeau <cfergeau redhat com> wrote:
> > Ok, so monitors_max is a guint, the second argument to g_ptr_array_set_size
> > is a gint, and the GPtrArray code doesn't seem to handle. A quick reading
> > of GPtrArray code didn't make me feel confident that it would do the right
> > thing when size > G_INT_MAX / sizeof(void *), which could cause a buffer
> > overflow when very huge values are used (I haven't carefully checked that).
> 
> G_INT_MAX / sizeof(void *) ? where did you get the / sizeof(void*) from?

You are using a GPtrArray, so my understanding is that the memory allocated
for it is num_elements * sizeof (void*). If num_elements > G_INT_MAX /
sizeof(void*), a quick reading suggests this will cause an overflow in
the amount of memory to allocate causing the allocated array to be much
smaller than what is expected. Maybe that is not an issue, but this is
worrying. My point was that you are saying that allocating too much memory
will only cause swapping and the OOM to trigger, and I wanted to point out
that in this case, the problem could not be easily simplified to "too much
memory allocated, but no buffer overflow".

> 
> Eh ok, so you want to check if max_monitors > G_INT_MAX.. :) a very
> unlikely condition that will crash the client anyway. But fine by me.

Not really, just trying to understand what the extent of the problem is,
and quite worried that you are basically saying this is not an issue at all
and that all is fine.

> 
> >> There is nothing wrong in OOM if the server tells us we have 1024
> >> maximum monitors on the guest for example,
> >
> > Is this max value coming from the server, or is it the guest qxl driver
> > telling the server about how many monitors it supports?
> 
> It's configured at xorg level, with NumHeads option, defaulting to 4.

So what we are trusting here is an arbitrary value provided by the guest
system?

Christophe

Attachment: pgpltL42c0ahF.pgp
Description: PGP signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]