On Thu, Dec 05, 2024 at 03:18:20PM +0200, Ilpo Järvinen wrote:
> On Thu, 5 Dec 2024, Kurt Borja wrote:
>
> > On Thu, Dec 05, 2024 at 12:17:01PM +0200, Ilpo Järvinen wrote:
> > > On Wed, 4 Dec 2024, Kurt Borja wrote:
> > >
> > > > Define zone_attrs statically with the use of helper macros and
> > > > initialize the zone_attribute_group with driver's .dev_groups.
> > > >
> > > > This makes match_zone() no longer needed, so drop it.
> > > >
> > > > Signed-off-by: Kurt Borja <kuurtb@xxxxxxxxx>
>
> > > > zone_data =
> > > > kcalloc(quirks->num_zones, sizeof(struct platform_zone),
> > > > GFP_KERNEL);
>
> You kcalloc() zone_data here for quirks->num_zones entries....
Totally missed this, thanks.
>
> > > > - for (zone = 0; zone < quirks->num_zones; zone++) {
> > > > - name = kasprintf(GFP_KERNEL, "zone%02hhX", zone);
> > > > - if (name == NULL)
> > > > - return 1;
> > > > - sysfs_attr_init(&zone_dev_attrs[zone].attr);
> > > > - zone_dev_attrs[zone].attr.name = name;
> > > > - zone_dev_attrs[zone].attr.mode = 0644;
> > > > - zone_dev_attrs[zone].show = zone_show;
> > > > - zone_dev_attrs[zone].store = zone_set;
> > > > + for (zone = 0; zone < 4; zone++)
> > > > zone_data[zone].location = zone;
> > >
> > > You allocate quirks->num_zones entries to zone_data above but use a
> > > literal here?
> >
> > I did this because quirks->num_zones controlls only visibility now.
>
> This kind of information would be useful to put into the commit message!
>
> It does not control only visibility, see the kcalloc() code above. Did you
> mean to alter the alloc too but forgot?
Yes I did not pay much attention when modifying this function. I'll fix
it.
>
> > I didn't feel comfortable leaving an out of bounds access on zone_show()
> > and zone_set() when they do `zone_data[location]`.
> >
> > Still those out of bounds accesses are hidden from user-space (right?) and
> > alienware_wmi_init() is getting dropped anyway so I should just leave it
> > as zone < quirks->num_zones.
>
> The assignment within this loop will write out of bounds if
> quirks->num_zones is less than 4.
>
> --
> i.
