[PATCH] tunables: Add IFUNC selection and cache sizes

[H.J. Lu]

On Wed, Jun 21, 2017 at 5:44 AM, Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
> On Wednesday 21 June 2017 05:55 PM, H.J. Lu wrote:
>>> Why do you need these to be inherited by children of setuid processes?
>>> If not, then you should remove the SXID_IGNORE and let the default (i.e.
>>> SXID_ERASE) prevail so that setuid processes and their children do not
>>> see these tunables.
>>
>> I want to be conservative.  The default ones should be good for 99.9%
>> of applications.
>
> The conservative (in terms of security) should be the default, i.e.
> disallow envvars to be read across setxid boundaries.  If an escalation
> of privileges is required, there should be a specifically stated reason
> to do so.  The current envvars need a thorough analysis too to figure
> out if they need to be read across setxid boundaries.

Done.

>>> Also you've specified these as size_t but the values seem to get stored
>>> in long int.  Please fix them.
>>
>> tunables supports:
>>
>>   TUNABLE_TYPE_INT_32,
>>   TUNABLE_TYPE_UINT_64,
>>   TUNABLE_TYPE_SIZE_T,
>>   TUNABLE_TYPE_STRING
>>
>> There is no long and on x86, long == size_t:
>>
>> [hjl@gnu-tools-1 tmp]$ cat f.c
>> #include <unistd.h>
>>
>> int x [(sizeof (size_t) == sizeof (long)) ? 1 : -1];
>> [hjl@gnu-tools-1 tmp]$ gcc -S f.c -m64
>> [hjl@gnu-tools-1 tmp]$ gcc -S f.c -m32
>> [hjl@gnu-tools-1 tmp]$ gcc -S f.c -mx32
>> [hjl@gnu-tools-1 tmp]$
>
> Sure, but one is signed and the other isn't, which messes with
> validation of the tunable.  If long is what you need, you could add
> ssize_t, but I don't see why the sizes should be negative.  It seems
> like instead of making the tunables signed, you should be making the
> caches unsigned.

Done.

>> On x86, there are CPU features, like SSE, AVX, ... and ARCH features,
>> like slow bsf.  X86 tunables allow users to disable AVX and slow bsf.
>> They don't change any CPU names.
>
> Ah OK then you're right, all of those should be hwcaps too and go into
> x86 - I thought you were suggesting something like the mtune options,
> i.e. corei7.  I'll add the glibc.tune.cpu myself.
>

The supported x86 platforms are listed in sysdeps/x86/dl-procinfo.c.
x86 tunables will influence setting of dl_platform, which shouldn't set
arbitrarily.

>>> Since #name is going to be a constant stirng, you could just use the
>>> is_name from dl-tunables.c.  Just pull it out into dl-tunables.h as a
>>> static inline.
>>
>> memcmp is used elsewhere in ld.so and it is much faster than
>> is_name in dl-tunables.c.  I prefer to keep memcmp here.
>
> Fair enough.
>

I am testing this patch.  OK for master if there are no regressions?


-- 
H.J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-x86-Rename-glibc.tune.ifunc-to-glibc.tune.hwcaps.patch
Type: text/x-patch
Size: 8005 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/libc-alpha/attachments/20170621/15404848/attachment.bin>