| Message ID | 20241202-iio-kmalloc-align-v1-2-aa9568c03937@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | iio: consumers: ensure read buffers for labels and ext_info are page aligned | expand |
On Mon, 02 Dec 2024 16:11:08 +0100 Matteo Martelli <matteomartelli3@gmail.com> wrote: > During channel configuration, the iio-mux driver allocates a page with > devm_kzalloc(PAGE_SIZE) to read channel ext_info. However, the resulting > buffer points to an offset of the page due to the devres header sitting > at the beginning of the allocated area. This leads to failure in the > provider driver when sysfs_emit* helpers are used to format the ext_info > attributes. > > Switch to plain kzalloc version. The devres version is not strictly > necessary as the buffer is only accessed during the channel > configuration phase. Rely on __free cleanup to deallocate the buffer. > Also, move the ext_info handling into a new function to have the page > buffer definition and assignment in one statement as suggested by > cleanup documentation. > > Signed-off-by: Matteo Martelli <matteomartelli3@gmail.com> This seems fine to me, but the diff ended up a bit complex, so I'd like Peter to take a look as well before I apply it. Do you have a board that is hitting this? If so, a fixes tag is definitely appropriate. I think it is probably appropriate even it not. Jonathan > --- > drivers/iio/multiplexer/iio-mux.c | 84 +++++++++++++++++++++------------------ > 1 file changed, 46 insertions(+), 38 deletions(-) > > diff --git a/drivers/iio/multiplexer/iio-mux.c b/drivers/iio/multiplexer/iio-mux.c > index 2953403bef53bbe47a97a8ab1c475ed88d7f86d2..c309d991490c63ba4299f1cda7102f10dcf54982 100644 > --- a/drivers/iio/multiplexer/iio-mux.c > +++ b/drivers/iio/multiplexer/iio-mux.c > @@ -7,6 +7,7 @@ > * Author: Peter Rosin <peda@axentia.se> > */ > > +#include <linux/cleanup.h> > #include <linux/err.h> > #include <linux/iio/consumer.h> > #include <linux/iio/iio.h> > @@ -237,49 +238,18 @@ static ssize_t mux_write_ext_info(struct iio_dev *indio_dev, uintptr_t private, > return ret; > } > > -static int mux_configure_channel(struct device *dev, struct mux *mux, > - u32 state, const char *label, int idx) > +static int mux_configure_chan_ext_info(struct device *dev, struct mux *mux, > + int idx, int num_ext_info) > { > struct mux_child *child = &mux->child[idx]; > - struct iio_chan_spec *chan = &mux->chan[idx]; > struct iio_chan_spec const *pchan = mux->parent->channel; > - char *page = NULL; > - int num_ext_info; > int i; > int ret; > > - chan->indexed = 1; > - chan->output = pchan->output; > - chan->datasheet_name = label; > - chan->ext_info = mux->ext_info; > - > - ret = iio_get_channel_type(mux->parent, &chan->type); > - if (ret < 0) { > - dev_err(dev, "failed to get parent channel type\n"); > - return ret; > - } > - > - if (iio_channel_has_info(pchan, IIO_CHAN_INFO_RAW)) > - chan->info_mask_separate |= BIT(IIO_CHAN_INFO_RAW); > - if (iio_channel_has_info(pchan, IIO_CHAN_INFO_SCALE)) > - chan->info_mask_separate |= BIT(IIO_CHAN_INFO_SCALE); > - > - if (iio_channel_has_available(pchan, IIO_CHAN_INFO_RAW)) > - chan->info_mask_separate_available |= BIT(IIO_CHAN_INFO_RAW); > - > - if (state >= mux_control_states(mux->control)) { > - dev_err(dev, "too many channels\n"); > - return -EINVAL; > - } > - > - chan->channel = state; > + char *page __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); > + if (!page) > + return -ENOMEM; > > - num_ext_info = iio_get_channel_ext_info_count(mux->parent); > - if (num_ext_info) { > - page = devm_kzalloc(dev, PAGE_SIZE, GFP_KERNEL); > - if (!page) > - return -ENOMEM; > - } > child->ext_info_cache = devm_kcalloc(dev, > num_ext_info, > sizeof(*child->ext_info_cache), > @@ -318,8 +288,46 @@ static int mux_configure_channel(struct device *dev, struct mux *mux, > child->ext_info_cache[i].size = ret; > } > > - if (page) > - devm_kfree(dev, page); > + return 0; > +} > + > +static int mux_configure_channel(struct device *dev, struct mux *mux, u32 state, > + const char *label, int idx) > +{ > + struct iio_chan_spec *chan = &mux->chan[idx]; > + struct iio_chan_spec const *pchan = mux->parent->channel; > + int num_ext_info; > + int ret; > + > + chan->indexed = 1; > + chan->output = pchan->output; > + chan->datasheet_name = label; > + chan->ext_info = mux->ext_info; > + > + ret = iio_get_channel_type(mux->parent, &chan->type); > + if (ret < 0) { > + dev_err(dev, "failed to get parent channel type\n"); > + return ret; > + } > + > + if (iio_channel_has_info(pchan, IIO_CHAN_INFO_RAW)) > + chan->info_mask_separate |= BIT(IIO_CHAN_INFO_RAW); > + if (iio_channel_has_info(pchan, IIO_CHAN_INFO_SCALE)) > + chan->info_mask_separate |= BIT(IIO_CHAN_INFO_SCALE); > + > + if (iio_channel_has_available(pchan, IIO_CHAN_INFO_RAW)) > + chan->info_mask_separate_available |= BIT(IIO_CHAN_INFO_RAW); > + > + if (state >= mux_control_states(mux->control)) { > + dev_err(dev, "too many channels\n"); > + return -EINVAL; > + } > + > + chan->channel = state; > + > + num_ext_info = iio_get_channel_ext_info_count(mux->parent); > + if (num_ext_info) > + return mux_configure_chan_ext_info(dev, mux, idx, num_ext_info); > > return 0; > } >
On Sun, 8 Dec 2024 18:15:31 +0000, Jonathan Cameron <jic23@kernel.org> wrote: > On Mon, 02 Dec 2024 16:11:08 +0100 > Matteo Martelli <matteomartelli3@gmail.com> wrote: > > > During channel configuration, the iio-mux driver allocates a page with > > devm_kzalloc(PAGE_SIZE) to read channel ext_info. However, the resulting > > buffer points to an offset of the page due to the devres header sitting > > at the beginning of the allocated area. This leads to failure in the > > provider driver when sysfs_emit* helpers are used to format the ext_info > > attributes. > > > > Switch to plain kzalloc version. The devres version is not strictly > > necessary as the buffer is only accessed during the channel > > configuration phase. Rely on __free cleanup to deallocate the buffer. > > Also, move the ext_info handling into a new function to have the page > > buffer definition and assignment in one statement as suggested by > > cleanup documentation. > > > > Signed-off-by: Matteo Martelli <matteomartelli3@gmail.com> > This seems fine to me, but the diff ended up a bit complex, so I'd like > Peter to take a look as well before I apply it. For a simpler diff I could go for devm_get_free_pages()+devm_free_pages(), but since devres doesn't seem necessary in this case, I think this patch provides a cleaner solution at the end. > > Do you have a board that is hitting this? If so, a fixes tag is definitely > appropriate. I think it is probably appropriate even it not. I am not sure if any existing board is affected as I encountered this issue while experimenting with consumer drivers, thus using a custom DT on top of sun50i-a64-pine64.dts just for testing. The following DT files might be affected but only if the iio channel controlled by the iio_mux multiplexer owns an ext_info attribute which is also exposed on sysfs. $ grep -Rl 'io-channel-mux' arch arch/arm/boot/dts/aspeed/aspeed-bmc-ampere-mtmitchell.dts arch/arm/boot/dts/aspeed/aspeed-bmc-ampere-mtjade.dts arch/arm/boot/dts/microchip/at91-tse850-3.dts arch/arm/boot/dts/microchip/at91-natte.dtsi arch/arm64/boot/dts/rockchip/rk3566-powkiddy-rk2023.dtsi arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg353x.dtsi arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg503.dts arch/arm64/boot/dts/rockchip/rk3326-odroid-go3.dts arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-h.dtb arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-h.dts I am also not sure what would be the reference commit for the Fixes tag. The related ext_info attributes handling was introduced in the first commit of the iio_mux implementation. If that applies, following the corresponding Fixes tag. Fixes: 7ba9df54b091 ("iio: multiplexer: new iio category and iio-mux driver") > > Jonathan > Best regards, Matteo > > --- > > drivers/iio/multiplexer/iio-mux.c | 84 +++++++++++++++++++++------------------ > > 1 file changed, 46 insertions(+), 38 deletions(-) > > > > diff --git a/drivers/iio/multiplexer/iio-mux.c b/drivers/iio/multiplexer/iio-mux.c > > index 2953403bef53bbe47a97a8ab1c475ed88d7f86d2..c309d991490c63ba4299f1cda7102f10dcf54982 100644 > > --- a/drivers/iio/multiplexer/iio-mux.c > > +++ b/drivers/iio/multiplexer/iio-mux.c > > @@ -7,6 +7,7 @@ > > * Author: Peter Rosin <peda@axentia.se> > > */ > > > > +#include <linux/cleanup.h> > > #include <linux/err.h> > > #include <linux/iio/consumer.h> > > #include <linux/iio/iio.h> > > @@ -237,49 +238,18 @@ static ssize_t mux_write_ext_info(struct iio_dev *indio_dev, uintptr_t private, > > return ret; > > } > > > > -static int mux_configure_channel(struct device *dev, struct mux *mux, > > - u32 state, const char *label, int idx) > > +static int mux_configure_chan_ext_info(struct device *dev, struct mux *mux, > > + int idx, int num_ext_info) > > { > > struct mux_child *child = &mux->child[idx]; > > - struct iio_chan_spec *chan = &mux->chan[idx]; > > struct iio_chan_spec const *pchan = mux->parent->channel; > > - char *page = NULL; > > - int num_ext_info; > > int i; > > int ret; > > > > - chan->indexed = 1; > > - chan->output = pchan->output; > > - chan->datasheet_name = label; > > - chan->ext_info = mux->ext_info; > > - > > - ret = iio_get_channel_type(mux->parent, &chan->type); > > - if (ret < 0) { > > - dev_err(dev, "failed to get parent channel type\n"); > > - return ret; > > - } > > - > > - if (iio_channel_has_info(pchan, IIO_CHAN_INFO_RAW)) > > - chan->info_mask_separate |= BIT(IIO_CHAN_INFO_RAW); > > - if (iio_channel_has_info(pchan, IIO_CHAN_INFO_SCALE)) > > - chan->info_mask_separate |= BIT(IIO_CHAN_INFO_SCALE); > > - > > - if (iio_channel_has_available(pchan, IIO_CHAN_INFO_RAW)) > > - chan->info_mask_separate_available |= BIT(IIO_CHAN_INFO_RAW); > > - > > - if (state >= mux_control_states(mux->control)) { > > - dev_err(dev, "too many channels\n"); > > - return -EINVAL; > > - } > > - > > - chan->channel = state; > > + char *page __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); > > + if (!page) > > + return -ENOMEM; > > > > - num_ext_info = iio_get_channel_ext_info_count(mux->parent); > > - if (num_ext_info) { > > - page = devm_kzalloc(dev, PAGE_SIZE, GFP_KERNEL); > > - if (!page) > > - return -ENOMEM; > > - } > > child->ext_info_cache = devm_kcalloc(dev, > > num_ext_info, > > sizeof(*child->ext_info_cache), > > @@ -318,8 +288,46 @@ static int mux_configure_channel(struct device *dev, struct mux *mux, > > child->ext_info_cache[i].size = ret; > > } > > > > - if (page) > > - devm_kfree(dev, page); > > + return 0; > > +} > > + > > +static int mux_configure_channel(struct device *dev, struct mux *mux, u32 state, > > + const char *label, int idx) > > +{ > > + struct iio_chan_spec *chan = &mux->chan[idx]; > > + struct iio_chan_spec const *pchan = mux->parent->channel; > > + int num_ext_info; > > + int ret; > > + > > + chan->indexed = 1; > > + chan->output = pchan->output; > > + chan->datasheet_name = label; > > + chan->ext_info = mux->ext_info; > > + > > + ret = iio_get_channel_type(mux->parent, &chan->type); > > + if (ret < 0) { > > + dev_err(dev, "failed to get parent channel type\n"); > > + return ret; > > + } > > + > > + if (iio_channel_has_info(pchan, IIO_CHAN_INFO_RAW)) > > + chan->info_mask_separate |= BIT(IIO_CHAN_INFO_RAW); > > + if (iio_channel_has_info(pchan, IIO_CHAN_INFO_SCALE)) > > + chan->info_mask_separate |= BIT(IIO_CHAN_INFO_SCALE); > > + > > + if (iio_channel_has_available(pchan, IIO_CHAN_INFO_RAW)) > > + chan->info_mask_separate_available |= BIT(IIO_CHAN_INFO_RAW); > > + > > + if (state >= mux_control_states(mux->control)) { > > + dev_err(dev, "too many channels\n"); > > + return -EINVAL; > > + } > > + > > + chan->channel = state; > > + > > + num_ext_info = iio_get_channel_ext_info_count(mux->parent); > > + if (num_ext_info) > > + return mux_configure_chan_ext_info(dev, mux, idx, num_ext_info); > > > > return 0; > > } > > >
On Mon, 09 Dec 2024 11:39:55 +0100 Matteo Martelli <matteomartelli3@gmail.com> wrote: > On Sun, 8 Dec 2024 18:15:31 +0000, Jonathan Cameron <jic23@kernel.org> wrote: > > On Mon, 02 Dec 2024 16:11:08 +0100 > > Matteo Martelli <matteomartelli3@gmail.com> wrote: > > > > > During channel configuration, the iio-mux driver allocates a page with > > > devm_kzalloc(PAGE_SIZE) to read channel ext_info. However, the resulting > > > buffer points to an offset of the page due to the devres header sitting > > > at the beginning of the allocated area. This leads to failure in the > > > provider driver when sysfs_emit* helpers are used to format the ext_info > > > attributes. > > > > > > Switch to plain kzalloc version. The devres version is not strictly > > > necessary as the buffer is only accessed during the channel > > > configuration phase. Rely on __free cleanup to deallocate the buffer. > > > Also, move the ext_info handling into a new function to have the page > > > buffer definition and assignment in one statement as suggested by > > > cleanup documentation. > > > > > > Signed-off-by: Matteo Martelli <matteomartelli3@gmail.com> > > This seems fine to me, but the diff ended up a bit complex, so I'd like > > Peter to take a look as well before I apply it. > > For a simpler diff I could go for devm_get_free_pages()+devm_free_pages(), > but since devres doesn't seem necessary in this case, I think this patch > provides a cleaner solution at the end. The approach is fine I think, I'd just like a second opinion so will give Peter some time to get to it before applying. > > > > > Do you have a board that is hitting this? If so, a fixes tag is definitely > > appropriate. I think it is probably appropriate even it not. > > I am not sure if any existing board is affected as I encountered this > issue while experimenting with consumer drivers, thus using a custom DT > on top of sun50i-a64-pine64.dts just for testing. The following DT files > might be affected but only if the iio channel controlled by the iio_mux > multiplexer owns an ext_info attribute which is also exposed on sysfs. > > $ grep -Rl 'io-channel-mux' arch > arch/arm/boot/dts/aspeed/aspeed-bmc-ampere-mtmitchell.dts > arch/arm/boot/dts/aspeed/aspeed-bmc-ampere-mtjade.dts > arch/arm/boot/dts/microchip/at91-tse850-3.dts > arch/arm/boot/dts/microchip/at91-natte.dtsi > arch/arm64/boot/dts/rockchip/rk3566-powkiddy-rk2023.dtsi > arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg353x.dtsi > arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg503.dts > arch/arm64/boot/dts/rockchip/rk3326-odroid-go3.dts > arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-h.dtb > arch/arm64/boot/dts/allwinner/sun50i-h700-anbernic-rg35xx-h.dts > > I am also not sure what would be the reference commit for the Fixes tag. > The related ext_info attributes handling was introduced in the first > commit of the iio_mux implementation. If that applies, following the > corresponding Fixes tag. > > Fixes: 7ba9df54b091 ("iio: multiplexer: new iio category and iio-mux driver") That works I think. Thanks, > > > > > Jonathan > > > > Best regards, > Matteo > > > --- > > > drivers/iio/multiplexer/iio-mux.c | 84 +++++++++++++++++++++------------------ > > > 1 file changed, 46 insertions(+), 38 deletions(-) > > > > > > diff --git a/drivers/iio/multiplexer/iio-mux.c b/drivers/iio/multiplexer/iio-mux.c > > > index 2953403bef53bbe47a97a8ab1c475ed88d7f86d2..c309d991490c63ba4299f1cda7102f10dcf54982 100644 > > > --- a/drivers/iio/multiplexer/iio-mux.c > > > +++ b/drivers/iio/multiplexer/iio-mux.c > > > @@ -7,6 +7,7 @@ > > > * Author: Peter Rosin <peda@axentia.se> > > > */ > > > > > > +#include <linux/cleanup.h> > > > #include <linux/err.h> > > > #include <linux/iio/consumer.h> > > > #include <linux/iio/iio.h> > > > @@ -237,49 +238,18 @@ static ssize_t mux_write_ext_info(struct iio_dev *indio_dev, uintptr_t private, > > > return ret; > > > } > > > > > > -static int mux_configure_channel(struct device *dev, struct mux *mux, > > > - u32 state, const char *label, int idx) > > > +static int mux_configure_chan_ext_info(struct device *dev, struct mux *mux, > > > + int idx, int num_ext_info) > > > { > > > struct mux_child *child = &mux->child[idx]; > > > - struct iio_chan_spec *chan = &mux->chan[idx]; > > > struct iio_chan_spec const *pchan = mux->parent->channel; > > > - char *page = NULL; > > > - int num_ext_info; > > > int i; > > > int ret; > > > > > > - chan->indexed = 1; > > > - chan->output = pchan->output; > > > - chan->datasheet_name = label; > > > - chan->ext_info = mux->ext_info; > > > - > > > - ret = iio_get_channel_type(mux->parent, &chan->type); > > > - if (ret < 0) { > > > - dev_err(dev, "failed to get parent channel type\n"); > > > - return ret; > > > - } > > > - > > > - if (iio_channel_has_info(pchan, IIO_CHAN_INFO_RAW)) > > > - chan->info_mask_separate |= BIT(IIO_CHAN_INFO_RAW); > > > - if (iio_channel_has_info(pchan, IIO_CHAN_INFO_SCALE)) > > > - chan->info_mask_separate |= BIT(IIO_CHAN_INFO_SCALE); > > > - > > > - if (iio_channel_has_available(pchan, IIO_CHAN_INFO_RAW)) > > > - chan->info_mask_separate_available |= BIT(IIO_CHAN_INFO_RAW); > > > - > > > - if (state >= mux_control_states(mux->control)) { > > > - dev_err(dev, "too many channels\n"); > > > - return -EINVAL; > > > - } > > > - > > > - chan->channel = state; > > > + char *page __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); > > > + if (!page) > > > + return -ENOMEM; > > > > > > - num_ext_info = iio_get_channel_ext_info_count(mux->parent); > > > - if (num_ext_info) { > > > - page = devm_kzalloc(dev, PAGE_SIZE, GFP_KERNEL); > > > - if (!page) > > > - return -ENOMEM; > > > - } > > > child->ext_info_cache = devm_kcalloc(dev, > > > num_ext_info, > > > sizeof(*child->ext_info_cache), > > > @@ -318,8 +288,46 @@ static int mux_configure_channel(struct device *dev, struct mux *mux, > > > child->ext_info_cache[i].size = ret; > > > } > > > > > > - if (page) > > > - devm_kfree(dev, page); > > > + return 0; > > > +} > > > + > > > +static int mux_configure_channel(struct device *dev, struct mux *mux, u32 state, > > > + const char *label, int idx) > > > +{ > > > + struct iio_chan_spec *chan = &mux->chan[idx]; > > > + struct iio_chan_spec const *pchan = mux->parent->channel; > > > + int num_ext_info; > > > + int ret; > > > + > > > + chan->indexed = 1; > > > + chan->output = pchan->output; > > > + chan->datasheet_name = label; > > > + chan->ext_info = mux->ext_info; > > > + > > > + ret = iio_get_channel_type(mux->parent, &chan->type); > > > + if (ret < 0) { > > > + dev_err(dev, "failed to get parent channel type\n"); > > > + return ret; > > > + } > > > + > > > + if (iio_channel_has_info(pchan, IIO_CHAN_INFO_RAW)) > > > + chan->info_mask_separate |= BIT(IIO_CHAN_INFO_RAW); > > > + if (iio_channel_has_info(pchan, IIO_CHAN_INFO_SCALE)) > > > + chan->info_mask_separate |= BIT(IIO_CHAN_INFO_SCALE); > > > + > > > + if (iio_channel_has_available(pchan, IIO_CHAN_INFO_RAW)) > > > + chan->info_mask_separate_available |= BIT(IIO_CHAN_INFO_RAW); > > > + > > > + if (state >= mux_control_states(mux->control)) { > > > + dev_err(dev, "too many channels\n"); > > > + return -EINVAL; > > > + } > > > + > > > + chan->channel = state; > > > + > > > + num_ext_info = iio_get_channel_ext_info_count(mux->parent); > > > + if (num_ext_info) > > > + return mux_configure_chan_ext_info(dev, mux, idx, num_ext_info); > > > > > > return 0; > > > } > > > > > >
On 12/2/24 9:11 AM, Matteo Martelli wrote: > During channel configuration, the iio-mux driver allocates a page with > devm_kzalloc(PAGE_SIZE) to read channel ext_info. However, the resulting > buffer points to an offset of the page due to the devres header sitting > at the beginning of the allocated area. This leads to failure in the > provider driver when sysfs_emit* helpers are used to format the ext_info > attributes. > > Switch to plain kzalloc version. The devres version is not strictly > necessary as the buffer is only accessed during the channel > configuration phase. Rely on __free cleanup to deallocate the buffer. > Also, move the ext_info handling into a new function to have the page > buffer definition and assignment in one statement as suggested by > cleanup documentation. > > Signed-off-by: Matteo Martelli <matteomartelli3@gmail.com> > --- Reviewed-by: David Lechner <dlechner@baylibre.com>
diff --git a/drivers/iio/multiplexer/iio-mux.c b/drivers/iio/multiplexer/iio-mux.c index 2953403bef53bbe47a97a8ab1c475ed88d7f86d2..c309d991490c63ba4299f1cda7102f10dcf54982 100644 --- a/drivers/iio/multiplexer/iio-mux.c +++ b/drivers/iio/multiplexer/iio-mux.c @@ -7,6 +7,7 @@ * Author: Peter Rosin <peda@axentia.se> */ +#include <linux/cleanup.h> #include <linux/err.h> #include <linux/iio/consumer.h> #include <linux/iio/iio.h> @@ -237,49 +238,18 @@ static ssize_t mux_write_ext_info(struct iio_dev *indio_dev, uintptr_t private, return ret; } -static int mux_configure_channel(struct device *dev, struct mux *mux, - u32 state, const char *label, int idx) +static int mux_configure_chan_ext_info(struct device *dev, struct mux *mux, + int idx, int num_ext_info) { struct mux_child *child = &mux->child[idx]; - struct iio_chan_spec *chan = &mux->chan[idx]; struct iio_chan_spec const *pchan = mux->parent->channel; - char *page = NULL; - int num_ext_info; int i; int ret; - chan->indexed = 1; - chan->output = pchan->output; - chan->datasheet_name = label; - chan->ext_info = mux->ext_info; - - ret = iio_get_channel_type(mux->parent, &chan->type); - if (ret < 0) { - dev_err(dev, "failed to get parent channel type\n"); - return ret; - } - - if (iio_channel_has_info(pchan, IIO_CHAN_INFO_RAW)) - chan->info_mask_separate |= BIT(IIO_CHAN_INFO_RAW); - if (iio_channel_has_info(pchan, IIO_CHAN_INFO_SCALE)) - chan->info_mask_separate |= BIT(IIO_CHAN_INFO_SCALE); - - if (iio_channel_has_available(pchan, IIO_CHAN_INFO_RAW)) - chan->info_mask_separate_available |= BIT(IIO_CHAN_INFO_RAW); - - if (state >= mux_control_states(mux->control)) { - dev_err(dev, "too many channels\n"); - return -EINVAL; - } - - chan->channel = state; + char *page __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); + if (!page) + return -ENOMEM; - num_ext_info = iio_get_channel_ext_info_count(mux->parent); - if (num_ext_info) { - page = devm_kzalloc(dev, PAGE_SIZE, GFP_KERNEL); - if (!page) - return -ENOMEM; - } child->ext_info_cache = devm_kcalloc(dev, num_ext_info, sizeof(*child->ext_info_cache), @@ -318,8 +288,46 @@ static int mux_configure_channel(struct device *dev, struct mux *mux, child->ext_info_cache[i].size = ret; } - if (page) - devm_kfree(dev, page); + return 0; +} + +static int mux_configure_channel(struct device *dev, struct mux *mux, u32 state, + const char *label, int idx) +{ + struct iio_chan_spec *chan = &mux->chan[idx]; + struct iio_chan_spec const *pchan = mux->parent->channel; + int num_ext_info; + int ret; + + chan->indexed = 1; + chan->output = pchan->output; + chan->datasheet_name = label; + chan->ext_info = mux->ext_info; + + ret = iio_get_channel_type(mux->parent, &chan->type); + if (ret < 0) { + dev_err(dev, "failed to get parent channel type\n"); + return ret; + } + + if (iio_channel_has_info(pchan, IIO_CHAN_INFO_RAW)) + chan->info_mask_separate |= BIT(IIO_CHAN_INFO_RAW); + if (iio_channel_has_info(pchan, IIO_CHAN_INFO_SCALE)) + chan->info_mask_separate |= BIT(IIO_CHAN_INFO_SCALE); + + if (iio_channel_has_available(pchan, IIO_CHAN_INFO_RAW)) + chan->info_mask_separate_available |= BIT(IIO_CHAN_INFO_RAW); + + if (state >= mux_control_states(mux->control)) { + dev_err(dev, "too many channels\n"); + return -EINVAL; + } + + chan->channel = state; + + num_ext_info = iio_get_channel_ext_info_count(mux->parent); + if (num_ext_info) + return mux_configure_chan_ext_info(dev, mux, idx, num_ext_info); return 0; }
During channel configuration, the iio-mux driver allocates a page with devm_kzalloc(PAGE_SIZE) to read channel ext_info. However, the resulting buffer points to an offset of the page due to the devres header sitting at the beginning of the allocated area. This leads to failure in the provider driver when sysfs_emit* helpers are used to format the ext_info attributes. Switch to plain kzalloc version. The devres version is not strictly necessary as the buffer is only accessed during the channel configuration phase. Rely on __free cleanup to deallocate the buffer. Also, move the ext_info handling into a new function to have the page buffer definition and assignment in one statement as suggested by cleanup documentation. Signed-off-by: Matteo Martelli <matteomartelli3@gmail.com> --- drivers/iio/multiplexer/iio-mux.c | 84 +++++++++++++++++++++------------------ 1 file changed, 46 insertions(+), 38 deletions(-)