[Spice-devel] [linux-agent v1 4/5] x11: Avoid dereference of a null pointer

[Victor Toso]

Hi,

On Tue, Dec 18, 2018 at 12:43:13PM -0500, Frediano Ziglio wrote:
> > 
> > From: Victor Toso <me at victortoso.com>
> > 
> > Found by coverity:
> >   | warning: Access to field 'next' results in a dereference of a
> >   | null pointer (loaded from variable 'prev_sel')
> > 
> > Signed-off-by: Victor Toso <victortoso at redhat.com>
> > ---
> >  src/vdagent/x11.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/vdagent/x11.c b/src/vdagent/x11.c
> > index c3c7a65..69eed4a 100644
> > --- a/src/vdagent/x11.c
> > +++ b/src/vdagent/x11.c
> > @@ -394,7 +394,7 @@ static void vdagent_x11_set_clipboard_owner(struct
> > vdagent_x11 *x11,
> >                  x11->selection_req_data_pos = 0;
> >                  x11->selection_req_data_size = 0;
> >                  x11->selection_req_atom = None;
> > -            } else {
> > +            } else if (prev_sel != NULL) {
> >                  prev_sel->next = next_sel;
> >              }
> >              free(curr_sel);
> 
> This is a false positive, if is not the first there's a previous.

Yes..

> Not that I have an easy suggestion on how to make Coverity understand this!
> One way would be to change prev_sel to a pointer to the next so
> initializing it like 
> 
>    struct vdagent_x11_selection_request **p_prev_next = &x11->selection_req;
> 
> and update with
> 
>    p_prev_next = &curr_sel->next;
> 
> Frediano

I still think this check is harmless and simple enough to shut
coverity.

Thanks,
Victor
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20181219/ff7c92df/attachment.sig>