Re: [PATCH 1/2] ima: Free IMA measurement buffer on error
On 1/22/21 2:30 PM, Thiago Jung Bauermann wrote: Hi Lakshmi, Lakshmi Ramasubramanian 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 Suggested-by: Tyler Hicks 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(); if (ret) { pr_err("Error passing over kexec measurement buffer.\n"); + vfree(kexec_buffer); return; } This is a good catch. Thanks. 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. Agreed. I'll post a new patch with the above change removed. thanks, -lakshmi
Re: [PATCH 1/2] ima: Free IMA measurement buffer on error
Hi Lakshmi, Lakshmi Ramasubramanian 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 > Suggested-by: Tyler Hicks > 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(); > 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
Re: [PATCH 1/2] ima: Free IMA measurement buffer on error
On 2021-01-21 09:30:02, Lakshmi Ramasubramanian wrote: > 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 > Suggested-by: Tyler Hicks > Fixes: 7b8589cc29e7 ("ima: on soft reboot, save the measurement list") Reviewed-by: Tyler Hicks Tyler > --- > 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(); > if (ret) { > pr_err("Error passing over kexec measurement buffer.\n"); > + vfree(kexec_buffer); > return; > } > > 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; > } > > -- > 2.30.0 >
[PATCH 1/2] ima: Free IMA measurement buffer on error
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 Suggested-by: Tyler Hicks 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(); if (ret) { pr_err("Error passing over kexec measurement buffer.\n"); + vfree(kexec_buffer); return; } 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; } -- 2.30.0