On Thu, Jan 27, 2022 at 02:24:29PM +0100, Greg Kroah-Hartman wrote:
> On Thu, Jan 27, 2022 at 02:02:18PM +0100, Mathias Krause wrote:
> > If the copy back to userland fails for the FASTRPC_IOCTL_ALLOC_DMA_BUFF
> > ioctl(), we shouldn't assume that 'buf->dmabuf' is still valid. In fact,
> > dma_buf_fd() called fd_install() before, i.e. "consumed" one reference,
> > leaving us with none.
> >
> > Calling dma_buf_put() will therefore put a reference we no longer own,
> > leading to a valid file descritor table entry for an already released
> > 'file' object which is a straight use-after-free.
> >
> > Simply avoid calling dma_buf_put() and rely on the process exit code to
> > do the necessary cleanup, if needed, i.e. if the file descriptor is
> > still valid.
> >
> > Fixes: 6cffd79504ce ("misc: fastrpc: Add support for dmabuf exporter")
> > Signed-off-by: Mathias Krause <minipli@xxxxxxxxxxxxxx>
> > ---
> > drivers/misc/fastrpc.c | 9 ++++++++-
> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> > index 4ccbf43e6bfa..aa1682b94a23 100644
> > --- a/drivers/misc/fastrpc.c
> > +++ b/drivers/misc/fastrpc.c
> > @@ -1288,7 +1288,14 @@ static int fastrpc_dmabuf_alloc(struct fastrpc_user *fl, char __user *argp)
> > }
> >
> > if (copy_to_user(argp, &bp, sizeof(bp))) {
> > - dma_buf_put(buf->dmabuf);
> > + /*
> > + * The usercopy failed, but we can't do much about it, as
> > + * dma_buf_fd() already called fd_install() and made the
> > + * file descriptor accessible for the current process. It
> > + * might already be closed and dmabuf no longer valid when
> > + * we reach this point. Therefore "leak" the fd and rely on
> > + * the process exit path to do any required cleanup.
> > + */
> > return -EFAULT;
> > }
> >
>
> This feels wrong. How do all other dma buf users handle this?
>
> And you forgot to cc: the dmabuf developers, I think get_maintainers.pl
> should have caught them on this patch.
Odd, it didn't, not your fault, my apologies.
DMA BUFFER maintainers, what happened to the MAINTAINERS regex that
caused the above patch to not catch you all?
thanks,
greg k-h
