Hi,
On 9/9/24 2:06 PM, Andy Shevchenko wrote:
On Mon, Sep 09, 2024 at 01:32:25PM +0200, Hans de Goede wrote:
The panasonic laptop code in various places uses the SINF array with index values of 0 - SINF_CUR_BRIGHT(0x0d) without checking that the SINF array is big enough.
Not all panasonic laptops have this many SINF array entries, for example the Toughbook CF-18 model only has 10 SINF array entries. So it only supports the AC+DC brightness entries and mute.
Check that the SINF array has a minimum size which covers all AC+DC brightness entries and refuse to load if the SINF array is smaller.
For higher SINF indexes hide the sysfs attributes when the SINF array does not contain an entry for that attribute, avoiding show()/store() accessing the array out of bounds and add bounds checking to the probe() and resume() code accessing these.
...
+static umode_t pcc_sysfs_is_visible(struct kobject *kobj, struct attribute *attr, int idx) +{
- struct device *dev = kobj_to_dev(kobj);
- struct acpi_device *acpi = to_acpi_device(dev);
- struct pcc_acpi *pcc = acpi_driver_data(acpi);
Isn't it the same as dev_get_drvdata()?
No I also thought so and I checked. It is not the same, struct acpi_device has its own driver_data member, which this gets.
- if (attr == &dev_attr_mute.attr)
return (pcc->num_sifr > SINF_MUTE) ? attr->mode : 0;- if (attr == &dev_attr_eco_mode.attr)
return (pcc->num_sifr > SINF_ECO_MODE) ? attr->mode : 0;- if (attr == &dev_attr_current_brightness.attr)
return (pcc->num_sifr > SINF_CUR_BRIGHT) ? attr->mode : 0;- return attr->mode;
+}
...
- /*
* pcc->sinf is expected to at least have the AC+DC brightness entries.* Accesses to higher SINF entries are checked against num_sifr.*/- if (num_sifr <= SINF_DC_CUR_BRIGHT || num_sifr > 255) {
pr_err("num_sifr %d out of range %d - 255\n", num_sifr, SINF_DC_CUR_BRIGHT + 1);acpi_handle_err() ?
The driver is using pr_err() already in 18 other places, so IMHO it is better to be consistent and also use it here.
Regards,
Hans