From patchwork Mon Dec 22 07:49:46 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dan Carpenter X-Patchwork-Id: 5526161 Return-Path: X-Original-To: patchwork-alsa-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 334119F30B for ; Mon, 22 Dec 2014 07:50:10 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 53FDD20160 for ; Mon, 22 Dec 2014 07:50:09 +0000 (UTC) Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) by mail.kernel.org (Postfix) with ESMTP id B1F8220176 for ; Mon, 22 Dec 2014 07:50:07 +0000 (UTC) Received: by alsa0.perex.cz (Postfix, from userid 1000) id 31C3526066A; Mon, 22 Dec 2014 08:50:06 +0100 (CET) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 Received: from alsa0.perex.cz (localhost [IPv6:::1]) by alsa0.perex.cz (Postfix) with ESMTP id 4C8F6260505; Mon, 22 Dec 2014 08:49:57 +0100 (CET) X-Original-To: alsa-devel@alsa-project.org Delivered-To: alsa-devel@alsa-project.org Received: by alsa0.perex.cz (Postfix, from userid 1000) id 9F12326065B; Mon, 22 Dec 2014 08:49:56 +0100 (CET) Received: from aserp1040.oracle.com (aserp1040.oracle.com [141.146.126.69]) by alsa0.perex.cz (Postfix) with ESMTP id 847672604EF for ; Mon, 22 Dec 2014 08:49:48 +0100 (CET) Received: from ucsinet22.oracle.com (ucsinet22.oracle.com [156.151.31.94]) by aserp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with ESMTP id sBM7nhO8013331 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 22 Dec 2014 07:49:44 GMT Received: from userz7021.oracle.com (userz7021.oracle.com [156.151.31.85]) by ucsinet22.oracle.com (8.14.5+Sun/8.14.5) with ESMTP id sBM7neEO005676 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=FAIL); Mon, 22 Dec 2014 07:49:41 GMT Received: from abhmp0016.oracle.com (abhmp0016.oracle.com [141.146.116.22]) by userz7021.oracle.com (8.14.4+Sun/8.14.4) with ESMTP id sBM7neNr004554; Mon, 22 Dec 2014 07:49:40 GMT Received: from mwanda (/154.0.139.178) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Sun, 21 Dec 2014 23:49:39 -0800 Date: Mon, 22 Dec 2014 10:49:46 +0300 From: Dan Carpenter To: Jaroslav Kysela Message-ID: <20141222074946.GA9737@mwanda> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) X-Source-IP: ucsinet22.oracle.com [156.151.31.94] Cc: Takashi Iwai , alsa-devel@alsa-project.org, kernel-janitors@vger.kernel.org, Eliot Blennerhassett Subject: [alsa-devel] [patch] ALSA: asihpi: fix an information leak in asihpi_hpi_ioctl() X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org X-Virus-Scanned: ClamAV using ClamSMTP So far as I can see "hr->h.size" is set to "res_max_size" which is a user controlled value between 12 and USHRT_MAX. If it's larger than sizeof(*hr), then that leads to an information leak. I am not very familiar with this code, my other question here is that on lines before we set "hr->h.size = sizeof(hr->h)". It think this is a bug. I also think this particular code is never executed and I added a comment to that effect. But we do it in earlier in the function as well: copy_to_user(puhr, hr, sizeof(hr->h)); It doesn't make sense to me. Anyway, I think my patch is safe and it seems to fix a real information leak. Signed-off-by: Dan Carpenter diff --git a/sound/pci/asihpi/hpioctl.c b/sound/pci/asihpi/hpioctl.c index 6aa677e..f88109a 100644 --- a/sound/pci/asihpi/hpioctl.c +++ b/sound/pci/asihpi/hpioctl.c @@ -283,6 +283,7 @@ long asihpi_hpi_ioctl(struct file *file, unsigned int cmd, unsigned long arg) goto out; } + /* FIXME: isn't this a no-op? */ if (hr->h.size > res_max_size) { HPI_DEBUG_LOG(ERROR, "response too big %d %d\n", hr->h.size, res_max_size); @@ -290,6 +291,9 @@ long asihpi_hpi_ioctl(struct file *file, unsigned int cmd, unsigned long arg) hr->h.specific_error = hr->h.size; hr->h.size = sizeof(hr->h); } + /* prevent an information leak */ + if (hr->h.size > sizeof(*hr)) + hr->h.size = sizeof(*hr); uncopied_bytes = copy_to_user(puhr, hr, hr->h.size); if (uncopied_bytes) {