Re: Bug#8482: Uninitialized memory read in gio.

[Owen Taylor <otaylor redhat com>]

Sebastian Wilhelmi <wilhelmi@ira.uka.de> writes:

> Hi Benjamin,
> 
> > It looks like g_io_unix_check is being called on a fd before it is
> > polled.  This means that revents is not being set and is thus being read
> > uninitialized.  No bad things have occured to me because of this, but
> > thought I should point it out.
> 
> That indeed is a bug. But actually it seems to reveal a problem inside
> g_main_iterate. After staring at the code for sometime I'm thinking, that
> this can only happen in the following case: 
> 
> g_main_iterate finds a source of priority 'current_priority' to be ready in
> 'prepare' and thus it doesn't check further for sources with smaller
> priorities. Then it polls all sources with priorities greater or equal to
> 'current_priority'. Then afterwards it searches for sources to be ready in
> 'check' and because (assumed here) no sources of priority greater or equal to
> 'current_priority' are ready wrt 'check', it continues searching for lower
> priorities, where the revents field wasn't set by g_poll (because g_poll
> didn't touch them at all). 

Hmmm, I don't think that would happen. To quote from gmain.c:

  /* Check to see what sources need to be dispatched */

  n_ready = 0;
  
  hook = g_hook_first_valid (&source_list, TRUE);
  while (hook)
    {
      GSource *source = (GSource *)hook;

      if ((n_ready > 0) && (source->priority > current_priority))
	{
	  g_hook_unref (&source_list, hook);
	  break;
	}

So, we won't check any sources with priority > current_priority.
(Note that priorities are like UNIX niceness values - numerically
smaller values have greater priority.) 

I have two theories.

 1) The app is threaded, and a source is being added during 
    to the poll. (I think to handle this properly we 
    need another source flag like G_SOURCE_PREPARED, since 
    there is is supposed to be an invariant that 
    a source will be prepared before any call to ->check().

 2) Someone is callign g_io_unix_add_watch() with 
    condition == NULL. This will produce the problem, since
    in g_main_poll() we have:

      if (pollrec->fd->events)
	{
	  pollrec->fd->revents = fd_array[i].revents;
	  i++;
	}

    This probably should be:

      if (pollrec->fd->events)
	{
	  pollrec->fd->revents = fd_array[i].revents;
	  i++;
	}
      else
        pollrec->fd->revents = 0;


  Benjamin - assuming that your app isn't threaded, could you 
  temporarily insert a g_return_val_if_fail (condition != 0, 0);
  in g_io_add_watch_full() to see if I've diagnosed this
  correctly?

Regards,
                                             Owen