From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f43.google.com (mail-wr1-f43.google.com [209.85.221.43]) (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 691AC41F5C4 for ; Tue, 18 Aug 2026 08:28:09 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.43 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787041690; cv=none; b=crhIuTLsjg7EWUQwdlmxSkMDBkMXvh3vBRagLZeHN7ZGKUmXsclw4hJa/X7sowCEdWUBM0/CG1VVq+doTfsN9+KyVA28nuU6Llb6hUL2WzXN7NlFeWkA3Pzu83H2Z0g2K8ce7MI7LOQm3GqTihtBD0e2SM4HATUryrTWCVRVTTA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787041690; c=relaxed/simple; bh=5WH9YNuH8fmW2zcH2Hrx09bNNHw+SgEvOlcVgPJoLxE=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=extUr3GCJl122DTlP/6dU2mnhR930DkRLNYFAOEi09erwE2TWr+GConvoOg4g9eU/rvCZD8Rt5LV7MHecAGT4/PNkZDAtMNzdvwU130qeGOK6VpbQKG/GBo2aMpjmaiGYc2OOOo8vmMmW+cLguFpy/Xwor98umPpTeJxGoCgZJU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=cm01I/na; arc=none smtp.client-ip=209.85.221.43 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="cm01I/na" Received: by mail-wr1-f43.google.com with SMTP id ffacd0b85a97d-480001972b8so1735727f8f.2 for ; Tue, 18 Aug 2026 01:28:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1787041688; x=1787646488; darn=vger.kernel.org; h=content-transfer-encoding:content-type:mime-version:references :in-reply-to:message-id:subject:cc:to:from:date:from:to:cc:subject :date:message-id:reply-to:content-type; bh=JqARlhjnju46OOH9kkzdURfO1R0DZex76yD1hra+3Ww=; b=cm01I/naHm1V16GIP98X7umB52jbnP3Tx7ilIp1rDlfLjmy/clEdNfL0iVthzI0mn0 BdIgCduVs0hajvpdjPWazW7kVOVd41OuZ3/lbhLCzUjSVRE5eMsl18DvkAgKkIWo/v68 /82gASzLKRuxS2vMbhe6sle+mNhFmZKQxSZl5jfEadDkGT6gUQjxNfsbn7q+GDgzenJG 4lNt1IXn2JMMk8AjYQoJaWn/lukF5z7VJMPgG0EZhSvRe9FGdzMfLyNkX6s1zIhU3sB/ a31qcFf8IBXAdMxT3PJCtaK1puWVr8uMhl30QcdFG4korWjOYCNDuijPdlCJIb7tg4p8 Gi5w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1787041688; x=1787646488; h=content-transfer-encoding:content-type:mime-version:references :in-reply-to:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=JqARlhjnju46OOH9kkzdURfO1R0DZex76yD1hra+3Ww=; b=SGbIYsm5nki57A9kw9FRcjxdxtUn81fKQji+/MSlR8VlCOsECoD51iAwRPeEjmcef6 ogRrEIL/B2YuuUXRGlXD+fRjT7EW13QZi6nW2/UzVGJ7lAw3HdyB7lagqXrbyDGHr/gU quzqSDgvRye9aBBOOJ7IngJiWolzWckDKZ3RiUDM5oMLWxkjvvs3XlUfqF8Ua0wGT7GJ U9Bz29Y6kf3kOE5dVWa7jnQO2Mda9ls5lRz6hvD2LVx4YC8E+7/4JZaonzT+jH7gtrRf UWh4wqUPSNJsDS7zU+B9tf0sPx1g2JrqWtbq5XEItQjszb3nhxg15LUF5il8/+5KMvhE u71A== X-Forwarded-Encrypted: i=1; AHgh+Rq0zKh2ukp5Qodw4zJdyNMpoeMEQgJd77dL3Qtm6pKSox3ZDk2orJ2AWLbGJY5V+jvhx0T4EnXDpvaP/Eg=@vger.kernel.org X-Gm-Message-State: AOJu0YyyJCIlERGJjFN+3oe0Z9ERNbGU99VhKM6jg4fAdSN1tnw6XFFY aEXOZj42eUc9wRz6Q4mPmm/JGgmx5kjvRG8/WCVCJwLO9J4YFpe9GzUS X-Gm-Gg: AR+sD12ZtJasCs5X8WflOxN4sISMxX/tZbSg3WIH14aRSl8C7Gq1U+GDfWhgGgqfLfy fw17fcCQ/31tDEdi1yhg2gon7P9CFbFjR85NEmZCwWYo4M9dUVetdyn0ZFYvv0QjTSWIGbXim+o vZwzxYL9vOCo3M2hgl10HOZ6eAbyeQ2nghsu1ATOkSzfHuf/q8O6HKxl0OfXC4S4Ql6mex/YoQr BJrAeXiITIRj4aExd3K4Q0cC34AblfdPxkMf9mGnVCKW4Gfyp2xEyGvfL30E63XeNPJ2FMFYEuD KbvIafXxNGeLLrlU23Nhjt2omVNDz4e9hQBNf5WriNunwqo2Qguqxp4MG36h9BLCLNmtbLti7EK 5gLVlDjDqnN8QNxqbLaSWNBO1jByswscCg38DlYwYt39VM3fNPdebQnI+gu6pDmU4Dlr32R010m p81DoFpIYoIQE9W3BWHRc3aEgdSj4kOKNU5plWAwXrj4cn5ah+UtZbPIdqmFSJwTylmqzwnsWvh 2ZdOpcosTDQ333dl3cqJP286Q== X-Received: by 2002:a5d:5009:0:b0:47f:e729:c588 with SMTP id ffacd0b85a97d-482a90cd751mr8657078f8f.14.1787041687385; Tue, 18 Aug 2026 01:28:07 -0700 (PDT) Received: from pumpkin (82-69-66-36.dsl.in-addr.zen.co.uk. [82.69.66.36]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-482a5a31543sm10988285f8f.4.2026.08.18.01.28.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 18 Aug 2026 01:28:05 -0700 (PDT) Date: Tue, 18 Aug 2026 09:28:03 +0100 From: David Laight To: Noam Ben Shimon Cc: laurent.pinchart@ideasonboard.com, hansg@kernel.org, mchehab@kernel.org, linux-media@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] media: uvcvideo: Fix integer overflow in frame buffer size calculation Message-ID: <20260818092803.4f51e6dc@pumpkin> In-Reply-To: <20260807101433.54886-1-noambs2999@gmail.com> References: <20260807101433.54886-1-noambs2999@gmail.com> X-Mailer: Claws Mail 4.1.1 (GTK 3.24.38; arm-unknown-linux-gnueabihf) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit On Fri, 7 Aug 2026 13:14:33 +0300 Noam Ben Shimon wrote: > In the function uvc_parse_frame(), it recomputes > dwMaxVideoFrameBufferSize for uncompressed formats. This helps working > around devices that report it wrong: > > frame->dwMaxVideoFrameBufferSize = format->bpp * frame->wWidth > * frame->wHeight / 8; > > These three arguments originate from the device's own descriptors, and > therefore can be decided by it. bpp is a u8 and wWidth and wHeight are > u16. The expression is evaluated in int, and the maximum value is > 255 * 65535 * 65535 (which is roughly 510 times INT_MAX). > A device that declares large dimensions therefore overflows a signed > int here. > The kernel is built using -fno-strict-overflow, so this wraps rather > than being miscompiled, but the wrapped value (which is often negative) > is then divided by 8 and stored in a u32 used as a size. > > Two examples for this (using legal field values): > > - 32 bpp, 16384x4096: the product is exactly 2^31 and wraps to > INT_MIN. After division and conversion to u32 the field has the > value 4026531840 rather than 268435456. > > - 16 bpp, 16384x16384: the product is exactly 2^32 and wraps to 0. > The field holds 0, rather than the correct 536870912. > > I don't think memory corruption is a consequence of this. The value > reaches uvc_queue_setup() as the vb2 buffer size, and every copy on the > decode path is bounded by buf->length, which uvc_buffer_prepare() gets > from vb2_plane_size() rather than this field. What a wrapped value does > instead is make the driver describe the stream inconsistently. > For an uncompressed format uvc_fixup_video_ctrl() copies it into > ctrl->dwMaxVideoFrameSize unconditionally, and that becomes the > sizeimage reported by VIDIOC_G_FMT. This is while width, height and > bytesperline continue to describe the full frame. > This also makes uvc_video_validate_buffer() mark error on all frames, > because it is comparing bytesused against the same number. > > Compute the size in 64-bit, and if the result does not fit in the u32 > field then reject the frame descriptor. An uncompressed frame this > large is probably not a real device and rejection is consistent with > the other checks over malformed-descriptors in this function. > > Fixes: c0efd232929c ("V4L/DVB (8145a): USB Video Class driver") > Signed-off-by: Noam Ben Shimon > --- > drivers/media/usb/uvc/uvc_driver.c | 18 +++++++++++++++--- > 1 file changed, 15 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c > index e289cc71ba98..d319368f9d21 100644 > --- a/drivers/media/usb/uvc/uvc_driver.c > +++ b/drivers/media/usb/uvc/uvc_driver.c > @@ -296,9 +296,21 @@ static int uvc_parse_frame(struct uvc_device *dev, > * information. For uncompressed formats this can be fixed by computing > * the value from the frame size. > */ > - if (!(format->flags & UVC_FMT_FLAG_COMPRESSED)) > - frame->dwMaxVideoFrameBufferSize = format->bpp * frame->wWidth > - * frame->wHeight / 8; > + if (!(format->flags & UVC_FMT_FLAG_COMPRESSED)) { > + u64 bufsize; > + > + bufsize = (u64)format->bpp * frame->wWidth * frame->wHeight / 8; I'd bet there is a requirement that width*bpp is a multiple of 8 (or even 32)? You definitely don't want the divide rounding down! > + if (bufsize > U32_MAX) { Should that be >= ? > + uvc_dbg(dev, DESCR, > + "device %d videostreaming interface %d FRAME %u: computed buffer size overflows\n", s/buffer/frame buffer/ ? > + dev->udev->devnum, > + alts->desc.bInterfaceNumber, > + frame->bFrameIndex); I'd include the bpp, width and height values in the trace. If the error happens the first thing you need the the three values. David > + return -EINVAL; > + } > + > + frame->dwMaxVideoFrameBufferSize = bufsize; > + } > > /* > * Clamp the default frame interval to the boundaries. A zero > > base-commit: f9a2394a23482bfd330911e9c8295b71724feacd