Function msm_gpu_crashstate_capture maybe called for several times, and then the state->bos is a potential memleak. Also the state->pos maybe alloc failed, but now without any handle. This change is to fix some potential memleak and add error handle when alloc failed.
I suggest to improve the provided information. How do you think about a wording variant like the following?
The function “msm_gpu_crashstate_capture” can be called multiple times. The members “comm”, “cmd” and “bos” of the data structure “msm_gpu_state” are reassigned with pointers according to dynamic memory allocations if the preprocessor symbol “CONFIG_DEV_COREDUMP” was defined. But the function “kfree” was not called for them before.
Thus add missing actions. * Release previous objects. * Use further null pointer checks. * Complete the corresponding exception handling.
Would you like to add the tag “Fixes” to the commit message? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Docu...
…
+++ b/drivers/gpu/drm/msm/msm_gpu.c @@ -366,8 +366,11 @@ static void msm_gpu_crashstate_capture(struct msm_gpu *gpu, if (!should_dump(submit, submit->cmd[i].idx)) nr++;
kfree(state->bos);if (!state->bos)return;
…
Will there be a need to reconsider the indentation for function call parameters in such source files? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Docu...
Regards, Markus
On Fri, Jun 12, 2020 at 09:36:09AM +0200, Markus Elfring wrote:
Function msm_gpu_crashstate_capture maybe called for several times, and then the state->bos is a potential memleak. Also the state->pos maybe alloc failed, but now without any handle. This change is to fix some potential memleak and add error handle when alloc failed.
I suggest to improve the provided information. How do you think about a wording variant like the following?
The function “msm_gpu_crashstate_capture” can be called multiple times. The members “comm”, “cmd” and “bos” of the data structure “msm_gpu_state” are reassigned with pointers according to dynamic memory allocations if the preprocessor symbol “CONFIG_DEV_COREDUMP” was defined. But the function “kfree” was not called for them before.
Thus add missing actions.
- Release previous objects.
- Use further null pointer checks.
- Complete the corresponding exception handling.
Would you like to add the tag “Fixes” to the commit message? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Docu...
Hi,
This is the semi-friendly patch-bot of Greg Kroah-Hartman.
Markus, you seem to have sent a nonsensical or otherwise pointless review comment to a patch submission on a Linux kernel developer mailing list. I strongly suggest that you not do this anymore. Please do not bother developers who are actively working to produce patches and features with comments that, in the end, are a waste of time.
Patch submitter, please ignore Markus's suggestion; you do not need to follow it at all. The person/bot/AI that sent it is being ignored by almost all Linux kernel maintainers for having a persistent pattern of behavior of producing distracting and pointless commentary, and inability to adapt to feedback. Please feel free to also ignore emails from them.
thanks,
greg k-h's patch email bot
dri-devel@lists.freedesktop.org
-
Greg KH -
Markus Elfring