[v2] block/mirror: fix NULL pointer dereference in mirror_wait_on_conflicts()

[PATCH v2] block/mirror: fix NULL pointer dereference in mirror_wait_on_conflicts()

Posted by Stefano Garzarella 2 years, 7 months ago

In mirror_iteration() we call mirror_wait_on_conflicts() with
`self` parameter set to NULL.

Starting from commit d44dae1a7c we dereference `self` pointer in
mirror_wait_on_conflicts() without checks if it is not NULL.

Backtrace:
  Program terminated with signal SIGSEGV, Segmentation fault.
  #0  mirror_wait_on_conflicts (self=0x0, s=<optimized out>, offset=<optimized out>, bytes=<optimized out>)
      at ../block/mirror.c:172
  172	                self->waiting_for_op = op;
  [Current thread is 1 (Thread 0x7f0908931ec0 (LWP 380249))]
  (gdb) bt
  #0  mirror_wait_on_conflicts (self=0x0, s=<optimized out>, offset=<optimized out>, bytes=<optimized out>)
      at ../block/mirror.c:172
  #1  0x00005610c5d9d631 in mirror_run (job=0x5610c76a2c00, errp=<optimized out>) at ../block/mirror.c:491
  #2  0x00005610c5d58726 in job_co_entry (opaque=0x5610c76a2c00) at ../job.c:917
  #3  0x00005610c5f046c6 in coroutine_trampoline (i0=<optimized out>, i1=<optimized out>)
      at ../util/coroutine-ucontext.c:173
  #4  0x00007f0909975820 in ?? () at ../sysdeps/unix/sysv/linux/x86_64/__start_context.S:91
      from /usr/lib64/libc.so.6

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2001404
Fixes: d44dae1a7c ("block/mirror: fix active mirror dead-lock in mirror_wait_on_conflicts")
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
v2:
- moved "if (op->waiting_for_op) {continue;}" code into "if (self)" too.
  [Vladimir]
---
 block/mirror.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index 98fc66eabf..85b781bc21 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -160,18 +160,25 @@ static void coroutine_fn mirror_wait_on_conflicts(MirrorOp *self,
             if (ranges_overlap(self_start_chunk, self_nb_chunks,
                                op_start_chunk, op_nb_chunks))
             {
-                /*
-                 * If the operation is already (indirectly) waiting for us, or
-                 * will wait for us as soon as it wakes up, then just go on
-                 * (instead of producing a deadlock in the former case).
-                 */
-                if (op->waiting_for_op) {
-                    continue;
+                if (self) {
+                    /*
+                     * If the operation is already (indirectly) waiting for us,
+                     * or will wait for us as soon as it wakes up, then just go
+                     * on (instead of producing a deadlock in the former case).
+                     */
+                    if (op->waiting_for_op) {
+                        continue;
+                    }
+
+                    self->waiting_for_op = op;
                 }
 
-                self->waiting_for_op = op;
                 qemu_co_queue_wait(&op->waiting_requests, NULL);
-                self->waiting_for_op = NULL;
+
+                if (self) {
+                    self->waiting_for_op = NULL;
+                }
+
                 break;
             }
         }
-- 
2.31.1

Re: [PATCH v2] block/mirror: fix NULL pointer dereference in mirror_wait_on_conflicts()

Posted by Vladimir Sementsov-Ogievskiy 2 years, 7 months ago

10.09.2021 15:45, Stefano Garzarella wrote:
> In mirror_iteration() we call mirror_wait_on_conflicts() with
> `self` parameter set to NULL.
> 
> Starting from commit d44dae1a7c we dereference `self` pointer in
> mirror_wait_on_conflicts() without checks if it is not NULL.
> 
> Backtrace:
>    Program terminated with signal SIGSEGV, Segmentation fault.
>    #0  mirror_wait_on_conflicts (self=0x0, s=<optimized out>, offset=<optimized out>, bytes=<optimized out>)
>        at ../block/mirror.c:172
>    172	                self->waiting_for_op = op;
>    [Current thread is 1 (Thread 0x7f0908931ec0 (LWP 380249))]
>    (gdb) bt
>    #0  mirror_wait_on_conflicts (self=0x0, s=<optimized out>, offset=<optimized out>, bytes=<optimized out>)
>        at ../block/mirror.c:172
>    #1  0x00005610c5d9d631 in mirror_run (job=0x5610c76a2c00, errp=<optimized out>) at ../block/mirror.c:491
>    #2  0x00005610c5d58726 in job_co_entry (opaque=0x5610c76a2c00) at ../job.c:917
>    #3  0x00005610c5f046c6 in coroutine_trampoline (i0=<optimized out>, i1=<optimized out>)
>        at ../util/coroutine-ucontext.c:173
>    #4  0x00007f0909975820 in ?? () at ../sysdeps/unix/sysv/linux/x86_64/__start_context.S:91
>        from /usr/lib64/libc.so.6
> 
> Buglink:https://bugzilla.redhat.com/show_bug.cgi?id=2001404
> Fixes: d44dae1a7c ("block/mirror: fix active mirror dead-lock in mirror_wait_on_conflicts")
> Signed-off-by: Stefano Garzarella<sgarzare@redhat.com>

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

-- 
Best regards,
Vladimir

Re: [PATCH v2] block/mirror: fix NULL pointer dereference in mirror_wait_on_conflicts()

Posted by Hanna Reitz 2 years, 7 months ago

On 10.09.21 14:45, Stefano Garzarella wrote:
> In mirror_iteration() we call mirror_wait_on_conflicts() with
> `self` parameter set to NULL.
>
> Starting from commit d44dae1a7c we dereference `self` pointer in
> mirror_wait_on_conflicts() without checks if it is not NULL.
>
> Backtrace:
>    Program terminated with signal SIGSEGV, Segmentation fault.
>    #0  mirror_wait_on_conflicts (self=0x0, s=<optimized out>, offset=<optimized out>, bytes=<optimized out>)
>        at ../block/mirror.c:172
>    172	                self->waiting_for_op = op;
>    [Current thread is 1 (Thread 0x7f0908931ec0 (LWP 380249))]
>    (gdb) bt
>    #0  mirror_wait_on_conflicts (self=0x0, s=<optimized out>, offset=<optimized out>, bytes=<optimized out>)
>        at ../block/mirror.c:172
>    #1  0x00005610c5d9d631 in mirror_run (job=0x5610c76a2c00, errp=<optimized out>) at ../block/mirror.c:491
>    #2  0x00005610c5d58726 in job_co_entry (opaque=0x5610c76a2c00) at ../job.c:917
>    #3  0x00005610c5f046c6 in coroutine_trampoline (i0=<optimized out>, i1=<optimized out>)
>        at ../util/coroutine-ucontext.c:173
>    #4  0x00007f0909975820 in ?? () at ../sysdeps/unix/sysv/linux/x86_64/__start_context.S:91
>        from /usr/lib64/libc.so.6
>
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2001404
> Fixes: d44dae1a7c ("block/mirror: fix active mirror dead-lock in mirror_wait_on_conflicts")
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
> v2:
> - moved "if (op->waiting_for_op) {continue;}" code into "if (self)" too.
>    [Vladimir]
> ---
>   block/mirror.c | 25 ++++++++++++++++---------
>   1 file changed, 16 insertions(+), 9 deletions(-)

Thanks, applied to my block branch:

https://github.com/XanClic/qemu/commits/block

Hanna