Re: [PATCH 1/2] iio: consumers: ensure read buffers for labels and ext_info are page aligned

Jonathan Cameron
2024-12-08T18:16:41Z

On Mon, 02 Dec 2024 16:11:07 +0100
Matteo Martelli <matteomartelli3@gmail.com> wrote:

> Attributes of iio providers are exposed via sysfs. Typically, providers
> pass attribute values to the iio core, which handles formatting and
> printing to sysfs. However, some attributes, such as labels or extended
> info, are directly formatted and printed to sysfs by provider drivers
> using sysfs_emit() and sysfs_emit_at(). These helpers assume the read
> buffer, allocated by sysfs fop, is page-aligned. When these attributes
> are accessed by consumer drivers, the read buffer is allocated by the
> consumer and may not be page-aligned, leading to failures in the
> provider's callback that utilizes sysfs_emit*.
>
> Add a check to ensure that read buffers for labels and external info
> attributes are page-aligned. Update the prototype documentation as well.
>
> Signed-off-by: Matteo Martelli <matteomartelli3@gmail.com>
This is good hardening independent of fixing any issues so I've picked this
patch up for the togreg branch of iio.git

Thanks,

Jonathan

> ---
> drivers/iio/inkern.c | 11 +++++++++++
> include/linux/iio/consumer.h | 4 ++--
> 2 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index 7f325b3ed08fae6674245312cf8f57bb151006c0..63707ed98e1d7aca1e446122bbf69c85c0dd06a2 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -7,6 +7,7 @@
> #include <linux/err.h>
> #include <linux/export.h>
> #include <linux/minmax.h>
> +#include <linux/mm.h>
> #include <linux/mutex.h>
> #include <linux/property.h>
> #include <linux/slab.h>
> @@ -989,6 +990,11 @@ ssize_t iio_read_channel_ext_info(struct iio_channel *chan,
> {
> const struct iio_chan_spec_ext_info *ext_info;
>
> + if (!buf || offset_in_page(buf)) {
> + pr_err("iio: invalid ext_info read buffer\n");
> + return -EINVAL;
> + }
> +
> ext_info = iio_lookup_ext_info(chan, attr);
> if (!ext_info)
> return -EINVAL;
> @@ -1014,6 +1020,11 @@ EXPORT_SYMBOL_GPL(iio_write_channel_ext_info);
>
> ssize_t iio_read_channel_label(struct iio_channel *chan, char *buf)
> {
> + if (!buf || offset_in_page(buf)) {
> + pr_err("iio: invalid label read buffer\n");
> + return -EINVAL;
> + }
> +
> return do_iio_read_channel_label(chan->indio_dev, chan->channel, buf);
> }
> EXPORT_SYMBOL_GPL(iio_read_channel_label);
> diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
> index 333d1d8ccb37f387fe531577ac5e0bfc7f752cec..6a44796164792b2dd930f8168b14de327a80a6f7 100644
> --- a/include/linux/iio/consumer.h
> +++ b/include/linux/iio/consumer.h
> @@ -418,7 +418,7 @@ unsigned int iio_get_channel_ext_info_count(struct iio_channel *chan);
> * @chan: The channel being queried.
> * @attr: The ext_info attribute to read.
> * @buf: Where to store the attribute value. Assumed to hold
> - * at least PAGE_SIZE bytes.
> + * at least PAGE_SIZE bytes and to be aligned at PAGE_SIZE.
> *
> * Returns the number of bytes written to buf (perhaps w/o zero termination;
> * it need not even be a string), or an error code.
> @@ -445,7 +445,7 @@ ssize_t iio_write_channel_ext_info(struct iio_channel *chan, const char *attr,
> * iio_read_channel_label() - read label for a given channel
> * @chan: The channel being queried.
> * @buf: Where to store the attribute value. Assumed to hold
> - * at least PAGE_SIZE bytes.
> + * at least PAGE_SIZE bytes and to be aligned at PAGE_SIZE.
> *
> * Returns the number of bytes written to buf, or an error code.
> */
>