[bug report] ALSA: compress_offload: introduce accel operation mode

[Dan Carpenter <dan.carpenter@xxxxxxxxxx>]

Hello Jaroslav Kysela,

Commit 04177158cf98 ("ALSA: compress_offload: introduce accel
operation mode") from Oct 2, 2024 (linux-next), leads to the
following Smatch static checker warning:

	sound/core/compress_offload.c:1059 snd_compr_task_new()
	warn: fd used after fd_install() 'task->input->file'

sound/core/compress_offload.c
    1025 static int snd_compr_task_new(struct snd_compr_stream *stream, struct snd_compr_task *utask)
    1026 {
    1027         struct snd_compr_task_runtime *task;
    1028         int retval;
    1029 
    1030         if (stream->runtime->total_tasks >= stream->runtime->fragments)
    1031                 return -EBUSY;
    1032         if (utask->origin_seqno != 0 || utask->input_size != 0)
    1033                 return -EINVAL;
    1034         task = kzalloc(sizeof(*task), GFP_KERNEL);
    1035         if (task == NULL)
    1036                 return -ENOMEM;
    1037         task->seqno = utask->seqno = snd_compr_seqno_next(stream);
    1038         task->input_size = utask->input_size;
    1039         retval = stream->ops->task_create(stream, task);
    1040         if (retval < 0)
    1041                 goto cleanup;
    1042         utask->input_fd = dma_buf_fd(task->input, O_WRONLY|O_CLOEXEC);

dma_buf_fd() calls fd_install() on task->input->file.  That exposes the fd
to userspace.

    1043         if (utask->input_fd < 0) {
    1044                 retval = utask->input_fd;
    1045                 goto cleanup;
    1046         }
    1047         utask->output_fd = dma_buf_fd(task->output, O_RDONLY|O_CLOEXEC);
    1048         if (utask->output_fd < 0) {
    1049                 retval = utask->output_fd;
    1050                 goto cleanup;

Let's imagine this fails.

    1051         }
    1052         /* keep dmabuf reference until freed with task free ioctl */
    1053         dma_buf_get(utask->input_fd);
    1054         dma_buf_get(utask->output_fd);
    1055         list_add_tail(&task->list, &stream->runtime->tasks);
    1056         stream->runtime->total_tasks++;
    1057         return 0;
    1058 cleanup:
--> 1059         snd_compr_task_free(task);

This calls fput() on task->input->file.  The user can mess with the fd
and make it point to something else which leads to a use after free.
I believe there are two options to deal with this.
1) Don't call fd_install() until we know we're going to succeed.
2) Just leak the fd on failure.  It will eventually get cleaned up
   when the process ends.

    1060         return retval;
    1061 }

regards,
dan carpenter