From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f47.google.com (mail-wr1-f47.google.com [209.85.221.47]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 7B60D14B094 for ; Sun, 5 Jan 2025 23:58:44 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.47 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736121526; cv=none; b=W4Ts5bHLgMntQrrRkt0YXMkWBxi0ddKpabGyH/Uj0k0afrJ1btTu2cCSkMl0AxfAtdSGedMCig1jQZZbXUlgbsv8lahI2wgMPzOnrLep64e+hhYzZYt/wKG9nZgfdsfoUBdC+RT9XZvTi783cDKBuoaT1ToPvuC6D1zaNcUH2TA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736121526; c=relaxed/simple; bh=/oB79DLaXODS/gy0IycdhaP00aBIZCZoIFqlOL0QNUE=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=QwaXxW86NZtkSZGFC2My2+b3SUOL+jsxE6C+cyZWAEvDhvi4VDZKYY1gRbFUaGC2+zSVp5x8zHKJkjd5gEyXAQEF0I8Yp+GYyffYTHsSe3Dlk0/N6/jjZ/RYuf6SNtGwJUMj84pgHl/14lee6tFkPWV/Rp+9b4h8qGCiBrIqUAA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org; spf=pass smtp.mailfrom=linaro.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b=ZARwvtBM; arc=none smtp.client-ip=209.85.221.47 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linaro.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="ZARwvtBM" Received: by mail-wr1-f47.google.com with SMTP id ffacd0b85a97d-3862d6d5765so8112112f8f.3 for ; Sun, 05 Jan 2025 15:58:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1736121523; x=1736726323; 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=QbNsBvpoFk/1ISttIYoBxKkguUBs6TizEP3xjLgdI3c=; b=ZARwvtBMc9l9LUR3QyT8VfX+NWVX3LiEsgxZljjLUdImEa5brnRuIBhSu2deTwe7gD Muhs0bG+DkDkE+MygREuGoDMYsl8iDYxSfjP6pCPe64vuovD/k/OnbbDRHEgLLWt3vUZ 1KjXFZf7Y2gM3IU/e64iGd+bhtpWbmRJmFib2xTGUdLLl1eoIqOZmcJF1cXFdXcj2WVN 1uHd9fVtnDweF0AkImXCJeTL/Aye2dS2nZZe+nenFJiLhXouHQAjV7QBavBla5hcsQZH BlAYSRK6d4ppjAJ9OrNEQAO1IH9wDS5BYW5n+O9YXFn9Lof1E+6PVR71U5FBM2KClWIb EFFA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736121523; x=1736726323; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=QbNsBvpoFk/1ISttIYoBxKkguUBs6TizEP3xjLgdI3c=; b=seZq0AVHfl1ixTl6ZkY6hiW2GgEmpoyNAjZJdvH5oakE0m0ze/B6A4VbNp16pEqaR0 Uk8rwgVlX7MDGzNApuG6ZWmivp3k+HJ0zxwIU9k6FkhBNX2LN+P0xfQ0vY2CVwjPi4Rf VtO5nyl1NUewfeUEfKZgyszwVgtUKr7Fq2beLcz7iszl937MjPKeFfM4AC1/0HZd58+o l0/N+n2r6bxgQfF9+CnKuNl6XbKSI/6IeKpG9BsTAhIpEF18yzi2sDyIZGF2OTxQPYRg RnxpymQaDmv5jRarj28ZJilJERRAhTCr//P3p7+3iKDNthr0YUAsgODEQlWfBwUYcwCg oIyw== X-Forwarded-Encrypted: i=1; AJvYcCWq8y30ruSp4EPXWSWN3HKW4mx/PI7fT7fjJuwmzK4A+FRsCfGA8IfbsP2kKV6MtoUbjWg67y3jQ+kUcbM=@vger.kernel.org X-Gm-Message-State: AOJu0YxRyapAooKwYDm8t/oIlz/vFaoqGN5c+GtR0XH/wd26gAasImP4 U8IUwmQsQTkd7Qb846NEmWT0jSatoz+DH70vgdb6eEboKMGSh298/p+hYyUkqFAAlvWikYqfk70 7 X-Gm-Gg: ASbGncsGRQcvbh+YZRfjYfxI6xwy0S0QR86B7wQKf6RAVUZ1J10neAz0b80bwQXW7Bi roTFGFTiXEt+SqCBxjTRN411B7qLLcIxBWrDPsOhsuxC49My/4OPe9iR68Kl688iQpjFmFWwDxA aCITiTJem2qY14lsVQ9CpGXFI3PwIije7qP810uLNGobDJxg0gjB7uz5RrjNzCwuW1Tu77Jf/4a JP0TjV4dEW/aa73cqdn4VFaQdRxwjwCZp4Nzbq56Sgxn3SHSwf6Heq8gE02BTLyE7oXYQ== X-Google-Smtp-Source: AGHT+IG6j9C9E/4LQM/2K35FNp7NLqsvOtrbIvhYHwapMRYFDdicWc8WyEtxiJGPCxEYcdYg4mD91g== X-Received: by 2002:a5d:59ac:0:b0:385:db11:badf with SMTP id ffacd0b85a97d-38a221fb47amr46144030f8f.22.1736121522802; Sun, 05 Jan 2025 15:58:42 -0800 (PST) Received: from [192.168.0.43] ([176.61.106.227]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-38a1c833280sm46693179f8f.40.2025.01.05.15.58.40 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 05 Jan 2025 15:58:41 -0800 (PST) Message-ID: Date: Sun, 5 Jan 2025 23:58:40 +0000 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 1/2] media: venus: fix OOB read issue due to double read To: Vedang Nagar , Stanimir Varbanov , Vikash Garodia , Mauro Carvalho Chehab Cc: linux-media@vger.kernel.org, linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org References: <20250104-venus-security-fixes-v1-0-9d0dd4594cb4@quicinc.com> <20250104-venus-security-fixes-v1-1-9d0dd4594cb4@quicinc.com> Content-Language: en-US From: Bryan O'Donoghue In-Reply-To: <20250104-venus-security-fixes-v1-1-9d0dd4594cb4@quicinc.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 04/01/2025 05:41, Vedang Nagar wrote: > During message queue read, the address is being read twice > from the shared memory. The first read is validated against > the size of the packet, however the second read is not > being validated. A brief scan of this code doesn't really show the base case you assert here. Could you be a bit more explicit. Therefore, it's possible for firmware to > modify the value to a bigger invalid value which can lead > to OOB read access issue while reading the packet. > Added fix to reupdate the size of the packet which was > read for the first time. > > Signed-off-by: Vedang Nagar > --- > drivers/media/platform/qcom/venus/hfi_venus.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c b/drivers/media/platform/qcom/venus/hfi_venus.c > index f9437b6412b91c2483670a2b11f4fd43f3206404..64cc9e916f53e5a9c82b1ff25c4475d622ebc321 100644 > --- a/drivers/media/platform/qcom/venus/hfi_venus.c > +++ b/drivers/media/platform/qcom/venus/hfi_venus.c > @@ -298,6 +298,7 @@ static int venus_read_queue(struct venus_hfi_device *hdev, > memcpy(pkt, rd_ptr, len); > memcpy(pkt + len, queue->qmem.kva, new_rd_idx << 2); > } > + memcpy(pkt, (u32 *)(queue->qmem.kva + (rd_idx << 2)), sizeof(u32)); I'm not entirely following your reasoning here. Here's how the code looks after your change: if (new_rd_idx < qsize) { memcpy(pkt, rd_ptr, dwords << 2); } else { size_t len; new_rd_idx -= qsize; len = (dwords - new_rd_idx) << 2; memcpy(pkt, rd_ptr, len); memcpy(pkt + len, queue->qmem.kva, new_rd_idx << 2); } memcpy(pkt, (u32 *)(queue->qmem.kva + (rd_idx << 2)), sizeof(u32)); Which of the above memcpy() operations is subject to a pointer that firmware can influence exactly ? Is this a real problem you've seen if so please add a backtrace to this commit log. > } else { > /* bad packet received, dropping */ > new_rd_idx = qhdr->write_idx; > If this is a fix it requires a Fixes: tag. Please add. Still finding the reasoning you are outlining here not obvious. --- bod