Messages in this thread | | | Date | Tue, 19 Apr 2022 19:55:19 -0700 | From | Kees Cook <> | Subject | Re: [RFC PATCH 11/11] driver/int3400_thermal: Fix prototype matching |
| |
On Tue, Apr 19, 2022 at 05:42:41PM -0700, joao@overdrivepizza.com wrote: > From: Joao Moreira <joao@overdrivepizza.com> > > The function attr_dev_show directly invokes functions from drivers > expecting an specific prototype. The driver for int3400_thermal > implements the given show function using a different prototype than what > is expected. This violates the prototype-based fine-grained CFI policy. > > Make the function prototype compliant and cast the respective assignement > so it can be properly user together with fine-grained CFI.
Does this trip on regular CFI? See below, but this all looks correct to me in the original code.
> (FWIIW, there should be a less ugly patch for this, but I don't know > enough about the touched source code). > > Signed-off-by: Joao Moreira <joao@overdrivepizza.com> > --- > .../thermal/intel/int340x_thermal/int3400_thermal.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c > index 4954800b9850..4bd95a2016b7 100644 > --- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c > +++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c > @@ -311,12 +311,13 @@ static int int3400_thermal_get_uuids(struct int3400_thermal_priv *priv) > return result; > } > > -static ssize_t odvp_show(struct kobject *kobj, struct kobj_attribute *attr, > +static ssize_t odvp_show(struct device *kobj, struct device_attribute *attr, > char *buf) > { > + struct kobj_attribute *kattr = (struct kobj_attribute *) attr; > struct odvp_attr *odvp_attr; > > - odvp_attr = container_of(attr, struct odvp_attr, attr); > + odvp_attr = container_of(kattr, struct odvp_attr, attr); > > return sprintf(buf, "%d\n", odvp_attr->priv->odvp[odvp_attr->odvp]); > } > @@ -388,7 +389,10 @@ static int evaluate_odvp(struct int3400_thermal_priv *priv) > goto out_err; > } > odvp->attr.attr.mode = 0444;
Eww, this function has a masked "odvp" variable here. One should be likely renamed.
But anyway, odvp is:
struct odvp_attr { int odvp; struct int3400_thermal_priv *priv; struct kobj_attribute attr; };
The original code looks correct to me (besides the masked variable name). kobj_attribute is part of odvp, the odvp_show callback has the correct prototype, and performs the correct container_of() to get odvp_attr.
Where/why is the mismatch happening?
-Kees
> - odvp->attr.show = odvp_show; > + odvp->attr.show = (ssize_t (*) > + (struct kobject *, > + struct kobj_attribute *, > + char *)) odvp_show; > odvp->attr.store = NULL; > ret = sysfs_create_file(&priv->pdev->dev.kobj, > &odvp->attr.attr); > -- > 2.35.1 >
-- Kees Cook
| |