Re: [PATCH v1 0/2] Add destructor hook to LSM modules
From: Greg Kroah-Hartman
Date: Tue Mar 14 2023 - 08:06:11 EST
On Tue, Mar 14, 2023 at 01:05:25PM +0200, Andy Shevchenko wrote:
> On Mon, Mar 13, 2023 at 04:27:42PM -0400, Paul Moore wrote:
> > On Mon, Mar 13, 2023 at 5:33 AM Andy Shevchenko
> > <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> > > On Sat, Mar 11, 2023 at 09:59:17AM -0500, Paul Moore wrote:
> > > > On Fri, Mar 10, 2023 at 5:53 PM Mirsad Goran Todorovac
> > > > <mirsad.todorovac@xxxxxxxxxxxx> wrote:
>
> ...
>
> > > > 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.
> > >
> > > Currently the security module so secure that even adds an unneeded noise to
> > > the debugging output.
> > >
> > > At very least it would be nice to add a stub and put a big comment
> > > (on your taste) explaining why we do not do anything there.
> > >
> > > Agree?
> >
> > No.
>
> Are you sure? I'm proposing to add a stub which is no-op, but with a comment
> inside explaining why. In such case we:
>
> 1) shut the kobject infra up;
Please do not do anything just to "shut up" the kobject code, that
warning is in there on purpose, fix the problem properly and do not try
to work around it as that is totally wrong.
greg k-h