Hi Geert Am 10.03.23 um 09:24 schrieb Geert Uytterhoeven:
Hi Thomas, On Thu, Mar 9, 2023 at 5:02 PM Thomas Zimmermann <tzimmermann@xxxxxxx> wrote:Introduce struct option_iter and helpers to parse command-line options with comma-separated key-value pairs. Then convert fbdev drivers to the new interface. Fixes a memory leak in the parsing of the video= option. Before commit 73ce73c30ba9 ("fbdev: Transfer video= option strings to caller; clarify ownership"), a call to fb_get_options() either returned an internal string or a duplicated string; hence ownership of the string's memory buffer was not well defined, but depended on how users specified the video= option on the kernel command line. For global settings, the caller owned the returned memory and for per-driver settings, fb_get_options() owned the memory. As calling drivers were unable to detect the case, they had no option but to leak the the memory. Commit 73ce73c30ba9 ("fbdev: Transfer video= option strings to caller; clarify ownership") changed semantics to caller-owned strings. Drivers still leaked the memory, but at least ownership was clear.While I can find the actual patch[1], I cannot find this commit? Where was it applied?
It's currently in drm-misc-next. https://cgit.freedesktop.org/drm/drm-misc/commit/?id=73ce73c30ba9ae4d90fdfad7ebe9104001d5d851 Best regards Thomas
[1] https://lore.kernel.org/all/20230209135509.7786-3-tzimmermann@xxxxxxxThis patchset fixes the memory leak and changes string ownership back to fb_get_options(). Patch 1 introduces struct option_iter and a few helpers. The interface takes an option string, such as video=, in the common form value1,key2:value2,value3 etc and returns the individual comma-separated pairs. Various modules use this pattern, so the code is located under lib/. Patches 2 to 100 go through fbdev drivers and convert them to the new interface. This often requires a number of cleanups. A driver would typically refer to the option string's video mode. Such strings are now copied to driver-allocated memory so that drivers don't refer directly to the option string's memory. The option iterator then replaces manual parsing loops based on strsep(","). All driver-allocated memory is released by removing the device or unloading the module. Patch 101 finally changes the ownership of the option string to be internal to fb_get_option(); thereby fixing the memory leak. The option iterator holds its own copy of the string and is not affected by the change. Most fbdev drivers only support to parse option strings if they are built-in. I assume that's because of the original fuzzy semantics of fb_get_options(). A later patchset could change the driver to respect video= settings in any configuration. v2: * use kstrdup()/kfree() for video strings (Geert, Timur) * fix iterator docs (Randy) * update iterator interfaceThanks for the update, this looks much better! Gr{oetje,eeting}s, Geert
-- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
