debian/patches/pve/0057-PVE-Backup-ensure-jobs-in-di_list-are-referenced.patch

   1 From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
   2 From: Fabian Ebner <f.ebner@proxmox.com>
   3 Date: Wed, 25 May 2022 13:59:38 +0200
   4 Subject: [PATCH] PVE-Backup: ensure jobs in di_list are referenced
   5
   6 Ensures that qmp_backup_cancel doesn't pick a job that's already been
   7 freed. With unlucky timings it seems possible that:
   8 1. job_exit -> job_completed -> job_finalize_single starts
   9 2. pvebackup_co_complete_stream gets spawned in completion callback
  10 3. job finalize_single finishes -> job's refcount hits zero -> job is
  11    freed
  12 4. qmp_backup_cancel comes in and locks backup_state.backup_mutex
  13    before pvebackup_co_complete_stream can remove the job from the
  14    di_list
  15 5. qmp_backup_cancel will pick a job that's already been freed
  16
  17 Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
  18 Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
  19 ---
  20  pve-backup.c | 25 ++++++++++++++++++++-----
  21  1 file changed, 20 insertions(+), 5 deletions(-)
  22
  23 diff --git a/pve-backup.c b/pve-backup.c
  24 index 5bed6f4014..0c34428713 100644
  25 --- a/pve-backup.c
  26 +++ b/pve-backup.c
  27 @@ -316,6 +316,14 @@ static void coroutine_fn pvebackup_co_complete_stream(void *opaque)
  28          }
  29      }
  30
  31 +    if (di->job) {
  32 +        AioContext *ctx = di->job->job.aio_context;
  33 +        aio_context_acquire(ctx);
  34 +        job_unref(&di->job->job);
  35 +        di->job = NULL;
  36 +        aio_context_release(ctx);
  37 +    }
  38 +
  39      // remove self from job list
  40      backup_state.di_list = g_list_remove(backup_state.di_list, di);
  41
  42 @@ -491,9 +499,12 @@ static void create_backup_jobs_bh(void *opaque) {
  43              bitmap_mode, false, NULL, &perf, BLOCKDEV_ON_ERROR_REPORT, BLOCKDEV_ON_ERROR_REPORT,
  44              JOB_DEFAULT, pvebackup_complete_cb, di, backup_state.txn, &local_err);
  45
  46 -        aio_context_release(aio_context);
  47 -
  48          di->job = job;
  49 +        if (job) {
  50 +            job_ref(&job->job);
  51 +        }
  52 +
  53 +        aio_context_release(aio_context);
  54
  55          if (!job || local_err) {
  56              error_setg(errp, "backup_job_create failed: %s",
  57 @@ -521,12 +532,16 @@ static void create_backup_jobs_bh(void *opaque) {
  58                  di->target = NULL;
  59              }
  60
  61 -            if (!canceled && di->job) {
  62 +            if (di->job) {
  63                  AioContext *ctx = di->job->job.aio_context;
  64                  aio_context_acquire(ctx);
  65 -                job_cancel_sync(&di->job->job, true);
  66 +                if (!canceled) {
  67 +                    job_cancel_sync(&di->job->job, true);
  68 +                    canceled = true;
  69 +                }
  70 +                job_unref(&di->job->job);
  71 +                di->job = NULL;
  72                  aio_context_release(ctx);
  73 -                canceled = true;
  74              }
  75          }
  76      }