| Message ID | 20191008111350.68581-2-heikki.krogerus@linux.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: typec: Small API improvement | expand |
On Tue, Oct 08, 2019 at 02:13:42PM +0300, Heikki Krogerus wrote: > Copying everything from struct typec_capability to struct > typec_port during port registration. This will make sure > that under no circumstances the driver can change the values > in the struct typec_capability that the port uses. > > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > --- > drivers/usb/typec/class.c | 15 +++++++++------ > 1 file changed, 9 insertions(+), 6 deletions(-) > > diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c > index 94a3eda62add..0bbf10c8ad58 100644 > --- a/drivers/usb/typec/class.c > +++ b/drivers/usb/typec/class.c > @@ -52,6 +52,7 @@ struct typec_port { > struct typec_switch *sw; > struct typec_mux *mux; > > + const struct typec_capability *orig_cap; /* to be removed */ > const struct typec_capability *cap; > }; > > @@ -968,7 +969,7 @@ preferred_role_store(struct device *dev, struct device_attribute *attr, > return -EINVAL; > } > > - ret = port->cap->try_role(port->cap, role); > + ret = port->cap->try_role(port->orig_cap, role); > if (ret) > return ret; > > @@ -1014,7 +1015,7 @@ static ssize_t data_role_store(struct device *dev, > goto unlock_and_ret; > } > > - ret = port->cap->dr_set(port->cap, ret); > + ret = port->cap->dr_set(port->orig_cap, ret); > if (ret) > goto unlock_and_ret; > > @@ -1071,7 +1072,7 @@ static ssize_t power_role_store(struct device *dev, > goto unlock_and_ret; > } > > - ret = port->cap->pr_set(port->cap, ret); > + ret = port->cap->pr_set(port->orig_cap, ret); > if (ret) > goto unlock_and_ret; > > @@ -1119,7 +1120,7 @@ port_type_store(struct device *dev, struct device_attribute *attr, > goto unlock_and_ret; > } > > - ret = port->cap->port_type_set(port->cap, type); > + ret = port->cap->port_type_set(port->orig_cap, type); > if (ret) > goto unlock_and_ret; > > @@ -1184,7 +1185,7 @@ static ssize_t vconn_source_store(struct device *dev, > if (ret) > return ret; > > - ret = port->cap->vconn_set(port->cap, (enum typec_role)source); > + ret = port->cap->vconn_set(port->orig_cap, (enum typec_role)source); > if (ret) > return ret; > > @@ -1278,6 +1279,7 @@ static void typec_release(struct device *dev) > ida_destroy(&port->mode_ids); > typec_switch_put(port->sw); > typec_mux_put(port->mux); > + kfree(port->cap); > kfree(port); > } > > @@ -1579,9 +1581,10 @@ struct typec_port *typec_register_port(struct device *parent, > mutex_init(&port->port_type_lock); > > port->id = id; > - port->cap = cap; > + port->orig_cap = cap; > port->port_type = cap->type; > port->prefer_role = cap->prefer_role; > + port->cap = kmemdup(cap, sizeof(*cap), GFP_KERNEL); I just realized ... unfortunately kmemdup() can return NULL. > > device_initialize(&port->dev); > port->dev.class = typec_class; > -- > 2.23.0 >
On Tue, Oct 08, 2019 at 01:44:28PM -0700, Guenter Roeck wrote: > > @@ -1579,9 +1581,10 @@ struct typec_port *typec_register_port(struct device *parent, > > mutex_init(&port->port_type_lock); > > > > port->id = id; > > - port->cap = cap; > > + port->orig_cap = cap; > > port->port_type = cap->type; > > port->prefer_role = cap->prefer_role; > > + port->cap = kmemdup(cap, sizeof(*cap), GFP_KERNEL); > > I just realized ... unfortunately kmemdup() can return NULL. Of course. I'll fix that. thanks,
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c index 94a3eda62add..0bbf10c8ad58 100644 --- a/drivers/usb/typec/class.c +++ b/drivers/usb/typec/class.c @@ -52,6 +52,7 @@ struct typec_port { struct typec_switch *sw; struct typec_mux *mux; + const struct typec_capability *orig_cap; /* to be removed */ const struct typec_capability *cap; }; @@ -968,7 +969,7 @@ preferred_role_store(struct device *dev, struct device_attribute *attr, return -EINVAL; } - ret = port->cap->try_role(port->cap, role); + ret = port->cap->try_role(port->orig_cap, role); if (ret) return ret; @@ -1014,7 +1015,7 @@ static ssize_t data_role_store(struct device *dev, goto unlock_and_ret; } - ret = port->cap->dr_set(port->cap, ret); + ret = port->cap->dr_set(port->orig_cap, ret); if (ret) goto unlock_and_ret; @@ -1071,7 +1072,7 @@ static ssize_t power_role_store(struct device *dev, goto unlock_and_ret; } - ret = port->cap->pr_set(port->cap, ret); + ret = port->cap->pr_set(port->orig_cap, ret); if (ret) goto unlock_and_ret; @@ -1119,7 +1120,7 @@ port_type_store(struct device *dev, struct device_attribute *attr, goto unlock_and_ret; } - ret = port->cap->port_type_set(port->cap, type); + ret = port->cap->port_type_set(port->orig_cap, type); if (ret) goto unlock_and_ret; @@ -1184,7 +1185,7 @@ static ssize_t vconn_source_store(struct device *dev, if (ret) return ret; - ret = port->cap->vconn_set(port->cap, (enum typec_role)source); + ret = port->cap->vconn_set(port->orig_cap, (enum typec_role)source); if (ret) return ret; @@ -1278,6 +1279,7 @@ static void typec_release(struct device *dev) ida_destroy(&port->mode_ids); typec_switch_put(port->sw); typec_mux_put(port->mux); + kfree(port->cap); kfree(port); } @@ -1579,9 +1581,10 @@ struct typec_port *typec_register_port(struct device *parent, mutex_init(&port->port_type_lock); port->id = id; - port->cap = cap; + port->orig_cap = cap; port->port_type = cap->type; port->prefer_role = cap->prefer_role; + port->cap = kmemdup(cap, sizeof(*cap), GFP_KERNEL); device_initialize(&port->dev); port->dev.class = typec_class;
Copying everything from struct typec_capability to struct typec_port during port registration. This will make sure that under no circumstances the driver can change the values in the struct typec_capability that the port uses. Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> --- drivers/usb/typec/class.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)