Thread (12 messages) 12 messages, 6 authors, 2023-03-14

Re: [PATCH v1 0/2] Add destructor hook to LSM modules

From: Paul Moore <paul@paul-moore.com>
Date: 2023-03-11 14:59:41
Also in: linux-integrity, lkml

On Fri, Mar 10, 2023 at 5:53 PM Mirsad Goran Todorovac
[off-list ref] wrote:
On 10. 03. 2023. 23:33, Paul Moore wrote:
quoted
On Fri, Mar 10, 2023 at 2:26 PM Mirsad Goran Todorovac
[off-list ref] wrote:
quoted
LSM security/integrity/iint.c had the case of kmem_cache_create() w/o a proper
kmem_cache_destroy() destructor.

Introducing the release() hook would enable LSMs to release allocated resources
on exit, and in proper order, rather than dying all together with kernel shutdown
in an undefined order.

Thanks,
        Mirsad

---
 include/linux/lsm_hooks.h | 1 +
 security/integrity/iint.c | 7 +++++++
 2 files changed, 8 insertions(+)
I only see the 1/2 patch, did you send the 2/2 patch to the LSM list?
If not, you need to do that
I will resend everything, because this first attempt was buggy and
incorrect regarding the credits. Will try this, Andy said that I wait
for the comments, but I did not expect them before the weekend.

Thank you guys for patience, I am just finishing the test of devres
patch and then I will proceed with integrity LSM patch submission.
Thank you for resending the patchset, although a couple of
administrative things to consider for your next posting:

* Each time you post a patchset it is generally considered a best
practice to increment the version number, e.g. "[PATCH vX 0/2]" with
'X' being the version number.  This makes it easier to identify
specific revisions and ensure that everyone is reviewing the latest
patchset.

* It is a good idea to use the "./scripts/get_maintainer.pl" script to
ensure you have included the right people and mailing lists on your
submissions as your latest resend did not include the LSM list when it
should have.

With that out of the way, I wanted to make a quick comment on the
patch itself.  Currently LSMs do not support unloading, or dynamic
loading for that matter.  There are several reasons for this, but
perhaps the most important is that in order to help meet the security
goals for several of the LSMs they need to be present in the kernel
from the very beginning and remain until the very end.  Adding a
proper "release" method to a LSM is going to be far more complicated
than what you've done with this patchset, involving a lot of
discussion both for the LSM layer itself and all of the currently
supported LSMs, and ultimately I don't believe it is something we will
want to support.

I appreciate your desire to help, and I want to thank you for your
patch and the effort behind it, but I don't believe the kobject memory
leak you saw at kernel shutdown was a real issue (it was only "leaked"
because the system was shutting down) and I'm not sure the current
behavior is something we want to change in the near future.

--
paul-moore.com
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help