From: Ian Campbell <ian.campbell@citrix.com>
To: George Dunlap <george.dunlap@eu.citrix.com>,
Harmandeep Kaur <write.harmandeep@gmail.com>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>,
Andrew Cooper <andrew.cooper3@citrix.com>,
Wei Liu <wei.liu2@citrix.com>,
xen-devel <xen-devel@lists.xen.org>
Subject: Re: New Defects reported by Coverity Scan for XenProject
Date: Thu, 25 Feb 2016 10:00:31 +0000 [thread overview]
Message-ID: <1456394431.6225.182.camel@citrix.com> (raw)
In-Reply-To: <56ce8ad13abd2_bd9abd33094410@ss1435.mail>
On Wed, 2016-02-24 at 21:02 -0800, scan-admin@coverity.com wrote:
> Hi,
>
> Please find the latest report on new defect(s) introduced to XenProject found with Coverity Scan.
>
> 2 new defect(s) introduced to XenProject found with Coverity Scan.
> 12 defect(s), reported by Coverity Scan earlier, were marked fixed in the recent build analyzed by Coverity Scan.
>
> New defect(s) Reported-by: Coverity Scan
> Showing 2 of 2 defect(s)
>
>
> ** CID 1354244: Null pointer dereferences (FORWARD_NULL)
> /tools/libxc/xc_tbuf.c: 72 in xc_tbuf_get_size()
>
>
> ________________________________________________________________________________________________________
> *** CID 1354244: Null pointer dereferences (FORWARD_NULL)
> /tools/libxc/xc_tbuf.c: 72 in xc_tbuf_get_size()
> 66 return rc;
> 67
> 68 t_info = xc_map_foreign_range(xch, DOMID_XEN,
> 69 sysctl.u.tbuf_op.size, PROT_READ | PROT_WRITE,
> 70 sysctl.u.tbuf_op.buffer_mfn);
> 71
> > > > CID 1354244: Null pointer dereferences (FORWARD_NULL)
> > > > Comparing "t_info" to null implies that "t_info" might be null.
> 72 if ( t_info == NULL || t_info->tbuf_size == 0 )
> 73 rc = -1;
> 74 else
> 75 *size = t_info->tbuf_size;
> 76
> 77 xenforeignmemory_unmap(xch->fmem, t_info, sysctl.u.tbuf_op.size);
This is complaining about the eventual munmap(t_info) => munmap(NULL) which
is behind xenforeignmemory_unmap().
Looks like it was newly added by the fix to CID 1351228 in 7c479883b04a
("libxc: fix leak of t_info in xc_tbuf_get_size()").
xenforeignmemory_unmap() should behave like munmap WRT tollerance of NULL,
I'm not 100% sure what that behaviour is since 0 is a valid address.
xenforeignmemory.h no doubt wants updating with the desired semantics and
either this code of the implementation adjusting to match.
While here I notice that using xc_map_*() to create the mapping and
xenforeignmemory_unmap() to destroy it is a bit odd since they are strictly
two separate APIs, even if one happens to be implemented in terms of the
other. Being libxc internal this code is at liberty to use xc_map_* but
should then use plain munmap to undo it, or it would also be reasonable to
port this code fully to the xenforeignmemory interface.
>
> ** CID 1354243: Control flow issues (DEADCODE)
> /tools/xentrace/xenalyze.c: 4148 in cr3_dump_list()
>
>
> ________________________________________________________________________________________________________
> *** CID 1354243: Control flow issues (DEADCODE)
> /tools/xentrace/xenalyze.c: 4148 in cr3_dump_list()
> 4142
> 4143 /* Count the number of elements */
> 4144 for(p=head; p; p=p->next)
> 4145 N++;
> 4146
> 4147 if(!N)
> > > > CID 1354243: Control flow issues (DEADCODE)
> > > > Execution cannot reach this statement: "return;".
Here it has observed that due to the (above, just out of the context given
here) "if (!head) return" that the for loop must run at least once, so N
cannot be 0.
My guess is that this is a prexisting issue which was exposed to coverities
beady eye somehow by 28ab9f3d0e7c ("tools/xenalyze: Fix build with clang").
Or maybe this was previous marked deliberate but the change has caused
coverity to think this is a different instance of the same thing, eitherway
I don't think the issue itself is new.
FWIW having both if (!head) return and if (!N) return looks redundant to
me, the other two similar looking instances (from grepping for N++) have
only the latter check.
Ian.
> 4148 return;
> 4149
> 4150 /* Alloc a struct of the right size */
> 4151 qsort_array = malloc(N * sizeof(struct eip_list_struct *));
> 4152
> 4153 /* Point the array into it */
>
>
> ________________________________________________________________________________________________________
> To view the defects in Coverity Scan visit, https://scan.coverity.com/projects/xenproject?tab=overview
>
> To manage Coverity Scan email notifications for "ian.campbell@citrix.com", click https://scan.coverity.com/subscriptions/edit?email=ian.campbell%40citrix.com&token=86eecf9a70a32d8ef6ac41e4c7cdaf58
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
next parent reply other threads:[~2016-02-25 10:00 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <56ce8ad13abd2_bd9abd33094410@ss1435.mail>
2016-02-25 10:00 ` Ian Campbell [this message]
2016-02-25 10:06 ` New Defects reported by Coverity Scan for XenProject George Dunlap
[not found] <664dc165759df_5e9362b92d249399c762@prd-scan-dashboard-0.mail>
2024-05-22 10:05 ` Jan Beulich
2024-05-22 13:49 ` Andrew Cooper
[not found] <6637576caf98c_10d9e42c57d37559ac60499@prd-scan-dashboard-0.mail>
2024-05-06 7:46 ` Jan Beulich
[not found] <6547674e54da3_1c3af2c62521719a8359bc@prd-scan-dashboard-0.mail>
2023-11-06 7:36 ` Jan Beulich
[not found] <64859cf3a1e46_712752abb10eab98834b9@prd-scan-dashboard-0.mail>
2023-06-12 10:54 ` Jan Beulich
2023-06-12 11:06 ` Andrew Cooper
[not found] <600d4d7f99bc3_241662b17c874cf6097f1@prd-scan-dashboard-0.mail>
2021-01-25 10:14 ` Jan Beulich
[not found] <5700f7b3e7d5c_3fdf4db3186252@ss1435.mail>
2016-04-04 15:07 ` Ian Jackson
[not found] <551be9e0474d8_2970d1331454394@scan.coverity.com.mail>
2015-04-02 14:32 ` Ian Campbell
2015-04-02 15:43 ` Charles Arnold
[not found] <E1Vgaam-0000UH-GS@build-l3.scan.coverity.com>
2013-11-13 13:51 ` Ian Campbell
2013-11-13 14:01 ` David Vrabel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1456394431.6225.182.camel@citrix.com \
--to=ian.campbell@citrix.com \
--cc=Ian.Jackson@eu.citrix.com \
--cc=andrew.cooper3@citrix.com \
--cc=george.dunlap@eu.citrix.com \
--cc=wei.liu2@citrix.com \
--cc=write.harmandeep@gmail.com \
--cc=xen-devel@lists.xen.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).