From patchwork Wed Jun 10 08:33:33 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sumit Semwal X-Patchwork-Id: 187743 Delivered-To: patch@linaro.org Received: by 2002:a92:cf06:0:0:0:0:0 with SMTP id c6csp282458ilo; Wed, 10 Jun 2020 01:33:46 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxj36HNt/LY6NCpYLX0KazeUobJojPUYCF1YGB6Lp/t2fwwpCKwzld92V07boZSG+iTqr3p X-Received: by 2002:a05:6402:1d84:: with SMTP id dk4mr1599263edb.22.1591778026642; Wed, 10 Jun 2020 01:33:46 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1591778026; cv=none; d=google.com; s=arc-20160816; b=lbgGurHtX0VMMl46+d0OLuENIPXLM5wLThaU/Uw0JzbQedjnkZYS1GPusv40ryxBOw Fu+u85ER96+6nsGjC7H9M3dbE1AxnKwlWFwtxCOlUZM3xFnSyGzrAlI7YAr3UKVKGmxn ZDSg7UwLYuvaDwiVR8RCuKaIz4PBV+7LRF16qSVWxgqoD5gO2w7CyxuIIJ36Lhp51AMR 8YZJfJQhvR4zAww3Ci6Dn+sIhoJVLgBKLxR82Zy8mSsn+ynI8wTed8Sbq0cYrJkZuKMg CNnsDS/YHY078f3kbzEJQE3PsCNNEqeaq4ErsjdTT30uvxqzXgb0n+DjsCvn8t7sxVAT 3CyA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=viTnGVmiLyU57YRhDhN8zDuiG6dPY/rXax8ClQE3VoA=; b=GswzBKGzWp/JClF8e1/1bmw8VbP9rWUAglgWp8Jw1XE0wF3HiLQA8lFbeoRzMuqlu9 XSwMuhJkx3ALgJKtszIYBVVcxyP/WVCc3yKXk1Y6ve2Utm8ajQ0e7TLf36kyd9jeL0OB Nfl1CCQwKWzJSvRXzi1dZZCufaQByRSAlmM+fkihpQAD2KZh7aeHkS/ccWk1IAEc2thq CK8iGH3ZsVGF9b4AzwCHxFl4hGYK0r+PdlRjanLMdEi8bIFZJ1dW0iqD7sjJ1LHy16IO R7WQwnws/Z+WCDabXTydtgs/5uOqm4/QSzX4Jg3FGopgZpRsGpjiAPJh9PHVcJcMhgnB OnuQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=W1K849gs; spf=pass (google.com: domain of stable-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=stable-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id j1si11981844ejy.734.2020.06.10.01.33.46; Wed, 10 Jun 2020 01:33:46 -0700 (PDT) Received-SPF: pass (google.com: domain of stable-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=W1K849gs; spf=pass (google.com: domain of stable-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=stable-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726814AbgFJIdo (ORCPT + 15 others); Wed, 10 Jun 2020 04:33:44 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36668 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726794AbgFJIdn (ORCPT ); Wed, 10 Jun 2020 04:33:43 -0400 Received: from mail-pj1-x1041.google.com (mail-pj1-x1041.google.com [IPv6:2607:f8b0:4864:20::1041]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C8240C03E96F for ; Wed, 10 Jun 2020 01:33:43 -0700 (PDT) Received: by mail-pj1-x1041.google.com with SMTP id ne5so565244pjb.5 for ; Wed, 10 Jun 2020 01:33:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=viTnGVmiLyU57YRhDhN8zDuiG6dPY/rXax8ClQE3VoA=; b=W1K849gsI1lcFSTB6RepoxDrdXeWXaq0G9ZXRaizv5Xh+XgQ/C+aBDCr+Lxx3C7h4y Vo25apgtLyuhLzGSMUs+Fy1iJmYpwqcxZ4/dAMVYUWOpe1xfThgrSJzr+MMKEm0eNdAU gakVOHPK4Gg6cpkKhqYdj+ls4TpFbzvpKv8qzN6x8MOM+C3ojLHAEb1my8N1Wjlhltuq xHHla5tPWw329KPgQHTAKdZ0upvMyblU44f3ZxHoi/Q16BUHaxbNIzoE79xuvjVfG7ke OBJOvIvRzLuqpyAoPZXHM/ZS5OWKV8XRTZ5ftFnxGkSlegROCIe4wey93YwW98cK34OJ I/Ew== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=viTnGVmiLyU57YRhDhN8zDuiG6dPY/rXax8ClQE3VoA=; b=Le4fASAYD1x4MpqRX/3KvpaV32VWf6MPk2DGTgauqyu77BxE5KsiQVjkkhFOZsbny5 PVl+I1Ec/r9+eysqkrB7YuEoYgMiBlzyoUxXadyoelosI8YZ2uVRyaFhirz4pKB1N8GH sqP7QtFsohk5Sm5DMD6C+S3SoWKAishrRCr82xDedKEPt2K/4Pyhrt+jjehY2ImSBK9c KvTzokSVPu5nYIM4EyS5VEF3yUPt+iFhpHCeTOxwNaWULUs4UBNDQIrQWF3vHPdv2c4w pb7YqtDzyToh7yul/iFXDuAF29A7vXtRfs7XW+gQ/H2FWJ947fBtRcpLVNcgytYWC0qo 1cmQ== X-Gm-Message-State: AOAM530vqoTYDRr6SX0sEnLQtmbpW3QC4aeH//kJ4yArVvzjy8rn3tz4 trPJEdRyPqePZLbz6nlJgH99aQ== X-Received: by 2002:a17:90b:190e:: with SMTP id mp14mr1931900pjb.198.1591778023046; Wed, 10 Jun 2020 01:33:43 -0700 (PDT) Received: from nagraj.local ([49.206.21.239]) by smtp.gmail.com with ESMTPSA id w18sm12231562pfq.121.2020.06.10.01.33.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 10 Jun 2020 01:33:42 -0700 (PDT) From: Sumit Semwal To: linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org Cc: Chenbo Feng , linux-kernel@vger.kernel.org, linaro-mm-sig@lists.linaro.org, Charan Teja Reddy , Sumit Semwal , syzbot+3643a18836bce555bff6@syzkaller.appspotmail.com, stable@vger.kernel.org, Arnd Bergmann Subject: [PATCH] dma-buf: Move dma_buf_release() from fops to dentry_ops Date: Wed, 10 Jun 2020 14:03:33 +0530 Message-Id: <20200610083333.455-1-sumit.semwal@linaro.org> X-Mailer: git-send-email 2.27.0 MIME-Version: 1.0 Sender: stable-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org Charan Teja reported a 'use-after-free' in dmabuffs_dname [1], which happens if the dma_buf_release() is called while the userspace is accessing the dma_buf pseudo fs's dmabuffs_dname() in another process, and dma_buf_release() releases the dmabuf object when the last reference to the struct file goes away. I discussed with Arnd Bergmann, and he suggested that rather than tying the dma_buf_release() to the file_operations' release(), we can tie it to the dentry_operations' d_release(), which will be called when the last ref to the dentry is removed. The path exercised by __fput() calls f_op->release() first, and then calls dput, which eventually calls d_op->d_release(). In the 'normal' case, when no userspace access is happening via dma_buf pseudo fs, there should be exactly one fd, file, dentry and inode, so closing the fd will kill of everything right away. In the presented case, the dentry's d_release() will be called only when the dentry's last ref is released. Therefore, lets move dma_buf_release() from fops->release() to d_ops->d_release(). Many thanks to Arnd for his FS insights :) [1]: https://lore.kernel.org/patchwork/patch/1238278/ Fixes: bb2bb9030425 ("dma-buf: add DMA_BUF_SET_NAME ioctls") Reported-by: syzbot+3643a18836bce555bff6@syzkaller.appspotmail.com Cc: [5.3+] Cc: Arnd Bergmann Reported-by: Charan Teja Reddy Signed-off-by: Sumit Semwal --- drivers/dma-buf/dma-buf.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) -- 2.27.0 Reviewed-by: Arnd Bergmann diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 01ce125f8e8d..92ba4b6ef3e7 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -54,8 +54,11 @@ static char *dmabuffs_dname(struct dentry *dentry, char *buffer, int buflen) dentry->d_name.name, ret > 0 ? name : ""); } +static void dma_buf_release(struct dentry *dentry); + static const struct dentry_operations dma_buf_dentry_ops = { .d_dname = dmabuffs_dname, + .d_release = dma_buf_release, }; static struct vfsmount *dma_buf_mnt; @@ -77,14 +80,14 @@ static struct file_system_type dma_buf_fs_type = { .kill_sb = kill_anon_super, }; -static int dma_buf_release(struct inode *inode, struct file *file) +static void dma_buf_release(struct dentry *dentry) { struct dma_buf *dmabuf; - if (!is_dma_buf_file(file)) - return -EINVAL; + if (dentry->d_op != &dma_buf_dentry_ops) + return; - dmabuf = file->private_data; + dmabuf = dentry->d_fsdata; BUG_ON(dmabuf->vmapping_counter); @@ -110,7 +113,6 @@ static int dma_buf_release(struct inode *inode, struct file *file) module_put(dmabuf->owner); kfree(dmabuf->name); kfree(dmabuf); - return 0; } static int dma_buf_mmap_internal(struct file *file, struct vm_area_struct *vma) @@ -412,7 +414,6 @@ static void dma_buf_show_fdinfo(struct seq_file *m, struct file *file) } static const struct file_operations dma_buf_fops = { - .release = dma_buf_release, .mmap = dma_buf_mmap_internal, .llseek = dma_buf_llseek, .poll = dma_buf_poll,