From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mx0b-0031df01.pphosted.com (mx0b-0031df01.pphosted.com [205.220.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C412C39FCDD for ; Wed, 4 Mar 2026 14:35:45 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=205.220.180.131 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772634947; cv=none; b=tPpbMgLqPSnkqJxuj4ijgpdNVfO35OgYP3wGRp8bDJ4O4mABSsM9f6Inw3TgezALBNEzoLqmT3DcFlfZOXDUD/OBYxoouHk5QGDYw9F4W0CQjaufdnncn8UJ0J/afC8Mcc7Sp6MIC4JXjcZG7OmWOMPGa7r71Fg7AUTmygWPVxs= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772634947; c=relaxed/simple; bh=DlVuAOzgEiAgO0TM9z9Ye9In2aDnn+6t/q7TmaChSvo=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=Vsw9str6cNWj4VK7WSdGXQDKMsveTNUXt8njuq2wtpj41dhjPoctRJxeZxPmgzBTPpGDNmMEogpBYenr3ek0s8YEE7JQHiy5khJs5TLz+DrN15a60s/NR0XSpBlNDZJw+AXLmgvo1Qqe1vGaa034+X+j+u+3H8qW6rjzb0plGTQ= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=oss.qualcomm.com; spf=pass smtp.mailfrom=oss.qualcomm.com; dkim=pass (2048-bit key) header.d=qualcomm.com header.i=@qualcomm.com header.b=Vcq0XTgA; dkim=pass (2048-bit key) header.d=oss.qualcomm.com header.i=@oss.qualcomm.com header.b=JKgeNFEd; arc=none smtp.client-ip=205.220.180.131 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=oss.qualcomm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=oss.qualcomm.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=qualcomm.com header.i=@qualcomm.com header.b="Vcq0XTgA"; dkim=pass (2048-bit key) header.d=oss.qualcomm.com header.i=@oss.qualcomm.com header.b="JKgeNFEd" Received: from pps.filterd (m0279870.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.18.1.11/8.18.1.11) with ESMTP id 624Clg7m2275412 for ; Wed, 4 Mar 2026 14:35:44 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=qualcomm.com; h= cc:content-transfer-encoding:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to; s=qcppdkim1; bh= VxKyQpAO52H/UKeLl72yH06DrNEkOADt/RkzWxe9EMo=; b=Vcq0XTgAihQCmCFd N5Tvc2S9+TrbaUhusiygGxg67r3mRI8yPoBgxXfSxFaEb2tglmaugwlxTlxDIMdo M/U9Zq5rrOeQfbZk2M2g+sU3xFu40Qebah9pjReVPwLEHizXQoaYYIPLPbeKtYow vDHIt2yPlOR8nCfPKM9VxjeyyHSyoHvIbq82xDRTJJ0rcXA4jS/1ebz569gFQOfO QTZD20lpAuAY/4FUkuN4KZYbdgoy94UnoSAnqPPE0IRr4SrpPncAVPh9TJtpTqy8 lHZSTj22Ta0ROipmSICVigfpteGfPZdQyBAN9fCCUifooYit+CQQYKbuwPtz16Ib zSGFaQ== Received: from mail-pj1-f70.google.com (mail-pj1-f70.google.com [209.85.216.70]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 4cpc4mt5b8-1 (version=TLSv1.3 cipher=TLS_AES_128_GCM_SHA256 bits=128 verify=NOT) for ; Wed, 04 Mar 2026 14:35:44 +0000 (GMT) Received: by mail-pj1-f70.google.com with SMTP id 98e67ed59e1d1-354c44bf176so6410398a91.0 for ; Wed, 04 Mar 2026 06:35:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oss.qualcomm.com; s=google; t=1772634943; x=1773239743; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=VxKyQpAO52H/UKeLl72yH06DrNEkOADt/RkzWxe9EMo=; b=JKgeNFEdz7U0txCoaGfZ8U+hIT1x1VWaXQDRrjuI2WxsvIoY+nzByXabYCCQjFvukv ErC6iGZMX4VUuWavgcS41vSm024Ll2Uh73qptBVoGAinZWuUcaCU0SH59FX/Kt34JNUa ZS+BvHXnXMStxBnOK3OwsCirGFtrTfZbc2L1mtzk9JWK1QZ64OEhVrntOGkFB90CF+IA pNj5YxRgtgjF2t2ZLBGyM66U0II/Zp5J/GW8tNtW3D7BQzVsZYZzAz066RR0h12w9KIE wQXWkLfe4ex1HuzL7XNfGrymx3E7OeS8D3A+Wv5IZWoHfd5g/PQ/w+ZOoI3wxgaVzcRj WLUQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1772634943; x=1773239743; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=VxKyQpAO52H/UKeLl72yH06DrNEkOADt/RkzWxe9EMo=; b=U16XwzJVZyyfeOyMALJPQW2M8/hve39vItTVTwjjimaZqUea+eRu2vCUnFgqcDaRbA Cp3/aABw4KcKBqKhaTWlxeLP1AoSG1yKsLpDvJFKl2Vw8lMUiNVtPjcWSJquUDMexCYh xZLqTEBEt65UpIGd8LsT1OIXfTClYur5DBpCIbRXV4nZMGvx0SqGLaJ477SQtnrbcvnV SPVeg6XummSyvZKUe0y87G2aIRDZhsmI5vvREibdPIwd8vtbVK/V6sRq+UiFIaEICi40 U5fO4Um90y0yT7TPvyusWy47DwoBXIpXBoOQRweROlqdW6Twm1kbW2SsRJynz9ytIEGa Ygbw== X-Forwarded-Encrypted: i=1; AJvYcCUgwluSWlqDIYACYCE/X5h61XO3LioryxHRZFEPCZ4mJqued7BVxGjrxpVYNnUB6J0Sesk8dXw3r1nqK8M=@vger.kernel.org X-Gm-Message-State: AOJu0Yy9HtEH09j5yrymizfXi6dZIzR4xlyAdx5JFdBxEcYuX688UKJT 2va9ewgZEi0zrgYX/+QhTMd0pNdNG3cX+T4XBZYTxmbBiQIknifjH15XbxfF2rBuTb2981f+v6J W4tX9naStNmVxxunxeZGWTQYEkjKpEZnsGlc/3LYBZuDWYFPLCS3+BSWsGUx9Stixe9E= X-Gm-Gg: ATEYQzx0e8n8gVPki9bIChcUsJOt4sx0mdJ8OuoBk91wKeewmKyTPKjwAtY6bkCFzU9 4kW8Ui+qB/a7+9pVklIXcC/wOz0FUuGoKfqHcv01MMwm5BIDqPYt698cbivXbtVRH/yr3HgMLyu 06isyQbjeIk8/4PjtZkXA44q64jubujqCjxhXbBTSNg2k+NRa5RQuJjyfNt/FjFFktY2O6YmX7X l/+wG9oftghgDOTK2sIQ6HhaVZGuve7OHVZUEx46rTM5TFS1FPfdz4mbWuGc1gQwMfzqDIJkWpp YTI+tHjAS1pR1Hl6UofqN7ADHFNliR6Q9F3fvczoT5xQ5h3sUFXKdexMVBKeVoZbT+Ok3lBu7fE vW1E6tmT5FFBblUi2arECZQfR7J0Wpla1FIN8YEqKf5FYJUw= X-Received: by 2002:a17:90a:d40d:b0:354:a065:ec3e with SMTP id 98e67ed59e1d1-359a6a66920mr1865460a91.26.1772634943395; Wed, 04 Mar 2026 06:35:43 -0800 (PST) X-Received: by 2002:a17:90a:d40d:b0:354:a065:ec3e with SMTP id 98e67ed59e1d1-359a6a66920mr1865427a91.26.1772634942654; Wed, 04 Mar 2026 06:35:42 -0800 (PST) Received: from [10.206.105.210] ([202.46.23.25]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-c70fa82cf71sm16879002a12.26.2026.03.04.06.35.38 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 04 Mar 2026 06:35:42 -0800 (PST) Message-ID: Date: Wed, 4 Mar 2026 20:05:36 +0530 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v1] misc: fastrpc: Add reference counting for fastrpc_user structure To: Bjorn Andersson Cc: srini@kernel.org, linux-arm-msm@vger.kernel.org, gregkh@linuxfoundation.org, quic_bkumar@quicinc.com, linux-kernel@vger.kernel.org, quic_chennak@quicinc.com, dri-devel@lists.freedesktop.org, arnd@arndb.de, ekansh.gupta@oss.qualcomm.com, stable@kernel.org References: <20260226151121.818852-1-anandu.e@oss.qualcomm.com> <07d585fe-dfd1-41c9-9c58-b2f9893e572e@oss.qualcomm.com> Content-Language: en-US From: Anandu Krishnan E In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Proofpoint-ORIG-GUID: bO0okCuNi_CvXbmC8_JCXhKklMMcLiWZ X-Proofpoint-GUID: bO0okCuNi_CvXbmC8_JCXhKklMMcLiWZ X-Proofpoint-Spam-Details-Enc: AW1haW4tMjYwMzA0MDExOCBTYWx0ZWRfX6+V2ieHTp6hy yu3HIQoBNXdjThvqO4TQEscGXNqHB5lrEGok+zS79EmI9Zz6nwKGnTeGyMJJUj8Qe91A/2jgmLN 6ZJ8IFQ8VcSTu8mG048xes8D5UiTLqHyrKwiNwEdEv6jmFH1QNTtAOsWRtx9JHG5/+ZavqpWR07 AZFvlfLmZogISDcy9atKkuAo33fdJM7I9gZSxIMJEyZBvRKkvDoC7IWyEYugdtJDXhArU8EW9Fy 2m0z8f52KC0KSveu1fCNkcYYrWoelIWMbottPMh6PV0YLpPdbP8Q2ajAnIT4S9SiUqRjQQtCmWL HPko0a9C5RcbfAolhwd6GJcJmHjctl9nBB4Lzvq+MSXGcxTiAvPBWtRc+OixFuftzZU6ODHH8FS v7eFRWhf5GMmcNAbGBeYs4XV5mA6DaSBaTc36LwoOsNTKh4N3nqcoMpf72hp82DNnfN9Sj5T7IF q14KCvf/fMjdALCmfWg== X-Authority-Analysis: v=2.4 cv=C67kCAP+ c=1 sm=1 tr=0 ts=69a84340 cx=c_pps a=0uOsjrqzRL749jD1oC5vDA==:117 a=ZePRamnt/+rB5gQjfz0u9A==:17 a=IkcTkHD0fZMA:10 a=Yq5XynenixoA:10 a=s4-Qcg_JpJYA:10 a=VkNPw1HP01LnGYTKEx00:22 a=u7WPNUs3qKkmUXheDGA7:22 a=gowsoOTTUOVcmtlkKump:22 a=VwQbUJbxAAAA:8 a=lEOYtOyc511SuodwwMIA:9 a=3ZKOabzyN94A:10 a=QEXdDO2ut3YA:10 a=mQ_c8vxmzFEMiUWkPHU9:22 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.293,Aquarius:18.0.1143,Hydra:6.1.51,FMLib:17.12.100.49 definitions=2026-03-04_06,2026-03-03_01,2025-10-01_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 adultscore=0 clxscore=1015 phishscore=0 malwarescore=0 bulkscore=0 priorityscore=1501 spamscore=0 impostorscore=0 suspectscore=0 lowpriorityscore=0 classifier=typeunknown authscore=0 authtc= authcc= route=outbound adjust=0 reason=mlx scancount=1 engine=8.22.0-2602130000 definitions=main-2603040118 On 2/28/2026 12:44 AM, Bjorn Andersson wrote: > On Fri, Feb 27, 2026 at 07:52:00PM +0530, Anandu Krishnan E wrote: >> >> On 2/26/2026 11:20 PM, Bjorn Andersson wrote: >>> On Thu, Feb 26, 2026 at 08:41:21PM +0530, Anandu Krishnan E wrote: >>>> Add reference counting using kref to the fastrpc_user structure to >>>> prevent use-after-free issues when contexts are freed from workqueue >>>> after device release. >>> Please follow >>> https://docs.kernel.org/process/submitting-patches.html#describe-your-changes >>> and start your commit message by clearly establishing the problem, once >>> that's done you can describe the technical solution. >> sure,  will update the commit message and send as patch v2. >>>> The issue occurs when fastrpc_device_release() frees the user structure >>>> while invoke contexts are still pending in the workqueue. When the >>>> workqueue later calls fastrpc_context_free(), it attempts to access >>>> buf->fl->cctx in fastrpc_buf_free(), leading to a use-after-free: >>> But why does it do that? >>> >>> The reason why we need buf->fl->cctx in this context is because we need >>> to mask out the DMA address from the buf->dma_addr (remove the SID bits). >>> >>> If we instead split "dma_addr" into two separate members of struct >>> fastrpc_buf, one dma_addr_t dma_addr and one u64 iova_with_sid (?), then >>> it would be clear throughout the driver which address space we're >>> talking about (is it the SID-adjusted iova space or the dma_addr_t in >>> the particular DMA context). >>> >>> In addition to making this aspect of the driver easier to follow, you no >>> longer need to call fastrpc_ipa_to_dma_addr() in fastrpc_buf_free() (or >>> anywhere else for that matter). >>> >>> You can just pass buf->dma_addr directly to dma_free_coherent(). >>> >>> You're isolating the fact that the firmware needs to see "SID | >>> dma_addr" to just two places in the driver. >> I agree with the refactoring direction you’re suggesting, and >> separating the address spaces does make the driver easier >> to reason about. >> >> That said, the UAF isn’t limited to the buffer >> free path. fastrpc_context_free() also calls fastrpc_map_put(), >> which reaches fastrpc_free_map() and still dereferences fl, so >> addressing only the buffer side doesn’t fully resolve the lifetime issue. >> So the reference counting is still needed. I will update the scenario in >> commit message as well. >> > I presume you're referring to the "vmid" we need to rebuild the > src_perms for use in fastrpc_free_map()? > > I think the relevant question to ask there is if it's really a property > of the "fastrpc file context". It seems to me that we could solve that > by storing the src_perms in the fastrpc_map once we've done the > qcom_scm_assign_mem() call in fastrpc_map_attach() - so that we can free > that object without having to reach out to objects of other lifetimes. > >> If you think it makes sense, I can take the address‑space refactoring >> as a separate follow‑up patch and keep this change focused on fixing >> the lifetime issue. > The chance of you fixing one lifetime issue by introducing one or more > worries me, I'm only familiar with the driver, so I wouldn't be able to > say with confidence without investing more time fully understand the > various lifetimes. So if we're going that path, I'd like someone else to > step up and tell me that it's good. > > On the other hand, the two changes I presented above are logically > simple to make, follow, and review - and they don't complicate the > driver further. So that would still be _my_ preferred choice. I understand your concerns about the ref count approach. But with the two above changes also UAF issue will still persist. In fastrpc_free_map() : if (map->fl) {     spin_lock(&map->fl->lock);     list_del(&map->node);     spin_unlock(&map->fl->lock);     map->fl = NULL; } we are using above logic to remove map node from the list. Here also we are using fl->lock to manage the map list and this map is part of fl->maps list as well. I suggested ref count change because it was handling all the scenarios we discussed above. I am open to any alternative suggestions as well. Regards, Anandu > > [..] >>>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c >>>> index 47356a5d5804..3ababcf327d7 100644 >>>> --- a/drivers/misc/fastrpc.c >>>> +++ b/drivers/misc/fastrpc.c >>>> @@ -310,6 +310,8 @@ struct fastrpc_user { >>>> spinlock_t lock; >>>> /* lock for allocations */ >>>> struct mutex mutex; >>>> + /* Reference count */ >>>> + struct kref refcount; >>>> }; >>>> /* Extract SMMU PA from consolidated IOVA */ >>>> @@ -497,15 +499,36 @@ static void fastrpc_channel_ctx_put(struct fastrpc_channel_ctx *cctx) >>>> kref_put(&cctx->refcount, fastrpc_channel_ctx_free); >>>> } >>>> +static void fastrpc_user_free(struct kref *ref) >>>> +{ >>>> + struct fastrpc_user *fl = container_of(ref, struct fastrpc_user, refcount); >>> Unrelated question, what does the "fl" abbreviation actually mean? I >>> presume 'f' is for "fastrpc", but what is 'l'? >> fl is short for fastrpc file. >> > Hmm, okay. Thank you for clarifying. > > Regards, > Bjorn > >> Regards, >> Anandu >>> Regards, >>> Bjorn >>>> + >>>> + fastrpc_channel_ctx_put(fl->cctx); >>>> + mutex_destroy(&fl->mutex); >>>> + kfree(fl); >>>> +} >>>> + >>>> +static void fastrpc_user_get(struct fastrpc_user *fl) >>>> +{ >>>> + kref_get(&fl->refcount); >>>> +} >>>> + >>>> +static void fastrpc_user_put(struct fastrpc_user *fl) >>>> +{ >>>> + kref_put(&fl->refcount, fastrpc_user_free); >>>> +} >>>> + >>>> static void fastrpc_context_free(struct kref *ref) >>>> { >>>> struct fastrpc_invoke_ctx *ctx; >>>> struct fastrpc_channel_ctx *cctx; >>>> + struct fastrpc_user *fl; >>>> unsigned long flags; >>>> int i; >>>> ctx = container_of(ref, struct fastrpc_invoke_ctx, refcount); >>>> cctx = ctx->cctx; >>>> + fl = ctx->fl; >>>> for (i = 0; i < ctx->nbufs; i++) >>>> fastrpc_map_put(ctx->maps[i]); >>>> @@ -521,6 +544,8 @@ static void fastrpc_context_free(struct kref *ref) >>>> kfree(ctx->olaps); >>>> kfree(ctx); >>>> + /* Release the reference taken in fastrpc_context_alloc() */ >>>> + fastrpc_user_put(fl); >>>> fastrpc_channel_ctx_put(cctx); >>>> } >>>> @@ -628,6 +653,8 @@ static struct fastrpc_invoke_ctx *fastrpc_context_alloc( >>>> /* Released in fastrpc_context_put() */ >>>> fastrpc_channel_ctx_get(cctx); >>>> + /* Take a reference to user, released in fastrpc_context_free() */ >>>> + fastrpc_user_get(user); >>>> ctx->sc = sc; >>>> ctx->retval = -1; >>>> @@ -658,6 +685,7 @@ static struct fastrpc_invoke_ctx *fastrpc_context_alloc( >>>> spin_lock(&user->lock); >>>> list_del(&ctx->node); >>>> spin_unlock(&user->lock); >>>> + fastrpc_user_put(user); >>>> fastrpc_channel_ctx_put(cctx); >>>> kfree(ctx->maps); >>>> kfree(ctx->olaps); >>>> @@ -1606,11 +1634,9 @@ static int fastrpc_device_release(struct inode *inode, struct file *file) >>>> } >>>> fastrpc_session_free(cctx, fl->sctx); >>>> - fastrpc_channel_ctx_put(cctx); >>>> - >>>> - mutex_destroy(&fl->mutex); >>>> - kfree(fl); >>>> file->private_data = NULL; >>>> + /* Release the reference taken in fastrpc_device_open */ >>>> + fastrpc_user_put(fl); >>>> return 0; >>>> } >>>> @@ -1654,6 +1680,7 @@ static int fastrpc_device_open(struct inode *inode, struct file *filp) >>>> spin_lock_irqsave(&cctx->lock, flags); >>>> list_add_tail(&fl->user, &cctx->users); >>>> spin_unlock_irqrestore(&cctx->lock, flags); >>>> + kref_init(&fl->refcount); >>>> return 0; >>>> } >>>> -- >>>> 2.34.1 >>>> >>>>