Re: [PATCH 12/14] powerpc/rtas: Close theoretical memory leak
From: Laurent Dufour <hidden>
Date: 2022-03-15 17:18:04
On 08/03/2022, 14:50:45, Nicholas Piggin wrote:
If buff_copy allocation failed then there was an error and the errbuf allocation succeeded, it will not be logged or freed.
Good catch! Since you're dealing with the error log buffer allocation/free, I think it would be better to not make this allocation in __fetch_rtas_last_error() and to rely on the caller to allocate it before taking the rtas lock. This way, the allocation is done without holding the rtas lock, as done in rtas(). This would simplify __fetch_rtas_last_error() and the caller logic to free the buffer too. Laurent.
quoted hunk ↗ jump to hunk
Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- arch/powerpc/kernel/rtas.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c index e047793cbb80..1fc22138e3ab 100644 --- a/arch/powerpc/kernel/rtas.c +++ b/arch/powerpc/kernel/rtas.c@@ -1239,9 +1239,10 @@ SYSCALL_DEFINE1(rtas, struct rtas_args __user *, uargs) unlock_rtas(flags); - if (buff_copy) { - if (errbuf) - log_error(errbuf, ERR_TYPE_RTAS_LOG, 0); + if (errbuf) { + log_error(errbuf, ERR_TYPE_RTAS_LOG, 0); + kfree(errbuf); + } else if (buff_copy) { kfree(buff_copy); }