Xen devel - Sep 2013 - More Coverity-reported issues.

Another bundle of issues from Coverity triage.
The first one is in x86/mm, and looks scarier than it is.  The others
are all in xen/drivers and AFAICT are pretty minor.

Cheers,

Tim.

Not a security bug, because in fact this is never called with a NULL
argument.

Coverity CID 1055955

Signed-off-by: Tim Deegan <tim@xen.org>
---
 xen/arch/x86/mm/p2m.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index f5ddd20..8f380ed 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -453,7 +453,7 @@ void p2m_teardown(struct p2m_domain *p2m)
  * We know we don''t have any extra mappings to these pages */
 {
     struct page_info *pg;
-    struct domain *d = p2m->domain;
+    struct domain *d;
     unsigned long gfn;
     p2m_type_t t;
     mfn_t mfn;
@@ -461,6 +461,8 @@ void p2m_teardown(struct p2m_domain *p2m)
     if (p2m == NULL)
         return;
 
+    d = p2m->domain;
+
     p2m_lock(p2m);
 
     /* Try to unshare any remaining shared p2m entries. Safeguard
-- 
1.7.10.4

The lock''s not used for anything, and AFAICT no locking is needed
since the IVRS tables are static after boot.

Coverity CID 1087199

Signed-off-by: Tim Deegan <tim@xen.org>
Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Cc: Jacob Shin <jacob.shin@amd.com>
---
 xen/drivers/passthrough/amd/iommu_intr.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/xen/drivers/passthrough/amd/iommu_intr.c
b/xen/drivers/passthrough/amd/iommu_intr.c
index 831f92a..213f4d7 100644
--- a/xen/drivers/passthrough/amd/iommu_intr.c
+++ b/xen/drivers/passthrough/amd/iommu_intr.c
@@ -443,7 +443,6 @@ static int update_intremap_entry_from_msi_msg(
      * devices.
      */
 
-    lock = get_intremap_lock(iommu->seg, alias_id);
     if ( ( req_id != alias_id ) &&
          get_ivrs_mappings(iommu->seg)[alias_id].intremap_table != NULL )
     {
-- 
1.7.10.4

Coverity CID 1055502

Signed-off-by: Tim Deegan <tim@xen.org>
Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Cc: Jacob Shin <jacob.shin@amd.com>
---
 xen/drivers/passthrough/amd/iommu_guest.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/xen/drivers/passthrough/amd/iommu_guest.c
b/xen/drivers/passthrough/amd/iommu_guest.c
index 85f2361..952600a 100644
--- a/xen/drivers/passthrough/amd/iommu_guest.c
+++ b/xen/drivers/passthrough/amd/iommu_guest.c
@@ -728,6 +728,7 @@ static void guest_iommu_mmio_write64(struct guest_iommu
*iommu,
         break;
     case IOMMU_EVENT_LOG_BASE_LOW_OFFSET:
         u64_to_reg(&iommu->event_log.reg_base, val);
+        break;
     case IOMMU_PPR_LOG_BASE_LOW_OFFSET:
         u64_to_reg(&iommu->ppr_log.reg_base, val);
         break;
-- 
1.7.10.4

Coverity''s parser chokes on seeing __section() before the type.

Coverity CID 1087190

Signed-off-by: Tim Deegan <tim@xen.org>
Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Cc: Jacob Shin <jacob.shin@amd.com>
---
 xen/drivers/passthrough/amd/iommu_acpi.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c
b/xen/drivers/passthrough/amd/iommu_acpi.c
index 89b359c..2bfe61e 100644
--- a/xen/drivers/passthrough/amd/iommu_acpi.c
+++ b/xen/drivers/passthrough/amd/iommu_acpi.c
@@ -633,7 +633,7 @@ static u16 __init parse_ivhd_device_extended_range(
     return dev_length;
 }
 
-static __initdata DECLARE_BITMAP(ioapic_cmdline, ARRAY_SIZE(ioapic_sbdf));
+static DECLARE_BITMAP(__initdata ioapic_cmdline, ARRAY_SIZE(ioapic_sbdf));
 
 static void __init parse_ivrs_ioapic(char *str)
 {
-- 
1.7.10.4

The def_sampling_rate() one is, I think, a real bug.  The others were
spotted at the same time and are probably not bugs until we start
dealing with 40GHz CPus.

Coverity CID 1055682
Coverity CID 1055683

Signed-off-by: Tim Deegan <tim@xen.org>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Liu Jinsong <jinsong.liu@intel.com>
---
 xen/drivers/cpufreq/cpufreq_ondemand.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/xen/drivers/cpufreq/cpufreq_ondemand.c
b/xen/drivers/cpufreq/cpufreq_ondemand.c
index b3f9ab8..7fdba03 100644
--- a/xen/drivers/cpufreq/cpufreq_ondemand.c
+++ b/xen/drivers/cpufreq/cpufreq_ondemand.c
@@ -144,7 +144,7 @@ static void dbs_check_cpu(struct cpu_dbs_info_s
*this_dbs_info)
     }
 
     /* Check for frequency increase */
-    if (max_load_freq > dbs_tuners_ins.up_threshold * policy->cur) {
+    if (max_load_freq > (uint64_t) dbs_tuners_ins.up_threshold *
policy->cur) {
         /* if we are already at full speed then break out early */
         if (policy->cur == max)
             return;
@@ -162,7 +162,8 @@ static void dbs_check_cpu(struct cpu_dbs_info_s
*this_dbs_info)
      * can support the current CPU usage without triggering the up
      * policy. To be safe, we focus 10 points under the threshold.
      */
-    if (max_load_freq < (dbs_tuners_ins.up_threshold - 10) * policy->cur)
{
+    if (max_load_freq
+        < (uint64_t) (dbs_tuners_ins.up_threshold - 10) * policy->cur) {
         uint64_t freq_next;
 
         freq_next = max_load_freq / (dbs_tuners_ins.up_threshold - 10);
@@ -246,7 +247,7 @@ int cpufreq_governor_dbs(struct cpufreq_policy *policy,
unsigned int event)
          * is used for first time
          */
         if ((dbs_enable == 1) && !dbs_tuners_ins.sampling_rate) {
-            def_sampling_rate = policy->cpuinfo.transition_latency *
+            def_sampling_rate = (uint64_t)
policy->cpuinfo.transition_latency *
                 DEF_SAMPLING_RATE_LATENCY_MULTIPLIER;
 
             if (def_sampling_rate < MIN_STAT_SAMPLING_RATE)
-- 
1.7.10.4

Coverity CID 1055131
Coverity CID 1055132

Signed-off-by: Tim Deegan <tim@xen.org>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Liu Jinsong <jinsong.liu@intel.com>
---
 xen/drivers/cpufreq/cpufreq.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/cpufreq/cpufreq.c b/xen/drivers/cpufreq/cpufreq.c
index 0de5d41..ab66884 100644
--- a/xen/drivers/cpufreq/cpufreq.c
+++ b/xen/drivers/cpufreq/cpufreq.c
@@ -471,8 +471,12 @@ int set_px_pminfo(uint32_t acpi_id, struct
xen_processor_performance *dom0_px_in
             ret = -ENOMEM;
             goto out;
         }
-        copy_from_guest(pxpt->states, dom0_px_info->states, 
-                                      dom0_px_info->state_count);
+        if ( copy_from_guest(pxpt->states, dom0_px_info->states,
+                             dom0_px_info->state_count) )
+        {
+            ret = -EFAULT;
+            goto out;
+        }
         pxpt->state_count = dom0_px_info->state_count;
 
         if ( cpufreq_verbose )
-- 
1.7.10.4

Unlikely to ever see hardware reporting 0 ports, but might as well
fail gracefully if we do.

Coverity CID 1055266

Signed-off-by: Tim Deegan <tim@xen.org>
Cc: Jan Beulich <jbeulich@suse.com>
---
 xen/drivers/char/ehci-dbgp.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/xen/drivers/char/ehci-dbgp.c b/xen/drivers/char/ehci-dbgp.c
index 2504979..0ac2dd9 100644
--- a/xen/drivers/char/ehci-dbgp.c
+++ b/xen/drivers/char/ehci-dbgp.c
@@ -1099,6 +1099,9 @@ try_next_port:
     dbgp_printk("n_ports:    %u\n", n_ports);
     ehci_dbgp_status(dbgp, "");
 
+    if ( n_ports == 0 )
+        return -1;
+
     for ( i = 1; i <= n_ports; i++ )
     {
         portsc = readl(&dbgp->ehci_regs->port_status[i-1]);
-- 
1.7.10.4

We can only reach this spot by breaking out of the scan loop,
so by construction ret > 0.

Coverity CID 1055259

Signed-off-by: Tim Deegan <tim@xen.org>
Cc: Jan Beulich <jbeulich@suse.com>
---
 xen/drivers/char/ehci-dbgp.c |    5 -----
 1 file changed, 5 deletions(-)

diff --git a/xen/drivers/char/ehci-dbgp.c b/xen/drivers/char/ehci-dbgp.c
index 0ac2dd9..b900d60 100644
--- a/xen/drivers/char/ehci-dbgp.c
+++ b/xen/drivers/char/ehci-dbgp.c
@@ -946,11 +946,6 @@ try_again:
         dbgp_printk("could not find attached debug device\n");
         goto err;
     }
-    if ( ret < 0 )
-    {
-        dbgp_printk("attached device is not a debug device\n");
-        goto err;
-    }
     dbgp->out.endpoint = dbgp_desc.bDebugOutEndpoint;
     dbgp->in.endpoint = dbgp_desc.bDebugInEndpoint;
 
-- 
1.7.10.4

These ''name'' strings are actually arrays in their structs.  So
the
address is never NULL: instead, we should check the first character to
detect cases where the field wasn''t initialized.

Coverity CID 1055633

Signed-off-by: Tim Deegan <tim@xen.org>
Cc: Jan Beulich <jbeulich@suse.com>
---
 xen/drivers/acpi/pmstat.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/acpi/pmstat.c b/xen/drivers/acpi/pmstat.c
index f8a9c85..daac2da 100644
--- a/xen/drivers/acpi/pmstat.c
+++ b/xen/drivers/acpi/pmstat.c
@@ -264,13 +264,13 @@ static int get_cpufreq_para(struct xen_sysctl_pm_op *op)
     op->u.get_para.scaling_max_freq = policy->max;
     op->u.get_para.scaling_min_freq = policy->min;
 
-    if ( cpufreq_driver->name )
+    if ( cpufreq_driver->name[0] )
         strlcpy(op->u.get_para.scaling_driver, 
             cpufreq_driver->name, CPUFREQ_NAME_LEN);
     else
         strlcpy(op->u.get_para.scaling_driver, "Unknown",
CPUFREQ_NAME_LEN);
 
-    if ( policy->governor->name )
+    if ( policy->governor->name[0] )
         strlcpy(op->u.get_para.scaling_governor, 
             policy->governor->name, CPUFREQ_NAME_LEN);
     else
-- 
1.7.10.4

On 12/09/13 13:15, Tim Deegan wrote:
> Another bundle of issues from Coverity triage.
> The first one is in x86/mm, and looks scarier than it is.  The others
> are all in xen/drivers and AFAICT are pretty minor.
>
> Cheers,
>
> Tim.
>
All Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

>>> On 12.09.13 at 14:15, Tim Deegan <tim@xen.org> wrote:
> The lock''s not used for anything, and AFAICT no locking is needed
> since the IVRS tables are static after boot.
> 
> Coverity CID 1087199
> 
> Signed-off-by: Tim Deegan <tim@xen.org>
That was an oversight of mine I think.

Reviewed-by: Jan Beulich <jbeulich@suse.com>
> --- a/xen/drivers/passthrough/amd/iommu_intr.c
> +++ b/xen/drivers/passthrough/amd/iommu_intr.c
> @@ -443,7 +443,6 @@ static int update_intremap_entry_from_msi_msg(
>       * devices.
>       */
>  
> -    lock = get_intremap_lock(iommu->seg, alias_id);
>      if ( ( req_id != alias_id ) &&
>           get_ivrs_mappings(iommu->seg)[alias_id].intremap_table != NULL
)
>      {
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org 
> http://lists.xen.org/xen-devel

>>> On 12.09.13 at 14:15, Tim Deegan <tim@xen.org> wrote:
> Coverity''s parser chokes on seeing __section() before the type.
> 
> Coverity CID 1087190
> 
> Signed-off-by: Tim Deegan <tim@xen.org>
> Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> Cc: Jacob Shin <jacob.shin@amd.com>
> ---
>  xen/drivers/passthrough/amd/iommu_acpi.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c 
> b/xen/drivers/passthrough/amd/iommu_acpi.c
> index 89b359c..2bfe61e 100644
> --- a/xen/drivers/passthrough/amd/iommu_acpi.c
> +++ b/xen/drivers/passthrough/amd/iommu_acpi.c
> @@ -633,7 +633,7 @@ static u16 __init parse_ivhd_device_extended_range(
>      return dev_length;
>  }
>  
> -static __initdata DECLARE_BITMAP(ioapic_cmdline, ARRAY_SIZE(ioapic_sbdf));
> +static DECLARE_BITMAP(__initdata ioapic_cmdline, ARRAY_SIZE(ioapic_sbdf));
But putting it inside the DECLARE_BITMAP() doesn''t seem that nice
either. I think the second best option - if we need to play such
games for Coverity in the first place - would be to put this at the end.

Jan

At 14:38 +0100 on 12 Sep (1378996715), Jan Beulich
wrote:
> >>> On 12.09.13 at 14:15, Tim Deegan <tim@xen.org> wrote:
> > Coverity''s parser chokes on seeing __section() before the
type.
> > 
> > Coverity CID 1087190
> > 
> > Signed-off-by: Tim Deegan <tim@xen.org>
> > Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> > Cc: Jacob Shin <jacob.shin@amd.com>
> > ---
> >  xen/drivers/passthrough/amd/iommu_acpi.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c 
> > b/xen/drivers/passthrough/amd/iommu_acpi.c
> > index 89b359c..2bfe61e 100644
> > --- a/xen/drivers/passthrough/amd/iommu_acpi.c
> > +++ b/xen/drivers/passthrough/amd/iommu_acpi.c
> > @@ -633,7 +633,7 @@ static u16 __init
parse_ivhd_device_extended_range(
> >      return dev_length;
> >  }
> >  
> > -static __initdata DECLARE_BITMAP(ioapic_cmdline,
ARRAY_SIZE(ioapic_sbdf));
> > +static DECLARE_BITMAP(__initdata ioapic_cmdline,
ARRAY_SIZE(ioapic_sbdf));
> 
> But putting it inside the DECLARE_BITMAP() doesn''t seem that nice
> either. I think the second best option - if we need to play such
> games for Coverity in the first place -
Yes, I think the question of how much we''re willing to accommodate
Coverity in our source code needs to be discussed.  On that subject I
have an RFC patch that introduces some annotation...
> would be to put this at the end.
Yes, that sounds better.  Thus?

index 89b359c..e967eba 100644
--- a/xen/drivers/passthrough/amd/iommu_acpi.c
+++ b/xen/drivers/passthrough/amd/iommu_acpi.c
@@ -633,7 +633,7 @@ static u16 __init parse_ivhd_device_extended_range(
     return dev_length;
 }
 
-static __initdata DECLARE_BITMAP(ioapic_cmdline, ARRAY_SIZE(ioapic_sbdf));
+static DECLARE_BITMAP(ioapic_cmdline, ARRAY_SIZE(ioapic_sbdf)) __initdata;
 
 static void __init parse_ivrs_ioapic(char *str)
 {

Tim.

>>> On 12.09.13 at 14:15, Tim Deegan <tim@xen.org> wrote:
> The def_sampling_rate() one is, I think, a real bug.  The others were
> spotted at the same time and are probably not bugs until we start
> dealing with 40GHz CPus.
> 
> Coverity CID 1055682
> Coverity CID 1055683
> 
> Signed-off-by: Tim Deegan <tim@xen.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
> Cc: Liu Jinsong <jinsong.liu@intel.com>
> ---
>  xen/drivers/cpufreq/cpufreq_ondemand.c |    7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/drivers/cpufreq/cpufreq_ondemand.c 
> b/xen/drivers/cpufreq/cpufreq_ondemand.c
> index b3f9ab8..7fdba03 100644
> --- a/xen/drivers/cpufreq/cpufreq_ondemand.c
> +++ b/xen/drivers/cpufreq/cpufreq_ondemand.c
> @@ -144,7 +144,7 @@ static void dbs_check_cpu(struct cpu_dbs_info_s 
> *this_dbs_info)
>      }
>  
>      /* Check for frequency increase */
> -    if (max_load_freq > dbs_tuners_ins.up_threshold * policy->cur) {
> +    if (max_load_freq > (uint64_t) dbs_tuners_ins.up_threshold *
policy->cur)
> {
>          /* if we are already at full speed then break out early */
>          if (policy->cur == max)
>              return;
> @@ -162,7 +162,8 @@ static void dbs_check_cpu(struct cpu_dbs_info_s 
> *this_dbs_info)
>       * can support the current CPU usage without triggering the up
>       * policy. To be safe, we focus 10 points under the threshold.
>       */
> -    if (max_load_freq < (dbs_tuners_ins.up_threshold - 10) *
policy->cur) {
> +    if (max_load_freq
> +        < (uint64_t) (dbs_tuners_ins.up_threshold - 10) *
policy->cur) {
>          uint64_t freq_next;
>  
>          freq_next = max_load_freq / (dbs_tuners_ins.up_threshold - 10);
> @@ -246,7 +247,7 @@ int cpufreq_governor_dbs(struct cpufreq_policy *policy,
> unsigned int event)
>           * is used for first time
>           */
>          if ((dbs_enable == 1) && !dbs_tuners_ins.sampling_rate) {
> -            def_sampling_rate = policy->cpuinfo.transition_latency *
> +            def_sampling_rate = (uint64_t)
policy->cpuinfo.transition_latency
> *
>                  DEF_SAMPLING_RATE_LATENCY_MULTIPLIER;
>  
>              if (def_sampling_rate < MIN_STAT_SAMPLING_RATE)
> -- 
> 1.7.10.4

>>> On 12.09.13 at 14:15, Tim Deegan <tim@xen.org> wrote:
> Coverity CID 1055131
> Coverity CID 1055132
> 
> Signed-off-by: Tim Deegan <tim@xen.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
> Cc: Liu Jinsong <jinsong.liu@intel.com>
> ---
>  xen/drivers/cpufreq/cpufreq.c |    8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/drivers/cpufreq/cpufreq.c b/xen/drivers/cpufreq/cpufreq.c
> index 0de5d41..ab66884 100644
> --- a/xen/drivers/cpufreq/cpufreq.c
> +++ b/xen/drivers/cpufreq/cpufreq.c
> @@ -471,8 +471,12 @@ int set_px_pminfo(uint32_t acpi_id, struct
xen_processor_performance *dom0_px_in
>              ret = -ENOMEM;
>              goto out;
>          }
> -        copy_from_guest(pxpt->states, dom0_px_info->states, 
> -                                      dom0_px_info->state_count);
> +        if ( copy_from_guest(pxpt->states, dom0_px_info->states,
> +                             dom0_px_info->state_count) )
> +        {
> +            ret = -EFAULT;
> +            goto out;
> +        }
>          pxpt->state_count = dom0_px_info->state_count;
>  
>          if ( cpufreq_verbose )
> -- 
> 1.7.10.4

>>> On 12.09.13 at 14:15, Tim Deegan <tim@xen.org> wrote:
> Unlikely to ever see hardware reporting 0 ports, but might as well
> fail gracefully if we do.
> 
> Coverity CID 1055266
> 
> Signed-off-by: Tim Deegan <tim@xen.org>
Well, that''s a really pointless change, but anyway:

Acked-by: Jan Beulich <jbeulich@suse.com>
> ---
>  xen/drivers/char/ehci-dbgp.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/xen/drivers/char/ehci-dbgp.c b/xen/drivers/char/ehci-dbgp.c
> index 2504979..0ac2dd9 100644
> --- a/xen/drivers/char/ehci-dbgp.c
> +++ b/xen/drivers/char/ehci-dbgp.c
> @@ -1099,6 +1099,9 @@ try_next_port:
>      dbgp_printk("n_ports:    %u\n", n_ports);
>      ehci_dbgp_status(dbgp, "");
>  
> +    if ( n_ports == 0 )
> +        return -1;
> +
>      for ( i = 1; i <= n_ports; i++ )
>      {
>          portsc = readl(&dbgp->ehci_regs->port_status[i-1]);
> -- 
> 1.7.10.4

>>> On 12.09.13 at 14:15, Tim Deegan <tim@xen.org> wrote:
> We can only reach this spot by breaking out of the scan loop,
> so by construction ret > 0.
> 
> Coverity CID 1055259
> 
> Signed-off-by: Tim Deegan <tim@xen.org>
Acked-by: Jan Beulich <jbeulich@suse.com>

But this should really be patched in the original (Linux) first...
> ---
>  xen/drivers/char/ehci-dbgp.c |    5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/xen/drivers/char/ehci-dbgp.c b/xen/drivers/char/ehci-dbgp.c
> index 0ac2dd9..b900d60 100644
> --- a/xen/drivers/char/ehci-dbgp.c
> +++ b/xen/drivers/char/ehci-dbgp.c
> @@ -946,11 +946,6 @@ try_again:
>          dbgp_printk("could not find attached debug device\n");
>          goto err;
>      }
> -    if ( ret < 0 )
> -    {
> -        dbgp_printk("attached device is not a debug device\n");
> -        goto err;
> -    }
>      dbgp->out.endpoint = dbgp_desc.bDebugOutEndpoint;
>      dbgp->in.endpoint = dbgp_desc.bDebugInEndpoint;
>  
> -- 
> 1.7.10.4

>>> On 12.09.13 at 14:15, Tim Deegan <tim@xen.org> wrote:
> These ''name'' strings are actually arrays in their
structs.  So the
> address is never NULL: instead, we should check the first character to
> detect cases where the field wasn''t initialized.
> 
> Coverity CID 1055633
> 
> Signed-off-by: Tim Deegan <tim@xen.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
> ---
>  xen/drivers/acpi/pmstat.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/drivers/acpi/pmstat.c b/xen/drivers/acpi/pmstat.c
> index f8a9c85..daac2da 100644
> --- a/xen/drivers/acpi/pmstat.c
> +++ b/xen/drivers/acpi/pmstat.c
> @@ -264,13 +264,13 @@ static int get_cpufreq_para(struct xen_sysctl_pm_op 
> *op)
>      op->u.get_para.scaling_max_freq = policy->max;
>      op->u.get_para.scaling_min_freq = policy->min;
>  
> -    if ( cpufreq_driver->name )
> +    if ( cpufreq_driver->name[0] )
>          strlcpy(op->u.get_para.scaling_driver, 
>              cpufreq_driver->name, CPUFREQ_NAME_LEN);
>      else
>          strlcpy(op->u.get_para.scaling_driver, "Unknown",
CPUFREQ_NAME_LEN);
>  
> -    if ( policy->governor->name )
> +    if ( policy->governor->name[0] )
>          strlcpy(op->u.get_para.scaling_governor, 
>              policy->governor->name, CPUFREQ_NAME_LEN);
>      else
> -- 
> 1.7.10.4

>>> On 12.09.13 at 15:44, Tim Deegan <tim@xen.org> wrote:
> At 14:38 +0100 on 12 Sep (1378996715), Jan Beulich wrote:
>> >>> On 12.09.13 at 14:15, Tim Deegan <tim@xen.org>
wrote:
>> > Coverity''s parser chokes on seeing __section() before the
type.
>> > 
>> > Coverity CID 1087190
>> > 
>> > Signed-off-by: Tim Deegan <tim@xen.org>
>> > Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>> > Cc: Jacob Shin <jacob.shin@amd.com>
>> > ---
>> >  xen/drivers/passthrough/amd/iommu_acpi.c |    2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> > 
>> > diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c 
>> > b/xen/drivers/passthrough/amd/iommu_acpi.c
>> > index 89b359c..2bfe61e 100644
>> > --- a/xen/drivers/passthrough/amd/iommu_acpi.c
>> > +++ b/xen/drivers/passthrough/amd/iommu_acpi.c
>> > @@ -633,7 +633,7 @@ static u16 __init
parse_ivhd_device_extended_range(
>> >      return dev_length;
>> >  }
>> >  
>> > -static __initdata DECLARE_BITMAP(ioapic_cmdline,
ARRAY_SIZE(ioapic_sbdf));
>> > +static DECLARE_BITMAP(__initdata ioapic_cmdline,
ARRAY_SIZE(ioapic_sbdf));
>> 
>> But putting it inside the DECLARE_BITMAP() doesn''t seem that
nice
>> either. I think the second best option - if we need to play such
>> games for Coverity in the first place -
> 
> Yes, I think the question of how much we''re willing to accommodate
> Coverity in our source code needs to be discussed.  On that subject I
> have an RFC patch that introduces some annotation...
> 
>> would be to put this at the end.
> 
> Yes, that sounds better.  Thus?
Ack.
> --- a/xen/drivers/passthrough/amd/iommu_acpi.c
> +++ b/xen/drivers/passthrough/amd/iommu_acpi.c
> @@ -633,7 +633,7 @@ static u16 __init parse_ivhd_device_extended_range(
>      return dev_length;
>  }
>  
> -static __initdata DECLARE_BITMAP(ioapic_cmdline, ARRAY_SIZE(ioapic_sbdf));
> +static DECLARE_BITMAP(ioapic_cmdline, ARRAY_SIZE(ioapic_sbdf)) __initdata;
>  
>  static void __init parse_ivrs_ioapic(char *str)
>  {
> 
> Tim.

Tim Deegan wrote:
> The def_sampling_rate() one is, I think, a real bug.  The others were
> spotted at the same time and are probably not bugs until we start
> dealing with 40GHz CPus.
> 
> Coverity CID 1055682
> Coverity CID 1055683
> 
> Signed-off-by: Tim Deegan <tim@xen.org>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Liu Jinsong <jinsong.liu@intel.com>
> ---
>  xen/drivers/cpufreq/cpufreq_ondemand.c |    7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/drivers/cpufreq/cpufreq_ondemand.c
> b/xen/drivers/cpufreq/cpufreq_ondemand.c index b3f9ab8..7fdba03 100644
> --- a/xen/drivers/cpufreq/cpufreq_ondemand.c
> +++ b/xen/drivers/cpufreq/cpufreq_ondemand.c
> @@ -144,7 +144,7 @@ static void dbs_check_cpu(struct cpu_dbs_info_s
>      *this_dbs_info) }
> 
>      /* Check for frequency increase */
> -    if (max_load_freq > dbs_tuners_ins.up_threshold * policy->cur) {
> +    if (max_load_freq > (uint64_t) dbs_tuners_ins.up_threshold *
>          policy->cur) { /* if we are already at full speed then break
>          out early */ if (policy->cur == max)
>              return;
> @@ -162,7 +162,8 @@ static void dbs_check_cpu(struct cpu_dbs_info_s
> *this_dbs_info) 
>       * can support the current CPU usage without triggering the up
>       * policy. To be safe, we focus 10 points under the threshold.
>       */
> -    if (max_load_freq < (dbs_tuners_ins.up_threshold - 10) *
> policy->cur) { +    if (max_load_freq
> +        < (uint64_t) (dbs_tuners_ins.up_threshold - 10) *
>          policy->cur) { uint64_t freq_next;
> 
>          freq_next = max_load_freq / (dbs_tuners_ins.up_threshold -
> 10); @@ -246,7 +247,7 @@ int cpufreq_governor_dbs(struct
> cpufreq_policy *policy, unsigned int event) 
>           * is used for first time
>           */
>          if ((dbs_enable == 1) && !dbs_tuners_ins.sampling_rate) {
> -            def_sampling_rate = policy->cpuinfo.transition_latency *
> +            def_sampling_rate = (uint64_t)
>                  policy->cpuinfo.transition_latency *
> DEF_SAMPLING_RATE_LATENCY_MULTIPLIER; 
> 
>              if (def_sampling_rate < MIN_STAT_SAMPLING_RATE)
Acked-by: Liu Jinsong <jinsong.liu@intel.com>

Tim Deegan wrote:
> Coverity CID 1055131
> Coverity CID 1055132
> 
> Signed-off-by: Tim Deegan <tim@xen.org>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Liu Jinsong <jinsong.liu@intel.com>
> ---
>  xen/drivers/cpufreq/cpufreq.c |    8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/drivers/cpufreq/cpufreq.c
> b/xen/drivers/cpufreq/cpufreq.c 
> index 0de5d41..ab66884 100644
> --- a/xen/drivers/cpufreq/cpufreq.c
> +++ b/xen/drivers/cpufreq/cpufreq.c
> @@ -471,8 +471,12 @@ int set_px_pminfo(uint32_t acpi_id, struct
>              xen_processor_performance *dom0_px_in ret = -ENOMEM;
>              goto out;
>          }
> -        copy_from_guest(pxpt->states, dom0_px_info->states,
> -                                      dom0_px_info->state_count);
> +        if ( copy_from_guest(pxpt->states, dom0_px_info->states,
> +                             dom0_px_info->state_count) )
> +        {
> +            ret = -EFAULT;
> +            goto out;
> +        }
>          pxpt->state_count = dom0_px_info->state_count;
> 
>          if ( cpufreq_verbose )
Acked-by: Liu Jinsong <jinsong.liu@intel.com>

On 09/12/2013 07:15 AM, Tim Deegan wrote:
> Coverity CID 1055502
>
> Signed-off-by: Tim Deegan <tim@xen.org>
> Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> Cc: Jacob Shin <jacob.shin@amd.com>
> ---
>   xen/drivers/passthrough/amd/iommu_guest.c |    1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/xen/drivers/passthrough/amd/iommu_guest.c
b/xen/drivers/passthrough/amd/iommu_guest.c
> index 85f2361..952600a 100644
> --- a/xen/drivers/passthrough/amd/iommu_guest.c
> +++ b/xen/drivers/passthrough/amd/iommu_guest.c
> @@ -728,6 +728,7 @@ static void guest_iommu_mmio_write64(struct guest_iommu
*iommu,
>           break;
>       case IOMMU_EVENT_LOG_BASE_LOW_OFFSET:
>           u64_to_reg(&iommu->event_log.reg_base, val);
> +        break;
>       case IOMMU_PPR_LOG_BASE_LOW_OFFSET:
>           u64_to_reg(&iommu->ppr_log.reg_base, val);
>           break;
Acked-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>

On 09/12/2013 09:03 AM, Jan Beulich wrote:
>>>> On 12.09.13 at 15:44, Tim Deegan <tim@xen.org> wrote:
>> At 14:38 +0100 on 12 Sep (1378996715), Jan Beulich wrote:
>>>>>> On 12.09.13 at 14:15, Tim Deegan <tim@xen.org>
wrote:
>>>> Coverity''s parser chokes on seeing __section() before
the type.
>>>>
>>>> Coverity CID 1087190
>>>>
>>>> Signed-off-by: Tim Deegan <tim@xen.org>
>>>> Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>>>> Cc: Jacob Shin <jacob.shin@amd.com>
>>>> ---
>>>>   xen/drivers/passthrough/amd/iommu_acpi.c |    2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c
>>>> b/xen/drivers/passthrough/amd/iommu_acpi.c
>>>> index 89b359c..2bfe61e 100644
>>>> --- a/xen/drivers/passthrough/amd/iommu_acpi.c
>>>> +++ b/xen/drivers/passthrough/amd/iommu_acpi.c
>>>> @@ -633,7 +633,7 @@ static u16 __init
parse_ivhd_device_extended_range(
>>>>       return dev_length;
>>>>   }
>>>>   
>>>> -static __initdata DECLARE_BITMAP(ioapic_cmdline,
ARRAY_SIZE(ioapic_sbdf));
>>>> +static DECLARE_BITMAP(__initdata ioapic_cmdline,
ARRAY_SIZE(ioapic_sbdf));
>>> But putting it inside the DECLARE_BITMAP() doesn''t seem
that nice
>>> either. I think the second best option - if we need to play such
>>> games for Coverity in the first place -
>> Yes, I think the question of how much we''re willing to
accommodate
>> Coverity in our source code needs to be discussed.  On that subject I
>> have an RFC patch that introduces some annotation...
>>
>>> would be to put this at the end.
>> Yes, that sounds better.  Thus?
> Ack.
>
>> --- a/xen/drivers/passthrough/amd/iommu_acpi.c
>> +++ b/xen/drivers/passthrough/amd/iommu_acpi.c
>> @@ -633,7 +633,7 @@ static u16 __init parse_ivhd_device_extended_range(
>>       return dev_length;
>>   }
>>   
>> -static __initdata DECLARE_BITMAP(ioapic_cmdline,
ARRAY_SIZE(ioapic_sbdf));
>> +static DECLARE_BITMAP(ioapic_cmdline, ARRAY_SIZE(ioapic_sbdf))
__initdata;
>>   
>>   static void __init parse_ivrs_ioapic(char *str)
>>   {
>>
>> Tim.
Acked-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>