| Message ID | 20220308135047.478297-13-npiggin@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | powerpc/rtas: various cleanups and improvements | expand |
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. > 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); > } >
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); }
If buff_copy allocation failed then there was an error and the errbuf allocation succeeded, it will not be logged or freed. Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- arch/powerpc/kernel/rtas.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)