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: Fiona Ebner <f.ebner@proxmox.com>
  18 Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
  19 [FE: adapt for new job lock mechanism replacing AioContext locks]
  20 Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
  21 ---
  22  pve-backup.c | 22 +++++++++++++++++++---
  23  1 file changed, 19 insertions(+), 3 deletions(-)
  24
  25 diff --git a/pve-backup.c b/pve-backup.c
  26 index 79d14d6a0b..67e2b99d74 100644
  27 --- a/pve-backup.c
  28 +++ b/pve-backup.c
  29 @@ -318,6 +318,13 @@ static void coroutine_fn pvebackup_co_complete_stream(void *opaque)
  30          }
  31      }
  32
  33 +    if (di->job) {
  34 +        WITH_JOB_LOCK_GUARD() {
  35 +            job_unref_locked(&di->job->job);
  36 +            di->job = NULL;
  37 +        }
  38 +    }
  39 +
  40      // remove self from job list
  41      backup_state.di_list = g_list_remove(backup_state.di_list, di);
  42
  43 @@ -493,6 +500,11 @@ static void create_backup_jobs_bh(void *opaque) {
  44          aio_context_release(aio_context);
  45
  46          di->job = job;
  47 +        if (job) {
  48 +            WITH_JOB_LOCK_GUARD() {
  49 +                job_ref_locked(&job->job);
  50 +            }
  51 +        }
  52
  53          if (!job || local_err) {
  54              error_setg(errp, "backup_job_create failed: %s",
  55 @@ -520,11 +532,15 @@ static void create_backup_jobs_bh(void *opaque) {
  56                  di->target = NULL;
  57              }
  58
  59 -            if (!canceled && di->job) {
  60 +            if (di->job) {
  61                  WITH_JOB_LOCK_GUARD() {
  62 -                    job_cancel_sync_locked(&di->job->job, true);
  63 +                    if (!canceled) {
  64 +                        job_cancel_sync_locked(&di->job->job, true);
  65 +                        canceled = true;
  66 +                    }
  67 +                    job_unref_locked(&di->job->job);
  68 +                    di->job = NULL;
  69                  }
  70 -                canceled = true;
  71              }
  72          }
  73      }