From: Al Viro <viro@zeniv.linux.org.uk>
To: linux-fsdevel@vger.kernel.org
Cc: "Jaroslav Kysela" <perex@perex.cz>,
"Amadeusz Sławiński" <amadeuszx.slawinski@linux.intel.com>,
"Takashi Iwai" <tiwai@suse.de>,
linux-sound@vger.kernel.org, "Vinod Koul" <vkoul@kernel.org>
Subject: [CFT][PATCH] fix descriptor uses in sound/core/compress_offload.c
Date: Thu, 26 Dec 2024 18:29:59 +0000 [thread overview]
Message-ID: <20241226182959.GU1977892@ZenIV> (raw)
[please, review and test]
1) uses of dma_buf_get() are racy - as soon as a reference has been inserted
into descriptor table, it's fair game for dup2(), etc.; we can no longer
count upon that descriptor resolving to the same file. get_dma_buf() should
be used instead (and before the insertions into table, lest we get hit with
use-after-free).
2) there's no cleanup possible past the successful dma_buf_fd() - again,
once it's in descriptor table, that's it. Just do fd_install() when
we are past all failure exits. As it is, failure in the second
dma_buf_fd() leads to task->input->file reference moved into
descriptor table *and* dropped by dma_buf_put() from snd_compr_task_free()
after goto cleanup. I.e. a dangling pointer left in descriptor table.
Frankly, dma_buf_fd() is an attractive nuisance - it's very easy to get
wrong.
Fixes: 04177158cf98 "ALSA: compress_offload: introduce accel operation mode"
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
index 86ed2fbee0c8..97526957d629 100644
--- a/sound/core/compress_offload.c
+++ b/sound/core/compress_offload.c
@@ -1026,6 +1026,7 @@ static int snd_compr_task_new(struct snd_compr_stream *stream, struct snd_compr_
{
struct snd_compr_task_runtime *task;
int retval;
+ int fd[2];
if (stream->runtime->total_tasks >= stream->runtime->fragments)
return -EBUSY;
@@ -1039,19 +1040,31 @@ static int snd_compr_task_new(struct snd_compr_stream *stream, struct snd_compr_
retval = stream->ops->task_create(stream, task);
if (retval < 0)
goto cleanup;
- utask->input_fd = dma_buf_fd(task->input, O_WRONLY|O_CLOEXEC);
- if (utask->input_fd < 0) {
- retval = utask->input_fd;
+ if (!task->input || !task->input->file ||
+ !task->output || !task->output->file) {
+ retval = -EINVAL;
goto cleanup;
}
- utask->output_fd = dma_buf_fd(task->output, O_RDONLY|O_CLOEXEC);
- if (utask->output_fd < 0) {
- retval = utask->output_fd;
+
+ fd[0] = get_unused_fd_flags(O_CLOEXEC);
+ if (unlikely(fd[0] < 0)) {
+ retval = fd[0];
+ goto cleanup;
+ }
+ fd[1] = get_unused_fd_flags(O_CLOEXEC);
+ if (unlikely(fd[1] < 0)) {
+ put_unused_fd(fd[0]);
+ retval = fd[1];
goto cleanup;
}
+
/* keep dmabuf reference until freed with task free ioctl */
- dma_buf_get(utask->input_fd);
- dma_buf_get(utask->output_fd);
+ get_dma_buf(task->input);
+ get_dma_buf(task->output);
+
+ fd_install(fd[0], task->input->file);
+ fd_install(fd[1], task->output->file);
+
list_add_tail(&task->list, &stream->runtime->tasks);
stream->runtime->total_tasks++;
return 0;
next reply other threads:[~2024-12-26 18:30 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-12-26 18:29 Al Viro [this message]
2024-12-26 19:00 ` [CFT][PATCH] fix descriptor uses in sound/core/compress_offload.c Jaroslav Kysela
2024-12-26 21:31 ` Al Viro
2024-12-26 22:17 ` Al Viro
2024-12-29 8:35 ` Takashi Iwai
2024-12-29 18:53 ` Al Viro
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=20241226182959.GU1977892@ZenIV \
--to=viro@zeniv.linux.org.uk \
--cc=amadeuszx.slawinski@linux.intel.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-sound@vger.kernel.org \
--cc=perex@perex.cz \
--cc=tiwai@suse.de \
--cc=vkoul@kernel.org \
/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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox