From patchwork Sun Sep 20 11:27:35 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Cameron X-Patchwork-Id: 11787465 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 C42446CB for ; Sun, 20 Sep 2020 11:28:25 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id AE43F222BB for ; Sun, 20 Sep 2020 11:28:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1600601305; bh=eCd5xjxSs8SS2HHXacJLuCqKsSdDpasJjlPPksFyf+A=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=wRbHZsIvAXvJu6v6W2GG4Wwx51B8NbTzxoCqxeG4XMK29gVvAlMh6qPE4pHzaidXv jpERIFKkshNJnPcOKrIxA/Ujp8MxG8/VQfCxsZmpI+zBBfWMM37QSDZ6xh3u3rOE5c qOiY9FXeVZ8rEYqSar2mMf84MJUreC8qiYueS9m8= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726273AbgITL2Z (ORCPT ); Sun, 20 Sep 2020 07:28:25 -0400 Received: from mail.kernel.org ([198.145.29.99]:39416 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726250AbgITL2Z (ORCPT ); Sun, 20 Sep 2020 07:28:25 -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 914F721BE5; Sun, 20 Sep 2020 11:28:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1600601304; bh=eCd5xjxSs8SS2HHXacJLuCqKsSdDpasJjlPPksFyf+A=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=qS1zgYD4h445cKNxAP38gjI8joo3INbwFGSM1hCmRFzoEyOswMLXg9x6qi+bGg0UJ dZmuH6xfQP0uq2Ks7xF9KuJ4SBTBVdzKAOuuc4FrKTig+g7cqEukPckxiehuRkmNpn pMzvziCMyEK0McVUwrAmLdzMNerbA+g3CCVKFjWk= From: Jonathan Cameron To: linux-iio@vger.kernel.org Cc: Andy Shevchenko , Jonathan Cameron , Mikko Koivunen Subject: [PATCH v4 1/8] iio:light:rpr0521: Fix timestamp alignment and prevent data leak. Date: Sun, 20 Sep 2020 12:27:35 +0100 Message-Id: <20200920112742.170751-2-jic23@kernel.org> X-Mailer: git-send-email 2.28.0 In-Reply-To: <20200920112742.170751-1-jic23@kernel.org> References: <20200920112742.170751-1-jic23@kernel.org> MIME-Version: 1.0 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 suitable structure in the iio_priv(). This data is allocated with kzalloc() so no data can leak apart from previous readings and in this case the status byte from the device. The forced alignment of ts is not necessary in this case but it potentially makes the code less fragile. From personal communications with Mikko: We could probably split the reading of the int register, but it would mean a significant performance cost of 20 i2c clock cycles. Fixes: e12ffd241c00 ("iio: light: rpr0521 triggered buffer") Cc: Mikko Koivunen Signed-off-by: Jonathan Cameron --- v4: Fixed typo. Also added to patch description Mikko's information drivers/iio/light/rpr0521.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/drivers/iio/light/rpr0521.c b/drivers/iio/light/rpr0521.c index aa2972b04833..31224a33bade 100644 --- a/drivers/iio/light/rpr0521.c +++ b/drivers/iio/light/rpr0521.c @@ -194,6 +194,17 @@ struct rpr0521_data { bool pxs_need_dis; struct regmap *regmap; + + /* + * Ensure correct naturally aligned timestamp. + * Note that the read will put garbage data into + * the padding but this should not be a problem + */ + struct { + __le16 channels[3]; + u8 garbage; + s64 ts __aligned(8); + } scan; }; static IIO_CONST_ATTR(in_intensity_scale_available, RPR0521_ALS_SCALE_AVAIL); @@ -449,8 +460,6 @@ static irqreturn_t rpr0521_trigger_consumer_handler(int irq, void *p) struct rpr0521_data *data = iio_priv(indio_dev); int err; - u8 buffer[16]; /* 3 16-bit channels + padding + ts */ - /* Use irq timestamp when reasonable. */ if (iio_trigger_using_own(indio_dev) && data->irq_timestamp) { pf->timestamp = data->irq_timestamp; @@ -461,11 +470,11 @@ static irqreturn_t rpr0521_trigger_consumer_handler(int irq, void *p) pf->timestamp = iio_get_time_ns(indio_dev); err = regmap_bulk_read(data->regmap, RPR0521_REG_PXS_DATA, - &buffer, + data->scan.channels, (3 * 2) + 1); /* 3 * 16-bit + (discarded) int clear reg. */ if (!err) iio_push_to_buffers_with_timestamp(indio_dev, - buffer, pf->timestamp); + &data->scan, pf->timestamp); else dev_err(&data->client->dev, "Trigger consumer can't read from sensor.\n"); From patchwork Sun Sep 20 11:27:36 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Cameron X-Patchwork-Id: 11787467 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 2C16E6CB for ; Sun, 20 Sep 2020 11:28:27 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 1252D221EC for ; Sun, 20 Sep 2020 11:28:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1600601307; bh=JKDqm3oz8WwgGan8wic+HdnAcdKtCjdQZz0V4syC/xc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=dyBkjxFfAz6JiaNKAoWCR8D3hQfpe38HfK4RKyiJVnwag2cWRNO6OlEp5rQjOJOe5 AJYUVDe2qsG6lNRjfMfolDM2CIcQKq3TD/7NhLg0EGU8Z1sHVl7QZx1/cqOcm8selb j4Sf1KD3dhkseViK42i4f/ORGcIY8o6vv4lhMkiE= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726279AbgITL20 (ORCPT ); Sun, 20 Sep 2020 07:28:26 -0400 Received: from mail.kernel.org ([198.145.29.99]:39452 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726250AbgITL20 (ORCPT ); Sun, 20 Sep 2020 07:28:26 -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 D79EF21D43; Sun, 20 Sep 2020 11:28:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1600601305; bh=JKDqm3oz8WwgGan8wic+HdnAcdKtCjdQZz0V4syC/xc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=OQ5bfiwtxubCnCETYxNHS5Y6LPlS48QRSv7Kdaa+NrmwTXe9GYIqLyk0Qffhv+YmV qIIvfoHrWKR/7qBsh9PW9E1HXR+UsE+IdmGCWOVIHM2rohq0mqPLsW2+Lce8qFXyce XdKncqAKrrCInYGnk/bPRiDl7YjMIB+SVM0VR9vk= From: Jonathan Cameron To: linux-iio@vger.kernel.org Cc: Andy Shevchenko , Jonathan Cameron , Lars-Peter Clausen , Lorenzo Bianconi Subject: [PATCH v4 2/8] iio:light:st_uvis25: Fix timestamp alignment and prevent data leak. Date: Sun, 20 Sep 2020 12:27:36 +0100 Message-Id: <20200920112742.170751-3-jic23@kernel.org> X-Mailer: git-send-email 2.28.0 In-Reply-To: <20200920112742.170751-1-jic23@kernel.org> References: <20200920112742.170751-1-jic23@kernel.org> MIME-Version: 1.0 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 suitable structure in the iio_priv() This data is allocated with kzalloc() so no data can leak apart from previous readings. A local unsigned int variable is used for the regmap call so it is clear there is no potential issue with writing into the padding of the structure. Fixes: 3025c8688c1e ("iio: light: add support for UVIS25 sensor") Reported-by: Lars-Peter Clausen Acked-by: Lorenzo Bianconi Signed-off-by: Jonathan Cameron --- v4: Drop the pointless masking. drivers/iio/light/st_uvis25.h | 5 +++++ drivers/iio/light/st_uvis25_core.c | 8 +++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/drivers/iio/light/st_uvis25.h b/drivers/iio/light/st_uvis25.h index 78bc56aad129..283086887caf 100644 --- a/drivers/iio/light/st_uvis25.h +++ b/drivers/iio/light/st_uvis25.h @@ -27,6 +27,11 @@ struct st_uvis25_hw { struct iio_trigger *trig; bool enabled; int irq; + /* Ensure timestamp is naturally aligned */ + struct { + u8 chan; + s64 ts __aligned(8); + } scan; }; extern const struct dev_pm_ops st_uvis25_pm_ops; diff --git a/drivers/iio/light/st_uvis25_core.c b/drivers/iio/light/st_uvis25_core.c index a18a82e6bbf5..1055594b2276 100644 --- a/drivers/iio/light/st_uvis25_core.c +++ b/drivers/iio/light/st_uvis25_core.c @@ -232,17 +232,19 @@ static const struct iio_buffer_setup_ops st_uvis25_buffer_ops = { static irqreturn_t st_uvis25_buffer_handler_thread(int irq, void *p) { - u8 buffer[ALIGN(sizeof(u8), sizeof(s64)) + sizeof(s64)]; struct iio_poll_func *pf = p; struct iio_dev *iio_dev = pf->indio_dev; struct st_uvis25_hw *hw = iio_priv(iio_dev); + unsigned int val; int err; - err = regmap_read(hw->regmap, ST_UVIS25_REG_OUT_ADDR, (int *)buffer); + err = regmap_read(hw->regmap, ST_UVIS25_REG_OUT_ADDR, &val); if (err < 0) goto out; - iio_push_to_buffers_with_timestamp(iio_dev, buffer, + hw->scan.chan = val; + + iio_push_to_buffers_with_timestamp(iio_dev, &hw->scan, iio_get_time_ns(iio_dev)); out: From patchwork Sun Sep 20 11:27:37 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Cameron X-Patchwork-Id: 11787469 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 922ED6CB for ; Sun, 20 Sep 2020 11:28:28 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 7992A235F9 for ; Sun, 20 Sep 2020 11:28:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1600601308; bh=k2jtqCM5EMsFrg6mZaM/3bhv5y3IDsm43+pl2qUen6c=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=KDwOzXq9aLwDug4d5WPB60tSmvdwLu0exG8FJMo4HbZvwpuObzhZpgJpTDu+J+P12 3yJmsPBKY/oDXPvo601MzRA55CHyCB78kCpEUBixlydBekPT+FHm2Iq4rhmYF/k7Nf i61HZF11z5VINrJhnmcQ59kDOa4+iXF0dCoVC890= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726290AbgITL22 (ORCPT ); Sun, 20 Sep 2020 07:28:28 -0400 Received: from mail.kernel.org ([198.145.29.99]:39466 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726250AbgITL21 (ORCPT ); Sun, 20 Sep 2020 07:28:27 -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 56565222BB; Sun, 20 Sep 2020 11:28:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1600601307; bh=k2jtqCM5EMsFrg6mZaM/3bhv5y3IDsm43+pl2qUen6c=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=l8iS+Eciq0eKBceh2kz40I4fV1SyG41f/B0l193XWgXnuILAbicScUttn70vizU/A ivov9zkbyG1FljvOIs3jCqe6fnp/4UrG7cLBKFRXkLo9mDFkyHkocg8S4db9iBruEH yicBoCfvRgPwmYmWvsDMAkxzGcGICl8FiczveSq0= From: Jonathan Cameron To: linux-iio@vger.kernel.org Cc: Andy Shevchenko , Jonathan Cameron , Lars-Peter Clausen Subject: [PATCH v4 3/8] iio:magnetometer:mag3110: Fix alignment and data leak issues. Date: Sun, 20 Sep 2020 12:27:37 +0100 Message-Id: <20200920112742.170751-4-jic23@kernel.org> X-Mailer: git-send-email 2.28.0 In-Reply-To: <20200920112742.170751-1-jic23@kernel.org> References: <20200920112742.170751-1-jic23@kernel.org> MIME-Version: 1.0 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 suitable structure in the iio_priv() data. This data is allocated with kzalloc() so no data can leak apart from previous readings. The explicit alignment of ts is not necessary in this case but does make the code slightly less fragile so I have included it. Fixes: 39631b5f9584 ("iio: Add Freescale mag3110 magnetometer driver") Reported-by: Lars-Peter Clausen Signed-off-by: Jonathan Cameron --- v4: Rename element to temperature to avoid confusion. drivers/iio/magnetometer/mag3110.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/drivers/iio/magnetometer/mag3110.c b/drivers/iio/magnetometer/mag3110.c index 838b13c8bb3d..c96415a1aead 100644 --- a/drivers/iio/magnetometer/mag3110.c +++ b/drivers/iio/magnetometer/mag3110.c @@ -56,6 +56,12 @@ struct mag3110_data { int sleep_val; struct regulator *vdd_reg; struct regulator *vddio_reg; + /* Ensure natural alignment of timestamp */ + struct { + __be16 channels[3]; + u8 temperature; + s64 ts __aligned(8); + } scan; }; static int mag3110_request(struct mag3110_data *data) @@ -387,10 +393,9 @@ static irqreturn_t mag3110_trigger_handler(int irq, void *p) struct iio_poll_func *pf = p; struct iio_dev *indio_dev = pf->indio_dev; struct mag3110_data *data = iio_priv(indio_dev); - u8 buffer[16]; /* 3 16-bit channels + 1 byte temp + padding + ts */ int ret; - ret = mag3110_read(data, (__be16 *) buffer); + ret = mag3110_read(data, data->scan.channels); if (ret < 0) goto done; @@ -399,10 +404,10 @@ static irqreturn_t mag3110_trigger_handler(int irq, void *p) MAG3110_DIE_TEMP); if (ret < 0) goto done; - buffer[6] = ret; + data->scan.temperature = ret; } - iio_push_to_buffers_with_timestamp(indio_dev, buffer, + iio_push_to_buffers_with_timestamp(indio_dev, &data->scan, iio_get_time_ns(indio_dev)); done: From patchwork Sun Sep 20 11:27:38 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Cameron X-Patchwork-Id: 11787471 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 786286CA for ; Sun, 20 Sep 2020 11:28:31 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 57EDB235FA for ; Sun, 20 Sep 2020 11:28:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1600601311; bh=fm7sgvvsHrieUik4TkM1qthsRkZi6ZzJbpsGAr/QaxI=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=CKnL4UmZgPOECP08BQuxLdZjqswx5fB4s3L+aug7qoUs165AF4Cvo3mgPQ4D+EFLP 2BECMgbruWD7zotHYALGdAEChevNSlE+4nS7TriyEmLiZyD3advvWiqTr9zdDfkS6a 9Dbx0TtAqyBcH1pkviccfFy0Tqr094fsh1lp4ruM= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726293AbgITL2a (ORCPT ); Sun, 20 Sep 2020 07:28:30 -0400 Received: from mail.kernel.org ([198.145.29.99]:39478 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726250AbgITL23 (ORCPT ); Sun, 20 Sep 2020 07:28:29 -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 B53C1221EC; Sun, 20 Sep 2020 11:28:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1600601308; bh=fm7sgvvsHrieUik4TkM1qthsRkZi6ZzJbpsGAr/QaxI=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=A2taVisEOEWaeYEiAbNQCEOtXVPivmsTYGy2FhU5ItM8+mlRdqQSuFeXS9qrHZGP5 c0vR05JLobfQaSpwS2D2Li7rNKLemTXcufh5IYrLQaxwYK90JkUdCM+2hRm0mlBfnq IteUBwHPvjtHoYTa8RAG0muxNzl8iDbJbR0jxjnc= From: Jonathan Cameron To: linux-iio@vger.kernel.org Cc: Andy Shevchenko , Jonathan Cameron , Daniel Baluta Subject: [PATCH v4 4/8] iio:imu:bmi160: Fix too large a buffer. Date: Sun, 20 Sep 2020 12:27:38 +0100 Message-Id: <20200920112742.170751-5-jic23@kernel.org> X-Mailer: git-send-email 2.28.0 In-Reply-To: <20200920112742.170751-1-jic23@kernel.org> References: <20200920112742.170751-1-jic23@kernel.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-iio@vger.kernel.org From: Jonathan Cameron The comment implies this device has 3 sensor types, but it only has an accelerometer and a gyroscope (both 3D). As such the buffer does not need to be as long as stated. Note I've separated this from the following patch which fixes the alignment for passing to iio_push_to_buffers_with_timestamp() as they are different issues even if they affect the same line of code. Signed-off-by: Jonathan Cameron Cc: Daniel Baluta --- v4: New patch drivers/iio/imu/bmi160/bmi160_core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/iio/imu/bmi160/bmi160_core.c b/drivers/iio/imu/bmi160/bmi160_core.c index 222ebb26f013..39fea58dd308 100644 --- a/drivers/iio/imu/bmi160/bmi160_core.c +++ b/drivers/iio/imu/bmi160/bmi160_core.c @@ -427,8 +427,8 @@ static irqreturn_t bmi160_trigger_handler(int irq, void *p) struct iio_poll_func *pf = p; struct iio_dev *indio_dev = pf->indio_dev; struct bmi160_data *data = iio_priv(indio_dev); - __le16 buf[16]; - /* 3 sens x 3 axis x __le16 + 3 x __le16 pad + 4 x __le16 tstamp */ + __le16 buf[12]; + /* 2 sens x 3 axis x __le16 + 2 x __le16 pad + 4 x __le16 tstamp */ int i, ret, j = 0, base = BMI160_REG_DATA_MAGN_XOUT_L; __le16 sample; From patchwork Sun Sep 20 11:27:39 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Cameron X-Patchwork-Id: 11787473 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 827BB6CB for ; Sun, 20 Sep 2020 11:28:32 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6ABFC21BE5 for ; Sun, 20 Sep 2020 11:28:32 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1600601312; bh=qKkiifeQgZBculEoovyDxk+Ss7PduJetxuA0LhIeAI8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=lynMo6JR5e/LUWfnKG7+938qrWtIHffSkhds+PpoLVcl3XGcya4Hb2V6OdukXrao/ h+rNAkxeg5N9tgnlPG03bBhPcjAdZxaBaszEWG4HyMznCh2qHbrVWkQ3IxxNTTqLmr szhOk4wkfEoYntmd3aT/4GmZJdiRqWsDZ+gHs5Xw= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726250AbgITL2b (ORCPT ); Sun, 20 Sep 2020 07:28:31 -0400 Received: from mail.kernel.org ([198.145.29.99]:39486 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726285AbgITL2b (ORCPT ); Sun, 20 Sep 2020 07:28:31 -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 1AD5B23119; Sun, 20 Sep 2020 11:28:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1600601310; bh=qKkiifeQgZBculEoovyDxk+Ss7PduJetxuA0LhIeAI8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=MuzWAxBvhRuTC9rYMPpMEd0amHWQ8AoNpplyG4V8y/OSnPF8sQD4Rp3NkGGysqLa0 SCVGB0YN+0AAvzSX/tVxvos91VmBtPO1e7A/B0Zr6IvQOxXTPotOp49w9izIvi0X8y 7F5T6rXfVS+RF7QT1qJ5nYJlV5poTJagHXkKzgt0= From: Jonathan Cameron To: linux-iio@vger.kernel.org Cc: Andy Shevchenko , Jonathan Cameron , Lars-Peter Clausen , Daniel Baluta , Daniel Baluta Subject: [PATCH v4 5/8] iio:imu:bmi160: Fix alignment and data leak issues Date: Sun, 20 Sep 2020 12:27:39 +0100 Message-Id: <20200920112742.170751-6-jic23@kernel.org> X-Mailer: git-send-email 2.28.0 In-Reply-To: <20200920112742.170751-1-jic23@kernel.org> References: <20200920112742.170751-1-jic23@kernel.org> MIME-Version: 1.0 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 suitable array in the iio_priv() data with alignment explicitly requested. This data is allocated with kzalloc() so no data can leak apart from previous readings. In this driver, depending on which channels are enabled, the timestamp can be in a number of locations. Hence we cannot use a structure to specify the data layout without it being misleading. Fixes: 77c4ad2d6a9b ("iio: imu: Add initial support for Bosch BMI160") Reported-by: Lars-Peter Clausen Signed-off-by: Jonathan Cameron Cc: Daniel Baluta Cc: Daniel Baluta --- v4: Improve comments and carry forwards shorter length. drivers/iio/imu/bmi160/bmi160.h | 7 +++++++ drivers/iio/imu/bmi160/bmi160_core.c | 6 ++---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/drivers/iio/imu/bmi160/bmi160.h b/drivers/iio/imu/bmi160/bmi160.h index a82e040bd109..32c2ea2d7112 100644 --- a/drivers/iio/imu/bmi160/bmi160.h +++ b/drivers/iio/imu/bmi160/bmi160.h @@ -10,6 +10,13 @@ struct bmi160_data { struct iio_trigger *trig; struct regulator_bulk_data supplies[2]; struct iio_mount_matrix orientation; + /* + * Ensure natural alignment for timestamp if present. + * Max length needed: 2 * 3 channels + 4 bytes padding + 8 byte ts. + * If fewer channels are enabled, less space may be needed, as + * long as the timestamp is still aligned to 8 bytes. + */ + __le16 buf[12] __aligned(8); }; extern const struct regmap_config bmi160_regmap_config; diff --git a/drivers/iio/imu/bmi160/bmi160_core.c b/drivers/iio/imu/bmi160/bmi160_core.c index 39fea58dd308..82f03a4dc47a 100644 --- a/drivers/iio/imu/bmi160/bmi160_core.c +++ b/drivers/iio/imu/bmi160/bmi160_core.c @@ -427,8 +427,6 @@ static irqreturn_t bmi160_trigger_handler(int irq, void *p) struct iio_poll_func *pf = p; struct iio_dev *indio_dev = pf->indio_dev; struct bmi160_data *data = iio_priv(indio_dev); - __le16 buf[12]; - /* 2 sens x 3 axis x __le16 + 2 x __le16 pad + 4 x __le16 tstamp */ int i, ret, j = 0, base = BMI160_REG_DATA_MAGN_XOUT_L; __le16 sample; @@ -438,10 +436,10 @@ static irqreturn_t bmi160_trigger_handler(int irq, void *p) &sample, sizeof(sample)); if (ret) goto done; - buf[j++] = sample; + data->buf[j++] = sample; } - iio_push_to_buffers_with_timestamp(indio_dev, buf, pf->timestamp); + iio_push_to_buffers_with_timestamp(indio_dev, data->buf, pf->timestamp); done: iio_trigger_notify_done(indio_dev->trig); return IRQ_HANDLED; From patchwork Sun Sep 20 11:27:40 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Cameron X-Patchwork-Id: 11787475 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 28252139F for ; Sun, 20 Sep 2020 11:28:33 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 10D5421BE5 for ; Sun, 20 Sep 2020 11:28:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1600601313; bh=FMt/MWnMDBB5Me8UPa7JHiF4FYkqXlGcj/+y+o7MUoM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=bLfG+duFjFcYYfsN4NWX9AFHCnUwqz9t8u82IZcCPYu+YwVu7Pu+RSW4uBF+eQNZD NMDOnLXsptkH8LK3PRDDpvFR1EBtV+RNuNeS3r3C6a3UFBW5cSgY4eIDgfWaMXH40p m0OJbMCvq9seB1ly0Y8a6xe1pfFpW1ke1S/BwxFI= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726311AbgITL2c (ORCPT ); Sun, 20 Sep 2020 07:28:32 -0400 Received: from mail.kernel.org ([198.145.29.99]:39500 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726285AbgITL2c (ORCPT ); Sun, 20 Sep 2020 07:28:32 -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 B5C7A216C4; Sun, 20 Sep 2020 11:28:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1600601311; bh=FMt/MWnMDBB5Me8UPa7JHiF4FYkqXlGcj/+y+o7MUoM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=hZvaPFwjzlLDwGM/3f+QzRqMOnE0PXjkhxLn0zegn4SSMRo9Zc+t4xYc4P3a/WaPu qXoO1ykiLVCzz4ufjv93Fk2dMi98OPJelSoqR9CZ5vYz/A4OshrCCmNguny3s1ERaQ SULgMILtszJhkqe/vMQU4hP1r/a0ygvmfKGHvqng= From: Jonathan Cameron To: linux-iio@vger.kernel.org Cc: Andy Shevchenko , Jonathan Cameron , Lars-Peter Clausen , Peter Meerwald Subject: [PATCH v4 6/8] iio:pressure:mpl3115: Force alignment of buffer Date: Sun, 20 Sep 2020 12:27:40 +0100 Message-Id: <20200920112742.170751-7-jic23@kernel.org> X-Mailer: git-send-email 2.28.0 In-Reply-To: <20200920112742.170751-1-jic23@kernel.org> References: <20200920112742.170751-1-jic23@kernel.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-iio@vger.kernel.org From: Jonathan Cameron Whilst this is another case of the issue Lars reported with an array of elements of smaller than 8 bytes being passed to iio_push_to_buffers_with_timestamp(), the solution here is a bit different from the other cases and relies on __aligned working on the stack (true since 4.6?) This one is unusual. We have to do an explicit memset() each time as we are reading 3 bytes into a potential 4 byte channel which may sometimes be a 2 byte channel depending on what is enabled. As such, moving the buffer to the heap in the iio_priv structure doesn't save us much. We can't use a nice explicit structure on the stack either as the data channels have different storage sizes and are all separately controlled. Fixes: cc26ad455f57 ("iio: Add Freescale MPL3115A2 pressure / temperature sensor driver") Reported-by: Lars-Peter Clausen Signed-off-by: Jonathan Cameron Reviewed-by: Andy Shevchenko Cc: Peter Meerwald --- v4: Added a 'special' comment as this one is unique. drivers/iio/pressure/mpl3115.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/iio/pressure/mpl3115.c b/drivers/iio/pressure/mpl3115.c index ccdb0b70e48c..1eb9e7b29e05 100644 --- a/drivers/iio/pressure/mpl3115.c +++ b/drivers/iio/pressure/mpl3115.c @@ -144,7 +144,14 @@ static irqreturn_t mpl3115_trigger_handler(int irq, void *p) struct iio_poll_func *pf = p; struct iio_dev *indio_dev = pf->indio_dev; struct mpl3115_data *data = iio_priv(indio_dev); - u8 buffer[16]; /* 32-bit channel + 16-bit channel + padding + ts */ + /* + * 32-bit channel + 16-bit channel + padding + ts + * Note that it is possible for only one of the first 2 + * channels to be enabled. If that happens, the first element + * of the buffer may be either 16 or 32-bits. As such we cannot + * use a simple structure definition to express this data layout. + */ + u8 buffer[16] __aligned(8); int ret, pos = 0; mutex_lock(&data->lock); From patchwork Sun Sep 20 11:27:41 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Cameron X-Patchwork-Id: 11787477 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 2EF906CA for ; Sun, 20 Sep 2020 11:28:34 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 17CCA235FA for ; Sun, 20 Sep 2020 11:28:34 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1600601314; bh=z73AgK2yfgNYXkFZdruydRBLaEMHDZNllP71Srw8ugY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=tuEq2CVfMq3RiI90iRUPo0GYBiDqKWlUC1XBFQ5qMEuNjFHxu4KatwzrfLmy9Ftyy h1ecEvYszJTpWtP8HhVZ7yllt4zkbfk/bXcTltkwRxO3Ii6Zje15IWzEyAZMCtxGIJ fh3ZjxlF5JDgVG2p0BDOahdKntAxtF4YxqTXsYGg= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726314AbgITL2d (ORCPT ); Sun, 20 Sep 2020 07:28:33 -0400 Received: from mail.kernel.org ([198.145.29.99]:39516 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726285AbgITL2d (ORCPT ); Sun, 20 Sep 2020 07:28:33 -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 2C9A820EDD; Sun, 20 Sep 2020 11:28:32 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1600601313; bh=z73AgK2yfgNYXkFZdruydRBLaEMHDZNllP71Srw8ugY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=ZZOirp5c4ZFjiY1uyJESkNFD+8++UcVihYOid3PIqc/UGJWA2HYyZykVtnm6Lk/cE QyIDNeOuMi8SB/6/MK4AFDlY5D97mvdJdZvRNiCZ5ScY5bX2pifCQdM/HXBDkNr26q fh6HR4RVYcLf/Hd2XMnPw+VmloMDS1TM/ZmLNd1k= From: Jonathan Cameron To: linux-iio@vger.kernel.org Cc: Andy Shevchenko , Jonathan Cameron , Dan Murphy Subject: [PATCH v4 7/8] iio:adc:ti-ads124s08: Fix buffer being too long. Date: Sun, 20 Sep 2020 12:27:41 +0100 Message-Id: <20200920112742.170751-8-jic23@kernel.org> X-Mailer: git-send-email 2.28.0 In-Reply-To: <20200920112742.170751-1-jic23@kernel.org> References: <20200920112742.170751-1-jic23@kernel.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-iio@vger.kernel.org From: Jonathan Cameron The buffer is expressed as a u32 array, yet the extra space for the s64 timestamp was expressed as sizeof(s64)/sizeof(u16). This will result in 2 extra u32 elements. Fix by dividing by sizeof(u32). Fixes: e717f8c6dfec ("iio: adc: Add the TI ads124s08 ADC code") Cc: Dan Murphy Signed-off-by: Jonathan Cameron --- v4: New patch drivers/iio/adc/ti-ads124s08.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iio/adc/ti-ads124s08.c b/drivers/iio/adc/ti-ads124s08.c index 4b4fbe33930c..eff4f9a9898e 100644 --- a/drivers/iio/adc/ti-ads124s08.c +++ b/drivers/iio/adc/ti-ads124s08.c @@ -269,7 +269,7 @@ static irqreturn_t ads124s_trigger_handler(int irq, void *p) struct iio_poll_func *pf = p; struct iio_dev *indio_dev = pf->indio_dev; struct ads124s_private *priv = iio_priv(indio_dev); - u32 buffer[ADS124S08_MAX_CHANNELS + sizeof(s64)/sizeof(u16)]; + u32 buffer[ADS124S08_MAX_CHANNELS + sizeof(s64)/sizeof(u32)]; int scan_index, j = 0; int ret; From patchwork Sun Sep 20 11:27:42 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Cameron X-Patchwork-Id: 11787479 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 80C0C6CB for ; Sun, 20 Sep 2020 11:28:36 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 692C0235FA for ; Sun, 20 Sep 2020 11:28:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1600601316; bh=1PzJMw9WIg1pCpshAZf3NIZK0fxLMlKYksdnKuROyjM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=Df+4nx9UxQqupXBy32zBW9DfW6uo/kS2B3einZ3ttaoY2oHZRmCVO/+3BsCllykl8 OLOEMBSIC14T/Na/Z346Lf2vAYNdhdIwioIYskkeQahj2kpplM7NHwZvHtkQSm3dKJ fFSBOaCuKiSyx3i+ImkdliXbpBAEHu56ALI7uAdQ= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726285AbgITL2g (ORCPT ); Sun, 20 Sep 2020 07:28:36 -0400 Received: from mail.kernel.org ([198.145.29.99]:39530 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726315AbgITL2f (ORCPT ); Sun, 20 Sep 2020 07:28:35 -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 78D3321BE5; Sun, 20 Sep 2020 11:28:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1600601314; bh=1PzJMw9WIg1pCpshAZf3NIZK0fxLMlKYksdnKuROyjM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=uoaQpjF4N7v2pS0P0any61MMFL6xBID8zfY+5iTcbSw7g2aGCWrk2IaBVJLPx0tKa uaQiQB9AnsSQvQEOXRdJyZRg1L/VCzpJqmxUnWUGac8GNds/nkaS9sILM24RZTNT5s bAC9I7mgoeN6lx9UMs12SHUdb9tWycLEo2XiAFcc= From: Jonathan Cameron To: linux-iio@vger.kernel.org Cc: Andy Shevchenko , Jonathan Cameron , Lars-Peter Clausen , Dan Murphy Subject: [PATCH v4 8/8] iio:adc:ti-ads124s08: Fix alignment and data leak issues. Date: Sun, 20 Sep 2020 12:27:42 +0100 Message-Id: <20200920112742.170751-9-jic23@kernel.org> X-Mailer: git-send-email 2.28.0 In-Reply-To: <20200920112742.170751-1-jic23@kernel.org> References: <20200920112742.170751-1-jic23@kernel.org> MIME-Version: 1.0 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 suitable structure in the iio_priv() data with alignment explicitly requested. This data is allocated with kzalloc() so no data can leak apart from previous readings. In this driver the timestamp can end up in various different locations depending on what other channels are enabled. As a result, we don't use a structure to specify it's position as that would be misleading. Fixes: e717f8c6dfec ("iio: adc: Add the TI ads124s08 ADC code") Reported-by: Lars-Peter Clausen Cc: Dan Murphy Signed-off-by: Jonathan Cameron --- v4: Expand comment to express the buffer length not all needed if not all channels are enabled. drivers/iio/adc/ti-ads124s08.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/drivers/iio/adc/ti-ads124s08.c b/drivers/iio/adc/ti-ads124s08.c index eff4f9a9898e..b4a128b19188 100644 --- a/drivers/iio/adc/ti-ads124s08.c +++ b/drivers/iio/adc/ti-ads124s08.c @@ -99,6 +99,14 @@ struct ads124s_private { struct gpio_desc *reset_gpio; struct spi_device *spi; struct mutex lock; + /* + * Used to correctly align data. + * Ensure timestamp is naturally aligned. + * Note that the full buffer length may not be needed if not + * all channels are enabled, as long as the alignment of the + * timestamp is maintained. + */ + u32 buffer[ADS124S08_MAX_CHANNELS + sizeof(s64)/sizeof(u32)] __aligned(8); u8 data[5] ____cacheline_aligned; }; @@ -269,7 +277,6 @@ static irqreturn_t ads124s_trigger_handler(int irq, void *p) struct iio_poll_func *pf = p; struct iio_dev *indio_dev = pf->indio_dev; struct ads124s_private *priv = iio_priv(indio_dev); - u32 buffer[ADS124S08_MAX_CHANNELS + sizeof(s64)/sizeof(u32)]; int scan_index, j = 0; int ret; @@ -284,7 +291,7 @@ static irqreturn_t ads124s_trigger_handler(int irq, void *p) if (ret) dev_err(&priv->spi->dev, "Start ADC conversions failed\n"); - buffer[j] = ads124s_read(indio_dev, scan_index); + priv->buffer[j] = ads124s_read(indio_dev, scan_index); ret = ads124s_write_cmd(indio_dev, ADS124S08_STOP_CONV); if (ret) dev_err(&priv->spi->dev, "Stop ADC conversions failed\n"); @@ -292,7 +299,7 @@ static irqreturn_t ads124s_trigger_handler(int irq, void *p) j++; } - iio_push_to_buffers_with_timestamp(indio_dev, buffer, + iio_push_to_buffers_with_timestamp(indio_dev, priv->buffer, pf->timestamp); iio_trigger_notify_done(indio_dev->trig);