On Sun, Sep 26, 2021 at 01:19:08PM +0200, Len Baker wrote: > As noted in the "Deprecated Interfaces, Language Features, Attributes, > and Conventions" documentation [1], size calculations (especially > multiplication) should not be performed in memory allocator (or similar) > function arguments due to the risk of them overflowing. This could lead > to values wrapping around and a smaller allocation being made than the > caller was expecting. Using those allocations could lead to linear > overflows of heap memory and other misbehaviors. > > So, to avoid open-coded arithmetic in the kzalloc() call inside the > create_attr_set() function the code must be refactored. Using the > struct_size() helper is the fast solution but it is better to switch > this code to common use of attributes. > > Then, remove all the custom code to manage hotkey attributes and use the > attribute_group structure instead, refactoring the code accordingly. > Also, to manage the optional hotkey attributes (hotkey_tablet_mode and > hotkey_radio_sw) use the is_visible callback from the same structure. > > Moreover, now the hotkey_init_tablet_mode() function never returns a > negative number. So, the check after the call can be safely removed. > > [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments > > Suggested-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> > Signed-off-by: Len Baker <len.baker@xxxxxxx> > --- > Hi, > > Following the suggestions made by Greg I have switch the code to common > use of attributes. However this code is untested. If someone could test > it would be great. Much better, thanks. But, I have a few questions here: > @@ -3161,9 +3106,7 @@ static void hotkey_exit(void) > hotkey_poll_stop_sync(); > mutex_unlock(&hotkey_mutex); > #endif > - > - if (hotkey_dev_attributes) > - delete_attr_set(hotkey_dev_attributes, &tpacpi_pdev->dev.kobj); > + sysfs_remove_group(&tpacpi_pdev->dev.kobj, &hotkey_attr_group); Why do you have to manually add/remove these groups still? A huge hint that something is going wrong is when you have to call a sysfs_*() call from within a driver. There should be proper driver_*() calls for you instead to get the job done. As this is a platform device, why not set the dev_groups variable in the platform_driver field so that these attribute groups get added and removed automatically? An example commit to look at that shows how this was converted for one driver is 5bd08a4ae3d0 ("platform: x86: hp-wmi: convert platform driver to use dev_groups"). See if that helps here as well. thanks, greg k-h
