False positive coverity bug id: 1351218

False positive coverity bug id: 1351218

[Harmandeep Kaur]

This is about a Coverity bug (included in the end), which I think is
a false positive. I don't think pagesize can be zero in any case.
pagesize = 1 << (((flags >> TMEM_POOL_PAGESIZE_SHIFT) &
                            TMEM_POOL_PAGESIZE_MASK) + 12);

Which means "pagesize > bufsize" will always be true and buf can
not be null in any case if it reaches line 464 (or call may terminate
if realloc(..) returns NULL).
-----------------------------------------------------------------------------------------
> ** CID 1351218:    (FORWARD_NULL)
> /tools/libxc/xc_tmem.c: 464 in xc_tmem_restore()
> /tools/libxc/xc_tmem.c: 427 in xc_tmem_restore()
>
> /tools/libxc/xc_tmem.c: 464 in xc_tmem_restore()
> 458                 if ( oid.oid[0] == -1L && oid.oid[1] == -1L && oid.oid[2] == -1L )
> 459                     break;
> 460                 if ( read_exact(io_fd, &index, sizeof(index)) )
> 461                     return -1;
> 462                 if ( read_exact(io_fd, buf, pagesize) )
> 463                     return -1;
> > > >     CID 1351218:    (FORWARD_NULL)
> > > >     Dereferencing null pointer "buf".
> 464                 checksum += *buf;
> 465                 if ( (rc = xc_tmem_control_oid(xch, pool_id,
> 466
pagesize = 1 << (((flags >> TMEM_POOL_PAGESIZE_SHIFT) &
                            TMEM_POOL_PAGESIZE_MASK) + 12);
XEN_SYSCTL_TMEM_OP_RESTORE_PUT_PAGE, dom,
> 467                                                bufsize, index, oid, buf)) <= 0 )
> 468                 {
> 469                     DPRINTF("xc_tmem_restore: putting page failed, rc=%d\n",rc);
> /tools/libxc/xc_tmem.c: 427 in xc_tmem_restore()
> 421         if ( read_exact(io_fd, &minusone, sizeof(minusone)) )
> 422             return -1;
> 423         while ( read_exact(io_fd, &pool_id, sizeof(pool_id)) == 0 && pool_id != -1 )
> 424         {
> 425             uint64_t uuid[2];
> 426             uint32_t n_pages;
> > > >     CID 1351218:    (FORWARD_NULL)
> > > >     Assigning: "buf" = "NULL".
> 427             char *buf = NULL;
> 428             int bufsize = 0, pagesize;
> 429             int j;
> 430
> 431             if ( read_exact(io_fd, &flags, sizeof(flags)) )
> 432                 return -1;

Re: False positive coverity bug id: 1351218

[Andrew Cooper]

On 18/02/16 15:36, Harmandeep Kaur wrote:
> This is about a Coverity bug (included in the end), which I think is
> a false positive. I don't think pagesize can be zero in any case.
> pagesize = 1 << (((flags >> TMEM_POOL_PAGESIZE_SHIFT) &
>                             TMEM_POOL_PAGESIZE_MASK) + 12);
>
> Which means "pagesize > bufsize" will always be true and buf can
> not be null in any case if it reaches line 464 (or call may terminate
> if realloc(..) returns NULL).

I would agree that given the "1 <<", pagesize will always be larger than
0, and therefore call realloc().

However, every iteration of the
"while ( read_exact(io_fd, &pool_id, sizeof(pool_id)) == 0 && pool_id != -1 )"
loop leaks buf, as do most of the error paths.

This function is currently orphaned code (since Xen 4.6), and in need of
some re-development before it can be used again.  I wouldn't worry too
much about fixing it up.

~Andrew