From patchwork Wed Oct 27 09:35:28 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Johan Hovold X-Patchwork-Id: 12586829 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 40886C4332F for ; Wed, 27 Oct 2021 09:37:00 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 2A971610A0 for ; Wed, 27 Oct 2021 09:37:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238954AbhJ0JjW (ORCPT ); Wed, 27 Oct 2021 05:39:22 -0400 Received: from mail.kernel.org ([198.145.29.99]:42320 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236741AbhJ0JjV (ORCPT ); Wed, 27 Oct 2021 05:39:21 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id B41AA61056; Wed, 27 Oct 2021 09:36:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1635327416; bh=A8O+IbmeT4Ni9leqpFCK7Sh5SliDMz7ykYVWOa64PM8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=TNzAfPjxky7ZjycSkbCRlLNsBmsn7qYHuJukCXyaImeI9a4NqXwCP6rxql0Uy2CN6 5lb8OpycM0+kA0IXo9iJvc3wvyV/GuWougHpDyJ3LNhmnaEb+TyNuII2TiPJKMJWLr GLRpdsJkbDCbpkB0uArV7KNn9g1/oN6agLJ/TQSDSpN99Vh0qL9lQG7Zt/giWVHeX8 9GSkuyv3q//I14VANGpdIbxYmgccXL5i2Cqc9Sv7ghotUvz7YAK0NiLKj9XVqd1r3r j2w27HJNVKpe8n6OERrNZirc2Argo5+kBkFhuTGC+vOhVoJ5QHDJXGKllQJ/prx3+8 rVneSCxfF1X7Q== Received: from johan by xi.lan with local (Exim 4.94.2) (envelope-from ) id 1mffMP-00083a-38; Wed, 27 Oct 2021 11:36:41 +0200 From: Johan Hovold To: Ian Abbott , H Hartley Sweeten Cc: Greg Kroah-Hartman , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, Johan Hovold , stable@vger.kernel.org, Luca Ellero Subject: [PATCH v2 1/2] comedi: ni_usb6501: fix NULL-deref in command paths Date: Wed, 27 Oct 2021 11:35:28 +0200 Message-Id: <20211027093529.30896-2-johan@kernel.org> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211027093529.30896-1-johan@kernel.org> References: <20211027093529.30896-1-johan@kernel.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org The driver uses endpoint-sized USB transfer buffers but had no sanity checks on the sizes. This can lead to zero-size-pointer dereferences or overflowed transfer buffers in ni6501_port_command() and ni6501_counter_command() if a (malicious) device has smaller max-packet sizes than expected (or when doing descriptor fuzz testing). Add the missing sanity checks to probe(). Fixes: a03bb00e50ab ("staging: comedi: add NI USB-6501 support") Cc: stable@vger.kernel.org # 3.18 Cc: Luca Ellero Signed-off-by: Johan Hovold Reviewed-by: Ian Abbott --- drivers/comedi/drivers/ni_usb6501.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/drivers/comedi/drivers/ni_usb6501.c b/drivers/comedi/drivers/ni_usb6501.c index 5b6d9d783b2f..c42987b74b1d 100644 --- a/drivers/comedi/drivers/ni_usb6501.c +++ b/drivers/comedi/drivers/ni_usb6501.c @@ -144,6 +144,10 @@ static const u8 READ_COUNTER_RESPONSE[] = {0x00, 0x01, 0x00, 0x10, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00}; +/* Largest supported packets */ +static const size_t TX_MAX_SIZE = sizeof(SET_PORT_DIR_REQUEST); +static const size_t RX_MAX_SIZE = sizeof(READ_PORT_RESPONSE); + enum commands { READ_PORT, WRITE_PORT, @@ -501,6 +505,12 @@ static int ni6501_find_endpoints(struct comedi_device *dev) if (!devpriv->ep_rx || !devpriv->ep_tx) return -ENODEV; + if (usb_endpoint_maxp(devpriv->ep_rx) < RX_MAX_SIZE) + return -ENODEV; + + if (usb_endpoint_maxp(devpriv->ep_tx) < TX_MAX_SIZE) + return -ENODEV; + return 0; } From patchwork Wed Oct 27 09:35:29 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Johan Hovold X-Patchwork-Id: 12586833 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 66460C433FE for ; Wed, 27 Oct 2021 09:37:01 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 4BCEF61056 for ; Wed, 27 Oct 2021 09:37:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S241235AbhJ0JjY (ORCPT ); Wed, 27 Oct 2021 05:39:24 -0400 Received: from mail.kernel.org ([198.145.29.99]:42322 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S238859AbhJ0JjV (ORCPT ); Wed, 27 Oct 2021 05:39:21 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id B79A161052; Wed, 27 Oct 2021 09:36:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1635327416; bh=w34sjWWnoH8USWkb6Lh268jrY7MkvGw0X2nThpeO9xA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=DkF4yEox6FYj5CQPjGSPS+w20CY1G4DBKhOEc4JVyouLq3zydt9aX4R81SSZW1IDl VJKYYr5iTwNq65yDsqz9EYGK1QFtYI+vMu3VE9mFdzn554rYg8u7/mYDXSHw5/8aq4 7lQXs92tcvu/a1IxngXuubjd66eSqEoyoIutS0u0uf1xjLSrX8M2N1IZlBayje6XB/ 25ro00xwNYSx12RR6YoUai48t9ylZ8FfOM/FPn7lS4fXCCI8Kp36JedTTKcuA8vJq9 0T1LHPaKn2y3R/ggWyWqXgf0LLdZH65GIcKxgCZXYXbi7CyRi50Argnr/lmqE82OSW u0q5QL40F2Qcg== Received: from johan by xi.lan with local (Exim 4.94.2) (envelope-from ) id 1mffMP-00083c-5x; Wed, 27 Oct 2021 11:36:41 +0200 From: Johan Hovold To: Ian Abbott , H Hartley Sweeten Cc: Greg Kroah-Hartman , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, Johan Hovold , stable@vger.kernel.org Subject: [PATCH v2 2/2] comedi: dt9812: fix DMA buffers on stack Date: Wed, 27 Oct 2021 11:35:29 +0200 Message-Id: <20211027093529.30896-3-johan@kernel.org> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211027093529.30896-1-johan@kernel.org> References: <20211027093529.30896-1-johan@kernel.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org USB transfer buffers are typically mapped for DMA and must not be allocated on the stack or transfers will fail. Allocate proper transfer buffers in the various command helpers and return an error on short transfers instead of acting on random stack data. Note that this also fixes a stack info leak on systems where DMA is not used as 32 bytes are always sent to the device regardless of how short the command is. Fixes: 63274cd7d38a ("Staging: comedi: add usb dt9812 driver") Cc: stable@vger.kernel.org # 2.6.29 Signed-off-by: Johan Hovold Reviewed-by: Ian Abbott --- drivers/comedi/drivers/dt9812.c | 115 ++++++++++++++++++++++++-------- 1 file changed, 86 insertions(+), 29 deletions(-) diff --git a/drivers/comedi/drivers/dt9812.c b/drivers/comedi/drivers/dt9812.c index 634f57730c1e..704b04d2980d 100644 --- a/drivers/comedi/drivers/dt9812.c +++ b/drivers/comedi/drivers/dt9812.c @@ -32,6 +32,7 @@ #include #include #include +#include #include #include "../comedi_usb.h" @@ -237,22 +238,42 @@ static int dt9812_read_info(struct comedi_device *dev, { struct usb_device *usb = comedi_to_usb_dev(dev); struct dt9812_private *devpriv = dev->private; - struct dt9812_usb_cmd cmd; + struct dt9812_usb_cmd *cmd; + size_t tbuf_size; int count, ret; + void *tbuf; - cmd.cmd = cpu_to_le32(DT9812_R_FLASH_DATA); - cmd.u.flash_data_info.address = + tbuf_size = max(sizeof(*cmd), buf_size); + + tbuf = kzalloc(tbuf_size, GFP_KERNEL); + if (!tbuf) + return -ENOMEM; + + cmd = tbuf; + + cmd->cmd = cpu_to_le32(DT9812_R_FLASH_DATA); + cmd->u.flash_data_info.address = cpu_to_le16(DT9812_DIAGS_BOARD_INFO_ADDR + offset); - cmd.u.flash_data_info.numbytes = cpu_to_le16(buf_size); + cmd->u.flash_data_info.numbytes = cpu_to_le16(buf_size); /* DT9812 only responds to 32 byte writes!! */ ret = usb_bulk_msg(usb, usb_sndbulkpipe(usb, devpriv->cmd_wr.addr), - &cmd, 32, &count, DT9812_USB_TIMEOUT); + cmd, sizeof(*cmd), &count, DT9812_USB_TIMEOUT); if (ret) - return ret; + goto out; + + ret = usb_bulk_msg(usb, usb_rcvbulkpipe(usb, devpriv->cmd_rd.addr), + tbuf, buf_size, &count, DT9812_USB_TIMEOUT); + if (!ret) { + if (count == buf_size) + memcpy(buf, tbuf, buf_size); + else + ret = -EREMOTEIO; + } +out: + kfree(tbuf); - return usb_bulk_msg(usb, usb_rcvbulkpipe(usb, devpriv->cmd_rd.addr), - buf, buf_size, &count, DT9812_USB_TIMEOUT); + return ret; } static int dt9812_read_multiple_registers(struct comedi_device *dev, @@ -261,22 +282,42 @@ static int dt9812_read_multiple_registers(struct comedi_device *dev, { struct usb_device *usb = comedi_to_usb_dev(dev); struct dt9812_private *devpriv = dev->private; - struct dt9812_usb_cmd cmd; + struct dt9812_usb_cmd *cmd; int i, count, ret; + size_t buf_size; + void *buf; - cmd.cmd = cpu_to_le32(DT9812_R_MULTI_BYTE_REG); - cmd.u.read_multi_info.count = reg_count; + buf_size = max_t(size_t, sizeof(*cmd), reg_count); + + buf = kzalloc(buf_size, GFP_KERNEL); + if (!buf) + return -ENOMEM; + + cmd = buf; + + cmd->cmd = cpu_to_le32(DT9812_R_MULTI_BYTE_REG); + cmd->u.read_multi_info.count = reg_count; for (i = 0; i < reg_count; i++) - cmd.u.read_multi_info.address[i] = address[i]; + cmd->u.read_multi_info.address[i] = address[i]; /* DT9812 only responds to 32 byte writes!! */ ret = usb_bulk_msg(usb, usb_sndbulkpipe(usb, devpriv->cmd_wr.addr), - &cmd, 32, &count, DT9812_USB_TIMEOUT); + cmd, sizeof(*cmd), &count, DT9812_USB_TIMEOUT); if (ret) - return ret; + goto out; + + ret = usb_bulk_msg(usb, usb_rcvbulkpipe(usb, devpriv->cmd_rd.addr), + buf, reg_count, &count, DT9812_USB_TIMEOUT); + if (!ret) { + if (count == reg_count) + memcpy(value, buf, reg_count); + else + ret = -EREMOTEIO; + } +out: + kfree(buf); - return usb_bulk_msg(usb, usb_rcvbulkpipe(usb, devpriv->cmd_rd.addr), - value, reg_count, &count, DT9812_USB_TIMEOUT); + return ret; } static int dt9812_write_multiple_registers(struct comedi_device *dev, @@ -285,19 +326,27 @@ static int dt9812_write_multiple_registers(struct comedi_device *dev, { struct usb_device *usb = comedi_to_usb_dev(dev); struct dt9812_private *devpriv = dev->private; - struct dt9812_usb_cmd cmd; + struct dt9812_usb_cmd *cmd; int i, count; + int ret; + + cmd = kzalloc(sizeof(*cmd), GFP_KERNEL); + if (!cmd) + return -ENOMEM; - cmd.cmd = cpu_to_le32(DT9812_W_MULTI_BYTE_REG); - cmd.u.read_multi_info.count = reg_count; + cmd->cmd = cpu_to_le32(DT9812_W_MULTI_BYTE_REG); + cmd->u.read_multi_info.count = reg_count; for (i = 0; i < reg_count; i++) { - cmd.u.write_multi_info.write[i].address = address[i]; - cmd.u.write_multi_info.write[i].value = value[i]; + cmd->u.write_multi_info.write[i].address = address[i]; + cmd->u.write_multi_info.write[i].value = value[i]; } /* DT9812 only responds to 32 byte writes!! */ - return usb_bulk_msg(usb, usb_sndbulkpipe(usb, devpriv->cmd_wr.addr), - &cmd, 32, &count, DT9812_USB_TIMEOUT); + ret = usb_bulk_msg(usb, usb_sndbulkpipe(usb, devpriv->cmd_wr.addr), + cmd, sizeof(*cmd), &count, DT9812_USB_TIMEOUT); + kfree(cmd); + + return ret; } static int dt9812_rmw_multiple_registers(struct comedi_device *dev, @@ -306,17 +355,25 @@ static int dt9812_rmw_multiple_registers(struct comedi_device *dev, { struct usb_device *usb = comedi_to_usb_dev(dev); struct dt9812_private *devpriv = dev->private; - struct dt9812_usb_cmd cmd; + struct dt9812_usb_cmd *cmd; int i, count; + int ret; + + cmd = kzalloc(sizeof(*cmd), GFP_KERNEL); + if (!cmd) + return -ENOMEM; - cmd.cmd = cpu_to_le32(DT9812_RMW_MULTI_BYTE_REG); - cmd.u.rmw_multi_info.count = reg_count; + cmd->cmd = cpu_to_le32(DT9812_RMW_MULTI_BYTE_REG); + cmd->u.rmw_multi_info.count = reg_count; for (i = 0; i < reg_count; i++) - cmd.u.rmw_multi_info.rmw[i] = rmw[i]; + cmd->u.rmw_multi_info.rmw[i] = rmw[i]; /* DT9812 only responds to 32 byte writes!! */ - return usb_bulk_msg(usb, usb_sndbulkpipe(usb, devpriv->cmd_wr.addr), - &cmd, 32, &count, DT9812_USB_TIMEOUT); + ret = usb_bulk_msg(usb, usb_sndbulkpipe(usb, devpriv->cmd_wr.addr), + cmd, sizeof(*cmd), &count, DT9812_USB_TIMEOUT); + kfree(cmd); + + return ret; } static int dt9812_digital_in(struct comedi_device *dev, u8 *bits)