Page MenuHomeSolus

Prevent dracut from adding avx2 or 32bit libraries
ClosedPublic

Authored by sunnyflunk on Aug 3 2018, 5:10 AM.

Details

Summary

Patch dracut to remove avx2 library paths when installing files to the initrd.

The last barrier towards T4719

Signed-off-by: Peter O'Connor <peter@solus-project.com>

Test Plan

Build initrd locally with repo dracut - includes glibc-2.28 haswell libs that
are installed locally
Build initrd locally with this dracut - no haswell libs included

Diff Detail

Repository
R661 dracut
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

sunnyflunk created this revision.Aug 3 2018, 5:10 AM
sunnyflunk requested review of this revision.Aug 3 2018, 5:10 AM
sunnyflunk added a comment.EditedAug 3 2018, 5:16 AM

Would like review of the C code since I don't have a clue what I'm doing. It suggested that perhaps I strdup somewhere I didn't need to on the valgrind. Small compilable version of the included code.

Updated from below to mostly remove memory leaks.

#include <stdio.h>
#include <string.h>

const char *strip_avx(const char *fullpath, const char *avxpath)
{
    int length;
    char *spntr = NULL;
    char npath[120] = "/usr/lib64/";

    // Get pointer to found path
    char *qpntr = strstr(fullpath, avxpath);

    if(qpntr)
    {
        // Remove leading path from the pointer
        length = strlen(avxpath);
        spntr = qpntr + length;

        // Join path to /usr/lib64 (dracut default)
        strcat(npath, spntr);
        char *resultpath = strdup(npath);
        return resultpath;
    }

    return fullpath;
}

int main()
{
    char *r = "/haswell/avx512_1/";
    char *p = "/haswell/";
    const char *result = NULL;
    const char *resulttmp = NULL;
    char *lib = "/usr/lib64/haswell/avx512_1/libm.so.6";
    printf("%s\n", lib);

    resulttmp = strip_avx(lib, r);
    result = strip_avx(resulttmp, p);
    printf("%s\n", result);
    return 0;
}

The diff of a filesize/file build for old and new dracut.

--- resultsold  2018-08-03 13:30:05.088640215 +1000
+++ resultsnew  2018-08-03 13:30:11.816640421 +1000
@@ -87,7 +87,7 @@
 0etc/fstab.empty
 251etc/group
 25etc/initrd-release
-10384etc/ld.so.cache
+9802etc/ld.so.cache
 0etc/lvm
 44etc/lvm/lvm.conf
 0etc/machine-id
@@ -891,26 +891,7 @@
 16usr/bin/umount
 3080096usr/bin/vim
 3usr/bin/vi
-0usr/lib32
-0usr/lib32/haswell
-171188usr/lib32/ld-2.28.so
-10usr/lib32/ld-linux.so.2
-2151824usr/lib32/libc-2.28.so
-252usr/lib32/libc.so
-12usr/lib32/libc.so.6
-132usr/lib32/libgcc_s.so
-116100usr/lib32/libgcc_s.so.1
-50772usr/lib32/libnss_files-2.28.so
-20usr/lib32/libnss_files.so.2
-20usr/lib32/libnss_files.so
 0usr/lib64
-0usr/lib64/haswell
-2049744usr/lib64/haswell/libc-2.28.so
-43296usr/lib64/haswell/libcrypt-2.28.so
-16usr/lib64/haswell/libcrypt.so.1
-12usr/lib64/haswell/libc.so.6
-1567096usr/lib64/haswell/libm-2.28.so
-12usr/lib64/haswell/libm.so.6
 135usr/lib64/initrd-release
 173616usr/lib64/ld-2.28.so
 10usr/lib64/ld-linux-x86-64.so.2
@@ -965,7 +946,6 @@
 17usr/lib64/libexpat.so.1
 332176usr/lib64/libext2fs.so.2.4
 16usr/lib64/libext2fs.so.2
-0usr/lib64/libfakeroot
 34880usr/lib64/libffi.so.6.0.4
 15usr/lib64/libffi.so.6
 15usr/lib64/libffi.so

Don't be a drongo and push the final version of the patch...

I haven't been using pure C for quite some time but:

  1. nested functions aren't in C standard, clang will rebuke you for that even with GNU extensions enabled
  2. void * strip_avx_path(..) is a lie, it works but you should express what type you want to return (char*)
  3. yep that use of strdup() is a leak, you can see it with clang -g review.c && clang --analyze review.c or valgrind --leak-check=full ./review.out

Maybe somebody better at C could help, this modified code has just one leak compared to 6 it previously had:

#include <stdio.h>
#include <string.h>

const char *strip_avx(const char *fullpath, const char *avxpath)
{
    int length;
    char *spntr = NULL, *fpath = NULL, *apath = NULL;
    char npath[120] = "/usr/lib64/";

    if(strstr(fullpath, avxpath))
    {
        // Get pointer to found path
        char *qpntr = strstr(fullpath, avxpath);

        // Remove leading path from the pointer
        length = strlen(avxpath);
        spntr = qpntr + length;

        // Join path to /usr/lib64 (dracut default)
        strcat(npath, spntr);
        char *resultpath = strdup(npath);
        return resultpath;
    }

    return fullpath;
}

int main()
{
    char *r = "/haswell/avx512_1/";
    char *p = "/haswell/";
    const char *result = NULL;
    const char *resulttmp = NULL;
    char *lib = "/usr/lib64/haswell/avx512_1/libm.so.6";
    printf("%s\n", lib);

    resulttmp = strip_avx(lib, r);
    result = strip_avx(resulttmp, p);
    printf("%s\n", result);
    return 0;
}
  1. void * strip_avx_path(..) is a lie, it works but you should express what type you want to return (char*)

I originally didn't use void and strdup, but at some point in having nfi what I was doing they worked around [courtesy of Google search] whatever issue I was having (I guess one of the returns wasn't a proper char variable). I guess once I tidied everything up, it was actually doing what I had originally intended.

Can also strip out apath and fpath from being initialized now.

Some fixes based on @mati865 comments

If even I know they're better, then they really must be!

I originally didn't use void and strdup, but at some point in having nfi what I was doing they worked around [courtesy of Google search] whatever issue I was having (I guess one of the returns wasn't a proper char variable). I guess once I tidied everything up, it was actually doing what I had originally intended.

Know that pain, you spent a lot time investigating why simple code doesn't work and create complicated workaround. Then somebody else removes few lines and the code looks before but this time it works...

Can also strip out apath and fpath from being initialized now.

Ah, just missed it.

files/0001-Exclude-unwanted-paths-from-initrd-creation.patch
76–77

Those would be const char* now (like in modified reduced code), but I haven't tested if dracut builds.

constantly create const char chew c correct consequences

sunnyflunk marked an inline comment as done.Aug 3 2018, 12:20 PM

Have built and validated dracut with the same results as before.

Only need to call strstr(fullpath, avxpath) once to save those picoseconds

This revision was not accepted when it landed; it landed in state Needs Review.Oct 23 2018, 4:33 AM
This revision was automatically updated to reflect the committed changes.