On Wed, Mar 22, 2017 at 07:05:27AM +0000, Ed Schouten wrote:
> Author: ed
> Date: Wed Mar 22 07:05:27 2017
> New Revision: 315701
> URL: https://svnweb.freebsd.org/changeset/base/315701
> 
> Log:
>   Set the interpreter path to /nonexistent.
>   
>   CloudABI executables are statically linked and don't have an
>   interpreter. Setting the interpreter path to NULL used to work
>   previously, but r314851 introduced code that checks the string
>   unconditionally. Running CloudABI executables now causes a null pointer
>   dereference.
You could have just reported that the revision breaks cloudabi.

>   
>   Looking at the rest of imgact_elf.c, it seems various other codepaths
>   already leaned on the fact that the interpreter path is set. Let's just
>   go ahead and pick an obviously incorrect interpreter path to appease
>   imgact_elf.c.

I believe that we should move in the reverse direction, in particular,
best would be to allow brand to specify that only a statically linked
binaries can be handled by it.  My reasoning is coming from a desire
to make brand matching as exact as possible, after dealing with the
bugs due to too vague matching.

Could you test the following ?  It supposedly fixes the NULL issue, and
adds the flag marking brands as only allowing static (really PT_INTERP-less)
binaries.

diff --git a/sys/amd64/cloudabi32/cloudabi32_sysvec.c 
b/sys/amd64/cloudabi32/cloudabi32_sysvec.c
index fe93385e4de..eca42873f8f 100644
--- a/sys/amd64/cloudabi32/cloudabi32_sysvec.c
+++ b/sys/amd64/cloudabi32/cloudabi32_sysvec.c
@@ -228,5 +228,5 @@ Elf32_Brandinfo cloudabi32_brand = {
        .machine        = EM_386,
        .sysvec         = &cloudabi32_elf_sysvec,
        .compat_3_brand = "CloudABI",
-       .interp_path    = "/nonexistent",
+       .flags          = BI_BRAND_NOTE_ONLY_STATIC,
 };
diff --git a/sys/amd64/cloudabi64/cloudabi64_sysvec.c 
b/sys/amd64/cloudabi64/cloudabi64_sysvec.c
index 34277f488fc..642516cc797 100644
--- a/sys/amd64/cloudabi64/cloudabi64_sysvec.c
+++ b/sys/amd64/cloudabi64/cloudabi64_sysvec.c
@@ -214,5 +214,5 @@ Elf64_Brandinfo cloudabi64_brand = {
        .sysvec         = &cloudabi64_elf_sysvec,
        .flags          = BI_CAN_EXEC_DYN,
        .compat_3_brand = "CloudABI",
-       .interp_path    = "/nonexistent",
+       .flags          = BI_BRAND_NOTE_ONLY_STATIC,
 };
diff --git a/sys/arm/cloudabi32/cloudabi32_sysvec.c 
b/sys/arm/cloudabi32/cloudabi32_sysvec.c
index aea4b822ddc..b1130c92bcd 100644
--- a/sys/arm/cloudabi32/cloudabi32_sysvec.c
+++ b/sys/arm/cloudabi32/cloudabi32_sysvec.c
@@ -190,5 +190,5 @@ Elf32_Brandinfo cloudabi32_brand = {
        .machine        = EM_ARM,
        .sysvec         = &cloudabi32_elf_sysvec,
        .compat_3_brand = "CloudABI",
-       .interp_path    = "/nonexistent",
+       .flags          = BI_BRAND_NOTE_ONLY_STATIC,
 };
diff --git a/sys/arm64/cloudabi64/cloudabi64_sysvec.c 
b/sys/arm64/cloudabi64/cloudabi64_sysvec.c
index 66d569b8f32..af23ffda9a0 100644
--- a/sys/arm64/cloudabi64/cloudabi64_sysvec.c
+++ b/sys/arm64/cloudabi64/cloudabi64_sysvec.c
@@ -183,5 +183,5 @@ Elf64_Brandinfo cloudabi64_brand = {
        .sysvec         = &cloudabi64_elf_sysvec,
        .flags          = BI_CAN_EXEC_DYN,
        .compat_3_brand = "CloudABI",
-       .interp_path    = "/nonexistent",
+       .flags          = BI_BRAND_NOTE_ONLY_STATIC,
 };
diff --git a/sys/i386/cloudabi32/cloudabi32_sysvec.c 
b/sys/i386/cloudabi32/cloudabi32_sysvec.c
index 2658c4f9ed6..d03c9e267ab 100644
--- a/sys/i386/cloudabi32/cloudabi32_sysvec.c
+++ b/sys/i386/cloudabi32/cloudabi32_sysvec.c
@@ -201,5 +201,5 @@ Elf32_Brandinfo cloudabi32_brand = {
        .machine        = EM_386,
        .sysvec         = &cloudabi32_elf_sysvec,
        .compat_3_brand = "CloudABI",
-       .interp_path    = "/nonexistent",
+       .flags          = BI_BRAND_NOTE_ONLY_STATIC,
 };
