From patchwork Sun Dec 20 20:11:24 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sakari Ailus X-Patchwork-Id: 11983995 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_GIT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8F566C3526C for ; Sun, 20 Dec 2020 20:12:47 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 4E95E23107 for ; Sun, 20 Dec 2020 20:12:47 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727300AbgLTUMa (ORCPT ); Sun, 20 Dec 2020 15:12:30 -0500 Received: from mga02.intel.com ([134.134.136.20]:8342 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726584AbgLTUMa (ORCPT ); Sun, 20 Dec 2020 15:12:30 -0500 IronPort-SDR: d3nrCirMbuAKJ0+J5HzpOp6uuOKvE7X4kAmiqhY6OVWqMSj4Q9T2FEezoK+AjwiTj6iSx9GrGx q66/seukIT4Q== X-IronPort-AV: E=McAfee;i="6000,8403,9841"; a="162706637" X-IronPort-AV: E=Sophos;i="5.78,435,1599548400"; d="scan'208";a="162706637" Received: from orsmga007.jf.intel.com ([10.7.209.58]) by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Dec 2020 12:10:45 -0800 IronPort-SDR: l7wnsqon7n7xqWvJ11/MdT1mrSzpQJ/N7oAsPJrM+TK0lxUPr3aQ4LTOilMEUxqpT8jcnbceTp sq0q3q7Yq0WQ== X-IronPort-AV: E=Sophos;i="5.78,435,1599548400"; d="scan'208";a="381596029" Received: from paasikivi.fi.intel.com ([10.237.72.42]) by orsmga007-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Dec 2020 12:10:43 -0800 Received: from punajuuri.localdomain (punajuuri.localdomain [192.168.240.130]) by paasikivi.fi.intel.com (Postfix) with ESMTP id 600C420204; Sun, 20 Dec 2020 22:10:41 +0200 (EET) Received: from sailus by punajuuri.localdomain with local (Exim 4.92) (envelope-from ) id 1kr537-0003ZU-0g; Sun, 20 Dec 2020 22:11:25 +0200 From: Sakari Ailus To: linux-media@vger.kernel.org Cc: Arnd Bergmann , syzbot , Arnd Bergmann , Hans Verkuil , Laurent Pinchart , "linux-kernel@vger.kernel.org" , Mauro Carvalho Chehab , syzkaller-bugs Subject: [PATCH v2 1/1] v4l: ioctl: Fix memory leak in video_usercopy Date: Sun, 20 Dec 2020 22:11:24 +0200 Message-Id: <20201220201124.13688-1-sakari.ailus@linux.intel.com> X-Mailer: git-send-email 2.20.1 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org When an IOCTL with argument size larger than 128 that also used array arguments were handled, two memory allocations were made but alas, only the latter one of them was released. This happened because there was only a single local variable to hold such a temporary allocation. Fix this by adding separate variables to hold the pointers to the temporary allocations. Reported-by: Arnd Bergmann Reported-by: syzbot+1115e79c8df6472c612b@syzkaller.appspotmail.com Fixes: d14e6d76ebf7 ("[media] v4l: Add multi-planar ioctl handling code") Cc: stable@vger.kernel.org Signed-off-by: Sakari Ailus Reviewed-by: Laurent Pinchart --- drivers/media/v4l2-core/v4l2-ioctl.c | 32 ++++++++++++---------------- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index 3198abdd538c..9906b41004e9 100644 --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -3283,7 +3283,7 @@ video_usercopy(struct file *file, unsigned int orig_cmd, unsigned long arg, v4l2_kioctl func) { char sbuf[128]; - void *mbuf = NULL; + void *mbuf = NULL, *array_buf = NULL; void *parg = (void *)arg; long err = -EINVAL; bool has_array_args; @@ -3318,27 +3318,21 @@ video_usercopy(struct file *file, unsigned int orig_cmd, unsigned long arg, has_array_args = err; if (has_array_args) { - /* - * When adding new types of array args, make sure that the - * parent argument to ioctl (which contains the pointer to the - * array) fits into sbuf (so that mbuf will still remain - * unused up to here). - */ - mbuf = kvmalloc(array_size, GFP_KERNEL); + array_buf = kvmalloc(array_size, GFP_KERNEL); err = -ENOMEM; - if (NULL == mbuf) + if (array_buf == NULL) goto out_array_args; err = -EFAULT; if (in_compat_syscall()) - err = v4l2_compat_get_array_args(file, mbuf, user_ptr, - array_size, orig_cmd, - parg); + err = v4l2_compat_get_array_args(file, array_buf, + user_ptr, array_size, + orig_cmd, parg); else - err = copy_from_user(mbuf, user_ptr, array_size) ? + err = copy_from_user(array_buf, user_ptr, array_size) ? -EFAULT : 0; if (err) goto out_array_args; - *kernel_ptr = mbuf; + *kernel_ptr = array_buf; } /* Handles IOCTL */ @@ -3360,12 +3354,13 @@ video_usercopy(struct file *file, unsigned int orig_cmd, unsigned long arg, if (in_compat_syscall()) { int put_err; - put_err = v4l2_compat_put_array_args(file, user_ptr, mbuf, - array_size, orig_cmd, - parg); + put_err = v4l2_compat_put_array_args(file, user_ptr, + array_buf, + array_size, + orig_cmd, parg); if (put_err) err = put_err; - } else if (copy_to_user(user_ptr, mbuf, array_size)) { + } else if (copy_to_user(user_ptr, array_buf, array_size)) { err = -EFAULT; } goto out_array_args; @@ -3381,6 +3376,7 @@ video_usercopy(struct file *file, unsigned int orig_cmd, unsigned long arg, if (video_put_user((void __user *)arg, parg, cmd, orig_cmd)) err = -EFAULT; out: + kvfree(array_buf); kvfree(mbuf); return err; }