[CFT][PATCH] fix descriptor uses in sound/core/compress_offload.c

[Al Viro <viro@xxxxxxxxxxxxxxxxxx>]

[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@xxxxxxxxxxxxxxxxxx>
---
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;