From patchwork Sun Jun 7 15:53:55 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Cameron X-Patchwork-Id: 11591697 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 16476913 for ; Sun, 7 Jun 2020 15:56:42 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id F3A432077D for ; Sun, 7 Jun 2020 15:56:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1591545402; bh=1ProLWL3iF3ZlhoS+5Y3dYFaj2FebM+LgdJaSC3sAJk=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=vLvVHyxyRXR/QEdWppGObJs0dBURUvAuCTujBrofFlXumJzewiUIMpiOAFWjKanTD mN68txG83GjaS5OedjKhbrxFYDyShaigXuGdhvDWJXyZNCavBVuL0eZsnmkt8gv/Jo dpX1KVtfVGr1fkTZy4sQloQfnTlbVKbBRZ3QSYl0= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726745AbgFGP4l (ORCPT ); Sun, 7 Jun 2020 11:56:41 -0400 Received: from mail.kernel.org ([198.145.29.99]:57514 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726703AbgFGP4l (ORCPT ); Sun, 7 Jun 2020 11:56:41 -0400 Received: from localhost.localdomain (cpc149474-cmbg20-2-0-cust94.5-4.cable.virginm.net [82.4.196.95]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 79B28206F6; Sun, 7 Jun 2020 15:56:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1591545400; bh=1ProLWL3iF3ZlhoS+5Y3dYFaj2FebM+LgdJaSC3sAJk=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=aW5hMztsvsquiRlppQJWKyKy4LtpaCCw3bFz+Hy4T0nMhDCt446sk3qfx/+Kq9cjG tZVRTVNzIfhXJQwuYH7t01r95SgVQhbUepZhF+y+1fPP08jKpL/+VRxn38OdClIEjx S0FdLoWxtHbpnPFg7oBe/hpyli6iaPd7n06JzToU= From: Jonathan Cameron To: linux-iio@vger.kernel.org Cc: Andy Shevchenko , Jonathan Cameron , Lars-Peter Clausen , Lorenzo Bianconi Subject: [PATCH 19/32] iio:imu:st_lsm6dsx Fix alignment and data leak issues Date: Sun, 7 Jun 2020 16:53:55 +0100 Message-Id: <20200607155408.958437-20-jic23@kernel.org> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200607155408.958437-1-jic23@kernel.org> References: <20200607155408.958437-1-jic23@kernel.org> MIME-Version: 1.0 Sender: linux-iio-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-iio@vger.kernel.org From: Jonathan Cameron One of a class of bugs pointed out by Lars in a recent review. iio_push_to_buffers_with_timestamp assumes the buffer used is aligned to the size of the timestamp (8 bytes). This is not guaranteed in this driver which uses an array of smaller elements on the stack. As Lars also noted this anti pattern can involve a leak of data to userspace and that indeed can happen here. We close both issues by moving to a set of suitable structures in the iio_priv() data. This data is allocated with kzalloc so no data can leak apart from previous readings. For the tagged path the data is aligned by using __aligned(8) for the buffer on the stack. There has been a lot of churn in this driver, so likely backports may be needed for stable. Fixes: 290a6ce11d93 ("iio: imu: add support to lsm6dsx driver") Reported-by: Lars-Peter Clausen Cc: Lorenzo Bianconi Signed-off-by: Jonathan Cameron Acked-by: Lorenzo Bianconi --- drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h | 5 +++ .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 36 ++++++++++--------- 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h index b56df409ed0f..5f821ef467da 100644 --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h @@ -411,6 +411,11 @@ struct st_lsm6dsx_hw { const struct st_lsm6dsx_settings *settings; struct iio_mount_matrix orientation; + /* Ensure natural alignment of buffer elements */ + struct { + __le16 channels[3]; + s64 ts __aligned(8); + } gyro_scan, acc_scan, ext_scan; }; static __maybe_unused const struct iio_event_spec st_lsm6dsx_event = { diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c index afd00daeefb2..bebbc2bb37f7 100644 --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c @@ -341,9 +341,6 @@ int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw) int err, sip, acc_sip, gyro_sip, ts_sip, ext_sip, read_len, offset; u16 fifo_len, pattern_len = hw->sip * ST_LSM6DSX_SAMPLE_SIZE; u16 fifo_diff_mask = hw->settings->fifo_ops.fifo_diff.mask; - u8 gyro_buff[ST_LSM6DSX_IIO_BUFF_SIZE]; - u8 acc_buff[ST_LSM6DSX_IIO_BUFF_SIZE]; - u8 ext_buff[ST_LSM6DSX_IIO_BUFF_SIZE]; bool reset_ts = false; __le16 fifo_status; s64 ts = 0; @@ -404,19 +401,22 @@ int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw) while (acc_sip > 0 || gyro_sip > 0 || ext_sip > 0) { if (gyro_sip > 0 && !(sip % gyro_sensor->decimator)) { - memcpy(gyro_buff, &hw->buff[offset], - ST_LSM6DSX_SAMPLE_SIZE); - offset += ST_LSM6DSX_SAMPLE_SIZE; + memcpy(hw->gyro_scan.channels, + &hw->buff[offset], + sizeof(hw->gyro_scan.channels)); + offset += sizeof(hw->gyro_scan.channels); } if (acc_sip > 0 && !(sip % acc_sensor->decimator)) { - memcpy(acc_buff, &hw->buff[offset], - ST_LSM6DSX_SAMPLE_SIZE); - offset += ST_LSM6DSX_SAMPLE_SIZE; + memcpy(hw->acc_scan.channels, + &hw->buff[offset], + sizeof(hw->acc_scan.channels)); + offset += sizeof(hw->acc_scan.channels); } if (ext_sip > 0 && !(sip % ext_sensor->decimator)) { - memcpy(ext_buff, &hw->buff[offset], - ST_LSM6DSX_SAMPLE_SIZE); - offset += ST_LSM6DSX_SAMPLE_SIZE; + memcpy(hw->ext_scan.channels, + &hw->buff[offset], + sizeof(hw->ext_scan.channels)); + offset += sizeof(hw->ext_scan.channels); } if (ts_sip-- > 0) { @@ -446,19 +446,22 @@ int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw) if (gyro_sip > 0 && !(sip % gyro_sensor->decimator)) { iio_push_to_buffers_with_timestamp( hw->iio_devs[ST_LSM6DSX_ID_GYRO], - gyro_buff, gyro_sensor->ts_ref + ts); + &hw->gyro_scan, + gyro_sensor->ts_ref + ts); gyro_sip--; } if (acc_sip > 0 && !(sip % acc_sensor->decimator)) { iio_push_to_buffers_with_timestamp( hw->iio_devs[ST_LSM6DSX_ID_ACC], - acc_buff, acc_sensor->ts_ref + ts); + &hw->acc_scan, + acc_sensor->ts_ref + ts); acc_sip--; } if (ext_sip > 0 && !(sip % ext_sensor->decimator)) { iio_push_to_buffers_with_timestamp( hw->iio_devs[ST_LSM6DSX_ID_EXT0], - ext_buff, ext_sensor->ts_ref + ts); + &hw->ext_scan, + ext_sensor->ts_ref + ts); ext_sip--; } sip++; @@ -543,7 +546,8 @@ int st_lsm6dsx_read_tagged_fifo(struct st_lsm6dsx_hw *hw) { u16 pattern_len = hw->sip * ST_LSM6DSX_TAGGED_SAMPLE_SIZE; u16 fifo_len, fifo_diff_mask; - u8 iio_buff[ST_LSM6DSX_IIO_BUFF_SIZE], tag; + u8 iio_buff[ST_LSM6DSX_IIO_BUFF_SIZE] __aligned(8); + u8 tag; bool reset_ts = false; int i, err, read_len; __le16 fifo_status;