On Thu, Jan 09, 2020 at 10:46:25AM +0100, Greg Kroah-Hartman wrote:
> On Mon, Dec 30, 2019 at 05:26:00PM +0300, Heikki Krogerus wrote:
> > For now the read-writable attribute files have made a check
> > in their store callback function to see does the underlying
> > port interface support changing the value or not, and when
> > it didn't, the callbacks returned -EOPNOTSUPP. From user
> > perspective that is not ideal, as there is no way to know is
> > changing the value possible beforehand.
> >
> > Instead of returning -EOPNOTSUPP, making the attribute file
> > read-only when the operation is not supported.
>
> As fun as this is, if someone then changes the permission on the sysfs
> file, trying to write to them will now crash the kernel, right?
>
> So I think you need to leave this check in:
>
> > diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> > index 08923637cd88..3abcfa09ecdf 100644
> > --- a/drivers/usb/typec/class.c
> > +++ b/drivers/usb/typec/class.c
> > @@ -373,12 +373,9 @@ static ssize_t active_store(struct device *dev, struct device_attribute *attr,
> > }
> > }
> >
> > - /* Note: If there is no driver, the mode will not be entered */
> > - if (adev->ops && adev->ops->activate) {
> > - ret = adev->ops->activate(adev, enter);
> > - if (ret)
> > - return ret;
> > - }
> > + ret = adev->ops->activate(adev, enter);
> > + if (ret)
> > + return ret;
> >
> > return size;
> > }
>
> We had to go add this type of check to some subsystems recently that had
> this same problem, I don't want to have to go and add it back for this
> one as well :)
You are correct. I'll leave the check in.
thanks,
--
heikki