diff --git a/sys/kern/imgact_elf.c b/sys/kern/imgact_elf.c
index f33dc4cbeae..3036f814faf 100644
--- a/sys/kern/imgact_elf.c
+++ b/sys/kern/imgact_elf.c
@@ -273,6 +273,9 @@ __elfN(get_brandinfo)(struct image_params *imgp, const char 
*interp,
                bi = elf_brand_list[i];
                if (bi == NULL)
                        continue;
+               if (interp != NULL &&
+                   (bi->flags & BI_BRAND_NOTE_ONLY_STATIC) != 0)
+                       continue;
                if (hdr->e_machine == bi->machine && (bi->flags &
                    (BI_BRAND_NOTE|BI_BRAND_NOTE_MANDATORY)) != 0) {
                        ret = __elfN(check_note)(imgp, bi->brand_note, osrel);
@@ -305,7 +308,9 @@ __elfN(get_brandinfo)(struct image_params *imgp, const char 
*interp,
        /* If the executable has a brand, search for it in the brand list. */
        for (i = 0; i < MAX_BRANDS; i++) {
                bi = elf_brand_list[i];
-               if (bi == NULL || bi->flags & BI_BRAND_NOTE_MANDATORY)
+               if (bi == NULL || (bi->flags & BI_BRAND_NOTE_MANDATORY) != 0 ||
+                   (interp != NULL && (bi->flags &
+                   BI_BRAND_NOTE_ONLY_STATIC) != 0))
                        continue;
                if (hdr->e_machine == bi->machine &&
                    (hdr->e_ident[EI_OSABI] == bi->brand ||
@@ -318,7 +323,11 @@ __elfN(get_brandinfo)(struct image_params *imgp, const 
char *interp,
                                 * Again, prefer strictly matching
                                 * interpreter path.
                                 */
-                               if (strlen(bi->interp_path) + 1 ==
+                               if (interp_name_len == 0 &&
+                                   bi->interp_path == NULL)
+                                       return (bi);
+                               if (bi->interp_path != NULL &&
+                                   strlen(bi->interp_path) + 1 ==
                                    interp_name_len && strncmp(interp,
                                    bi->interp_path, interp_name_len) == 0)
                                        return (bi);
@@ -347,7 +356,8 @@ __elfN(get_brandinfo)(struct image_params *imgp, const char 
*interp,
        if (interp != NULL) {
                for (i = 0; i < MAX_BRANDS; i++) {
                        bi = elf_brand_list[i];
-                       if (bi == NULL || bi->flags & BI_BRAND_NOTE_MANDATORY)
+                       if (bi == NULL || (bi->flags & (BI_BRAND_NOTE_MANDATORY 
|
+                           BI_BRAND_NOTE_ONLY_STATIC)) != 0)
                                continue;
                        if (hdr->e_machine == bi->machine &&
                            /* ELF image p_filesz includes terminating zero */
@@ -361,7 +371,9 @@ __elfN(get_brandinfo)(struct image_params *imgp, const char 
*interp,
        /* Lacking a recognized interpreter, try the default brand */
        for (i = 0; i < MAX_BRANDS; i++) {
                bi = elf_brand_list[i];
-               if (bi == NULL || bi->flags & BI_BRAND_NOTE_MANDATORY)
+               if (bi == NULL || (bi->flags & BI_BRAND_NOTE_MANDATORY) != 0 ||
+                   (interp != NULL && (bi->flags &
+                   BI_BRAND_NOTE_ONLY_STATIC) != 0))
                        continue;
                if (hdr->e_machine == bi->machine &&
                    __elfN(fallback_brand) == bi->brand)
diff --git a/sys/sys/imgact_elf.h b/sys/sys/imgact_elf.h
index beb5f96fc03..83570ae2399 100644
--- a/sys/sys/imgact_elf.h
+++ b/sys/sys/imgact_elf.h
@@ -80,6 +80,7 @@ typedef struct {
 #define        BI_CAN_EXEC_DYN         0x0001
 #define        BI_BRAND_NOTE           0x0002  /* May have note.ABI-tag 
section. */
 #define        BI_BRAND_NOTE_MANDATORY 0x0004  /* Must have note.ABI-tag 
section. */
+#define        BI_BRAND_NOTE_ONLY_STATIC 0x0008
 } __ElfN(Brandinfo);
 
 __ElfType(Auxargs);
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to