Received-SPF: pass (google.com: domain of linux-kernel+bounces-37758-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
From: "Kasireddy, Vivek" <vivek.kasireddy@intel.com>
To: Andrew Davis <afd@ti.com>, Gerd Hoffmann <kraxel@redhat.com>, Sumit Semwal
	<sumit.semwal@linaro.org>, =?iso-8859-1?Q?Christian_K=F6nig?=
	<christian.koenig@amd.com>, Paul Cercueil <paul@crapouillou.net>
CC: "linaro-mm-sig@lists.linaro.org" <linaro-mm-sig@lists.linaro.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org>,
	"linux-media@vger.kernel.org" <linux-media@vger.kernel.org>
Subject: RE: [PATCH 1/3] udmabuf: Keep track current device mappings
Thread-Topic: [PATCH 1/3] udmabuf: Keep track current device mappings
Thread-Index: AQHaTklQejyxD8ekVUytWta26SfJq7DpjYjQ
Date: Wed, 24 Jan 2024 22:36:40 +0000
Message-ID: <IA0PR11MB7185C76FC248EDDC5FBC49EFF87B2@IA0PR11MB7185.namprd11.prod.outlook.com>
References: <20240123221227.868341-1-afd@ti.com>
In-Reply-To: <20240123221227.868341-1-afd@ti.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?iso-8859-1?Q?jPLRuqTEA1Kw2GxJksSXiKvkMiYvSHWrg2dBsJlTB8VUQ4xdMGr3Ojo0G2?=
 =?iso-8859-1?Q?trVc1Ijah0fG1jFyPuX2Dm8sao56vR7U4AUb8LTiEBCuWPPpc62kLestn8?=
 =?iso-8859-1?Q?/aeOV493/JuAXsZyxXFNj5OcqFQXQdEc2A8OGYpa3a3APIckXVS6zJt+rw?=
 =?iso-8859-1?Q?Q4bGla8XC2saMqI+5Mk9wix/ZDSPz1lxE9ZXekXVD+k7inLvH6JL/+fJEZ?=
 =?iso-8859-1?Q?oA3FfBDbzFfXyb5tuKihNbmqtTJTR9nqirifY43VmTqmcdLF2XYzS5k+81?=
 =?iso-8859-1?Q?ykAMhx7i5V4KKJqByhvCyhnQxLQkenpCtPsxtdbE221Iy9GMaJsCjlgr4z?=
 =?iso-8859-1?Q?7E7Jr+iWFnuIGdU1XJo8Sy/c6ka/c1t0iXYsXRRbVO5a28Me7DsCXVMBSU?=
 =?iso-8859-1?Q?bmo/NDjnape2TPhbifm6LmAFm30DSBpK58rkhFEVa0B1YZB0QOXa52QKjO?=
 =?iso-8859-1?Q?etMhgAeYAAcPWVu+8t9ZRnbkub7VCPxO9zA33goCVJdE4v+42ji5b3HwN2?=
 =?iso-8859-1?Q?z4P0jDd6ObR1UAeHbmiFU0YpkpzQxI7V6nCPwngotPw9UWUkygiooYdLFz?=
 =?iso-8859-1?Q?fBOJIjEM285DOrVid7nyzDo+WLMWRMCOJFGorSESsg1YCaGNdMyfJ4O1wD?=
 =?iso-8859-1?Q?EC0PSOuN5aoXoCwkeoD8KuacenUlUhYdimJxuU6zP+I5csdP6U+6lFG0vE?=
 =?iso-8859-1?Q?94YJPO1Z0uDfcnSP1u6wHNFC+IPLFN+OTbbBFUeTOdhbos55mj9TQOzul3?=
 =?iso-8859-1?Q?92py+mBYNvOs3tnftNEwWKZgkKiC56qzOKkZzO4K1f+/V3XVIt798Di9XP?=
 =?iso-8859-1?Q?PEOA+WLv92cDH46T44j/zE54jOl1NfRxJHNcFXhn5ACtq1iG4ip08r723R?=
 =?iso-8859-1?Q?HK06CqvihiFRveaUCYe0yBfZg3djVMnjhTackeVlBgmlvZuVs1IGK8OXOJ?=
 =?iso-8859-1?Q?qE8F7crviG0vaVvDKMQcLuW01Fh+LnpJxlZTptUu2xN/x0uDkZpWC31Wff?=
 =?iso-8859-1?Q?/aiq1SZmh0YbLQyWx44QpIU6SPGNqaofOpvlhO0ka/IOUcPn336bkTLWPG?=
 =?iso-8859-1?Q?MZZGLjtmC7bpXvi7X4NaAPdo+Glsmex/hvwoq54gOddkQo/kmOIDjLkZWn?=
 =?iso-8859-1?Q?Pv6MDOOIQ55xBEVz0S3Wd+qHng92UU0WRW2XEhF+xnOyR7dLE1UMyJo0zU?=
 =?iso-8859-1?Q?AujIOP9r8ZN8yyob0VbY1Nk/dbFQb76ViKRYoNC7c9sWCTGK+dolGktCSx?=
 =?iso-8859-1?Q?O+uPVWqlvHxKuywzT7ifSt26kDut9MwBJ4zJVBj6gam/vwcZzzWQNj2btJ?=
 =?iso-8859-1?Q?5eXqXJ8iF5w1oe9+Rd2OIDsZHz4NX5628lCFcikBs1xjhKnJosFPkZlUA4?=
 =?iso-8859-1?Q?J1jA5F7t0vfn59+KsIen9/wHj9Nxsw8ZQOcE7l7wM90G5U7am/yH43vFXM?=
 =?iso-8859-1?Q?DyzYb/bHj85PJc4ehwP5VRr9tesmVba2OzuV+Mw3+fjE3jhGnE7OtPAfO+?=
 =?iso-8859-1?Q?WAH4S3THDkx5EcIoV+0pwzVvxVICbR+Fie3GYPZBd5qDa1wHMifNeBuMfF?=
 =?iso-8859-1?Q?e1lAgSPfc1WLJ5AwcEvDqiMUpVq99Mh0g5Rh1nHuGpB+hlw1Dbpdzoz7m7?=
 =?iso-8859-1?Q?t5KfrrfdcSoNJPgCwnsh8jG5RtLafJKgWJ?=
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: IA0PR11MB7185.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 9622ae00-7b3b-454c-7a54-08dc1d2ceecf
X-MS-Exchange-CrossTenant-originalarrivaltime: 24 Jan 2024 22:36:40.2025
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: jm7OM9ozEW8uOWYgEOtAltCCfIenk0hjKXLr/KQ457jKDYifKf7m2azRzvgIEqHk0OsLGA194B8/pUjyHGNQ1rkUdWD2fW/t2uEBR2QWW+4=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA0PR11MB4592
X-OriginatorOrg: intel.com

Hi Andrew,

> When a device attaches to and maps our buffer we need to keep track
> of this mapping/device. This is needed for synchronization with these
> devices when beginning and ending CPU access for instance. Add a list
> that tracks device mappings as part of {map,unmap}_udmabuf().
>=20
> Signed-off-by: Andrew Davis <afd@ti.com>
> ---
>  drivers/dma-buf/udmabuf.c | 43
> +++++++++++++++++++++++++++++++++++++--
>  1 file changed, 41 insertions(+), 2 deletions(-)
>=20
> diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c
> index c406459996489..3a23f0a7d112a 100644
> --- a/drivers/dma-buf/udmabuf.c
> +++ b/drivers/dma-buf/udmabuf.c
> @@ -28,6 +28,14 @@ struct udmabuf {
>  	struct page **pages;
>  	struct sg_table *sg;
>  	struct miscdevice *device;
> +	struct list_head attachments;
> +	struct mutex lock;
> +};
> +
> +struct udmabuf_attachment {
> +	struct device *dev;
> +	struct sg_table *table;
> +	struct list_head list;
>  };
>=20
>  static vm_fault_t udmabuf_vm_fault(struct vm_fault *vmf)
> @@ -120,14 +128,42 @@ static void put_sg_table(struct device *dev, struct
> sg_table *sg,
>  static struct sg_table *map_udmabuf(struct dma_buf_attachment *at,
>  				    enum dma_data_direction direction)
>  {
> -	return get_sg_table(at->dev, at->dmabuf, direction);
> +	struct udmabuf *ubuf =3D at->dmabuf->priv;
> +	struct udmabuf_attachment *a;
> +
> +	a =3D kzalloc(sizeof(*a), GFP_KERNEL);
> +	if (!a)
> +		return ERR_PTR(-ENOMEM);
> +
> +	a->table =3D get_sg_table(at->dev, at->dmabuf, direction);
> +	if (IS_ERR(a->table)) {
> +		kfree(a);
> +		return a->table;
Isn't that a use-after-free bug?
Rest of the patch lgtm.

Thanks,
Vivek

> +	}
> +
> +	a->dev =3D at->dev;
> +
> +	mutex_lock(&ubuf->lock);
> +	list_add(&a->list, &ubuf->attachments);
> +	mutex_unlock(&ubuf->lock);
> +
> +	return a->table;
>  }
>=20
>  static void unmap_udmabuf(struct dma_buf_attachment *at,
>  			  struct sg_table *sg,
>  			  enum dma_data_direction direction)
>  {
> -	return put_sg_table(at->dev, sg, direction);
> +	struct udmabuf_attachment *a =3D at->priv;
> +	struct udmabuf *ubuf =3D at->dmabuf->priv;
> +
> +	mutex_lock(&ubuf->lock);
> +	list_del(&a->list);
> +	mutex_unlock(&ubuf->lock);
> +
> +	put_sg_table(at->dev, sg, direction);
> +
> +	kfree(a);
>  }
>=20
>  static void release_udmabuf(struct dma_buf *buf)
> @@ -263,6 +299,9 @@ static long udmabuf_create(struct miscdevice
> *device,
>  		memfd =3D NULL;
>  	}
>=20
> +	INIT_LIST_HEAD(&ubuf->attachments);
> +	mutex_init(&ubuf->lock);
> +
>  	exp_info.ops  =3D &udmabuf_ops;
>  	exp_info.size =3D ubuf->pagecount << PAGE_SHIFT;
>  	exp_info.priv =3D ubuf;
> --
> 2.39.2