* [PATCH 1/2] ima: Free IMA measurement buffer on error
@ 2021-01-21 17:30 Lakshmi Ramasubramanian
2021-01-21 17:30 ` [PATCH 2/2] ima: Free IMA measurement buffer after kexec syscall Lakshmi Ramasubramanian
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Lakshmi Ramasubramanian @ 2021-01-21 17:30 UTC (permalink / raw)
To: zohar, bauerman, dmitry.kasatkin, ebiederm, gregkh, sashal, tyhicks
Cc: linux-integrity, linux-kernel, linuxppc-dev
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;
}
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] ima: Free IMA measurement buffer after kexec syscall
2021-01-21 17:30 [PATCH 1/2] ima: Free IMA measurement buffer on error Lakshmi Ramasubramanian
@ 2021-01-21 17:30 ` Lakshmi Ramasubramanian
2021-01-21 17:37 ` Tyler Hicks
2021-01-22 22:31 ` Thiago Jung Bauermann
2021-01-21 17:35 ` [PATCH 1/2] ima: Free IMA measurement buffer on error Tyler Hicks
2021-01-22 22:30 ` Thiago Jung Bauermann
2 siblings, 2 replies; 8+ messages in thread
From: Lakshmi Ramasubramanian @ 2021-01-21 17:30 UTC (permalink / raw)
To: zohar, bauerman, dmitry.kasatkin, ebiederm, gregkh, sashal, tyhicks
Cc: linux-integrity, linux-kernel, linuxppc-dev
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. This buffer is not freed before
completing the kexec system call resulting in memory leak.
Add ima_buffer field in "struct kimage" to store the virtual address
of the buffer allocated for the IMA measurement list.
Free the memory allocated for the IMA measurement list in
kimage_file_post_load_cleanup() 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")
---
include/linux/kexec.h | 5 +++++
kernel/kexec_file.c | 5 +++++
security/integrity/ima/ima_kexec.c | 2 ++
3 files changed, 12 insertions(+)
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 9e93bef52968..5f61389f5f36 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -300,6 +300,11 @@ struct kimage {
/* Information for loading purgatory */
struct purgatory_info purgatory_info;
#endif
+
+#ifdef CONFIG_IMA_KEXEC
+ /* Virtual address of IMA measurement buffer for kexec syscall */
+ void *ima_buffer;
+#endif
};
/* kexec interface functions */
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index b02086d70492..5c3447cf7ad5 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -166,6 +166,11 @@ void kimage_file_post_load_cleanup(struct kimage *image)
vfree(pi->sechdrs);
pi->sechdrs = NULL;
+#ifdef CONFIG_IMA_KEXEC
+ vfree(image->ima_buffer);
+ image->ima_buffer = NULL;
+#endif /* CONFIG_IMA_KEXEC */
+
/* See if architecture has anything to cleanup post load */
arch_kimage_file_post_load_cleanup(image);
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 212145008a01..8eadd0674629 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -130,6 +130,8 @@ void ima_add_kexec_buffer(struct kimage *image)
return;
}
+ image->ima_buffer = kexec_buffer;
+
pr_debug("kexec measurement buffer for the loaded kernel at 0x%lx.\n",
kbuf.mem);
}
--
2.30.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] ima: Free IMA measurement buffer on error
2021-01-21 17:30 [PATCH 1/2] ima: Free IMA measurement buffer on error Lakshmi Ramasubramanian
2021-01-21 17:30 ` [PATCH 2/2] ima: Free IMA measurement buffer after kexec syscall Lakshmi Ramasubramanian
@ 2021-01-21 17:35 ` Tyler Hicks
2021-01-22 22:30 ` Thiago Jung Bauermann
2 siblings, 0 replies; 8+ messages in thread
From: Tyler Hicks @ 2021-01-21 17:35 UTC (permalink / raw)
To: Lakshmi Ramasubramanian
Cc: zohar, bauerman, dmitry.kasatkin, ebiederm, gregkh, sashal,
linux-integrity, linux-kernel, linuxppc-dev
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 <nramas@linux.microsoft.com>
> Suggested-by: Tyler Hicks <tyhicks@linux.microsoft.com>
> Fixes: 7b8589cc29e7 ("ima: on soft reboot, save the measurement list")
Reviewed-by: Tyler Hicks <tyhicks@linux.microsoft.com>
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(&kbuf);
> 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
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] ima: Free IMA measurement buffer after kexec syscall
2021-01-21 17:30 ` [PATCH 2/2] ima: Free IMA measurement buffer after kexec syscall Lakshmi Ramasubramanian
@ 2021-01-21 17:37 ` Tyler Hicks
2021-01-22 22:31 ` Thiago Jung Bauermann
1 sibling, 0 replies; 8+ messages in thread
From: Tyler Hicks @ 2021-01-21 17:37 UTC (permalink / raw)
To: Lakshmi Ramasubramanian
Cc: zohar, bauerman, dmitry.kasatkin, ebiederm, gregkh, sashal,
linux-integrity, linux-kernel, linuxppc-dev
On 2021-01-21 09:30:03, 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. This buffer is not freed before
> completing the kexec system call resulting in memory leak.
>
> Add ima_buffer field in "struct kimage" to store the virtual address
> of the buffer allocated for the IMA measurement list.
> Free the memory allocated for the IMA measurement list in
> kimage_file_post_load_cleanup() 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")
Reviewed-by: Tyler Hicks <tyhicks@linux.microsoft.com>
Tyler
> ---
> include/linux/kexec.h | 5 +++++
> kernel/kexec_file.c | 5 +++++
> security/integrity/ima/ima_kexec.c | 2 ++
> 3 files changed, 12 insertions(+)
>
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index 9e93bef52968..5f61389f5f36 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -300,6 +300,11 @@ struct kimage {
> /* Information for loading purgatory */
> struct purgatory_info purgatory_info;
> #endif
> +
> +#ifdef CONFIG_IMA_KEXEC
> + /* Virtual address of IMA measurement buffer for kexec syscall */
> + void *ima_buffer;
> +#endif
> };
>
> /* kexec interface functions */
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index b02086d70492..5c3447cf7ad5 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -166,6 +166,11 @@ void kimage_file_post_load_cleanup(struct kimage *image)
> vfree(pi->sechdrs);
> pi->sechdrs = NULL;
>
> +#ifdef CONFIG_IMA_KEXEC
> + vfree(image->ima_buffer);
> + image->ima_buffer = NULL;
> +#endif /* CONFIG_IMA_KEXEC */
> +
> /* See if architecture has anything to cleanup post load */
> arch_kimage_file_post_load_cleanup(image);
>
> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> index 212145008a01..8eadd0674629 100644
> --- a/security/integrity/ima/ima_kexec.c
> +++ b/security/integrity/ima/ima_kexec.c
> @@ -130,6 +130,8 @@ void ima_add_kexec_buffer(struct kimage *image)
> return;
> }
>
> + image->ima_buffer = kexec_buffer;
> +
> pr_debug("kexec measurement buffer for the loaded kernel at 0x%lx.\n",
> kbuf.mem);
> }
> --
> 2.30.0
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] ima: Free IMA measurement buffer on error
2021-01-21 17:30 [PATCH 1/2] ima: Free IMA measurement buffer on error Lakshmi Ramasubramanian
2021-01-21 17:30 ` [PATCH 2/2] ima: Free IMA measurement buffer after kexec syscall Lakshmi Ramasubramanian
2021-01-21 17:35 ` [PATCH 1/2] ima: Free IMA measurement buffer on error Tyler Hicks
@ 2021-01-22 22:30 ` Thiago Jung Bauermann
2021-02-03 17:47 ` Lakshmi Ramasubramanian
2 siblings, 1 reply; 8+ messages in thread
From: Thiago Jung Bauermann @ 2021-01-22 22:30 UTC (permalink / raw)
To: Lakshmi Ramasubramanian
Cc: zohar, dmitry.kasatkin, ebiederm, gregkh, sashal, tyhicks,
linux-integrity, linux-kernel, linuxppc-dev
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] ima: Free IMA measurement buffer after kexec syscall
2021-01-21 17:30 ` [PATCH 2/2] ima: Free IMA measurement buffer after kexec syscall Lakshmi Ramasubramanian
2021-01-21 17:37 ` Tyler Hicks
@ 2021-01-22 22:31 ` Thiago Jung Bauermann
2021-02-03 17:46 ` Lakshmi Ramasubramanian
1 sibling, 1 reply; 8+ messages in thread
From: Thiago Jung Bauermann @ 2021-01-22 22:31 UTC (permalink / raw)
To: Lakshmi Ramasubramanian
Cc: zohar, dmitry.kasatkin, ebiederm, gregkh, sashal, tyhicks,
linux-integrity, linux-kernel, linuxppc-dev
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. This buffer is not freed before
> completing the kexec system call resulting in memory leak.
>
> Add ima_buffer field in "struct kimage" to store the virtual address
> of the buffer allocated for the IMA measurement list.
> Free the memory allocated for the IMA measurement list in
> kimage_file_post_load_cleanup() 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")
Good catch.
Reviewed-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] ima: Free IMA measurement buffer after kexec syscall
2021-01-22 22:31 ` Thiago Jung Bauermann
@ 2021-02-03 17:46 ` Lakshmi Ramasubramanian
0 siblings, 0 replies; 8+ messages in thread
From: Lakshmi Ramasubramanian @ 2021-02-03 17:46 UTC (permalink / raw)
To: Thiago Jung Bauermann
Cc: zohar, dmitry.kasatkin, ebiederm, gregkh, sashal, tyhicks,
linux-integrity, linux-kernel, linuxppc-dev
On 1/22/21 2:31 PM, Thiago Jung Bauermann wrote:
>
> 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. This buffer is not freed before
>> completing the kexec system call resulting in memory leak.
>>
>> Add ima_buffer field in "struct kimage" to store the virtual address
>> of the buffer allocated for the IMA measurement list.
>> Free the memory allocated for the IMA measurement list in
>> kimage_file_post_load_cleanup() 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")
>
> Good catch.
>
> Reviewed-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
>
Thanks Thiago.
-lakshmi
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] ima: Free IMA measurement buffer on error
2021-01-22 22:30 ` Thiago Jung Bauermann
@ 2021-02-03 17:47 ` Lakshmi Ramasubramanian
0 siblings, 0 replies; 8+ messages in thread
From: Lakshmi Ramasubramanian @ 2021-02-03 17:47 UTC (permalink / raw)
To: Thiago Jung Bauermann
Cc: zohar, dmitry.kasatkin, ebiederm, gregkh, sashal, tyhicks,
linux-integrity, linux-kernel, linuxppc-dev
On 1/22/21 2:30 PM, Thiago Jung Bauermann wrote:
>
> 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.
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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-02-03 17:48 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-21 17:30 [PATCH 1/2] ima: Free IMA measurement buffer on error Lakshmi Ramasubramanian
2021-01-21 17:30 ` [PATCH 2/2] ima: Free IMA measurement buffer after kexec syscall Lakshmi Ramasubramanian
2021-01-21 17:37 ` Tyler Hicks
2021-01-22 22:31 ` Thiago Jung Bauermann
2021-02-03 17:46 ` Lakshmi Ramasubramanian
2021-01-21 17:35 ` [PATCH 1/2] ima: Free IMA measurement buffer on error Tyler Hicks
2021-01-22 22:30 ` Thiago Jung Bauermann
2021-02-03 17:47 ` Lakshmi Ramasubramanian
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).