public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>
To: DJ Delorie <dj@redhat.com>
Cc: libc-alpha@sourceware.org
Subject: Re: RFC: system-wide default tunables
Date: Fri, 6 Oct 2023 16:14:40 -0300	[thread overview]
Message-ID: <fb605521-c9af-4427-8bae-3da51d2a59e0@linaro.org> (raw)
In-Reply-To: <xnpm1ry57m.fsf@greed.delorie.com>



On 06/10/23 15:29, DJ Delorie wrote:
> Adhemerval Zanella Netto <adhemerval.zanella@linaro.org> writes:
>> The idea sounds ok, but adding on ld.so.cache means it would not work 
>> for static.  I don't think this is really an issue, static PIE is really
>> tricky because self-relocation happens after tunable (because tunables 
>> itself might change ifunc selection); and trying to add support for 
>> static PIE would require a lot of messy refactoring (all ld.so.cache 
>> loading would need to be annotated hidden, no external function calling,
>> even mem* ones; etc.).
> 
> The idea behind using this is that admins are used to running ldconfig
> and managing ld.so.cache, so there would be nothing new to learn.  The
> cache already has some hwcaps-related info in it, too, for selecting
> suitable libraries.

The hwcap-related are still used for library selection, the original
intention of the ld.so.cache.  What I am afraid is ld.so.cache start to
being an umbrella for different and unrelated settings.

> 
>>> Ideas:
>>>
>>> * Specify some file or files in /etc that contain tunables settings.
>>>   Follow the ld.so.conf patterns, allow subdirectories, etc.
>>>
>>> * Store tunables info in /etc/ld.so.cache in a new slot at the end,
>>>   with a new enum for the chunk.  This way older glibc will just
>>>   ignore it.  Parsing and storing will be done via ldconfig.
>>
>> It means that we will have to always load the ld.so.cache, not only when
>> we will actually have to load an ET_DYN .  It should be ok, but runtimes
>> that usually only link against libc.so (like rust) will have additional
>> overhead on startup.
> 
> We don't have to "load" it just map it and read the aux data at the end.
> But, yeah.

By 'load' I man mmap the ld.so.cache, which still adds extra commit charge.

> 
>> We also have DF_1_NODEFLIB to inhibit loader cache search, should we add
>> another flag to inhibit the global tunables?  It does not make sense to
>> set per-library, but it still might be useful to set on ET_EXEC.
> 
> If there were a system-wide tunable that caused improper action, it
> wouldn't be a "tunable" it would be a "stoppable".  If a single program
> was incompatible with system-wide tunables, it would need the environmen
> variable set to tune them for the specific program.

It is not improper action that I worried, but security and performance
implications.  For instance, the malloc tunables change performance depending
of the workload; a system-wide tunable might not the best option for all
programs.

The DF_ is just a shortcut to avoid the need to know which tunables are
set system-wide (assuming that the environment variables has precedence
over the system-wide). But I am assuming that system-wide has preference
over user-defined, which does not seems the case for your proposal.

> 
>> So I am not fully sure adding the global tunable setting on ld.so.cache is
>> the correct approach.  However, adding on an external file will add
>> another open/mmap/close on each program; plus the extra mmap.
> 
> Yup.  And worst case, those apps that don't get the tunable...  act like
> they do now, with no system-wide tunables.
> 
> If the settings were mandatory, I'd worry more.

I think what is not clear is if the idea is to mimic the 'security_level'
of tunable (which I intend to remove), in a way that we might have tunable
that might overridden and/or disabled by the process (through the envvar).

> 
>>> * Values in ld.so.cache will be parsed but not range checked; that's
>>>   dependent on what the glibc app expects.
>>
>> Importing the range information on tunable definition is straightforward, 
> 
> Unless it differs between applications.  Or if the cache were generated
> with one version of glibc and the application runs with a different
> version.

But in this case, and invalid/unknown value should be ignored if the program
is using a different glibc version.  Setting an invalid/unknown tunable still
make little sense to me, it means that it will always be ignored by some target 
if you are running different glibc with different tunable setting support.
And I don't see why we should not make it explicit for the administrator. 

