From: Ian Jackson <Ian.Jackson@eu.citrix.com>
To: coverity@xenproject.org
Cc: xen-devel@lists.xenproject.org
Subject: Re: New Defects reported by Coverity Scan for XenProject
Date: Mon, 4 Apr 2016 16:07:50 +0100 [thread overview]
Message-ID: <22274.33606.593395.44137@mariner.uk.xensource.com> (raw)
In-Reply-To: <5700f7b3e7d5c_3fdf4db3186252@ss1435.mail>
scan-admin@coverity.com writes ("New Defects reported by Coverity Scan for XenProject"):
> Please find the latest report on new defect(s) introduced to XenProject found with Coverity Scan.
>
> 35 new defect(s) introduced to XenProject found with Coverity Scan.
> 2 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 20 of 35 defect(s)
I have been through the tools reports here. None of them are security
problems in released branches.
I would like some advice from Coverity experts on the two below:
> ** CID 1358088: Concurrent data access violations (MISSING_LOCK)
Applies to 1358086..1358105 inclusive. Here is a sample:
> *** CID 1358088: Concurrent data access violations (MISSING_LOCK)
> /tools/libxl/libxl_event.c: 2189 in libxl__ao_progress_report()
> 2183 } else if (how->callback) {
> 2184 libxl__aop_occurred *aop = libxl__zalloc(&egc->gc, sizeof(*aop));
> 2185 ao->progress_reports_outstanding++;
> 2186 aop->ao = ao;
> 2187 aop->ev = ev;
> 2188 aop->how = how;
> >>> CID 1358088: Concurrent data access violations (MISSING_LOCK)
> >>> Accessing "egc->aops_for_callback.tqh_last" without holding lock "libxl__ctx.lock". Elsewhere, "libxl__egc.aops_for_callback.tqh_last" is accessed with "libxl__ctx.lock" held 34 out of 44 times.
> 2189 LIBXL_TAILQ_INSERT_TAIL(&egc->aops_for_callback, aop, entry);
> 2190 LOG(DEBUG,"ao %p: progress report: callback queued aop=%p",ao,aop);
> 2191 } else {
> 2192 LOG(DEBUG,"ao %p: progress report: event queued ev=%p type=%s",
> 2193 ao, ev, libxl_event_type_to_string(ev->type));
> 2194 libxl__event_occurred(egc, ev);
This is a false positive. An egc is always allocated on the stack of
the thread that uses it. It is only ever used by that thread. So
does not need to be protected by a lock.
Is there a way we can teach Coverity about this ?
> ** CID 1358085: Incorrect expression (IDENTICAL_BRANCHES)
> /tools/libxl/_libxl_types.c: 5611 in libxl_device_usbdev_gen_json()
Applies to 1358081..1358084 inclusive.
Here is a sample:
> *** CID 1358085: Incorrect expression (IDENTICAL_BRANCHES)
> /tools/libxl/_libxl_types.c: 5611 in libxl_device_usbdev_gen_json()
> 5605 if (s != yajl_gen_status_ok)
> 5606 goto out;
> 5607 break;
> 5608 }
> 5609 }
> 5610 s = yajl_gen_map_close(hand);
> >>> CID 1358085: Incorrect expression (IDENTICAL_BRANCHES)
> >>> The same code is executed when the condition "s != yajl_gen_status_ok" is true or false, because the code in the if-then branch and after the if statement is identical. Should the if statement be removed?
> 5611 if (s != yajl_gen_status_ok)
> 5612 goto out;
> 5613 out:
> 5614 return s;
This is a false positive arising from autogenerated code. The
autogenerated code naturally has a completely systematic error
handling pattern, which leads to it sometimes doing this:
if (error)
goto out;
out:
/* exit path */
Is there a way to arrange to always persuade Coverity that this is
intentional ?
Thanks,
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
next parent reply other threads:[~2016-04-04 15:08 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <5700f7b3e7d5c_3fdf4db3186252@ss1435.mail>
2016-04-04 15:07 ` Ian Jackson [this message]
[not found] <664dc165759df_5e9362b92d249399c762@prd-scan-dashboard-0.mail>
2024-05-22 10:05 ` New Defects reported by Coverity Scan for XenProject 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] <56ce8ad13abd2_bd9abd33094410@ss1435.mail>
2016-02-25 10:00 ` Ian Campbell
2016-02-25 10:06 ` George Dunlap
[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=22274.33606.593395.44137@mariner.uk.xensource.com \
--to=ian.jackson@eu.citrix.com \
--cc=coverity@xenproject.org \
--cc=xen-devel@lists.xenproject.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).