Thread (28 messages) 28 messages, 3 authors, 2023-02-17

Re: [PATCH 02/11] fbdev: Transfer video= option strings to caller; clarify ownership

From: Javier Martinez Canillas <javierm@redhat.com>
Date: 2023-02-17 08:37:54
Also in: dri-devel, linuxppc-dev

Thomas Zimmermann [off-list ref] writes:
In fb_get_options(), always duplicate the returned option string and
transfer ownership of the memory to the function's caller.

Until now, only the global option string got duplicated and transferred
to the caller; the per-driver options were owned by fb_get_options().
In the end, it was impossible for the function's caller to detect if
it had to release the string's memory buffer. Hence, all calling drivers
leak the memory buffer. The leaks have existed ever since, but drivers
only call fb_get_option() once as part of module initialization. So the
amount of leaked memory is not significant.

Fix the semantics of fb_get_option() by unconditionally transferring
ownership of the memory buffer to the caller. Later patches can resolve
the memory leaks in the fbdev drivers.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
[...]
+	if (option) {
+		if (options)
+			*option = kstrdup(options, GFP_KERNEL);
+		else
+			*option = NULL;
+	}
I know the old code wasn't checking if kstrdup() succeeded, but you should
do it here and let the caller know. And same if (!options). So I guess the
following check can be added (to be consistent with the rest of the code):

	if (!*option)
		retval = 1;
 	return retval;
 }
-- 
2.39.1
Best regards,
Javier
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help