[Qemu-devel] [PATCH 0/2] A couple of IPMI bugs

[Qemu-devel] [PATCH 0/2] A couple of IPMI bugs

[minyard]

Found by Paulo and Coverity.

[Qemu-devel] [PATCH 1/2] ipmi_bmc_sim: Fix off by one in check.

[minyard]

From: Corey Minyard <cminyard@mvista.com>

Found by Paolo.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Shannon Zhao <zhaoshenglong@huawei.com>
Cc: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 hw/ipmi/ipmi_bmc_sim.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
index 0a59e53..3ecf02e 100644
--- a/hw/ipmi/ipmi_bmc_sim.c
+++ b/hw/ipmi/ipmi_bmc_sim.c
@@ -546,7 +546,7 @@ static void ipmi_init_sensors_from_sdrs(IPMIBmcSim *s)
 static int ipmi_register_netfn(IPMIBmcSim *s, unsigned int netfn,
                                const IPMINetfn *netfnd)
 {
-    if ((netfn & 1) || (netfn > MAX_NETFNS) || (s->netfns[netfn / 2])) {
+    if ((netfn & 1) || (netfn >= MAX_NETFNS) || (s->netfns[netfn / 2])) {
         return -1;
     }
     s->netfns[netfn / 2] = netfnd;
-- 
2.5.0

[Qemu-devel] [PATCH 2/2] ipmi_bmc_sim: Add break to correct watchdog NMI check

[minyard]

From: Corey Minyard <cminyard@mvista.com>

It was falling through when it should have been a break.  Found by
Coverity.  The logic could be simplified a bit with a fallthrough,
probably the original thought, but that would be less clear, I think.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Shannon Zhao <zhaoshenglong@huawei.com>
Cc: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 hw/ipmi/ipmi_bmc_sim.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
index 3ecf02e..e522ffa 100644
--- a/hw/ipmi/ipmi_bmc_sim.c
+++ b/hw/ipmi/ipmi_bmc_sim.c
@@ -1101,6 +1101,8 @@ static void set_watchdog_timer(IPMIBmcSim *ibs,
             rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
             goto out;
         }
+        break;
+
     default:
         /* We don't support PRE_SMI */
         rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
-- 
2.5.0

Re: [Qemu-devel] [PATCH 0/2] A couple of IPMI bugs

[Paolo Bonzini]

Michael, these were missing in your pull request, probably because you
weren't CCed.  Do you want me to pick them instead?

Thanks,

Paolo

Re: [Qemu-devel] [PATCH 1/2] ipmi_bmc_sim: Fix off by one in check.

[Michael S. Tsirkin]

On Mon, Jan 11, 2016 at 07:32:31AM -0600, minyard@acm.org wrote:
> From: Corey Minyard <cminyard@mvista.com>
> 
> Found by Paolo.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Shannon Zhao <zhaoshenglong@huawei.com>
> Cc: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Corey Minyard <cminyard@mvista.com>

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  hw/ipmi/ipmi_bmc_sim.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
> index 0a59e53..3ecf02e 100644
> --- a/hw/ipmi/ipmi_bmc_sim.c
> +++ b/hw/ipmi/ipmi_bmc_sim.c
> @@ -546,7 +546,7 @@ static void ipmi_init_sensors_from_sdrs(IPMIBmcSim *s)
>  static int ipmi_register_netfn(IPMIBmcSim *s, unsigned int netfn,
>                                 const IPMINetfn *netfnd)
>  {
> -    if ((netfn & 1) || (netfn > MAX_NETFNS) || (s->netfns[netfn / 2])) {
> +    if ((netfn & 1) || (netfn >= MAX_NETFNS) || (s->netfns[netfn / 2])) {
>          return -1;
>      }
>      s->netfns[netfn / 2] = netfnd;
> -- 
> 2.5.0

Re: [Qemu-devel] [PATCH 2/2] ipmi_bmc_sim: Add break to correct watchdog NMI check

[Michael S. Tsirkin]

On Mon, Jan 11, 2016 at 07:32:32AM -0600, minyard@acm.org wrote:
> From: Corey Minyard <cminyard@mvista.com>
> 
> It was falling through when it should have been a break.  Found by
> Coverity.  The logic could be simplified a bit with a fallthrough,
> probably the original thought, but that would be less clear, I think.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Shannon Zhao <zhaoshenglong@huawei.com>
> Cc: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Corey Minyard <cminyard@mvista.com>

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  hw/ipmi/ipmi_bmc_sim.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
> index 3ecf02e..e522ffa 100644
> --- a/hw/ipmi/ipmi_bmc_sim.c
> +++ b/hw/ipmi/ipmi_bmc_sim.c
> @@ -1101,6 +1101,8 @@ static void set_watchdog_timer(IPMIBmcSim *ibs,
>              rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
>              goto out;
>          }
> +        break;
> +
>      default:
>          /* We don't support PRE_SMI */
>          rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
> -- 
> 2.5.0

Re: [Qemu-devel] [PATCH 0/2] A couple of IPMI bugs

[Michael S. Tsirkin]

On Mon, Feb 08, 2016 at 10:14:14AM +0100, Paolo Bonzini wrote:
> Michael, these were missing in your pull request, probably because you
> weren't CCed.  Do you want me to pick them instead?

Sure, go ahead.
Thanks!

> Thanks,
> 
> Paolo