Message ID | 20180517090550.GB4250@mwanda (mailing list archive) |
---|---|
State | Superseded, archived |
Delegated to: | Hans Verkuil |
Headers |
Received: from vger.kernel.org ([209.132.180.67]) by www.linuxtv.org with esmtp (Exim 4.84_2) (envelope-from <linux-media-owner@vger.kernel.org>) id 1fJErq-0000OL-2m; Thu, 17 May 2018 09:06:34 +0000 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751985AbeEQJG1 (ORCPT <rfc822;mkrufky@linuxtv.org> + 1 other); Thu, 17 May 2018 05:06:27 -0400 Received: from userp2120.oracle.com ([156.151.31.85]:59182 "EHLO userp2120.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751914AbeEQJGW (ORCPT <rfc822;linux-media@vger.kernel.org>); Thu, 17 May 2018 05:06:22 -0400 Received: from pps.filterd (userp2120.oracle.com [127.0.0.1]) by userp2120.oracle.com (8.16.0.22/8.16.0.22) with SMTP id w4H90bhT177617; Thu, 17 May 2018 09:06:09 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=date : from : to : cc : subject : message-id : mime-version : content-type; s=corp-2017-10-26; bh=Zore8WPFHconLUuRsXZR5yQUL40gEBLPfsskywJ74qk=; b=qC0AnKyGp34/lM90AZAa0L/1N18GyvLkCt5o1wjV8fV34i/EKlz5WqhaLvBTwjApxPUr bP+05Ert3cFy+9NLfzgcZawRHlLIQQnZD99Xkm15Z3rP5c+qrvCs+ArPhEAi83Spa9zs ohnSWTLeZJNliXfHfwznn7OKUnTekvi6QQp0WAZP2sBQz2piLq/S10FIRkAr2QbjzFbG 9ElapsmFyvQdCpOp21lka+UgkvdRkPglzxKKgfutbaJaINtey2QjU6CFOKOn2ji+6YpE 7zZ8RrccInGas3AiQE0bpvBT7d2/leiCPpBTAw6HIAOq0JAlXzNqCOTUus0xc5FC6Ar9 iw== Received: from userv0022.oracle.com (userv0022.oracle.com [156.151.31.74]) by userp2120.oracle.com with ESMTP id 2hx29w899j-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 17 May 2018 09:06:09 +0000 Received: from aserv0121.oracle.com (aserv0121.oracle.com [141.146.126.235]) by userv0022.oracle.com (8.14.4/8.14.4) with ESMTP id w4H968B9019282 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 17 May 2018 09:06:08 GMT Received: from abhmp0006.oracle.com (abhmp0006.oracle.com [141.146.116.12]) by aserv0121.oracle.com (8.14.4/8.13.8) with ESMTP id w4H9674R028450; Thu, 17 May 2018 09:06:07 GMT Received: from mwanda (/197.254.35.146) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Thu, 17 May 2018 02:06:07 -0700 Date: Thu, 17 May 2018 12:05:51 +0300 From: Dan Carpenter <dan.carpenter@oracle.com> To: Mauro Carvalho Chehab <mchehab@kernel.org>, Manjunath Hadli <manjunath.hadli@ti.com> Cc: Hans Verkuil <hans.verkuil@cisco.com>, Sakari Ailus <sakari.ailus@linux.intel.com>, Sylwester Nawrocki <s.nawrocki@samsung.com>, Tim Harvey <tharvey@gateworks.com>, Guennadi Liakhovetski <guennadi.liakhovetski@intel.com>, Smitha T Murthy <smitha.t@samsung.com>, Sami Tolvanen <samitolvanen@google.com>, linux-media@vger.kernel.org, kernel-janitors@vger.kernel.org Subject: [PATCH] media: v4l2-ioctl: prevent underflow in v4l_enumoutput() Message-ID: <20180517090550.GB4250@mwanda> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Mailer: git-send-email haha only kidding User-Agent: Mutt/1.9.4 (2018-02-28) X-Proofpoint-Virus-Version: vendor=nai engine=5900 definitions=8895 signatures=668698 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 malwarescore=0 phishscore=0 bulkscore=0 spamscore=0 mlxscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1711220000 definitions=main-1805170084 Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org |
Commit Message
Dan Carpenter
May 17, 2018, 9:05 a.m. UTC
My Smatch allmodconfig build only detects one function implementing
vpbe_device_ops->enum_outputs and that's vpbe_enum_outputs(). The
problem really happens in that function when we do:
int temp_index = output->index;
if (temp_index >= cfg->num_outputs)
return -EINVAL;
Unfortunately, both temp_index and cfg->num_outputs are type int so we
have a potential read before the start of the array if "temp_index" is
negative.
I could have fixed the bug in that function but it's more secure and
future proof to block that bug earlier in a central place. There is no
one who need p->index to be more than INT_MAX.
Fixes: 66715cdc3224 ("[media] davinci vpbe: VPBE display driver")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Comments
On 17/05/18 11:05, Dan Carpenter wrote: > My Smatch allmodconfig build only detects one function implementing > vpbe_device_ops->enum_outputs and that's vpbe_enum_outputs(). The > problem really happens in that function when we do: > > int temp_index = output->index; > > if (temp_index >= cfg->num_outputs) > return -EINVAL; > > Unfortunately, both temp_index and cfg->num_outputs are type int so we > have a potential read before the start of the array if "temp_index" is > negative. Why not fix it in this driver? Make num_outputs unsigned, as it should be. I really don't like having a random index check in the core. If we ever want to do such things in the core, then it needs to be implemented consistently for all ioctls that do something similar. Regards, Hans > > I could have fixed the bug in that function but it's more secure and > future proof to block that bug earlier in a central place. There is no > one who need p->index to be more than INT_MAX. > > Fixes: 66715cdc3224 ("[media] davinci vpbe: VPBE display driver") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c > index a40dbec271f1..115757ab8bc0 100644 > --- a/drivers/media/v4l2-core/v4l2-ioctl.c > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c > @@ -1099,6 +1099,9 @@ static int v4l_enumoutput(const struct v4l2_ioctl_ops *ops, > if (is_valid_ioctl(vfd, VIDIOC_S_STD)) > p->capabilities |= V4L2_OUT_CAP_STD; > > + if (p->index > INT_MAX) > + return -EINVAL; > + > return ops->vidioc_enum_output(file, fh, p); > } > >
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index a40dbec271f1..115757ab8bc0 100644 --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -1099,6 +1099,9 @@ static int v4l_enumoutput(const struct v4l2_ioctl_ops *ops, if (is_valid_ioctl(vfd, VIDIOC_S_STD)) p->capabilities |= V4L2_OUT_CAP_STD; + if (p->index > INT_MAX) + return -EINVAL; + return ops->vidioc_enum_output(file, fh, p); }