Re: some fixes to musl

[Rich Felker <dalias@aerifal.cx>]

On Sun, Jul 24, 2011 at 01:39:11PM +0400, Vasiliy Kulikov wrote:
> > It should be obvious that a bogus LSM will create gaping security
> > holes if it's allowed such power.
> 
> I don't say such handling is perfect, just want to show it can be
> somehow justified:
> 
> Most of LSM hooks maintain some security context associated with
> file/task/socket/etc.  Some actions may require (re)allocation of this
> context.  If the allocation fails (e.g. OOM), it's wrong to allow the
> task to continue the work with the object, which has an outdated context
> (not updated against some recent activity).  So, unexpected ENOMEM is
> returned.

Are you saying that all operations on the associated object
(file/task/socket/etc.) are automatically first subjected to an LSM
hook? Or just that the LSM module author might set up a hook?

In the latter case, I would say it's a bug and potential gaping
vulnerability for a module to install a hook on any
resource-deallocation function.

If it's the former, I would say it's a bug and gaping security hole in
the whole LSM architecture. Failure to account for dependence of a
critical operation on resource allocation is a classic programming
error. To fix it, I would imagine the allocation either needs to be
moved to the action that caused (re)allocation to be needed, or LSM
hooks need to be skipped for resource deallocation functions, or LSM
hooks need to fallback to success rather than failure when resource
(re)allocation fails during a resource deallocation operation. But
since I'm not familiar with the LSM architecture, this is only
speculation.

In short, I think you've raised really good points, and LSM probably
needs an audit...

> I totally agree with you that injecting such faults where a program doesn't
> expect it is wrong, but sometimes it can be not obvious for kernel
> developers that it breaks something.

Not being aware that you're breaking something isn't a very good
excuse. In any case, following a principle of "deallocation cannot
fail" should not be too hard, and it's required for C++ RAII apps to
work anyway (what can you do if an operation fails during a
destructor?!)

> Even if they don't want to
> introduce such cases, it can be unintendedly introduced by a bug
> (e.g. sendmail setuid vulnerability).  Such bugs may be tricky to catch
> and may appear in very rare situations.  Trying to catch such cases would
> make the program much more robust against kernel bugs/changes (unless the
> error handling code complicates the whole logic and/or is not well
> tested).

I agree that it's better to check for failure and handle it where you
can. What bothers me is the creation of error cases that are usually
impossible to handle.

Rich