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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 19 Apr 2022 19:55:19 -0700 From: Kees Cook <keescook@...omium.org> To: joao@...rdrivepizza.com Cc: linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org, peterz@...radead.org, jpoimboe@...hat.com, andrew.cooper3@...rix.com, samitolvanen@...gle.com, mark.rutland@....com, hjl.tools@...il.com, alyssa.milburn@...ux.intel.com, ndesaulniers@...gle.com, gabriel.gomes@...ux.intel.com, rick.p.edgecombe@...el.com Subject: Re: [RFC PATCH 11/11] driver/int3400_thermal: Fix prototype matching On Tue, Apr 19, 2022 at 05:42:41PM -0700, joao@...rdrivepizza.com wrote: > From: Joao Moreira <joao@...rdrivepizza.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@...rdrivepizza.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
Powered by blists - more mailing lists