Messages in this thread | | | From | Thiago Jung Bauermann <> | Subject | Re: [PATCH 1/2] ima: Free IMA measurement buffer on error | Date | Fri, 22 Jan 2021 19:30:20 -0300 |
| |
Hi Lakshmi,
Lakshmi Ramasubramanian <nramas@linux.microsoft.com> writes:
> IMA allocates kernel virtual memory to carry forward the measurement > list, from the current kernel to the next kernel on kexec system call, > in ima_add_kexec_buffer() function. In error code paths this memory > is not freed resulting in memory leak. > > Free the memory allocated for the IMA measurement list in > the error code paths in ima_add_kexec_buffer() function. > > Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com> > Suggested-by: Tyler Hicks <tyhicks@linux.microsoft.com> > Fixes: 7b8589cc29e7 ("ima: on soft reboot, save the measurement list") > --- > security/integrity/ima/ima_kexec.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c > index 121de3e04af2..212145008a01 100644 > --- a/security/integrity/ima/ima_kexec.c > +++ b/security/integrity/ima/ima_kexec.c > @@ -119,12 +119,14 @@ void ima_add_kexec_buffer(struct kimage *image) > ret = kexec_add_buffer(&kbuf); > if (ret) { > pr_err("Error passing over kexec measurement buffer.\n"); > + vfree(kexec_buffer); > return; > }
This is a good catch.
> > ret = arch_ima_add_kexec_buffer(image, kbuf.mem, kexec_segment_size); > if (ret) { > pr_err("Error passing over kexec measurement buffer.\n"); > + vfree(kexec_buffer); > return; > }
But this would cause problems, because the buffer is still there in the kimage and would cause kimage_load_segment() to access invalid memory.
There's no function to undo a kexec_add_buffer() to avoid this problem, so I'd suggest just accepting the leak in this case. Fortunately, the current implementations of arch_ima_add_kexec_buffer() are very simple and cannot fail, so this is a theoretical problem.
-- Thiago Jung Bauermann IBM Linux Technology Center
| |