lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAD=FV=WVEfi2u-uHcZAoMd4HXPcZrwb95HQzTE8V6YmAW9mhPA@mail.gmail.com>
Date:   Mon, 23 Jan 2023 15:12:06 -0800
From:   Doug Anderson <dianders@...omium.org>
To:     Konrad Dybcio <konrad.dybcio@...aro.org>
Cc:     Jonathan Cameron <jic23@...nel.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Jonathan Cameron <Jonathan.Cameron@...wei.com>,
        Nuno Sá <nuno.sa@...log.com>,
        linux-arm-msm@...r.kernel.org, linux-iio@...r.kernel.org,
        linux-kernel@...r.kernel.org, Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Marijn Suijten <marijn.suijten@...ainline.org>,
        Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        Matthias Kaehlcke <mka@...omium.org>
Subject: Re: [PATCH v2 1/1] iio: adc: qcom-spmi-adc5: Fix the channel name

Hi,

On Mon, Jan 23, 2023 at 8:35 AM Konrad Dybcio <konrad.dybcio@...aro.org> wrote:
>
> On 22.01.2023 18:24, Jonathan Cameron wrote:
> > On Wed, 18 Jan 2023 12:06:23 +0200
> > Andy Shevchenko <andriy.shevchenko@...ux.intel.com> wrote:
> >
> >> The node name can contain an address part which is unused
> >> by the driver. Moreover, this string is propagated into
> >> the userspace label, sysfs filenames *and breaking ABI*.
> >>
> >> Cut the address part out before assigning the channel name.
> >>
> >> Fixes: 4f47a236a23d ("iio: adc: qcom-spmi-adc5: convert to device properties")
> >> Reported-by: Marijn Suijten <marijn.suijten@...ainline.org>
> >> Signed-off-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> >
> > LGTM, but given it will have ABI impact, I'd like to hear from
> > Andy, Bjorn or Konrad as maintainers and /or Dmitry as someone
> > who has touched this driver fairly recently.
> + Doug
>
> Unless the Chromium folks relied on the old names (they're the
> only ones I can think of that actually could have tapped into
> this), I say green light!

Thanks for the CC. I _think_ the only place we use these ADCs is for
certain thermistors and I think that those are all just hooked up in
the device tree, so the channel name doesn't matter. I'll also note
that no Qualcomm Chromebooks are shipping with anything newer than
kernel 5.15 right now, and (I checked) the ChromeOS 5.15 tree doesn't
have commit 4f47a236a23d ("iio: adc: qcom-spmi-adc5: convert to device
properties"). Thus, even if I'm wrong and the name is used someplace
hidden then the "old" name would be better for us. I haven't tested
the patch myself, but it sounds as if ${SUBJECT} patch is actually
moving us back to the old name.

+Matthias to keep me honest since he's spent more time with the ADCs.

> Konrad
> >
> > Mostly I want to be sure they know this exists before it causes surprise.
> >
> > Jonathan
> >
> >> ---
> >> v2: rephrased commit message (Marijn), fixed compilation issue (Marijin)
> >>  drivers/iio/adc/qcom-spmi-adc5.c | 10 +++++++++-
> >>  1 file changed, 9 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/iio/adc/qcom-spmi-adc5.c b/drivers/iio/adc/qcom-spmi-adc5.c
> >> index e90c299c913a..c2d5e06f137a 100644
> >> --- a/drivers/iio/adc/qcom-spmi-adc5.c
> >> +++ b/drivers/iio/adc/qcom-spmi-adc5.c
> >> @@ -628,12 +628,20 @@ static int adc5_get_fw_channel_data(struct adc5_chip *adc,
> >>                                  struct fwnode_handle *fwnode,
> >>                                  const struct adc5_data *data)
> >>  {
> >> -    const char *name = fwnode_get_name(fwnode), *channel_name;
> >> +    const char *channel_name;
> >> +    char *name;
> >>      u32 chan, value, varr[2];
> >>      u32 sid = 0;
> >>      int ret;
> >>      struct device *dev = adc->dev;
> >>
> >> +    name = devm_kasprintf(dev, GFP_KERNEL, "%pfwP", fwnode);
> >> +    if (!name)
> >> +            return -ENOMEM;
> >> +
> >> +    /* Cut the address part */
> >> +    name[strchrnul(name, '@') - name] = '\0';
> >> +
> >>      ret = fwnode_property_read_u32(fwnode, "reg", &chan);
> >>      if (ret) {
> >>              dev_err(dev, "invalid channel number %s\n", name);
> >

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