From patchwork Wed Jul 22 15:50: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: 11678693 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 234D113A4 for ; Wed, 22 Jul 2020 15:53:11 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 00E5A207DD for ; Wed, 22 Jul 2020 15:53:11 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433191; bh=wuvjViPkxYN+MygFOS8mGclaAqsSpkro5stAKw7+5AQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=SVFwZAo+pbotYzwC/IdFBU/2LH5a/j3OMgNixaXlVcS5beVMXRiw+oaF9z3ZAoh6h P3t1Yx5v+HrIEqFne8aBKEcnCTTZKyH0caS75o/GHlHvVS+i5UqOx/EwZPVfdatKHY TPgLjQzI5tAWNwauGQ9QaGKdZ0E+hyHnaqK/u89g= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727863AbgGVPxK (ORCPT ); Wed, 22 Jul 2020 11:53:10 -0400 Received: from mail.kernel.org ([198.145.29.99]:35506 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726427AbgGVPxK (ORCPT ); Wed, 22 Jul 2020 11:53:10 -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 DB29E20717; Wed, 22 Jul 2020 15:53:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433189; bh=wuvjViPkxYN+MygFOS8mGclaAqsSpkro5stAKw7+5AQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=sIi2hgoeUPynoiTJlozJQD0uohod0vh0puXLgHqelGivf8knwsrqInMg6UkkhNjbc P6ZzLDTBs+uBJyKIsitCyzZulPImSsytbWvmwIuIgFxsC5mRELdbX0Kl7qTjwhBZIa dDRhfjZUSVmgLOoiWjaPyhcNf8QwMRqUknctFDZA= From: Jonathan Cameron To: linux-iio@vger.kernel.org Cc: Andy Shevchenko , Lars-Peter Clausen , Peter Meerwald , Jonathan Cameron Subject: [PATCH v3 01/27] iio: accel: kxsd9: Fix alignment of local buffer. Date: Wed, 22 Jul 2020 16:50:37 +0100 Message-Id: <20200722155103.979802-2-jic23@kernel.org> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200722155103.979802-1-jic23@kernel.org> References: <20200722155103.979802-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 iio_push_to_buffers_with_timestamp assumes 8 byte alignment which is not guaranteed by an array of smaller elements. Note that whilst in this particular case the alignment forcing of the ts element is not strictly necessary it acts as good documentation. Doing this where not necessary should cut down on the number of cut and paste introduced errors elsewhere. Reported-by: Lars-Peter Clausen Signed-off-by: Jonathan Cameron --- drivers/iio/accel/kxsd9.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c index 66b2e4cf24cf..0e18b92e2099 100644 --- a/drivers/iio/accel/kxsd9.c +++ b/drivers/iio/accel/kxsd9.c @@ -209,14 +209,20 @@ static irqreturn_t kxsd9_trigger_handler(int irq, void *p) const struct iio_poll_func *pf = p; struct iio_dev *indio_dev = pf->indio_dev; struct kxsd9_state *st = iio_priv(indio_dev); + /* + * Ensure correct positioning and alignment of timestamp. + * No need to zero initialize as all elements written. + */ + struct { + __be16 chan[4]; + s64 ts __aligned(8); + } hw_values; int ret; - /* 4 * 16bit values AND timestamp */ - __be16 hw_values[8]; ret = regmap_bulk_read(st->map, KXSD9_REG_X, - &hw_values, - 8); + hw_values.chan, + sizeof(hw_values.chan)); if (ret) { dev_err(st->dev, "error reading data\n"); @@ -224,7 +230,7 @@ static irqreturn_t kxsd9_trigger_handler(int irq, void *p) } iio_push_to_buffers_with_timestamp(indio_dev, - hw_values, + &hw_values, iio_get_time_ns(indio_dev)); iio_trigger_notify_done(indio_dev->trig); From patchwork Wed Jul 22 15:50: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: 11678695 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 6464F13A4 for ; Wed, 22 Jul 2020 15:53:12 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 4D1B7207E8 for ; Wed, 22 Jul 2020 15:53:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433192; bh=Zmnk4xZ1pGh8I1k4TrpFM63YFE/SARDLHj6dZZhfs7k=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=z5IOV0sicy/YxcfBVTcgStmbka2rYnwVpLRomAHh8EtvmI6d5hf7IMl0vUi8vt6BV 65H/iXoNlMb+kGejaA1m3bMBEd0mG/7ANAHUIhIf0wu85SNlG6pF3VHVjvR0+J3e6A 7MNbvkycq9TuCJo5yScdmOX7eaVkg+MWtd28svkA= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727891AbgGVPxM (ORCPT ); Wed, 22 Jul 2020 11:53:12 -0400 Received: from mail.kernel.org ([198.145.29.99]:35526 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726427AbgGVPxL (ORCPT ); Wed, 22 Jul 2020 11:53:11 -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 63368207CD; Wed, 22 Jul 2020 15:53:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433191; bh=Zmnk4xZ1pGh8I1k4TrpFM63YFE/SARDLHj6dZZhfs7k=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=kM2IC8p45ZZPnBIF2msHFKqXlBSdv32Hmrbq2l8ikpfGnUAgUdqVcBXHomyYL/ALD NyHN9bqdlQmeYasYeDXZvfjElEr9J4UQkcmDbfp6oHL0+WAfL3ITQVncN+8WczjvzM QFIo1tG/QO800k6gGDoiiREOqKCcry4Dy3ACNT40= From: Jonathan Cameron To: linux-iio@vger.kernel.org Cc: Andy Shevchenko , Lars-Peter Clausen , Peter Meerwald , Jonathan Cameron Subject: [PATCH v3 02/27] iio:accel:mma8452: Fix timestamp alignment and prevent data leak. Date: Wed, 22 Jul 2020 16:50:38 +0100 Message-Id: <20200722155103.979802-3-jic23@kernel.org> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200722155103.979802-1-jic23@kernel.org> References: <20200722155103.979802-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 a 16 byte u8 array 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 ensured by use of an explicit c structure. This data is allocated with kzalloc so no data can leak appart from previous readings. The additional forcing of the 8 byte alignment of the timestamp is not strictly necessary but makes the code less fragile by making this explicit. Fixes: c7eeea93ac60 ("iio: Add Freescale MMA8452Q 3-axis accelerometer driver") Reported-by: Lars-Peter Clausen Cc: Peter Meerwald Signed-off-by: Jonathan Cameron --- drivers/iio/accel/mma8452.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c index ba27f8673131..1cf2b5db26ca 100644 --- a/drivers/iio/accel/mma8452.c +++ b/drivers/iio/accel/mma8452.c @@ -110,6 +110,12 @@ struct mma8452_data { int sleep_val; struct regulator *vdd_reg; struct regulator *vddio_reg; + + /* Ensure correct alignment of time stamp when present */ + struct { + __be16 channels[3]; + s64 ts __aligned(8); + } buffer; }; /** @@ -1091,14 +1097,13 @@ static irqreturn_t mma8452_trigger_handler(int irq, void *p) struct iio_poll_func *pf = p; struct iio_dev *indio_dev = pf->indio_dev; struct mma8452_data *data = iio_priv(indio_dev); - u8 buffer[16]; /* 3 16-bit channels + padding + ts */ int ret; - ret = mma8452_read(data, (__be16 *)buffer); + ret = mma8452_read(data, data->buffer.channels); if (ret < 0) goto done; - iio_push_to_buffers_with_timestamp(indio_dev, buffer, + iio_push_to_buffers_with_timestamp(indio_dev, &data->buffer, iio_get_time_ns(indio_dev)); done: From patchwork Wed Jul 22 15:50: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: 11678697 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 29CB613A4 for ; Wed, 22 Jul 2020 15:53:14 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 113D0207E8 for ; Wed, 22 Jul 2020 15:53:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433194; bh=kgN38xlg5rt6u1iO3jr5SziroURYQCtFvw17Kfczz1w=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=qnUXl7hsMlQB28+XE3X3jt69W135LPaemWMGB0rmqojmohwDjwCQ/w2LtJV3TnGPP V6sYw2igfiTCVulQ6VDbjKSgzYENhmeiApvo1tqvkj3NegjS9Q4MxCZYuSq+rTvTr/ 96dMobGBM8Iatnrgjy6tqmNkHv5dQsf/qvXL0MTs= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727933AbgGVPxN (ORCPT ); Wed, 22 Jul 2020 11:53:13 -0400 Received: from mail.kernel.org ([198.145.29.99]:35538 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726427AbgGVPxN (ORCPT ); Wed, 22 Jul 2020 11:53:13 -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 CCA3320771; Wed, 22 Jul 2020 15:53:11 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433193; bh=kgN38xlg5rt6u1iO3jr5SziroURYQCtFvw17Kfczz1w=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=izPF1xw4VRlUhMEJ5ONliN0OyLUC+8HT/0UFoyUMTTMPhUVHoaZdKqsKutmPSqii6 x8RfLcz0+7SnVWdrtS7ldlv4mDqLRFGbPxbNbOLAdSYIX500NzslJVIdU1V2tvFuQq 9bdIq6o7XyNWtlrVx6q9d3lxSE3V/MqesZP6ln7E= From: Jonathan Cameron To: linux-iio@vger.kernel.org Cc: Andy Shevchenko , Lars-Peter Clausen , Peter Meerwald , Jonathan Cameron , Srinivas Pandruvada Subject: [PATCH v3 03/27] iio:accel:bmc150-accel: Fix timestamp alignment and prevent data leak. Date: Wed, 22 Jul 2020 16:50:39 +0100 Message-Id: <20200722155103.979802-4-jic23@kernel.org> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200722155103.979802-1-jic23@kernel.org> References: <20200722155103.979802-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 a 16 byte 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 ensured by use of an explicit c structure. This data is allocated with kzalloc so no data can leak appart from previous readings. Fixes tag is beyond some major refactoring so likely manual backporting would be needed to get that far back. Whilst the force alignment of the ts is not strictly necessary, it does make the code less fragile. Fixes: 3bbec9773389 ("iio: bmc150_accel: add support for hardware fifo") Reported-by: Lars-Peter Clausen Cc: Srinivas Pandruvada Signed-off-by: Jonathan Cameron Acked-by: Srinivas Pandruvada --- drivers/iio/accel/bmc150-accel-core.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c index 24864d9dfab5..48435865fdaf 100644 --- a/drivers/iio/accel/bmc150-accel-core.c +++ b/drivers/iio/accel/bmc150-accel-core.c @@ -189,6 +189,14 @@ struct bmc150_accel_data { struct mutex mutex; u8 fifo_mode, watermark; s16 buffer[8]; + /* + * Ensure there is sufficient space and correct alignment for + * the timestamp if enabled + */ + struct { + __le16 channels[3]; + s64 ts __aligned(8); + } scan; u8 bw_bits; u32 slope_dur; u32 slope_thres; @@ -922,15 +930,16 @@ static int __bmc150_accel_fifo_flush(struct iio_dev *indio_dev, * now. */ for (i = 0; i < count; i++) { - u16 sample[8]; int j, bit; j = 0; for_each_set_bit(bit, indio_dev->active_scan_mask, indio_dev->masklength) - memcpy(&sample[j++], &buffer[i * 3 + bit], 2); + memcpy(&data->scan.channels[j++], &buffer[i * 3 + bit], + sizeof(data->scan.channels[0])); - iio_push_to_buffers_with_timestamp(indio_dev, sample, tstamp); + iio_push_to_buffers_with_timestamp(indio_dev, &data->scan, + tstamp); tstamp += sample_period; } From patchwork Wed Jul 22 15:50: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: 11678699 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 7BF8C14E3 for ; Wed, 22 Jul 2020 15:53:15 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 633402080D for ; Wed, 22 Jul 2020 15:53:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433195; bh=JPjPQTwurZOmRt6qpeWPLAIdDax89TZBB7giNHldGRQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=gw20CJXxAf1uD5iAYuWC5Htgfy6ZAWwNcHOFUq5yk4tFImLiqy8Kp4+xvLdJXncrg ULR27DHjMpyWjTJq4j2hsRN3cfg4/o2VFlI2TifvgnhHF3ETktDHv1auZBeipow6Cg sLyO4/BygFAJAIhNtVInjKpqkAfjR+83bokQSn9E= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727985AbgGVPxP (ORCPT ); Wed, 22 Jul 2020 11:53:15 -0400 Received: from mail.kernel.org ([198.145.29.99]:35554 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726427AbgGVPxO (ORCPT ); Wed, 22 Jul 2020 11:53:14 -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 6E18C207DD; Wed, 22 Jul 2020 15:53:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433194; bh=JPjPQTwurZOmRt6qpeWPLAIdDax89TZBB7giNHldGRQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=D64YmlOreutlTCHiIY6pWAweLbo9Yw3fefHZ7sCqC2dAfx2Dixsp5UM7kjjC7HFFn 2hOq/pE5UyBs8ryXNnRakcYl8H4sqB86aG05aogmILeXIfdnsF0b184tHcWdtXax7B JMDwVWH8e2VeHbAXbOdzYQ6R4NgFKsdkSJf8HWtw= From: Jonathan Cameron To: linux-iio@vger.kernel.org Cc: Andy Shevchenko , Lars-Peter Clausen , Peter Meerwald , Jonathan Cameron Subject: [PATCH v3 04/27] iio:accel:mma7455: Fix timestamp alignment and prevent data leak. Date: Wed, 22 Jul 2020 16:50:40 +0100 Message-Id: <20200722155103.979802-5-jic23@kernel.org> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200722155103.979802-1-jic23@kernel.org> References: <20200722155103.979802-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 a 16 byte u8 array 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 ensured by use of an explicit c structure. This data is allocated with kzalloc so no data can leak appart from previous readings. The force alignment of ts is not strictly necessary in this particularly case but does make the code less fragile. Fixes: a84ef0d181d9 ("iio: accel: add Freescale MMA7455L/MMA7456L 3-axis accelerometer driver") Reported-by: Lars-Peter Clausen Signed-off-by: Jonathan Cameron --- drivers/iio/accel/mma7455_core.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/drivers/iio/accel/mma7455_core.c b/drivers/iio/accel/mma7455_core.c index 7e99bcb3398d..922bd38ff6ea 100644 --- a/drivers/iio/accel/mma7455_core.c +++ b/drivers/iio/accel/mma7455_core.c @@ -52,6 +52,14 @@ struct mma7455_data { struct regmap *regmap; + /* + * Used to reorganize data. Will ensure correct alignment of + * the timestamp if present + */ + struct { + __le16 channels[3]; + s64 ts __aligned(8); + } scan; }; static int mma7455_drdy(struct mma7455_data *mma7455) @@ -82,19 +90,19 @@ static irqreturn_t mma7455_trigger_handler(int irq, void *p) struct iio_poll_func *pf = p; struct iio_dev *indio_dev = pf->indio_dev; struct mma7455_data *mma7455 = iio_priv(indio_dev); - u8 buf[16]; /* 3 x 16-bit channels + padding + ts */ int ret; ret = mma7455_drdy(mma7455); if (ret) goto done; - ret = regmap_bulk_read(mma7455->regmap, MMA7455_REG_XOUTL, buf, - sizeof(__le16) * 3); + ret = regmap_bulk_read(mma7455->regmap, MMA7455_REG_XOUTL, + mma7455->scan.channels, + sizeof(mma7455->scan.channels)); if (ret) goto done; - iio_push_to_buffers_with_timestamp(indio_dev, buf, + iio_push_to_buffers_with_timestamp(indio_dev, &mma7455->scan, iio_get_time_ns(indio_dev)); done: From patchwork Wed Jul 22 15:50: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: 11678701 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 9A3EC13A4 for ; Wed, 22 Jul 2020 15:53:17 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 7923E208E4 for ; Wed, 22 Jul 2020 15:53:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433197; bh=Qx6VAp2IInsTmEtO1B2pSFla9Ntl+3Z1V9YEjYutsus=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=bJXjXdpBFzR6LsDnyS7B8mU/3SwthnWPeHVjjn2Agd5qOkI/NOxzQj9inLyTczOSS vUW7lDYKGdh+UnpW75jze7+hjnoPaFqwcpLkTu5smf9FMTS55d3Zzu+n3ZB/7hmHtp i+6RSMMHwxHFTt8YDQ+IqivwzHnQlx/7whi8ozlY= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728186AbgGVPxR (ORCPT ); Wed, 22 Jul 2020 11:53:17 -0400 Received: from mail.kernel.org ([198.145.29.99]:35568 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726427AbgGVPxQ (ORCPT ); Wed, 22 Jul 2020 11:53:16 -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 DC505207E8; Wed, 22 Jul 2020 15:53:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433195; bh=Qx6VAp2IInsTmEtO1B2pSFla9Ntl+3Z1V9YEjYutsus=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=kkIjRr9Lc2aQh5AZ7grdw+/cBraMCehS4Hj32vj5TkHabY8k7VDeckqRQqAVHUU6G ryUANl9HX5HR4GjkAhUnoULjFM4pwQjK4tT0k7asvcIklWaIv76+wVJG7MHfV1ADJB 2DxHjaB0G+U0x83PEjqfEBlKYE/Ci2rci27dl03s= From: Jonathan Cameron To: linux-iio@vger.kernel.org Cc: Andy Shevchenko , Lars-Peter Clausen , Peter Meerwald , Jonathan Cameron Subject: [PATCH v3 05/27] iio:gyro:itg3200: Fix timestamp alignment and prevent data leak. Date: Wed, 22 Jul 2020 16:50:41 +0100 Message-Id: <20200722155103.979802-6-jic23@kernel.org> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200722155103.979802-1-jic23@kernel.org> References: <20200722155103.979802-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 a 16 byte array of smaller elements on the stack. This is fixed by using an explicit c structure. As there are no holes in the structure, there is no possiblity of data leakage in this case. The explicit alignment of ts is not strictly necessary but potentially makes the code slightly less fragile. It also removes the possibility of this being cut and paste into another driver where the alignment isn't already true. Fixes: 36e0371e7764 ("iio:itg3200: Use iio_push_to_buffers_with_timestamp()") Reported-by: Lars-Peter Clausen Signed-off-by: Jonathan Cameron --- drivers/iio/gyro/itg3200_buffer.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/drivers/iio/gyro/itg3200_buffer.c b/drivers/iio/gyro/itg3200_buffer.c index d3fbe9d86467..1c3c1bd53374 100644 --- a/drivers/iio/gyro/itg3200_buffer.c +++ b/drivers/iio/gyro/itg3200_buffer.c @@ -46,13 +46,20 @@ static irqreturn_t itg3200_trigger_handler(int irq, void *p) struct iio_poll_func *pf = p; struct iio_dev *indio_dev = pf->indio_dev; struct itg3200 *st = iio_priv(indio_dev); - __be16 buf[ITG3200_SCAN_ELEMENTS + sizeof(s64)/sizeof(u16)]; - - int ret = itg3200_read_all_channels(st->i2c, buf); + /* + * Ensure correct alignment and padding including for the + * timestamp that may be inserted. + */ + struct { + __be16 buf[ITG3200_SCAN_ELEMENTS]; + s64 ts __aligned(8); + } scan; + + int ret = itg3200_read_all_channels(st->i2c, scan.buf); if (ret < 0) goto error_ret; - iio_push_to_buffers_with_timestamp(indio_dev, buf, pf->timestamp); + iio_push_to_buffers_with_timestamp(indio_dev, &scan, pf->timestamp); iio_trigger_notify_done(indio_dev->trig); From patchwork Wed Jul 22 15:50: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: 11678703 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 6681A159A for ; Wed, 22 Jul 2020 15:53:18 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 4EBC920B1F for ; Wed, 22 Jul 2020 15:53:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433198; bh=5QuK84qbMXTvOmisdS/5lYhQHBC9xeUyd2YS0HocAi8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=T8HWQ6tFsJjc58633jW1GR51r1GBUEmS3qJsygl8WUPcCgn92TfwuB+rbhW+qx+QC oIv8Lx1d45q+SxIhq5POKWgygwOLK+zicNDr9MJxKMtwt0QXCJ78QINp5xUFxQEZPY waj0zWZXto/K++OfD6rhEwB0bx7XCKF151J5GBYk= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728670AbgGVPxS (ORCPT ); Wed, 22 Jul 2020 11:53:18 -0400 Received: from mail.kernel.org ([198.145.29.99]:35578 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726427AbgGVPxR (ORCPT ); Wed, 22 Jul 2020 11:53:17 -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 45D9B2080D; Wed, 22 Jul 2020 15:53:16 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433197; bh=5QuK84qbMXTvOmisdS/5lYhQHBC9xeUyd2YS0HocAi8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=RYlV+JJvRdEjuZSGJbMQfO1g9MSPnI9P1+caY1CknQaURJgoXPoE2t0bCdjX2wdhT FDox1yoniV8O9+/+/xWFklWKMu2yXOQCREnViwx5k5QycgV6unTPa0X8WIBztGbhbe JLMp7PtTXnFpR2JOPpYrXIe+9Z6IKlMpZsTqeeCE= From: Jonathan Cameron To: linux-iio@vger.kernel.org Cc: Andy Shevchenko , Lars-Peter Clausen , Peter Meerwald , Jonathan Cameron , Andreas Klinger Subject: [PATCH v3 06/27] iio:proximity:mb1232: Fix timestamp alignment and prevent data leak. Date: Wed, 22 Jul 2020 16:50:42 +0100 Message-Id: <20200722155103.979802-7-jic23@kernel.org> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200722155103.979802-1-jic23@kernel.org> References: <20200722155103.979802-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 a 16 byte s16 array 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 ensured by use of an explicit c structure. This data is allocated with kzalloc so no data can leak appart from previous readings. In this case the forced alignment of the ts is necessary to ensure correct padding on x86_32 where the s64 would only be 4 byte aligned. Fixes: 16b05261537e ("mb1232.c: add distance iio sensor with i2c") Reported-by: Lars-Peter Clausen Cc: Andreas Klinger Signed-off-by: Jonathan Cameron --- drivers/iio/proximity/mb1232.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/drivers/iio/proximity/mb1232.c b/drivers/iio/proximity/mb1232.c index 654564c45248..ad4b1fb2607a 100644 --- a/drivers/iio/proximity/mb1232.c +++ b/drivers/iio/proximity/mb1232.c @@ -40,6 +40,11 @@ struct mb1232_data { */ struct completion ranging; int irqnr; + /* Ensure correct alignment of data to push to IIO buffer */ + struct { + s16 distance; + s64 ts __aligned(8); + } scan; }; static irqreturn_t mb1232_handle_irq(int irq, void *dev_id) @@ -113,17 +118,13 @@ static irqreturn_t mb1232_trigger_handler(int irq, void *p) struct iio_poll_func *pf = p; struct iio_dev *indio_dev = pf->indio_dev; struct mb1232_data *data = iio_priv(indio_dev); - /* - * triggered buffer - * 16-bit channel + 48-bit padding + 64-bit timestamp - */ - s16 buffer[8] = { 0 }; - buffer[0] = mb1232_read_distance(data); - if (buffer[0] < 0) + data->scan.distance = mb1232_read_distance(data); + if (data->scan.distance < 0) goto err; - iio_push_to_buffers_with_timestamp(indio_dev, buffer, pf->timestamp); + iio_push_to_buffers_with_timestamp(indio_dev, &data->scan, + pf->timestamp); err: iio_trigger_notify_done(indio_dev->trig); From patchwork Wed Jul 22 15:50:43 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Cameron X-Patchwork-Id: 11678705 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 23E5B13A4 for ; Wed, 22 Jul 2020 15:53:20 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 0C75D208E4 for ; Wed, 22 Jul 2020 15:53:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433200; bh=uAyTlGFNquqTUFcruiE3JO6ffy37QQf5YgapKAU32WA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=PstG5Cd499ZI0/1hjSDTfDdWsqR2ZVoFFI5ZHgS7fdYSs2H1NsMYGyyeVsI1y2OeP CtBnOdNqEkigC+i5tfBjNMD7EbYfjnA/+LUI+LDkAlfGHUTz8rPWo3UY9nJmJcn9L+ X1XbMB4I7uGCMoLSCoR3mwYvSIiZGVQwp7B4SYmQ= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728687AbgGVPxT (ORCPT ); Wed, 22 Jul 2020 11:53:19 -0400 Received: from mail.kernel.org ([198.145.29.99]:35596 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726427AbgGVPxT (ORCPT ); Wed, 22 Jul 2020 11:53:19 -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 CDB572084D; Wed, 22 Jul 2020 15:53:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433199; bh=uAyTlGFNquqTUFcruiE3JO6ffy37QQf5YgapKAU32WA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=d+EbHLCahSoyRJaQxeH9jDiXnZMONWnnQG2lPuhdgQN7KegfR7QlLPwfsUpmUBoq4 cFeAwY8PuTh/arWXV+ukqDTVGkdWnDe5uf18DeWImukEiEt7LoXZ0ZibRw9J+YP/IX D8Rc0SmURoRHMDhq3wrpDauXTVji+EEf0vj60xfY= From: Jonathan Cameron To: linux-iio@vger.kernel.org Cc: Andy Shevchenko , Lars-Peter Clausen , Peter Meerwald , Jonathan Cameron , Narcisa Ana Maria Vasile Subject: [PATCH v3 07/27] iio:chemical:ccs811: Fix timestamp alignment and prevent data leak. Date: Wed, 22 Jul 2020 16:50:43 +0100 Message-Id: <20200722155103.979802-8-jic23@kernel.org> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200722155103.979802-1-jic23@kernel.org> References: <20200722155103.979802-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 suitable structure in the iio_priv() data with alignment explicitly requested. This data is allocated with kzalloc so no data can leak appart from previous readings. The explicit alignment of ts is necessary to ensure consistent padding for x86_32 in which the ts would otherwise be 4 byte aligned. Fixes: 283d26917ad6 ("iio: chemical: ccs811: Add triggered buffer support") Reported-by: Lars-Peter Clausen Cc: Narcisa Ana Maria Vasile Signed-off-by: Jonathan Cameron --- drivers/iio/chemical/ccs811.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/drivers/iio/chemical/ccs811.c b/drivers/iio/chemical/ccs811.c index 2b007e7568b2..60dd87e96f5f 100644 --- a/drivers/iio/chemical/ccs811.c +++ b/drivers/iio/chemical/ccs811.c @@ -78,6 +78,11 @@ struct ccs811_data { struct iio_trigger *drdy_trig; struct gpio_desc *wakeup_gpio; bool drdy_trig_on; + /* Ensures correct alignment of timestamp if present */ + struct { + s16 channels[2]; + s64 ts __aligned(8); + } scan; }; static const struct iio_chan_spec ccs811_channels[] = { @@ -327,17 +332,17 @@ static irqreturn_t ccs811_trigger_handler(int irq, void *p) struct iio_dev *indio_dev = pf->indio_dev; struct ccs811_data *data = iio_priv(indio_dev); struct i2c_client *client = data->client; - s16 buf[8]; /* s16 eCO2 + s16 TVOC + padding + 8 byte timestamp */ int ret; - ret = i2c_smbus_read_i2c_block_data(client, CCS811_ALG_RESULT_DATA, 4, - (u8 *)&buf); + ret = i2c_smbus_read_i2c_block_data(client, CCS811_ALG_RESULT_DATA, + sizeof(data->scan.channels), + (u8 *)data->scan.channels); if (ret != 4) { dev_err(&client->dev, "cannot read sensor data\n"); goto err; } - iio_push_to_buffers_with_timestamp(indio_dev, buf, + iio_push_to_buffers_with_timestamp(indio_dev, &data->scan, iio_get_time_ns(indio_dev)); err: From patchwork Wed Jul 22 15:50:44 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Cameron X-Patchwork-Id: 11678707 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 831C713A4 for ; Wed, 22 Jul 2020 15:53:21 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6A67820B1F for ; Wed, 22 Jul 2020 15:53:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433201; bh=bygXAix11Yif9FOMNTJ8h+p55wt+ROUtmFRG85PnGR4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=TVzQZU4Kh7scY3xnSuAH+uTL7/DHhNwjG24b4ksJnYnEqavqlwXkh2hGsw6B9UDTE OJE8oUuUThDGFF+cQj4KtCDApZ+w2Lz7PcC0PV7Zxd1JwcM5AgsQ9KeDPV6FrDJjUY DrzUgJgNe/moH97eg+nudAw9ZkaHF9Z09CVHegsc= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728697AbgGVPxV (ORCPT ); Wed, 22 Jul 2020 11:53:21 -0400 Received: from mail.kernel.org ([198.145.29.99]:35614 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726427AbgGVPxU (ORCPT ); Wed, 22 Jul 2020 11:53:20 -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 5CA80206F5; Wed, 22 Jul 2020 15:53:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433200; bh=bygXAix11Yif9FOMNTJ8h+p55wt+ROUtmFRG85PnGR4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=LmjgHCUHKPjaeNrOj/F+ADsQ4rJqULAgrosOZBjZWJYUCicppfRk39gnXE1StVRpQ RtXHf6FCSWBelQCQM3+EZtKrR1d9z+r/r4Ke2wkLFO1Xw8fe3iL/8edMlVhVs54KnD oT95apDhYB7WQUE6XetDWscqHSln7UEhvuAlx0kQ= From: Jonathan Cameron To: linux-iio@vger.kernel.org Cc: Andy Shevchenko , Lars-Peter Clausen , Peter Meerwald , Jonathan Cameron Subject: [PATCH v3 08/27] iio:light:si1145: Fix timestamp alignment and prevent data leak. Date: Wed, 22 Jul 2020 16:50:44 +0100 Message-Id: <20200722155103.979802-9-jic23@kernel.org> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200722155103.979802-1-jic23@kernel.org> References: <20200722155103.979802-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 a 24 byte 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 appart from previous readings. Depending on the enabled channels, the location of the timestamp can be at various aligned offsets through the buffer. As such we any use of a structure to enforce this alignment would incorrectly suggest a single location for the timestamp. Fixes: ac45e57f1590 ("iio: light: Add driver for Silabs si1132, si1141/2/3 and si1145/6/7 ambient light, uv index and proximity sensors") Reported-by: Lars-Peter Clausen Cc: Peter Meerwald-Stadler Signed-off-by: Jonathan Cameron --- drivers/iio/light/si1145.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/iio/light/si1145.c b/drivers/iio/light/si1145.c index 155faaea8c72..e8bdc221d65b 100644 --- a/drivers/iio/light/si1145.c +++ b/drivers/iio/light/si1145.c @@ -179,6 +179,8 @@ struct si1145_data { bool autonomous; struct iio_trigger *trig; int meas_rate; + /* Ensure timestamp will be naturally aligned if present */ + u8 buffer[24] __aligned(8); }; /* @@ -445,7 +447,6 @@ static irqreturn_t si1145_trigger_handler(int irq, void *private) * 6*2 bytes channels data + 4 bytes alignment + * 8 bytes timestamp */ - u8 buffer[24]; int i, j = 0; int ret; u8 irq_status = 0; @@ -478,7 +479,7 @@ static irqreturn_t si1145_trigger_handler(int irq, void *private) ret = i2c_smbus_read_i2c_block_data_or_emulated( data->client, indio_dev->channels[i].address, - sizeof(u16) * run, &buffer[j]); + sizeof(u16) * run, &data->buffer[j]); if (ret < 0) goto done; j += run * sizeof(u16); @@ -493,7 +494,7 @@ static irqreturn_t si1145_trigger_handler(int irq, void *private) goto done; } - iio_push_to_buffers_with_timestamp(indio_dev, buffer, + iio_push_to_buffers_with_timestamp(indio_dev, data->buffer, iio_get_time_ns(indio_dev)); done: From patchwork Wed Jul 22 15:50:45 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Cameron X-Patchwork-Id: 11678709 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 0FF4E14E3 for ; Wed, 22 Jul 2020 15:53:23 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id EC9EA20B1F for ; Wed, 22 Jul 2020 15:53:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433203; bh=tJNRtj1pgUOXomhod3e9MpNDSMmcwEtsX1Km8Tcofnw=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=uaZ4QNZAifk0+CKPu+ytlRxKSY5SXhRx0LrgWzHwp13NonlIF0IcrWytAINQNDMw4 F2Te1FI/uWFyvOLQCUa+efTa71izno0daGjCzV/v3xr3wGBoH3wfmPAEzxaQEBKeHN c68cedYnYT/1yp7PGQuTIB5Q0obNbx6tiEv73SDA= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728936AbgGVPxW (ORCPT ); Wed, 22 Jul 2020 11:53:22 -0400 Received: from mail.kernel.org ([198.145.29.99]:35634 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726427AbgGVPxW (ORCPT ); Wed, 22 Jul 2020 11:53:22 -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 BB1C320717; Wed, 22 Jul 2020 15:53:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433201; bh=tJNRtj1pgUOXomhod3e9MpNDSMmcwEtsX1Km8Tcofnw=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=GYrGPZlmyZwwoISgUBXjlKBt0W4wzNM6+F2/7WMWudNa4+BK/11vsZLQhwDnAnsap fIv5z2+0G1rqUOB9+9agJ0I7Rgi3DaOr0dpUaVEU6YfxUqCC+TdthUVOabrWJpRoDG 1w9AW2Owp+caRosv52h2kz3Bl3rkwnZUqJZqnnRs= From: Jonathan Cameron To: linux-iio@vger.kernel.org Cc: Andy Shevchenko , Lars-Peter Clausen , Peter Meerwald , Jonathan Cameron Subject: [PATCH v3 09/27] iio:light:max44000 Fix timestamp alignment and prevent data leak. Date: Wed, 22 Jul 2020 16:50:45 +0100 Message-Id: <20200722155103.979802-10-jic23@kernel.org> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200722155103.979802-1-jic23@kernel.org> References: <20200722155103.979802-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 a 16 byte 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 appart from previous readings. It is necessary to force the alignment of ts to avoid the padding on x86_32 being different from 64 bit platorms (it alows for 4 bytes aligned 8 byte types. Fixes: 06ad7ea10e2b ("max44000: Initial triggered buffer support") Reported-by: Lars-Peter Clausen Signed-off-by: Jonathan Cameron --- drivers/iio/light/max44000.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/iio/light/max44000.c b/drivers/iio/light/max44000.c index aa8ed1e3e89a..b8e721bced5b 100644 --- a/drivers/iio/light/max44000.c +++ b/drivers/iio/light/max44000.c @@ -75,6 +75,11 @@ struct max44000_data { struct mutex lock; struct regmap *regmap; + /* Ensure naturally aligned timestamp */ + struct { + u16 channels[2]; + s64 ts __aligned(8); + } scan; }; /* Default scale is set to the minimum of 0.03125 or 1 / (1 << 5) lux */ @@ -488,7 +493,6 @@ static irqreturn_t max44000_trigger_handler(int irq, void *p) struct iio_poll_func *pf = p; struct iio_dev *indio_dev = pf->indio_dev; struct max44000_data *data = iio_priv(indio_dev); - u16 buf[8]; /* 2x u16 + padding + 8 bytes timestamp */ int index = 0; unsigned int regval; int ret; @@ -498,17 +502,17 @@ static irqreturn_t max44000_trigger_handler(int irq, void *p) ret = max44000_read_alsval(data); if (ret < 0) goto out_unlock; - buf[index++] = ret; + data->scan.channels[index++] = ret; } if (test_bit(MAX44000_SCAN_INDEX_PRX, indio_dev->active_scan_mask)) { ret = regmap_read(data->regmap, MAX44000_REG_PRX_DATA, ®val); if (ret < 0) goto out_unlock; - buf[index] = regval; + data->scan.channels[index] = regval; } mutex_unlock(&data->lock); - iio_push_to_buffers_with_timestamp(indio_dev, buf, + iio_push_to_buffers_with_timestamp(indio_dev, &data->scan, iio_get_time_ns(indio_dev)); iio_trigger_notify_done(indio_dev->trig); return IRQ_HANDLED; From patchwork Wed Jul 22 15:50:46 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Cameron X-Patchwork-Id: 11678711 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 4D49514E3 for ; Wed, 22 Jul 2020 15:53:24 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 3325020B1F for ; Wed, 22 Jul 2020 15:53:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433204; bh=TmMsXkQQ5gFHuUkqI6kxjNw5ilxRdJnc9DKZ4hNzOvs=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=hVKlf1j1V8OFIQYeTRUMGB2QXlIGLgP6wvIAMJ2tArV13Y8P7r+/HkZoYBBkShTZi cn9K6jbkfqr3xSD1vr+KnSrkF8Irwk+g4UgD2GFxPWjH6VXG9swfCqQr3Esf0na8FU fc/lSXBHiOzQClLHBnyqM7vPjZKoiFP4vBQJZjAU= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729096AbgGVPxX (ORCPT ); Wed, 22 Jul 2020 11:53:23 -0400 Received: from mail.kernel.org ([198.145.29.99]:35644 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726427AbgGVPxX (ORCPT ); Wed, 22 Jul 2020 11:53:23 -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 27568207CD; Wed, 22 Jul 2020 15:53:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433203; bh=TmMsXkQQ5gFHuUkqI6kxjNw5ilxRdJnc9DKZ4hNzOvs=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=tGQf0jj/X++nAdgNUzZjIWYDaim66KvJA6xvU/V/9+8kl4b25g+DSFtaY6tvDu2aQ H6LYg3HUChGycUC/KNOpFk+feYQKl206SnQawuf+38d6O/RQUt+xRG0hS/c2hHDwpw ojOG2CcuhMkDHIQEOK3b3qN63F5H6TiWr1Vg99Z4= From: Jonathan Cameron To: linux-iio@vger.kernel.org Cc: Andy Shevchenko , Lars-Peter Clausen , Peter Meerwald , Jonathan Cameron , Mikko Koivunen Subject: [PATCH v3 10/27] iio:light:rpr0521 Fix timestamp alignment and prevent data leak. Date: Wed, 22 Jul 2020 16:50:46 +0100 Message-Id: <20200722155103.979802-11-jic23@kernel.org> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200722155103.979802-1-jic23@kernel.org> References: <20200722155103.979802-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 suitable structure in the iio_priv(). This data is allocated with kzalloc so no data can leak appart 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. Fixes: e12ffd241c00 ("iio: light: rpr0521 triggered buffer") Cc: Mikko Koivunen Signed-off-by: Jonathan Cameron --- 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 Wed Jul 22 15:50:47 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Cameron X-Patchwork-Id: 11678713 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 403C913A4 for ; Wed, 22 Jul 2020 15:53:26 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 2880A20B1F for ; Wed, 22 Jul 2020 15:53:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433206; bh=TZ4FEWAjKvmXqgkVVlnaXf+ULLHl8K8W0YsSzflV640=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=ixRMzhNGwrLo+5M3spsmUfNlJO+hhVu5xKhkA+LSUmxTldbhWJsbB6elo0ulRqBCf Y1/0NgItrsh32mRBcED/+W/uQOdvLRfgeWt19QjfXf9nW7eylQkTi068laes5Qds8o 6uQljWUfZ9HmTxmkQeORdMox/Ed+IypsFZ8um0Dw= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729201AbgGVPxZ (ORCPT ); Wed, 22 Jul 2020 11:53:25 -0400 Received: from mail.kernel.org ([198.145.29.99]:35662 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726427AbgGVPxZ (ORCPT ); Wed, 22 Jul 2020 11:53: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 BBFFB20771; Wed, 22 Jul 2020 15:53:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433204; bh=TZ4FEWAjKvmXqgkVVlnaXf+ULLHl8K8W0YsSzflV640=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=BQsfQx43RSPWtWCsBn4eIozzwpTJ3IZigMLU737MCM+WGIV6Sm3saFkyr9TXPr7ii W/FOktrsstQztFB6wBabyoemT9vK4tYEjCq5r1nfWrytdJ2fqS5VhDkemN3/sCNLYB 6lzVewp4pX/W41PWpr4VZqI0qwfGySQT3ZFcEI0A= From: Jonathan Cameron To: linux-iio@vger.kernel.org Cc: Andy Shevchenko , Lars-Peter Clausen , Peter Meerwald , Jonathan Cameron , Lorenzo Bianconi Subject: [PATCH v3 11/27] iio:light:st_uvis25 Fix timestamp alignment and prevent data leak. Date: Wed, 22 Jul 2020 16:50:47 +0100 Message-Id: <20200722155103.979802-12-jic23@kernel.org> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200722155103.979802-1-jic23@kernel.org> References: <20200722155103.979802-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 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 --- 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..50d9850256fc 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 & 0xFF; + + iio_push_to_buffers_with_timestamp(iio_dev, &hw->scan, iio_get_time_ns(iio_dev)); out: From patchwork Wed Jul 22 15:50:48 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Cameron X-Patchwork-Id: 11678715 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 56C1514E3 for ; Wed, 22 Jul 2020 15:53:27 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 3D08522BEF for ; Wed, 22 Jul 2020 15:53:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433207; bh=9/Nc7AOxF+tbHgakEOl8OJWkFEiVTOk+Wrzfl1AmJXY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=v0LnMLhxskAT+4zQzqyh2d0syB911o47MRnYQtr4PVbdYnF9Mh9HQfW+sZK2WVAp9 vI0oKU90HHsZywlfIEE8FSR+2XMYbeOOwyG4lGZj7+3GDRhMKzhL4shJfxoDCZdq2Z GPWbcHCQozmBw9/CnBhOQnXnFso2NSvyWnPP35ZE= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729249AbgGVPx0 (ORCPT ); Wed, 22 Jul 2020 11:53:26 -0400 Received: from mail.kernel.org ([198.145.29.99]:35684 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726427AbgGVPx0 (ORCPT ); Wed, 22 Jul 2020 11:53: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 4CBFC208E4; Wed, 22 Jul 2020 15:53:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433206; bh=9/Nc7AOxF+tbHgakEOl8OJWkFEiVTOk+Wrzfl1AmJXY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=JkXUXWygPfDnAdR3j742J3Qgzon7UggDjimF0s7zJ3u+bbBNTXKCd75He5NDjj+aM tUuOqIUmhEI3OE+onN4+SUvyubyIP64/oh5+J5GfPCrqKJ1Kln4+cuNpxobvA28u76 TfMdfJTYisCTFq4HmhIHk29vpWZQUAIyE62Qq5Os= From: Jonathan Cameron To: linux-iio@vger.kernel.org Cc: Andy Shevchenko , Lars-Peter Clausen , Peter Meerwald , Jonathan Cameron Subject: [PATCH v3 12/27] iio:light:ltr501 Fix timestamp alignment issue. Date: Wed, 22 Jul 2020 16:50:48 +0100 Message-Id: <20200722155103.979802-13-jic23@kernel.org> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200722155103.979802-1-jic23@kernel.org> References: <20200722155103.979802-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. Here we use a structure on the stack. The driver already did an explicit memset so no data leak was possible. Forced alignment of ts is not strictly necessary but probably makes the code slightly less fragile. Note there has been some rework in this driver of the years, so no way this will apply cleanly all the way back. Fixes: 2690be905123 ("iio: Add Lite-On ltr501 ambient light / proximity sensor driver") Reported-by: Lars-Peter Clausen Signed-off-by: Jonathan Cameron --- drivers/iio/light/ltr501.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c index 4bac0646398d..b4323d2db0b1 100644 --- a/drivers/iio/light/ltr501.c +++ b/drivers/iio/light/ltr501.c @@ -1243,13 +1243,16 @@ static irqreturn_t ltr501_trigger_handler(int irq, void *p) struct iio_poll_func *pf = p; struct iio_dev *indio_dev = pf->indio_dev; struct ltr501_data *data = iio_priv(indio_dev); - u16 buf[8]; + struct { + u16 channels[3]; + s64 ts __aligned(8); + } scan; __le16 als_buf[2]; u8 mask = 0; int j = 0; int ret, psdata; - memset(buf, 0, sizeof(buf)); + memset(&scan, 0, sizeof(scan)); /* figure out which data needs to be ready */ if (test_bit(0, indio_dev->active_scan_mask) || @@ -1268,9 +1271,9 @@ static irqreturn_t ltr501_trigger_handler(int irq, void *p) if (ret < 0) return ret; if (test_bit(0, indio_dev->active_scan_mask)) - buf[j++] = le16_to_cpu(als_buf[1]); + scan.channels[j++] = le16_to_cpu(als_buf[1]); if (test_bit(1, indio_dev->active_scan_mask)) - buf[j++] = le16_to_cpu(als_buf[0]); + scan.channels[j++] = le16_to_cpu(als_buf[0]); } if (mask & LTR501_STATUS_PS_RDY) { @@ -1278,10 +1281,10 @@ static irqreturn_t ltr501_trigger_handler(int irq, void *p) &psdata, 2); if (ret < 0) goto done; - buf[j++] = psdata & LTR501_PS_DATA_MASK; + scan.channels[j++] = psdata & LTR501_PS_DATA_MASK; } - iio_push_to_buffers_with_timestamp(indio_dev, buf, + iio_push_to_buffers_with_timestamp(indio_dev, &scan, iio_get_time_ns(indio_dev)); done: From patchwork Wed Jul 22 15:50:49 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Cameron X-Patchwork-Id: 11678717 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 4CC8F14E3 for ; Wed, 22 Jul 2020 15:53:29 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 351742080D for ; Wed, 22 Jul 2020 15:53:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433209; bh=/E/fLONlk2D40Co8MwdPnHBs/wsbak7ZiF44SBUW+ek=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=jNVXWQJi4pnzuhb2klYDzqEkI8jclUJxUS+kmVakaLYT8Vt+VcYpTwqGXiBnkxEFj MlUvRfClEY3v/E2Baqfneu+Tuh8Pky8NDrI4Cd41Ch+reVrraKCtPed/hYZdnzvlxU M8Mv3PPQOHNbu+QsvbKlSpTcqBEHcrXPH/pvot5w= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729604AbgGVPx2 (ORCPT ); Wed, 22 Jul 2020 11:53:28 -0400 Received: from mail.kernel.org ([198.145.29.99]:35692 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729306AbgGVPx2 (ORCPT ); Wed, 22 Jul 2020 11:53:28 -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 AF6A2207DD; Wed, 22 Jul 2020 15:53:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433208; bh=/E/fLONlk2D40Co8MwdPnHBs/wsbak7ZiF44SBUW+ek=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=obwdA4t6YMZLMAjM7kG2orS2w6KPAYCYmrdLFHW+oiyPb5wjO9HclC5IyXof1nFRF Hv8yHP6nZVvvaErswx5+qFyfGh0DCmBt6llPiR9q4AUoS2vEjOz4y0JB6MRTsnxUcZ 5Z/A71udaBRaq0yYXOq3Ldl+n8F8t1n5j1iZtS68= From: Jonathan Cameron To: linux-iio@vger.kernel.org Cc: Andy Shevchenko , Lars-Peter Clausen , Peter Meerwald , Jonathan Cameron , Gregor Boirie , Andy Shevchenko , Linus Walleij Subject: [PATCH v3 13/27] iio:magnetometer:ak8975 Fix alignment and data leak issues. Date: Wed, 22 Jul 2020 16:50:49 +0100 Message-Id: <20200722155103.979802-14-jic23@kernel.org> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200722155103.979802-1-jic23@kernel.org> References: <20200722155103.979802-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 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 as by coincidence the padding will end up the same, however I consider it to make the code less fragile and have included it. Fixes: bc11ca4a0b84 ("iio:magnetometer:ak8975: triggered buffer support") Reported-by: Lars-Peter Clausen Cc: Gregor Boirie Cc: Andy Shevchenko Cc: Linus Walleij Signed-off-by: Jonathan Cameron --- drivers/iio/magnetometer/ak8975.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c index 03d71f796177..623766ff800b 100644 --- a/drivers/iio/magnetometer/ak8975.c +++ b/drivers/iio/magnetometer/ak8975.c @@ -366,6 +366,12 @@ struct ak8975_data { struct iio_mount_matrix orientation; struct regulator *vdd; struct regulator *vid; + + /* Ensure natural alignment of timestamp */ + struct { + s16 channels[3]; + s64 ts __aligned(8); + } scan; }; /* Enable attached power regulator if any. */ @@ -793,7 +799,6 @@ static void ak8975_fill_buffer(struct iio_dev *indio_dev) const struct i2c_client *client = data->client; const struct ak_def *def = data->def; int ret; - s16 buff[8]; /* 3 x 16 bits axis values + 1 aligned 64 bits timestamp */ __le16 fval[3]; mutex_lock(&data->lock); @@ -816,12 +821,13 @@ static void ak8975_fill_buffer(struct iio_dev *indio_dev) mutex_unlock(&data->lock); /* Clamp to valid range. */ - buff[0] = clamp_t(s16, le16_to_cpu(fval[0]), -def->range, def->range); - buff[1] = clamp_t(s16, le16_to_cpu(fval[1]), -def->range, def->range); - buff[2] = clamp_t(s16, le16_to_cpu(fval[2]), -def->range, def->range); + data->scan.channels[0] = clamp_t(s16, le16_to_cpu(fval[0]), -def->range, def->range); + data->scan.channels[1] = clamp_t(s16, le16_to_cpu(fval[1]), -def->range, def->range); + data->scan.channels[2] = clamp_t(s16, le16_to_cpu(fval[2]), -def->range, def->range); - iio_push_to_buffers_with_timestamp(indio_dev, buff, + iio_push_to_buffers_with_timestamp(indio_dev, &data->scan, iio_get_time_ns(indio_dev)); + return; unlock: From patchwork Wed Jul 22 15:50:50 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Cameron X-Patchwork-Id: 11678719 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 6579413A4 for ; Wed, 22 Jul 2020 15:53:31 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 4D82B22BEF for ; Wed, 22 Jul 2020 15:53:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433211; bh=ScHk/g2so9KptJyBAfekbi3R35PPSVX+BKTDj1tgQIo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=kzEy8yOpPZTxNZwnyeKYQzmiMEt3GPm60507syVwh7H0xFlCEAsE+O/O2bwTIhaRQ OhFIR5OT4H8KJz9WJahhX5Nl9bRiPPloy7DQ+0+464oWanoNoaZ/IH5qBSuyb0S+3r G/U5wZZfZ6/UIZs+DVSSz07TeWHXtP0yu4FtyT1M= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729642AbgGVPxa (ORCPT ); Wed, 22 Jul 2020 11:53:30 -0400 Received: from mail.kernel.org ([198.145.29.99]:35712 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726427AbgGVPxa (ORCPT ); Wed, 22 Jul 2020 11:53:30 -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 92F2720B1F; Wed, 22 Jul 2020 15:53:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433209; bh=ScHk/g2so9KptJyBAfekbi3R35PPSVX+BKTDj1tgQIo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=X5XNsDsRbVSOM8l/REPJsjmiI0+w9asSxuE/Sqw7SZvaGd9vTDub+epB6eEuIDkbV JDizWZXjHacqIlrDLOgsCmayVtoiUNhduQelWzkWZEPmIZJt88JMaAn0dHBchvcHQ1 8WCdultNosaMpkF5+huJ7UhI8YjU8eCzeAD0vrVQ= From: Jonathan Cameron To: linux-iio@vger.kernel.org Cc: Andy Shevchenko , Lars-Peter Clausen , Peter Meerwald , Jonathan Cameron Subject: [PATCH v3 14/27] iio:magnetometer:mag3110 Fix alignment and data leak issues. Date: Wed, 22 Jul 2020 16:50:50 +0100 Message-Id: <20200722155103.979802-15-jic23@kernel.org> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200722155103.979802-1-jic23@kernel.org> References: <20200722155103.979802-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 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 --- 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 4d305a21c379..e96113ca39bd 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 temp; + 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.temp = 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 Wed Jul 22 15:50:51 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Cameron X-Patchwork-Id: 11678721 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 1BC5413A4 for ; Wed, 22 Jul 2020 15:53:33 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 011D12084D for ; Wed, 22 Jul 2020 15:53:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433213; bh=bU/B4ZHBPu2q5zoajXKveNEYO5LK3+e8NB0+jBTWcak=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=2R/QuT9ffmE10mnGLdjobxMDGn4W5Lwmg2IrkxyXpurmevN6+QVevJzPgnHrEeiHj xaSfj8I1BBNKqVVHH0Inq702N776BZkbd9P9VUs+dfgwhxjg5BRPKfeonp5wRviglM OBGBwDOscnwxGfa+Q3DamvwVJVd2dXhmqwBjP/W0= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729178AbgGVPxc (ORCPT ); Wed, 22 Jul 2020 11:53:32 -0400 Received: from mail.kernel.org ([198.145.29.99]:35734 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726427AbgGVPxb (ORCPT ); Wed, 22 Jul 2020 11:53: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 0019E2080D; Wed, 22 Jul 2020 15:53:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433211; bh=bU/B4ZHBPu2q5zoajXKveNEYO5LK3+e8NB0+jBTWcak=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=y6BcVgGpPAeIUZ/oxXM+kS0H6Qqumluva5gkJsAN1lHAEHuadAas8pdGL0pU4ycV/ 9HpHLVsbQ4LlzcjWuKi2cPA/5/r6cdzk2+Rq/4A/cL98MJbqvunpl5VH1s23bn2dcp NE2pzHSLY6LO5h9qC3juevCcwUSyggz+7gMB4W9Y= From: Jonathan Cameron To: linux-iio@vger.kernel.org Cc: Andy Shevchenko , Lars-Peter Clausen , Peter Meerwald , Jonathan Cameron , Daniel Baluta Subject: [PATCH v3 15/27] iio:imu:bmi160 Fix alignment and data leak issues Date: Wed, 22 Jul 2020 16:50:51 +0100 Message-Id: <20200722155103.979802-16-jic23@kernel.org> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200722155103.979802-1-jic23@kernel.org> References: <20200722155103.979802-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 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 datalayout without it being missleading. Fixes: 77c4ad2d6a9b ("iio: imu: Add initial support for Bosch BMI160") Reported-by: Lars-Peter Clausen Signed-off-by: Jonathan Cameron Cc: Daniel Baluta Signed-off-by: Jonathan Cameron --- drivers/iio/imu/bmi160/bmi160.h | 2 ++ drivers/iio/imu/bmi160/bmi160_core.c | 5 ++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/iio/imu/bmi160/bmi160.h b/drivers/iio/imu/bmi160/bmi160.h index a82e040bd109..d29f1b5d1658 100644 --- a/drivers/iio/imu/bmi160/bmi160.h +++ b/drivers/iio/imu/bmi160/bmi160.h @@ -10,6 +10,8 @@ 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 */ + __le16 buf[16] __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 222ebb26f013..86cfd75ea125 100644 --- a/drivers/iio/imu/bmi160/bmi160_core.c +++ b/drivers/iio/imu/bmi160/bmi160_core.c @@ -427,7 +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[16]; /* 3 sens x 3 axis x __le16 + 3 x __le16 pad + 4 x __le16 tstamp */ int i, ret, j = 0, base = BMI160_REG_DATA_MAGN_XOUT_L; __le16 sample; @@ -438,10 +437,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 Wed Jul 22 15:50:52 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Cameron X-Patchwork-Id: 11678723 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 4556014E3 for ; Wed, 22 Jul 2020 15:53:34 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 2B72C207CD for ; Wed, 22 Jul 2020 15:53:34 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433214; bh=BrMikSOSP8rQfuWua//rncDQlSlapCNNxsWOW25/l14=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=jLqWmt3RV4tvcfb0TzaB2D3Ot6Ae0v67zAVJIr6pmHnP8XMQffFo00SMMVuGp/Y5J c75j88WFn2z2RyOfSjYN7hnNorhHGbKAQkR8lt3Rw9ZLr1bmG5Zho+3PkhE0MbYK7O ewE9uVONE8MIihwH6kT32wuEXaxNGZFNYRDca5OQ= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729866AbgGVPxd (ORCPT ); Wed, 22 Jul 2020 11:53:33 -0400 Received: from mail.kernel.org ([198.145.29.99]:35748 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726427AbgGVPxd (ORCPT ); Wed, 22 Jul 2020 11:53: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 85E95206F5; Wed, 22 Jul 2020 15:53:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433212; bh=BrMikSOSP8rQfuWua//rncDQlSlapCNNxsWOW25/l14=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=S4mtAlA83544RlA4UcAlrsCNvm6Lv/JzmG71IvQFUUacScFctovXqNauLKB9hyu7R FuBDnDPvuCETgeB18nuNT9MT8pNKSIY3ZhmBNmdO7wU+rOkg6IqKvhtwDvI7GPweD5 36jsa99k4GuQq7hDVHffTz/fSk5utbBAJ4Uhx36M= From: Jonathan Cameron To: linux-iio@vger.kernel.org Cc: Andy Shevchenko , Lars-Peter Clausen , Peter Meerwald , Jonathan Cameron , Lorenzo Bianconi Subject: [PATCH v3 16/27] iio:imu:st_lsm6dsx Fix alignment and data leak issues Date: Wed, 22 Jul 2020 16:50:52 +0100 Message-Id: <20200722155103.979802-17-jic23@kernel.org> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200722155103.979802-1-jic23@kernel.org> References: <20200722155103.979802-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 an array 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 --- drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h | 6 ++++ .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 36 ++++++++++--------- 2 files changed, 26 insertions(+), 16 deletions(-) diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h index d80ba2e688ed..9275346a9cc1 100644 --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h @@ -383,6 +383,7 @@ struct st_lsm6dsx_sensor { * @iio_devs: Pointers to acc/gyro iio_dev instances. * @settings: Pointer to the specific sensor settings in use. * @orientation: sensor chip orientation relative to main hardware. + * @scan: Temporary buffers used to align data before iio_push_to_buffers() */ struct st_lsm6dsx_hw { struct device *dev; @@ -411,6 +412,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); + } scan[3]; }; 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 7de10bd636ea..9e0404e3581d 100644 --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c @@ -353,9 +353,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; @@ -416,19 +413,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->scan[ST_LSM6DSX_ID_GYRO].channels, + &hw->buff[offset], + sizeof(hw->scan[ST_LSM6DSX_ID_GYRO].channels)); + offset += sizeof(hw->scan[ST_LSM6DSX_ID_GYRO].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->scan[ST_LSM6DSX_ID_ACC].channels, + &hw->buff[offset], + sizeof(hw->scan[ST_LSM6DSX_ID_ACC].channels)); + offset += sizeof(hw->scan[ST_LSM6DSX_ID_ACC].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->scan[ST_LSM6DSX_ID_EXT0].channels, + &hw->buff[offset], + sizeof(hw->scan[ST_LSM6DSX_ID_EXT0].channels)); + offset += sizeof(hw->scan[ST_LSM6DSX_ID_EXT0].channels); } if (ts_sip-- > 0) { @@ -458,19 +458,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->scan[ST_LSM6DSX_ID_GYRO], + 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->scan[ST_LSM6DSX_ID_ACC], + 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->scan[ST_LSM6DSX_ID_EXT0], + ext_sensor->ts_ref + ts); ext_sip--; } sip++; @@ -555,7 +558,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; From patchwork Wed Jul 22 15:50:53 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Cameron X-Patchwork-Id: 11678725 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 62D8613A4 for ; Wed, 22 Jul 2020 15:53:35 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 4B0FB207CD for ; Wed, 22 Jul 2020 15:53:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433215; bh=Q1+lA1qWyLMqAmlkrxNs+4cWTmkFY09nI45VNF2csog=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=ln/e9CisiZnLGqo+Y11XlE/Y0MrTtB7x6GM90sXedmxR7OsPZMRuUQuvie5IbAfa5 Uyrewg7AjxQJIZODdDQdz8Z847i4vgIOVcpLL7I7IYgZy0ZKmPIGX9K74g55r2JubU tb5sSpC/P1oCQ8U075ZlMPYHzWJydIdgWgu5XX3U= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730111AbgGVPxe (ORCPT ); Wed, 22 Jul 2020 11:53:34 -0400 Received: from mail.kernel.org ([198.145.29.99]:35770 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726427AbgGVPxe (ORCPT ); Wed, 22 Jul 2020 11:53:34 -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 19A7E22BEF; Wed, 22 Jul 2020 15:53:32 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433214; bh=Q1+lA1qWyLMqAmlkrxNs+4cWTmkFY09nI45VNF2csog=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=ULhzOULN8RQquvxDdHd6rTERpGFDni/XJzx3PBuC+X9szIjd58WgL7tLH8WvHAcsC HHYCNAl6+p+zE8UsJZTebQs2zfNFXy27McTNy9bVMG5mflFsik24GTrY1wZbfH6KxZ JB5huq1thciiIQedvQpg300X5Lfe9/rSgIzqlcro= From: Jonathan Cameron To: linux-iio@vger.kernel.org Cc: Andy Shevchenko , Lars-Peter Clausen , Peter Meerwald , Jonathan Cameron , Jean-Baptiste Maneyrol Subject: [PATCH v3 17/27] iio:imu:inv_mpu6050 Fix dma and ts alignment and data leak issues. Date: Wed, 22 Jul 2020 16:50:53 +0100 Message-Id: <20200722155103.979802-18-jic23@kernel.org> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200722155103.979802-1-jic23@kernel.org> References: <20200722155103.979802-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 This case is a bit different to the rest of the series. The driver was doing a regmap_bulk_read into a buffer that wasn't dma safe as it was on the stack with no guarantee of it being in a cacheline on it's own. Fixing that also dealt with the data leak and alignment issues that Lars-Peter pointed out. Also removed some unaligned handling as we are now aligned. Fixes tag is for the dma safe buffer issue. Potentially we would need to backport timestamp alignment futher but that is a totally different patch. Fixes: fd64df16f40e ("iio: imu: inv_mpu6050: Add SPI support for MPU6000") Reported-by: Lars-Peter Clausen Cc: Jean-Baptiste Maneyrol Signed-off-by: Jonathan Cameron Reviewed-by: Jean-Baptiste Maneyrol Reported-by: Lars-Peter Clausen Signed-off-by: Jonathan Cameron --- drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 8 +++++--- drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c | 12 +++++------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h index cd38b3fccc7b..e4df2d51b689 100644 --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h @@ -122,6 +122,9 @@ struct inv_mpu6050_chip_config { u8 user_ctrl; }; +/* 6 + 6 + 2 + 7 (for MPU9x50) = 21 round up to 24 and plus 8 */ +#define INV_MPU6050_OUTPUT_DATA_SIZE 32 + /** * struct inv_mpu6050_hw - Other important hardware information. * @whoami: Self identification byte from WHO_AM_I register @@ -165,6 +168,7 @@ struct inv_mpu6050_hw { * @magn_raw_to_gauss: coefficient to convert mag raw value to Gauss. * @magn_orient: magnetometer sensor chip orientation if available. * @suspended_sensors: sensors mask of sensors turned off for suspend + * @data: dma safe buffer used for bulk reads. */ struct inv_mpu6050_state { struct mutex lock; @@ -190,6 +194,7 @@ struct inv_mpu6050_state { s32 magn_raw_to_gauss[3]; struct iio_mount_matrix magn_orient; unsigned int suspended_sensors; + u8 data[INV_MPU6050_OUTPUT_DATA_SIZE] ____cacheline_aligned; }; /*register and associated bit definition*/ @@ -334,9 +339,6 @@ struct inv_mpu6050_state { #define INV_ICM20608_TEMP_OFFSET 8170 #define INV_ICM20608_TEMP_SCALE 3059976 -/* 6 + 6 + 2 + 7 (for MPU9x50) = 21 round up to 24 and plus 8 */ -#define INV_MPU6050_OUTPUT_DATA_SIZE 32 - #define INV_MPU6050_REG_INT_PIN_CFG 0x37 #define INV_MPU6050_ACTIVE_HIGH 0x00 #define INV_MPU6050_ACTIVE_LOW 0x80 diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c index b533fa2dad0a..d8e6b88ddffc 100644 --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c @@ -13,7 +13,6 @@ #include #include #include -#include #include "inv_mpu_iio.h" /** @@ -121,7 +120,6 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p) struct inv_mpu6050_state *st = iio_priv(indio_dev); size_t bytes_per_datum; int result; - u8 data[INV_MPU6050_OUTPUT_DATA_SIZE]; u16 fifo_count; s64 timestamp; int int_status; @@ -160,11 +158,11 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p) * read fifo_count register to know how many bytes are inside the FIFO * right now */ - result = regmap_bulk_read(st->map, st->reg->fifo_count_h, data, - INV_MPU6050_FIFO_COUNT_BYTE); + result = regmap_bulk_read(st->map, st->reg->fifo_count_h, + st->data, INV_MPU6050_FIFO_COUNT_BYTE); if (result) goto end_session; - fifo_count = get_unaligned_be16(&data[0]); + fifo_count = be16_to_cpup((__be16 *)&st->data[0]); /* * Handle fifo overflow by resetting fifo. @@ -182,7 +180,7 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p) inv_mpu6050_update_period(st, pf->timestamp, nb); for (i = 0; i < nb; ++i) { result = regmap_bulk_read(st->map, st->reg->fifo_r_w, - data, bytes_per_datum); + st->data, bytes_per_datum); if (result) goto flush_fifo; /* skip first samples if needed */ @@ -191,7 +189,7 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p) continue; } timestamp = inv_mpu6050_get_timestamp(st); - iio_push_to_buffers_with_timestamp(indio_dev, data, timestamp); + iio_push_to_buffers_with_timestamp(indio_dev, st->data, timestamp); } end_session: From patchwork Wed Jul 22 15:50:54 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Cameron X-Patchwork-Id: 11678727 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 0D1BF13A4 for ; Wed, 22 Jul 2020 15:53:37 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id DA307207CD for ; Wed, 22 Jul 2020 15:53:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433216; bh=aNShfbyrW0F2L3sXIPPxtNaDWjF+rIoFOTDsdMIaoIQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=XrUxAgT2DsWSTSKTyBV97f+xbs1detrADf646Dn0BElmPVxPZRK1OrNmxImYCPevG +5fImjfUq+UcIvKg6wVfc2dICVBXC5OxmfbvBt8bhpBbkrV9Vn+IeAq5J7F3UGAbl2 /1ycdBegIxZC/yxDRCMiNlzTOQc5GLvqKp36LGKw= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730293AbgGVPxg (ORCPT ); Wed, 22 Jul 2020 11:53:36 -0400 Received: from mail.kernel.org ([198.145.29.99]:35786 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726427AbgGVPxg (ORCPT ); Wed, 22 Jul 2020 11:53:36 -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 AFB2720717; Wed, 22 Jul 2020 15:53:34 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433215; bh=aNShfbyrW0F2L3sXIPPxtNaDWjF+rIoFOTDsdMIaoIQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=fxhoWybMtqxOk/FvrLOaqLl8o5uXi2xMxrBahurXloNoTeEnxGTinWygLGG6ougtG RfxV6IMfxsvEPYE4e0wmBs9eNJ66n0MyDx968a4vnX+76i5fcUHek6HMKjYCK9iTvO mtxCTbVXtxtJrO+hGbPGN3RLHNySW8UA6igvXgs4= From: Jonathan Cameron To: linux-iio@vger.kernel.org Cc: Andy Shevchenko , Lars-Peter Clausen , Peter Meerwald , Jonathan Cameron , Jean-Baptiste Maneyrol Subject: [PATCH v3 18/27] iio:imu:inv_mpu6050: Use regmap_noinc_read for fifo reads. Date: Wed, 22 Jul 2020 16:50:54 +0100 Message-Id: <20200722155103.979802-19-jic23@kernel.org> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200722155103.979802-1-jic23@kernel.org> References: <20200722155103.979802-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 We should not be assuming that we are reading a sequence of registers as here we are doing a read of a lot of data from a single register address. Suggested-by: Jean-Baptiste Maneyrol Signed-off-by: Jonathan Cameron Reviewed-by: Jean-Baptiste Maneyrol Signed-off-by: Jonathan Cameron --- drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c index d8e6b88ddffc..45c37525c2f1 100644 --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c @@ -179,8 +179,8 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p) nb = fifo_count / bytes_per_datum; inv_mpu6050_update_period(st, pf->timestamp, nb); for (i = 0; i < nb; ++i) { - result = regmap_bulk_read(st->map, st->reg->fifo_r_w, - st->data, bytes_per_datum); + result = regmap_noinc_read(st->map, st->reg->fifo_r_w, + st->data, bytes_per_datum); if (result) goto flush_fifo; /* skip first samples if needed */ From patchwork Wed Jul 22 15:50: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: 11678729 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 9A2D313A4 for ; Wed, 22 Jul 2020 15:53:38 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 77D29208E4 for ; Wed, 22 Jul 2020 15:53:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433218; bh=JvD7nhY9VfGsfS8vJGMtYvEtmAxdoF7RxvdVp+w7rTU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=xqt2gM2OWTdYox/CsCqJjM1H4+9HIZbnFBiE65gcxrQX5WbbH6pLtVlQpYBGfGO+R AKvLLvtM1d5JaaE5G88jSXsfbnFHUDl2I3sFoo8ftk6vFyAXsPfNM1x+eOJergS3QI uYuZdz0Ry5Ay46PmLW7kTHNv4bLiG2zre2nUkqCc= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730402AbgGVPxi (ORCPT ); Wed, 22 Jul 2020 11:53:38 -0400 Received: from mail.kernel.org ([198.145.29.99]:35806 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726427AbgGVPxh (ORCPT ); Wed, 22 Jul 2020 11:53:37 -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 4921B20771; Wed, 22 Jul 2020 15:53:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433217; bh=JvD7nhY9VfGsfS8vJGMtYvEtmAxdoF7RxvdVp+w7rTU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=cthvhKvdMEUCcZT8TKwECACI+hVNwxe4KXAXAmG0+vuBNcenFADQsAEAL6RjRSVLa zz5OOowPMPZwGqtgAwH2BC16xi3b1eoKQR0e4JWtOmGGBbiSuaWulnSmVx7WQGB3lq ZR8lkamqBKE+NgiUF8daMnSLEjZLFDkBEPfkWSw8= From: Jonathan Cameron To: linux-iio@vger.kernel.org Cc: Andy Shevchenko , Lars-Peter Clausen , Peter Meerwald , Jonathan Cameron Subject: [PATCH v3 19/27] iio:pressure:mpl3115 Force alignment of buffer Date: Wed, 22 Jul 2020 16:50:55 +0100 Message-Id: <20200722155103.979802-20-jic23@kernel.org> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200722155103.979802-1-jic23@kernel.org> References: <20200722155103.979802-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 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 Cc: Peter Meerwald Signed-off-by: Jonathan Cameron --- drivers/iio/pressure/mpl3115.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/iio/pressure/mpl3115.c b/drivers/iio/pressure/mpl3115.c index ccdb0b70e48c..8a481dbe808c 100644 --- a/drivers/iio/pressure/mpl3115.c +++ b/drivers/iio/pressure/mpl3115.c @@ -144,7 +144,8 @@ 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 */ + u8 buffer[16] __aligned(8); int ret, pos = 0; mutex_lock(&data->lock); From patchwork Wed Jul 22 15:50:56 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Cameron X-Patchwork-Id: 11678731 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 E01E314E3 for ; Wed, 22 Jul 2020 15:53:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id BCC97208E4 for ; Wed, 22 Jul 2020 15:53:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433219; bh=w3LGe39IerBHvYTv/VvKMnI3lLTCyXa1a7u1VSCcP78=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=gms4Y9QP7a6y6ts5qB5sd7KJiJvenCahGy2G7uadW/PcCeyvgRmGXFuUcVdGxvCxa V4aaw2RU80jHGpLRaCbNMqe0yzphYRpp25Is3aUUdr/6WwiI5FJGy3CZmHqGbceE6l RKMdwxwn/Eg3/3sI6nDEBqFjhn8LfwIfbIJfk4Oc= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730468AbgGVPxj (ORCPT ); Wed, 22 Jul 2020 11:53:39 -0400 Received: from mail.kernel.org ([198.145.29.99]:35822 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726427AbgGVPxj (ORCPT ); Wed, 22 Jul 2020 11:53:39 -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 ACF82207CD; Wed, 22 Jul 2020 15:53:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433218; bh=w3LGe39IerBHvYTv/VvKMnI3lLTCyXa1a7u1VSCcP78=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=lvTYj74jL5dcOiob8DEV5TcnWMhGdX0izdY0UUASib/Z7PuDJhygJI3tVl8DzsnJ0 BL+VwxlxBRhJB6mw3dya/0LAARmMUnXWrpthtLmr6XJxXVCl+PJdGp4waNJq62d68Q DUY989B5t6URjx2y/3Ko9uh1TXOcVSXuFGenDCnc= From: Jonathan Cameron To: linux-iio@vger.kernel.org Cc: Andy Shevchenko , Lars-Peter Clausen , Peter Meerwald , Jonathan Cameron Subject: [PATCH v3 20/27] iio:adc:ti-adc081c Fix alignment and data leak issues Date: Wed, 22 Jul 2020 16:50:56 +0100 Message-Id: <20200722155103.979802-21-jic23@kernel.org> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200722155103.979802-1-jic23@kernel.org> References: <20200722155103.979802-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 suitable structure in the iio_priv(). This data is allocated with kzalloc so no data can leak apart from previous readings. The eplicit alignment of ts is necessary to ensure correct padding on x86_32 where s64 is only aligned to 4 bytes. Fixes: 08e05d1fce5c (" ti-adc081c: Initial triggered buffer support") Reported-by: Lars-Peter Clausen Signed-off-by: Jonathan Cameron --- drivers/iio/adc/ti-adc081c.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/iio/adc/ti-adc081c.c b/drivers/iio/adc/ti-adc081c.c index 9426f70a8005..cf63983a54d9 100644 --- a/drivers/iio/adc/ti-adc081c.c +++ b/drivers/iio/adc/ti-adc081c.c @@ -33,6 +33,12 @@ struct adc081c { /* 8, 10 or 12 */ int bits; + + /* Ensure natural alignment of buffer elements */ + struct { + u16 channel; + s64 ts __aligned(8); + } scan; }; #define REG_CONV_RES 0x00 @@ -128,14 +134,13 @@ static irqreturn_t adc081c_trigger_handler(int irq, void *p) struct iio_poll_func *pf = p; struct iio_dev *indio_dev = pf->indio_dev; struct adc081c *data = iio_priv(indio_dev); - u16 buf[8]; /* 2 bytes data + 6 bytes padding + 8 bytes timestamp */ int ret; ret = i2c_smbus_read_word_swapped(data->i2c, REG_CONV_RES); if (ret < 0) goto out; - buf[0] = ret; - iio_push_to_buffers_with_timestamp(indio_dev, buf, + data->scan.channel = ret; + iio_push_to_buffers_with_timestamp(indio_dev, &data->scan, iio_get_time_ns(indio_dev)); out: iio_trigger_notify_done(indio_dev->trig); From patchwork Wed Jul 22 15:50:57 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Jonathan Cameron X-Patchwork-Id: 11678733 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 8775214E3 for ; Wed, 22 Jul 2020 15:53:41 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6D796208E4 for ; Wed, 22 Jul 2020 15:53:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433221; bh=Wlfui+VlY4bDZW0tV1inSNbem/cmMyKs1kjmlaZJbu4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=T2g42k+CzTMU8YvuXMrwM+Gg+b+vm1atvjKUFNopC5CZigOoe5Xb3bAkYDxbCQFRZ PtQ1F0CU4wFr/9dV+MPI7Bl4JvDN4EeijFt0Txi4SSuevFWOIUHUZju93sHTXD6u1j KFtz84mZxZtS6Q1YfCMR0t5TC5SC+J0ewPbcAeOM= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730581AbgGVPxl (ORCPT ); Wed, 22 Jul 2020 11:53:41 -0400 Received: from mail.kernel.org ([198.145.29.99]:35846 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726427AbgGVPxk (ORCPT ); Wed, 22 Jul 2020 11:53:40 -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 204B42084D; Wed, 22 Jul 2020 15:53:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433220; bh=Wlfui+VlY4bDZW0tV1inSNbem/cmMyKs1kjmlaZJbu4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Hq0TM+F6Nfz5kUB41o3SvMR94PglPhTEmg5+fEXWnIikrjxi8uKZewypwDd00PVnK NJ4lG/lBTFaAajHIXMuSfiDpcA8hk5i+8rNwW+bLbvGWPKBwWIP2jQRTurLRujsgx8 QVxmmqYMqfI1TaY+om7/Gg9BEiOrP8oNrYZvIuwM= From: Jonathan Cameron To: linux-iio@vger.kernel.org Cc: Andy Shevchenko , Lars-Peter Clausen , Peter Meerwald , Jonathan Cameron , =?utf-8?q?M=C3=A5rten_Linda?= =?utf-8?q?hl?= Subject: [PATCH v3 21/27] iio:adc:ti-adc084s021 Fix alignment and data leak issues. Date: Wed, 22 Jul 2020 16:50:57 +0100 Message-Id: <20200722155103.979802-22-jic23@kernel.org> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200722155103.979802-1-jic23@kernel.org> References: <20200722155103.979802-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 suitable structure in the iio_priv(). This data is allocated with kzalloc so no data can leak apart from previous readings. The force alignment of ts is not strictly necessary in this case but reduces the fragility of the code. Fixes: 3691e5a69449 ("iio: adc: add driver for the ti-adc084s021 chip") Reported-by: Lars-Peter Clausen Cc: Mårten Lindahl Signed-off-by: Jonathan Cameron --- drivers/iio/adc/ti-adc084s021.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/iio/adc/ti-adc084s021.c b/drivers/iio/adc/ti-adc084s021.c index 9017e1e24273..dfba34834a57 100644 --- a/drivers/iio/adc/ti-adc084s021.c +++ b/drivers/iio/adc/ti-adc084s021.c @@ -26,6 +26,11 @@ struct adc084s021 { struct spi_transfer spi_trans; struct regulator *reg; struct mutex lock; + /* Buffer used to align data */ + struct { + __be16 channels[4]; + s64 ts __aligned(8); + } scan; /* * DMA (thus cache coherency maintenance) requires the * transfer buffers to live in their own cache line. @@ -141,14 +146,13 @@ static irqreturn_t adc084s021_buffer_trigger_handler(int irq, void *pollfunc) struct iio_poll_func *pf = pollfunc; struct iio_dev *indio_dev = pf->indio_dev; struct adc084s021 *adc = iio_priv(indio_dev); - __be16 data[8] = {0}; /* 4 * 16-bit words of data + 8 bytes timestamp */ mutex_lock(&adc->lock); - if (adc084s021_adc_conversion(adc, &data) < 0) + if (adc084s021_adc_conversion(adc, adc->scan.channels) < 0) dev_err(&adc->spi->dev, "Failed to read data\n"); - iio_push_to_buffers_with_timestamp(indio_dev, data, + iio_push_to_buffers_with_timestamp(indio_dev, &adc->scan, iio_get_time_ns(indio_dev)); mutex_unlock(&adc->lock); iio_trigger_notify_done(indio_dev->trig); From patchwork Wed Jul 22 15:50:58 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Cameron X-Patchwork-Id: 11678735 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 D8B7413A4 for ; Wed, 22 Jul 2020 15:53:42 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id BC6F320B1F for ; Wed, 22 Jul 2020 15:53:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433222; bh=6gc1X0dWiUoU4cSntRDo7o1SLHOB2YUmtLeKTrCA+6M=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=w1E93dNTU99Dii5qQilJH0mU1nIoITNm40vUQVvKEolR4tL3TlTIweLIN6zKhgfqq 1qoH4u9q4tgbD/f4+uiBa2u5n85iKi9Cf+LFJzi7mQuQ8mYAgfL5QzsT/B/ewVBFuQ F2eJNDrX9LGvvS4oQyod88IXDhUOCaDbfj5S2NOI= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730657AbgGVPxm (ORCPT ); Wed, 22 Jul 2020 11:53:42 -0400 Received: from mail.kernel.org ([198.145.29.99]:35858 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726427AbgGVPxm (ORCPT ); Wed, 22 Jul 2020 11:53:42 -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 A3293207E8; Wed, 22 Jul 2020 15:53:40 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433221; bh=6gc1X0dWiUoU4cSntRDo7o1SLHOB2YUmtLeKTrCA+6M=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=uJEmWCMTjJVgBlY5Gr3sN62AdonyKPTpDx+ENZ5XVEvSmJL217qLUIGFlYFYPWlg1 k8/RjgnCSnpIEysxBy3ukabOKq3Y83pwaUlm6djk8hqJR1NzRAYYq8KIwZx2YRpv8/ I/3mjRVcxKRyF0gvpsjikcHyNWop6fFQR41trMNM= From: Jonathan Cameron To: linux-iio@vger.kernel.org Cc: Andy Shevchenko , Lars-Peter Clausen , Peter Meerwald , Jonathan Cameron Subject: [PATCH v3 22/27] iio:adc:ti-adc084s021 Tidy up endian types Date: Wed, 22 Jul 2020 16:50:58 +0100 Message-Id: <20200722155103.979802-23-jic23@kernel.org> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200722155103.979802-1-jic23@kernel.org> References: <20200722155103.979802-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 By adding a few local variables and avoiding a void * for a parameter we can easily make all the endian types explicit and get rid of the warnings from sparse: CHECK drivers/iio/adc/ti-adc084s021.c drivers/iio/adc/ti-adc084s021.c:84:26: warning: incorrect type in assignment (different base types) drivers/iio/adc/ti-adc084s021.c:84:26: expected unsigned short [usertype] drivers/iio/adc/ti-adc084s021.c:84:26: got restricted __be16 drivers/iio/adc/ti-adc084s021.c:115:24: warning: cast to restricted __be16 drivers/iio/adc/ti-adc084s021.c:115:24: warning: cast to restricted __be16 drivers/iio/adc/ti-adc084s021.c:115:24: warning: cast to restricted __be16 drivers/iio/adc/ti-adc084s021.c:115:24: warning: cast to restricted __be16 Signed-off-by: Jonathan Cameron --- drivers/iio/adc/ti-adc084s021.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/iio/adc/ti-adc084s021.c b/drivers/iio/adc/ti-adc084s021.c index dfba34834a57..fb14b92fa6e7 100644 --- a/drivers/iio/adc/ti-adc084s021.c +++ b/drivers/iio/adc/ti-adc084s021.c @@ -70,11 +70,10 @@ static const struct iio_chan_spec adc084s021_channels[] = { * @adc: The ADC SPI data. * @data: Buffer for converted data. */ -static int adc084s021_adc_conversion(struct adc084s021 *adc, void *data) +static int adc084s021_adc_conversion(struct adc084s021 *adc, __be16 *data) { int n_words = (adc->spi_trans.len >> 1) - 1; /* Discard first word */ int ret, i = 0; - u16 *p = data; /* Do the transfer */ ret = spi_sync(adc->spi, &adc->message); @@ -82,7 +81,7 @@ static int adc084s021_adc_conversion(struct adc084s021 *adc, void *data) return ret; for (; i < n_words; i++) - *(p + i) = adc->rx_buf[i + 1]; + *(data + i) = adc->rx_buf[i + 1]; return ret; } @@ -93,6 +92,7 @@ static int adc084s021_read_raw(struct iio_dev *indio_dev, { struct adc084s021 *adc = iio_priv(indio_dev); int ret; + __be16 be_val; switch (mask) { case IIO_CHAN_INFO_RAW: @@ -107,13 +107,13 @@ static int adc084s021_read_raw(struct iio_dev *indio_dev, } adc->tx_buf[0] = channel->channel << 3; - ret = adc084s021_adc_conversion(adc, val); + ret = adc084s021_adc_conversion(adc, &be_val); iio_device_release_direct_mode(indio_dev); regulator_disable(adc->reg); if (ret < 0) return ret; - *val = be16_to_cpu(*val); + *val = be16_to_cpu(be_val); *val = (*val >> channel->scan_type.shift) & 0xff; return IIO_VAL_INT; From patchwork Wed Jul 22 15:50:59 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Cameron X-Patchwork-Id: 11678737 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 E6BD414E3 for ; Wed, 22 Jul 2020 15:53:44 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id CDEAA207DD for ; Wed, 22 Jul 2020 15:53:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433224; bh=BIatcbE+CfKNYNAPsaxb4Ys2/kUbi6raTVuQg4Z0Ct0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=TIvb+DhyE+//jq8JlBMwFDthVN5XiUdhVqlqmPBUGYUb9KVBz3CDl9Cqz1jsvECgQ wlULBEn42vTTVD9jiDhihHvLgMs+nj+ywVNCCblABltBPueQaW12UBJk9rafbyrSoH 5bnd2egfZ3IjbT6UaPF1yLI16IK07ciIvKvpwrD4= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730591AbgGVPxo (ORCPT ); Wed, 22 Jul 2020 11:53:44 -0400 Received: from mail.kernel.org ([198.145.29.99]:35872 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726427AbgGVPxo (ORCPT ); Wed, 22 Jul 2020 11:53:44 -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 1C4562080D; Wed, 22 Jul 2020 15:53:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433223; bh=BIatcbE+CfKNYNAPsaxb4Ys2/kUbi6raTVuQg4Z0Ct0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=GQsZ1n0WU3Ta4+FUBTZlfeY+NFRMf6wyNYFvgaL23fAKaHOdxrX6xDaQ5G5ayNRnO SKRZUYgtMwsMKEvfoCtORjg8seS4etiH4eTh+/gasi7/rAhe7F3E9Fumz4ZdC9yyYI e7RLa52gzgeVkPSO3NlcgTWIKMqlZSzHihVZcg+M= From: Jonathan Cameron To: linux-iio@vger.kernel.org Cc: Andy Shevchenko , Lars-Peter Clausen , Peter Meerwald , Jonathan Cameron , Dan Murphy Subject: [PATCH v3 23/27] iio:adc:ti-ads124s08 Fix alignment and data leak issues. Date: Wed, 22 Jul 2020 16:50:59 +0100 Message-Id: <20200722155103.979802-24-jic23@kernel.org> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200722155103.979802-1-jic23@kernel.org> References: <20200722155103.979802-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 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 missleading. Fixes: e717f8c6dfec ("iio: adc: Add the TI ads124s08 ADC code") Reported-by: Lars-Peter Clausen Cc: Dan Murphy Signed-off-by: Jonathan Cameron --- drivers/iio/adc/ti-ads124s08.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/iio/adc/ti-ads124s08.c b/drivers/iio/adc/ti-ads124s08.c index 4b4fbe33930c..734ee5d82ff6 100644 --- a/drivers/iio/adc/ti-ads124s08.c +++ b/drivers/iio/adc/ti-ads124s08.c @@ -99,6 +99,11 @@ 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. + */ + u32 buffer[ADS124S08_MAX_CHANNELS + sizeof(s64)/sizeof(u16)] __aligned(8); u8 data[5] ____cacheline_aligned; }; @@ -269,7 +274,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(u16)]; int scan_index, j = 0; int ret; @@ -284,7 +288,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 +296,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); From patchwork Wed Jul 22 15:51:00 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Cameron X-Patchwork-Id: 11678739 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 BE44A14E3 for ; Wed, 22 Jul 2020 15:53:45 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A11B120B1F for ; Wed, 22 Jul 2020 15:53:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433225; bh=5eJCV0Z/hSjdqvckepKd6JHihVEQ5bBMA871o1ru3k4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=dsrRf9snu+Xyn2etaq4iQZyqSjmQ+At5ijZRWiN4pNpRyeGOQiM5EcBwMIoAVvcrU n4jt5SJIUdjNC4TfS9Qa/a21H5nd88voOf58KMzMNjDhQDN1Mi2myt9z0MyWRkF4v8 LY4Lf9CRmrkT2/oLbWsJbRAIffxZqETSciUPTEa0= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730870AbgGVPxp (ORCPT ); Wed, 22 Jul 2020 11:53:45 -0400 Received: from mail.kernel.org ([198.145.29.99]:35894 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726427AbgGVPxp (ORCPT ); Wed, 22 Jul 2020 11:53:45 -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 AE769206F5; Wed, 22 Jul 2020 15:53:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433224; bh=5eJCV0Z/hSjdqvckepKd6JHihVEQ5bBMA871o1ru3k4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=P+RmeCY8NGMEWXxLQ6LqX/No9e7k4jee+0gRhZjr7z06UuaTE6LlhV0RLbMgQ82B+ 1zRci3oB5grDkzy7g8I4IWlQ3qNbgU1HK8nLnu0Mf35/7/rSdCQXxl8STyhGHieTbP QFc32IZ0liJWJqq5IU/G46G+9qgUfPmC3ZEvE0N0= From: Jonathan Cameron To: linux-iio@vger.kernel.org Cc: Andy Shevchenko , Lars-Peter Clausen , Peter Meerwald , Jonathan Cameron , Akinobu Mita Subject: [PATCH v3 24/27] iio:adc:ti-adc0832 Fix alignment issue with timestamp Date: Wed, 22 Jul 2020 16:51:00 +0100 Message-Id: <20200722155103.979802-25-jic23@kernel.org> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200722155103.979802-1-jic23@kernel.org> References: <20200722155103.979802-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. We fix this 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. Note that previously no data could leak 'including' previous readings but I don't think it is an issue to potentially leak them like this now does. In this case the postioning of the timestamp is depends on what other channels are enabled. As such we cannot use a structure to make the alignment explicit as it would be missleading by suggesting only one possible location for the timestamp. Fixes: 815bbc87462a ("iio: ti-adc0832: add triggered buffer support") Reported-by: Lars-Peter Clausen Cc: Akinobu Mita Signed-off-by: Jonathan Cameron --- drivers/iio/adc/ti-adc0832.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/iio/adc/ti-adc0832.c b/drivers/iio/adc/ti-adc0832.c index c7a085dce1f4..71d49f97ab9c 100644 --- a/drivers/iio/adc/ti-adc0832.c +++ b/drivers/iio/adc/ti-adc0832.c @@ -29,6 +29,8 @@ struct adc0832 { struct regulator *reg; struct mutex lock; u8 mux_bits; + /* 16x 1 byte ADC data + 8 bytes timestamp */ + u8 data[24] __aligned(8); u8 tx_buf[2] ____cacheline_aligned; u8 rx_buf[2]; @@ -200,7 +202,6 @@ static irqreturn_t adc0832_trigger_handler(int irq, void *p) struct iio_poll_func *pf = p; struct iio_dev *indio_dev = pf->indio_dev; struct adc0832 *adc = iio_priv(indio_dev); - u8 data[24] = { }; /* 16x 1 byte ADC data + 8 bytes timestamp */ int scan_index; int i = 0; @@ -218,10 +219,10 @@ static irqreturn_t adc0832_trigger_handler(int irq, void *p) goto out; } - data[i] = ret; + adc->data[i] = ret; i++; } - iio_push_to_buffers_with_timestamp(indio_dev, data, + iio_push_to_buffers_with_timestamp(indio_dev, adc->data, iio_get_time_ns(indio_dev)); out: mutex_unlock(&adc->lock); From patchwork Wed Jul 22 15:51:01 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Cameron X-Patchwork-Id: 11678741 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 E5A3B13A4 for ; Wed, 22 Jul 2020 15:53:47 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C8F1620B1F for ; Wed, 22 Jul 2020 15:53:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433227; bh=yK695yUBjHPXnG6jLtv82svb8xNb9ne3lwXtsMRgg7U=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=CSsSv8ozNSBOo7JDaunmkn+OayCxOdIoWJfGR58ExNQfc1jLJQ+nPdGYYdO2d/Va8 gM67yjnUj4gsdkDW+eU7zggEzgxYAtRZvjoq7Xe0ge7h2MrcCwsd/XjmKlK6BUa7TS X3HBm0JS0VnBYe+2Rgj3I3/EcDOpyUgCVS3APKH4= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732391AbgGVPxr (ORCPT ); Wed, 22 Jul 2020 11:53:47 -0400 Received: from mail.kernel.org ([198.145.29.99]:35908 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726427AbgGVPxr (ORCPT ); Wed, 22 Jul 2020 11:53:47 -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 41B52207DD; Wed, 22 Jul 2020 15:53:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433226; bh=yK695yUBjHPXnG6jLtv82svb8xNb9ne3lwXtsMRgg7U=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=DBkcvShdEsbxFCaqxn+W821wbSCkzYD9VJnm8x9fvUgdRcgL70drj05hRDJ2kU7IJ hkSFEmBxVyG3uFelzjilLgpPW0hOelQY4nL/sPLPafrV+SAVKJCcQiHFcrASYNlV6s NlbUS4+aaL0RWkefLNs/X1b/UBe0KLJdKnhyzRA4= From: Jonathan Cameron To: linux-iio@vger.kernel.org Cc: Andy Shevchenko , Lars-Peter Clausen , Peter Meerwald , Jonathan Cameron , Akinobu Mita Subject: [PATCH v3 25/27] iio:adc:ti-adc12138 Fix alignment issue with timestamp Date: Wed, 22 Jul 2020 16:51:01 +0100 Message-Id: <20200722155103.979802-26-jic23@kernel.org> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200722155103.979802-1-jic23@kernel.org> References: <20200722155103.979802-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. We move 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. Note that previously no leak at all could occur, but previous readings should never be a problem. In this case the timestamp location depends on what other channels are enabled. As such we can't use a structure without misleading by suggesting only one possible timestamp location. Fixes: 50a6edb1b6e0 ("iio: adc: add ADC12130/ADC12132/ADC12138 ADC driver") Reported-by: Lars-Peter Clausen Cc: Akinobu Mita Signed-off-by: Jonathan Cameron --- drivers/iio/adc/ti-adc12138.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/iio/adc/ti-adc12138.c b/drivers/iio/adc/ti-adc12138.c index e485719cd2c4..16f4fd7a04d9 100644 --- a/drivers/iio/adc/ti-adc12138.c +++ b/drivers/iio/adc/ti-adc12138.c @@ -47,6 +47,8 @@ struct adc12138 { struct completion complete; /* The number of cclk periods for the S/H's acquisition time */ unsigned int acquisition_time; + /* 16x 2 bytes ADC data + 8 bytes timestamp */ + __be16 data[20] __aligned(8); u8 tx_buf[2] ____cacheline_aligned; u8 rx_buf[2]; @@ -329,7 +331,6 @@ static irqreturn_t adc12138_trigger_handler(int irq, void *p) struct iio_poll_func *pf = p; struct iio_dev *indio_dev = pf->indio_dev; struct adc12138 *adc = iio_priv(indio_dev); - __be16 data[20] = { }; /* 16x 2 bytes ADC data + 8 bytes timestamp */ __be16 trash; int ret; int scan_index; @@ -345,7 +346,7 @@ static irqreturn_t adc12138_trigger_handler(int irq, void *p) reinit_completion(&adc->complete); ret = adc12138_start_and_read_conv(adc, scan_chan, - i ? &data[i - 1] : &trash); + i ? &adc->data[i - 1] : &trash); if (ret) { dev_warn(&adc->spi->dev, "failed to start conversion\n"); @@ -362,7 +363,7 @@ static irqreturn_t adc12138_trigger_handler(int irq, void *p) } if (i) { - ret = adc12138_read_conv_data(adc, &data[i - 1]); + ret = adc12138_read_conv_data(adc, &adc->data[i - 1]); if (ret) { dev_warn(&adc->spi->dev, "failed to get conversion data\n"); @@ -370,7 +371,7 @@ static irqreturn_t adc12138_trigger_handler(int irq, void *p) } } - iio_push_to_buffers_with_timestamp(indio_dev, data, + iio_push_to_buffers_with_timestamp(indio_dev, adc->data, iio_get_time_ns(indio_dev)); out: mutex_unlock(&adc->lock); From patchwork Wed Jul 22 15:51:02 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Jonathan Cameron X-Patchwork-Id: 11678743 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 3A39D13A4 for ; Wed, 22 Jul 2020 15:53:49 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 1F8EC20B1F for ; Wed, 22 Jul 2020 15:53:49 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433229; bh=dkvt+oaphgkTDUnzSW8KJtLMYyCSAN74BaQkzRu7QWc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=dknuRIoYKZMVQZEpXdb4A3GISXZc9+anKljfs5F0MBsQQ+ECI3/WdD4/5jHDaaK5p 6UXwo3TDzB8CBmFOQiRmkoXfJGFIe28fEFgV5/0nx5t2e1xIJThNL1Zs0D2Zf4lsVZ oBLMxdWQcSPxQ6PfZVkk3Df2Mikiy9Ei5AWu6cFo= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732726AbgGVPxs (ORCPT ); Wed, 22 Jul 2020 11:53:48 -0400 Received: from mail.kernel.org ([198.145.29.99]:35928 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732624AbgGVPxs (ORCPT ); Wed, 22 Jul 2020 11:53:48 -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 CCF5320717; Wed, 22 Jul 2020 15:53:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433228; bh=dkvt+oaphgkTDUnzSW8KJtLMYyCSAN74BaQkzRu7QWc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=m5T7Sps7jm+JVRfwvn7PtYKDI9gSbAT2mb9e+LA+OFTop2O/C/sU2hRwFLxcjCud3 ePxJSkDUOCUzVH9MpsebHL9j4UJTrNU9dYGVXUhIlhrpBnBe8lcbd7k/LbfTL5vE03 brIiem1If4E2r6jXExr+5bKcQEsWnr1Rl9uwkXY0= From: Jonathan Cameron To: linux-iio@vger.kernel.org Cc: Andy Shevchenko , Lars-Peter Clausen , Peter Meerwald , Jonathan Cameron , =?utf-8?q?Stefan_Br=C3=BCns?= , Marc Titinger Subject: [PATCH v3 26/27] iio:adc:ina2xx Fix timestamp alignment issue. Date: Wed, 22 Jul 2020 16:51:02 +0100 Message-Id: <20200722155103.979802-27-jic23@kernel.org> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200722155103.979802-1-jic23@kernel.org> References: <20200722155103.979802-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 a 32 byte 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. The explicit alignment isn't technically needed here, but it reduced fragility and avoids cut and paste into drivers where it will be needed. If we want this in older stables will need manual backport due to driver reworks. Fixes: c43a102e67db ("iio: ina2xx: add support for TI INA2xx Power Monitors") Reported-by: Lars-Peter Clausen Cc: Stefan Brüns Cc: Marc Titinger Signed-off-by: Jonathan Cameron --- drivers/iio/adc/ina2xx-adc.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c index 5ed63e874292..b573ec60a8b8 100644 --- a/drivers/iio/adc/ina2xx-adc.c +++ b/drivers/iio/adc/ina2xx-adc.c @@ -146,6 +146,11 @@ struct ina2xx_chip_info { int range_vbus; /* Bus voltage maximum in V */ int pga_gain_vshunt; /* Shunt voltage PGA gain */ bool allow_async_readout; + /* data buffer needs space for channel data and timestamp */ + struct { + u16 chan[4]; + u64 ts __aligned(8); + } scan; }; static const struct ina2xx_config ina2xx_config[] = { @@ -738,8 +743,6 @@ static int ina2xx_conversion_ready(struct iio_dev *indio_dev) static int ina2xx_work_buffer(struct iio_dev *indio_dev) { struct ina2xx_chip_info *chip = iio_priv(indio_dev); - /* data buffer needs space for channel data and timestap */ - unsigned short data[4 + sizeof(s64)/sizeof(short)]; int bit, ret, i = 0; s64 time; @@ -758,10 +761,10 @@ static int ina2xx_work_buffer(struct iio_dev *indio_dev) if (ret < 0) return ret; - data[i++] = val; + chip->scan.chan[i++] = val; } - iio_push_to_buffers_with_timestamp(indio_dev, data, time); + iio_push_to_buffers_with_timestamp(indio_dev, &chip->scan, time); return 0; }; From patchwork Wed Jul 22 15:51:03 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Cameron X-Patchwork-Id: 11678745 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 03A9114E3 for ; Wed, 22 Jul 2020 15:53:51 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id DA34220B1F for ; Wed, 22 Jul 2020 15:53:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433230; bh=RNNl6bAHVwJ30y4Xg0mqag0ifFcEY3vNyFLgoVTF9e8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=dVb+gOWI9YZbXSSDHj2+JfPvusy8zrXupQ9uicmpQlQIrp+OezCm1mhSpEzbrmRb2 jA7CAT9kj0dc0ze4GsoIB92GmpHJIP8EvnDqS87x3V/p7lAqbnIPkO32uaqUsK+fWH wD8RK6hEYstVOwUVf4vudW9zY1n2exgD4PU1gDuY= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730116AbgGVPxu (ORCPT ); Wed, 22 Jul 2020 11:53:50 -0400 Received: from mail.kernel.org ([198.145.29.99]:35946 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726427AbgGVPxu (ORCPT ); Wed, 22 Jul 2020 11:53:50 -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 8EA5E207CD; Wed, 22 Jul 2020 15:53:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595433229; bh=RNNl6bAHVwJ30y4Xg0mqag0ifFcEY3vNyFLgoVTF9e8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=ybLJKj3SbMSTMD396ZTBozMWNBgmZOrsWCQJZ+NzwbfZXcdcesd9sFGvDA+G28HZX OSxq43Zqn43+oFCW7dpH/5w+IjXTFTU7URJXO0qlcPmpoM1fspmTr7lwK8OOjgE0bg AmTZAMjXPwN1eYz1uycrQJ7dGsgaZpOOrYSg4VNA= From: Jonathan Cameron To: linux-iio@vger.kernel.org Cc: Andy Shevchenko , Lars-Peter Clausen , Peter Meerwald , Jonathan Cameron , Akinobu Mita Subject: [PATCH v3 27/27] iio:adc:max1118 Fix alignment of timestamp and data leak issues Date: Wed, 22 Jul 2020 16:51:03 +0100 Message-Id: <20200722155103.979802-28-jic23@kernel.org> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200722155103.979802-1-jic23@kernel.org> References: <20200722155103.979802-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 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 necessary to ensure correct padding on architectures where s64 is only 4 bytes aligned such as x86_32. Fixes: a9e9c7153e96 ("iio: adc: add max1117/max1118/max1119 ADC driver") Reported-by: Lars-Peter Clausen Cc: Akinobu Mita Signed-off-by: Jonathan Cameron --- drivers/iio/adc/max1118.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/iio/adc/max1118.c b/drivers/iio/adc/max1118.c index 01b20e420ac4..6efb0b43d938 100644 --- a/drivers/iio/adc/max1118.c +++ b/drivers/iio/adc/max1118.c @@ -36,6 +36,11 @@ struct max1118 { struct spi_device *spi; struct mutex lock; struct regulator *reg; + /* Ensure natural alignment of buffer elements */ + struct { + u8 channels[2]; + s64 ts __aligned(8); + } scan; u8 data ____cacheline_aligned; }; @@ -166,7 +171,6 @@ static irqreturn_t max1118_trigger_handler(int irq, void *p) struct iio_poll_func *pf = p; struct iio_dev *indio_dev = pf->indio_dev; struct max1118 *adc = iio_priv(indio_dev); - u8 data[16] = { }; /* 2x 8-bit ADC data + padding + 8 bytes timestamp */ int scan_index; int i = 0; @@ -184,10 +188,10 @@ static irqreturn_t max1118_trigger_handler(int irq, void *p) goto out; } - data[i] = ret; + adc->scan.channels[i] = ret; i++; } - iio_push_to_buffers_with_timestamp(indio_dev, data, + iio_push_to_buffers_with_timestamp(indio_dev, &adc->scan, iio_get_time_ns(indio_dev)); out: mutex_unlock(&adc->lock);