* [PATCH] ipmi:smbus: Add a check around a memcpy
@ 2022-07-31 23:02 minyard
2022-08-01 6:15 ` Michael S. Tsirkin
2022-08-01 8:39 ` Peter Maydell
0 siblings, 2 replies; 3+ messages in thread
From: minyard @ 2022-07-31 23:02 UTC (permalink / raw)
To: Peter Maydell
Cc: QEMU Developers, Michael S . Tsirkin, Igor Mammedov,
M : Marcel Apfelbaum, David Gibson, Paolo Bonzini,
Marc-André Lureau, Corey Minyard
From: Corey Minyard <cminyard@mvista.com>
In one case:
memcpy(sid->inmsg + sid->inlen, buf, len);
if len == 0 then sid->inmsg + sig->inlen can point to one past the inmsg
array if the array is full. We have to allow len == 0 due to some
vagueness in the spec, but we don't have to call memcpy.
Found by Coverity. This is not a problem in practice, but the results
are technically (maybe) undefined. So make Coverity happy.
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
hw/ipmi/smbus_ipmi.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
I think this should do it.
diff --git a/hw/ipmi/smbus_ipmi.c b/hw/ipmi/smbus_ipmi.c
index 9ef9112dd5..d0991ab7f9 100644
--- a/hw/ipmi/smbus_ipmi.c
+++ b/hw/ipmi/smbus_ipmi.c
@@ -281,7 +281,9 @@ static int ipmi_write_data(SMBusDevice *dev, uint8_t *buf, uint8_t len)
*/
send = true;
}
- memcpy(sid->inmsg + sid->inlen, buf, len);
+ if (len > 0) {
+ memcpy(sid->inmsg + sid->inlen, buf, len);
+ }
sid->inlen += len;
break;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] ipmi:smbus: Add a check around a memcpy
2022-07-31 23:02 [PATCH] ipmi:smbus: Add a check around a memcpy minyard
@ 2022-08-01 6:15 ` Michael S. Tsirkin
2022-08-01 8:39 ` Peter Maydell
1 sibling, 0 replies; 3+ messages in thread
From: Michael S. Tsirkin @ 2022-08-01 6:15 UTC (permalink / raw)
To: minyard
Cc: Peter Maydell, QEMU Developers, Igor Mammedov,
M : Marcel Apfelbaum, David Gibson, Paolo Bonzini,
Marc-André Lureau, Corey Minyard
On Sun, Jul 31, 2022 at 06:02:46PM -0500, minyard@acm.org wrote:
> From: Corey Minyard <cminyard@mvista.com>
>
> In one case:
>
> memcpy(sid->inmsg + sid->inlen, buf, len);
>
> if len == 0 then sid->inmsg + sig->inlen can point to one past the inmsg
> array if the array is full. We have to allow len == 0 due to some
> vagueness in the spec, but we don't have to call memcpy.
>
> Found by Coverity. This is not a problem in practice, but the results
> are technically (maybe) undefined. So make Coverity happy.
>
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Corey Minyard <cminyard@mvista.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> hw/ipmi/smbus_ipmi.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> I think this should do it.
>
> diff --git a/hw/ipmi/smbus_ipmi.c b/hw/ipmi/smbus_ipmi.c
> index 9ef9112dd5..d0991ab7f9 100644
> --- a/hw/ipmi/smbus_ipmi.c
> +++ b/hw/ipmi/smbus_ipmi.c
> @@ -281,7 +281,9 @@ static int ipmi_write_data(SMBusDevice *dev, uint8_t *buf, uint8_t len)
> */
> send = true;
> }
> - memcpy(sid->inmsg + sid->inlen, buf, len);
> + if (len > 0) {
> + memcpy(sid->inmsg + sid->inlen, buf, len);
> + }
> sid->inlen += len;
> break;
> }
> --
> 2.25.1
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] ipmi:smbus: Add a check around a memcpy
2022-07-31 23:02 [PATCH] ipmi:smbus: Add a check around a memcpy minyard
2022-08-01 6:15 ` Michael S. Tsirkin
@ 2022-08-01 8:39 ` Peter Maydell
1 sibling, 0 replies; 3+ messages in thread
From: Peter Maydell @ 2022-08-01 8:39 UTC (permalink / raw)
To: minyard
Cc: QEMU Developers, Michael S . Tsirkin, Igor Mammedov,
M : Marcel Apfelbaum, David Gibson, Paolo Bonzini,
Marc-André Lureau, Corey Minyard
On Mon, 1 Aug 2022 at 00:03, <minyard@acm.org> wrote:
>
> From: Corey Minyard <cminyard@mvista.com>
>
> In one case:
>
> memcpy(sid->inmsg + sid->inlen, buf, len);
>
> if len == 0 then sid->inmsg + sig->inlen can point to one past the inmsg
> array if the array is full. We have to allow len == 0 due to some
> vagueness in the spec, but we don't have to call memcpy.
>
> Found by Coverity. This is not a problem in practice, but the results
> are technically (maybe) undefined. So make Coverity happy.
>
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> ---
> hw/ipmi/smbus_ipmi.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> I think this should do it.
>
> diff --git a/hw/ipmi/smbus_ipmi.c b/hw/ipmi/smbus_ipmi.c
> index 9ef9112dd5..d0991ab7f9 100644
> --- a/hw/ipmi/smbus_ipmi.c
> +++ b/hw/ipmi/smbus_ipmi.c
> @@ -281,7 +281,9 @@ static int ipmi_write_data(SMBusDevice *dev, uint8_t *buf, uint8_t len)
> */
> send = true;
> }
> - memcpy(sid->inmsg + sid->inlen, buf, len);
> + if (len > 0) {
> + memcpy(sid->inmsg + sid->inlen, buf, len);
> + }
> sid->inlen += len;
> break;
> }
> --
> 2.25.1
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-08-01 8:42 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-07-31 23:02 [PATCH] ipmi:smbus: Add a check around a memcpy minyard
2022-08-01 6:15 ` Michael S. Tsirkin
2022-08-01 8:39 ` Peter Maydell
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 NNTP newsgroup(s).