Re: [PATCH] fjes: fix memleaks in fjes_hw_setup
From: Simon Horman <horms@kernel.org>
Date: 2024-01-22 21:05:42
Also in:
lkml
On Tue, Jan 23, 2024 at 01:24:42AM +0800, Zhipeng Lu wrote:
In fjes_hw_setup, it allocates several memory and delay the deallocation
to the fjes_hw_exit in fjes_probe through the following call chain:
fjes_probe
|-> fjes_hw_init
|-> fjes_hw_setup
|-> fjes_hw_exit
However, when fjes_hw_setup fails, fjes_hw_exit won't be called and thus
all the resources allocated in fjes_hw_setup will be leaked. In this
patch, we free those resources in fjes_hw_setup and prevents such leaks.
Fixes: 2fcbca687702 ("fjes: platform_driver's .probe and .remove routine")
Signed-off-by: Zhipeng Lu <redacted>Hi Zhipeng Lu, It looks like the last non-trivial change to this driver was in 2016. So perhaps it is better to leave it be. But if not, this patch does look correct to me. Reviewed-by: Simon Horman <horms@kernel.org> ...
quoted hunk ↗ jump to hunk
@@ -273,6 +277,25 @@ static int fjes_hw_setup(struct fjes_hw *hw) fjes_hw_init_command_registers(hw, ¶m); return 0; + +free_epbuf: + for (epidx = 0; epidx < hw->max_epid ; epidx++) { + if (epidx == hw->my_epid) + continue; + fjes_hw_free_epbuf(&hw->ep_shm_info[epidx].tx); + fjes_hw_free_epbuf(&hw->ep_shm_info[epidx].rx); + } + fjes_hw_free_shared_status_region(hw); +free_res_buf: + kfree(hw->hw_info.res_buf); + hw->hw_info.res_buf = NULL; +free_req_buf: + kfree(hw->hw_info.req_buf); + hw->hw_info.req_buf = NULL; +free_ep_info: + kfree(hw->ep_shm_info); + hw->ep_shm_info = NULL; + return result;
FWIIW, I'm not sure it is necessary to set these pointers NULL, although it doesn't do any harm. Also, if this function returns an error, does the caller (fjes_hw_init()) leak hw->hw_info.trace?
} static void fjes_hw_cleanup(struct fjes_hw *hw) -- 2.34.1