[Qemu-devel] [Qemu-devel PATCH v3 0/2] fix incorrect assertions in sd and main-loop

[Qemu-devel] [Qemu-devel PATCH v3 0/2] fix incorrect assertions in sd and main-loop

[Lidong Chen]

This v3 added Philippe's Reviewed-by in patch2 (main-loop.c) 
I also included Philippe's previous comment about patch1 (sd.c)
in this cover: 

--------
Not sure via which tree this patch is going (trivial?).
To the maintainer, please consider adding when applying:

"This access can not happen. Fix to silent static analyzer warnings."

As confirmed by Lidong in v1 here:
https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg01337.html

Thanks,

Phil.
-------

Lidong Chen (2):
  sd: Fix out-of-bounds assertions
  util/main-loop: Fix incorrect assertion

 hw/sd/sd.c       | 4 ++--
 util/main-loop.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [Qemu-devel PATCH v3 1/2] sd: Fix out-of-bounds assertions

[Lidong Chen]

Due to an off-by-one error, the assert statements allow an
out-of-bound array access.

Signed-off-by: Lidong Chen <lidong.chen@oracle.com>
Reviewed-by: Liam Merwick <liam.merwick@oracle.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Li Qiang <liq3ea@gmail.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
---
 hw/sd/sd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index aaab15f..818f86c 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -144,7 +144,7 @@ static const char *sd_state_name(enum SDCardStates state)
     if (state == sd_inactive_state) {
         return "inactive";
     }
-    assert(state <= ARRAY_SIZE(state_name));
+    assert(state < ARRAY_SIZE(state_name));
     return state_name[state];
 }
 
@@ -165,7 +165,7 @@ static const char *sd_response_name(sd_rsp_type_t rsp)
     if (rsp == sd_r1b) {
         rsp = sd_r1;
     }
-    assert(rsp <= ARRAY_SIZE(response_name));
+    assert(rsp < ARRAY_SIZE(response_name));
     return response_name[rsp];
 }
 
-- 
1.8.3.1

[Qemu-devel] [Qemu-devel PATCH v3 2/2] util/main-loop: Fix incorrect assertion

[Lidong Chen]

The check for poll_fds in g_assert() was incorrect. The correct assertion
should check "n_poll_fds + w->num <= ARRAY_SIZE(poll_fds)" because the
subsequent for-loop is doing access to poll_fds[n_poll_fds + i] where i
is in [0, w->num).

Signed-off-by: Lidong Chen <lidong.chen@oracle.com>
Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Suggested-by: Liam Merwick <liam.merwick@oracle.com>
Reviewed-by: Liran Alon <liran.alon@oracle.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Li Qiang <liq3ea@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 util/main-loop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util/main-loop.c b/util/main-loop.c
index e1e349c..a9f4e8d 100644
--- a/util/main-loop.c
+++ b/util/main-loop.c
@@ -422,7 +422,7 @@ static int os_host_main_loop_wait(int64_t timeout)
     g_main_context_prepare(context, &max_priority);
     n_poll_fds = g_main_context_query(context, max_priority, &poll_timeout,
                                       poll_fds, ARRAY_SIZE(poll_fds));
-    g_assert(n_poll_fds <= ARRAY_SIZE(poll_fds));
+    g_assert(n_poll_fds + w->num <= ARRAY_SIZE(poll_fds));
 
     for (i = 0; i < w->num; i++) {
         poll_fds[n_poll_fds + i].fd = (DWORD_PTR)w->events[i];
-- 
1.8.3.1

Re: [Qemu-devel] [Qemu-devel PATCH v3 0/2] fix incorrect assertions in sd and main-loop

[Philippe Mathieu-Daudé]

On 6/19/19 9:14 PM, Lidong Chen wrote:
> This v3 added Philippe's Reviewed-by in patch2 (main-loop.c) 

Thanks.

> I also included Philippe's previous comment about patch1 (sd.c)
> in this cover: 
> 
> --------
> Not sure via which tree this patch is going (trivial?).
> To the maintainer, please consider adding when applying:
> 
> "This access can not happen. Fix to silent static analyzer warnings."

What I asked you is to add this in the patch1 description.
If you were not to respin, then I'd ask the maintainer who takes this
series to amend this comment to the patch.

> 
> As confirmed by Lidong in v1 here:
> https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg01337.html
> 
> Thanks,
> 
> Phil.
> -------
> 
> Lidong Chen (2):
>   sd: Fix out-of-bounds assertions
>   util/main-loop: Fix incorrect assertion
> 
>  hw/sd/sd.c       | 4 ++--
>  util/main-loop.c | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 

Re: [Qemu-devel] [Qemu-devel PATCH v3 0/2] fix incorrect assertions in sd and main-loop

[Paolo Bonzini]

On 19/06/19 21:14, Lidong Chen wrote:
> This v3 added Philippe's Reviewed-by in patch2 (main-loop.c) 
> I also included Philippe's previous comment about patch1 (sd.c)
> in this cover: 
> 
> --------
> Not sure via which tree this patch is going (trivial?).
> To the maintainer, please consider adding when applying:
> 
> "This access can not happen. Fix to silent static analyzer warnings."
> 
> As confirmed by Lidong in v1 here:
> https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg01337.html
> 
> Thanks,
> 
> Phil.
> -------
> 
> Lidong Chen (2):
>   sd: Fix out-of-bounds assertions
>   util/main-loop: Fix incorrect assertion
> 
>  hw/sd/sd.c       | 4 ++--
>  util/main-loop.c | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> -- 1.8.3.1

Queued, thanks.

Paolo