From: trix@redhat.com To: Felix.Kuehling@amd.com, alexander.deucher@amd.com, christian.koenig@amd.com, Xinhui.Pan@amd.com, airlied@linux.ie, daniel@ffwll.ch, nathan@kernel.org, ndesaulniers@google.com, david.yatsin@amd.com, rajneesh.bhardwaj@amd.com Cc: amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, llvm@lists.linux.dev, Tom Rix <trix@redhat.com> Subject: [PATCH] drm/amdkfd: rework criu_restore_bos error handling Date: Fri, 18 Feb 2022 09:39:13 -0800 [thread overview] Message-ID: <20220218173913.3376948-1-trix@redhat.com> (raw) From: Tom Rix <trix@redhat.com> Clang static analysis reports this problem kfd_chardev.c:2327:2: warning: 1st function call argument is an uninitialized value kvfree(bo_privs); ^~~~~~~~~~~~~~~~ If the copy_from_users(bo_buckets, ...) fails, there is a jump to the generic error handler at exit:. The freeing of bo_privs and unwinding of the dmabuf_fd loop do not need to be done. Add some specific labels for the early failures. Reorder the frees to be the reverse of their allocs. Move the initialize of 'i' back to the loop. The problem with the early frees predates the loop unwinding problem. Fixes: 73fa13b6a511 ("drm/amdkfd: CRIU Implement KFD restore ioctl") Signed-off-by: Tom Rix <trix@redhat.com> --- drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c index 965af2a08bc0..1d5f41ac3832 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c @@ -2102,7 +2102,7 @@ static int criu_restore_bos(struct kfd_process *p, const bool criu_resume = true; bool flush_tlbs = false; int ret = 0, j = 0; - uint32_t i = 0; + uint32_t i; if (*priv_offset + (args->num_bos * sizeof(*bo_privs)) > max_priv_data_size) return -EINVAL; @@ -2119,13 +2119,13 @@ static int criu_restore_bos(struct kfd_process *p, if (ret) { pr_err("Failed to copy BOs information from user\n"); ret = -EFAULT; - goto exit; + goto free_buckets; } bo_privs = kvmalloc_array(args->num_bos, sizeof(*bo_privs), GFP_KERNEL); if (!bo_privs) { ret = -ENOMEM; - goto exit; + goto free_buckets; } ret = copy_from_user(bo_privs, (void __user *)args->priv_data + *priv_offset, @@ -2133,12 +2133,12 @@ static int criu_restore_bos(struct kfd_process *p, if (ret) { pr_err("Failed to copy BOs information from user\n"); ret = -EFAULT; - goto exit; + goto free_privs; } *priv_offset += args->num_bos * sizeof(*bo_privs); /* Create and map new BOs */ - for (; i < args->num_bos; i++) { + for (i = 0; i < args->num_bos; i++) { struct kfd_criu_bo_bucket *bo_bucket; struct kfd_criu_bo_priv_data *bo_priv; struct kfd_dev *dev; @@ -2323,8 +2323,11 @@ static int criu_restore_bos(struct kfd_process *p, if (bo_buckets[i].alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM) close_fd(bo_buckets[i].dmabuf_fd); } - kvfree(bo_buckets); +free_privs: kvfree(bo_privs); +free_buckets: + kvfree(bo_buckets); + return ret; } -- 2.26.3
WARNING: multiple messages have this Message-ID (diff)
From: trix@redhat.com To: Felix.Kuehling@amd.com, alexander.deucher@amd.com, christian.koenig@amd.com, Xinhui.Pan@amd.com, airlied@linux.ie, daniel@ffwll.ch, nathan@kernel.org, ndesaulniers@google.com, david.yatsin@amd.com, rajneesh.bhardwaj@amd.com Cc: Tom Rix <trix@redhat.com>, llvm@lists.linux.dev, dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org Subject: [PATCH] drm/amdkfd: rework criu_restore_bos error handling Date: Fri, 18 Feb 2022 09:39:13 -0800 [thread overview] Message-ID: <20220218173913.3376948-1-trix@redhat.com> (raw) From: Tom Rix <trix@redhat.com> Clang static analysis reports this problem kfd_chardev.c:2327:2: warning: 1st function call argument is an uninitialized value kvfree(bo_privs); ^~~~~~~~~~~~~~~~ If the copy_from_users(bo_buckets, ...) fails, there is a jump to the generic error handler at exit:. The freeing of bo_privs and unwinding of the dmabuf_fd loop do not need to be done. Add some specific labels for the early failures. Reorder the frees to be the reverse of their allocs. Move the initialize of 'i' back to the loop. The problem with the early frees predates the loop unwinding problem. Fixes: 73fa13b6a511 ("drm/amdkfd: CRIU Implement KFD restore ioctl") Signed-off-by: Tom Rix <trix@redhat.com> --- drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c index 965af2a08bc0..1d5f41ac3832 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c @@ -2102,7 +2102,7 @@ static int criu_restore_bos(struct kfd_process *p, const bool criu_resume = true; bool flush_tlbs = false; int ret = 0, j = 0; - uint32_t i = 0; + uint32_t i; if (*priv_offset + (args->num_bos * sizeof(*bo_privs)) > max_priv_data_size) return -EINVAL; @@ -2119,13 +2119,13 @@ static int criu_restore_bos(struct kfd_process *p, if (ret) { pr_err("Failed to copy BOs information from user\n"); ret = -EFAULT; - goto exit; + goto free_buckets; } bo_privs = kvmalloc_array(args->num_bos, sizeof(*bo_privs), GFP_KERNEL); if (!bo_privs) { ret = -ENOMEM; - goto exit; + goto free_buckets; } ret = copy_from_user(bo_privs, (void __user *)args->priv_data + *priv_offset, @@ -2133,12 +2133,12 @@ static int criu_restore_bos(struct kfd_process *p, if (ret) { pr_err("Failed to copy BOs information from user\n"); ret = -EFAULT; - goto exit; + goto free_privs; } *priv_offset += args->num_bos * sizeof(*bo_privs); /* Create and map new BOs */ - for (; i < args->num_bos; i++) { + for (i = 0; i < args->num_bos; i++) { struct kfd_criu_bo_bucket *bo_bucket; struct kfd_criu_bo_priv_data *bo_priv; struct kfd_dev *dev; @@ -2323,8 +2323,11 @@ static int criu_restore_bos(struct kfd_process *p, if (bo_buckets[i].alloc_flags & KFD_IOC_ALLOC_MEM_FLAGS_VRAM) close_fd(bo_buckets[i].dmabuf_fd); } - kvfree(bo_buckets); +free_privs: kvfree(bo_privs); +free_buckets: + kvfree(bo_buckets); + return ret; } -- 2.26.3
next reply other threads:[~2022-02-18 17:39 UTC|newest] Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-02-18 17:39 trix [this message] 2022-02-18 17:39 ` [PATCH] drm/amdkfd: rework criu_restore_bos error handling trix 2022-02-18 18:35 ` Felix Kuehling 2022-02-18 18:35 ` Felix Kuehling 2022-02-19 2:34 ` Tom Rix 2022-02-19 2:34 ` Tom Rix 2022-02-19 3:03 ` Felix Kuehling 2022-02-19 3:03 ` Felix Kuehling 2022-02-21 7:09 ` Christian König 2022-02-21 7:09 ` Christian König
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20220218173913.3376948-1-trix@redhat.com \ --to=trix@redhat.com \ --cc=Felix.Kuehling@amd.com \ --cc=Xinhui.Pan@amd.com \ --cc=airlied@linux.ie \ --cc=alexander.deucher@amd.com \ --cc=amd-gfx@lists.freedesktop.org \ --cc=christian.koenig@amd.com \ --cc=daniel@ffwll.ch \ --cc=david.yatsin@amd.com \ --cc=dri-devel@lists.freedesktop.org \ --cc=linux-kernel@vger.kernel.org \ --cc=llvm@lists.linux.dev \ --cc=nathan@kernel.org \ --cc=ndesaulniers@google.com \ --cc=rajneesh.bhardwaj@amd.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.