Maybe the best way is to make the range/key check as default and add an
option to force to set invalid values.


> 
>> so I think we should add the range check on ldconfig ld.so.cache setup.
>> There is no error checking on env var tunable, invalid values are just
>> ignored without any user feedback.  Since we will do pre-processing,
>> I think it would be valuable to at least show any possible invalid range,
>> specially because this is a administration setting.
> 
> I see no problem with validating at cache time, but I don't want to skip
> range checking at run time.  Doing so leads to security bugs.

I agree, and I was not suggesting otherwise.

> 
>>> * read those, do range checking, and call callbacks at runtime
>>>
>>> * To speed processing, encode a hash for each tunable name, both in
>>>   glibc's table (which is built at glibc build time) and in
>>>   /etc/ld.so.cache.  Comparing the hash typically fails but avoids a
>>>   string compare.  Matching hashes are followed by a string compare to
>>>   verify.  The hash need not be crypographically secure.
> 
>> Do we really need this optimization?  Internally, the tunables are already
>> accessed through a enum,
> 
> That enum might change from version to version.  We're already seeing
> enum-related issues with RHEL upgrades.

Yes, Carlos has brought the awk sorting problem.  I think this is fixable
(maybe porting the tunables generation to python, run the sysdeps search
in a predictable way, set enum value based on tunable ordering) and we can
add test to check if tunable enum changes over releases.

I still think this is simpler than data structure your are proposing, and 
with less performance issues.  But I don't have a strong preference.

> 
>>> * I'm not going to try to add some "syntax" to specify if a tunable is
>>>   overridable or not; this is a simple default-only change.
>>
>> How should we handle the envvar GLIBC_TUNABLEs in the presence of a system-wide
>> tunable?
> 
> The variable overrides the cache.

Right.  Do we allow security opt-in option as well? I am asking because
in this case, it is expected that the system-wide should not be overridden.

> 
>>> * Tunables set by these defaults will not be disabled for setuid
>>>   programs; it's assumed they're a "trusted source".
>>
>> This seems reasonable, and with this rationale should we add an option
>> to allow some tunable to be disabled or overriden? I take this is an extra
>> complexity that we should not pursuit. 
> 
> Agreed; we want a simple system.
> 
>> Another possible feature that comes from the tunable discussion is
>> whether make sense to add a per-process tunables.
> 
> Er, isn't that the environment variable?
> 

It is a easier way to configure per-process tunable, it avoid messing with FS 
that either are mounted RO or do not allow adding wrappers to setup GLIBC_TUNABLES.

Also, my idea is to just remove the tunable setting for setuid/setguid; so 
per-process is a way to setup specific rules for security sensitive processes
(assuming that we will support non-overriden security tunables).

  reply	other threads:[~2023-10-06 19:14 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-04 20:55 DJ Delorie
2023-10-06 14:44 ` Adhemerval Zanella Netto
2023-10-06 17:12   ` Adhemerval Zanella Netto
2023-10-06 18:29   ` DJ Delorie
2023-10-06 19:14     ` Adhemerval Zanella Netto [this message]
2023-10-06 20:25       ` DJ Delorie
2023-10-17 14:10         ` Adhemerval Zanella Netto
2023-10-17 14:17           ` Siddhesh Poyarekar
2023-10-17 14:37             ` Siddhesh Poyarekar
2023-10-17 15:43             ` DJ Delorie
2023-10-17 15:58               ` Siddhesh Poyarekar
2023-10-17 16:45             ` DJ Delorie
2023-10-17 16:55               ` Siddhesh Poyarekar
2023-10-17 17:14                 ` DJ Delorie
2023-10-18 14:20                   ` Siddhesh Poyarekar
2023-10-17 17:40           ` Zack Weinberg
2023-10-17 17:47             ` DJ Delorie
2023-10-17 18:17               ` Zack Weinberg
2023-10-17 18:21                 ` DJ Delorie
2023-10-06 22:04       ` DJ Delorie

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fb605521-c9af-4427-8bae-3da51d2a59e0@linaro.org \
    --to=adhemerval.zanella@linaro.org \
    --cc=dj@redhat.com \
    --cc=libc-alpha@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).