linux-kernel - Re: [kernel-hardening] [PATCH 3/4] Make static usermode helper binaries constant

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1481846720.1054.1.camel@gmail.com>
Date:   Thu, 15 Dec 2016 19:05:20 -0500
From:   Daniel Micay <danielmicay@...il.com>
To:     kernel-hardening@...ts.openwall.com
Cc:     linux-kernel@...r.kernel.org
Subject: Re: [kernel-hardening] [PATCH 3/4] Make static usermode helper
 binaries constant

> Thanks for the explanation.  I don't think we need to worry about
> merging these strings, but I'll keep it in mind.
> 
> However, the "folklore" of the kernel was to never do:
> 	char *foo = "bar";
> but instead do:
> 	char foo[] = "bar";
> to save on the extra variable that the former creates.  Is that no
> longer the case and we really should be using '*' to allow gcc to be
> smarter about optimizations?
> 
> > The 'const' qualifier for pointers doesn't really do anything, it's
> > when
> > it's used on the variable (after the pointer) that it can do more
> > than
> > acting as a programming guide.
> 
> Many thanks for the explanations,
> 
> greg k-h

Can see what the compiler has to work with pretty easily from LLVM IR:

    char *const constant_string_constant = "string";
    char *const constant_string_constant2 = "string";
    char *non_constant_string_constant = "string";
    char *non_constant_string_constant2 = "string";
    char non_constant_string_array[] = "string";
    char non_constant_string_array2[] = "string";
    const char constant_string_array[] = "string";
    const char constant_string_array2[] = "string";

Becomes:

    @.str = private unnamed_addr constant [7 x i8] c"string\00", align 1
    @constant_string_constant = constant i8* getelementptr inbounds ([7 x i8], [7 x i8]* @.str, i32 0, i32 0), align 8
    @constant_string_constant2 = constant i8* getelementptr inbounds ([7 x i8], [7 x i8]* @.str, i32 0, i32 0), align 8
    @non_constant_string_constant = global i8* getelementptr inbounds ([7 x i8], [7 x i8]* @.str, i32 0, i32 0), align 8
    @non_constant_string_constant2 = global i8* getelementptr inbounds ([7 x i8], [7 x i8]* @.str, i32 0, i32 0), align 8
    @non_constant_string_array = global [7 x i8] c"string\00", align 1
    @non_constant_string_array2 = global [7 x i8] c"string\00", align 1
    @constant_string_array = constant [7 x i8] c"string\00", align 1
    @constant_string_array2 = constant [7 x i8] c"string\00", align 1

And with optimization:

    @constant_string_constant = local_unnamed_addr constant i8* getelementptr inbounds ([7 x i8], [7 x i8]* @constant_string_array, i64 0, i64 0), align 8
    @constant_string_constant2 = local_unnamed_addr constant i8* getelementptr inbounds ([7 x i8], [7 x i8]* @constant_string_array, i64 0, i64 0), align 8
    @non_constant_string_constant = local_unnamed_addr global i8* getelementptr inbounds ([7 x i8], [7 x i8]* @constant_string_array, i64 0, i64 0), align 8
    @non_constant_string_constant2 = local_unnamed_addr global i8* getelementptr inbounds ([7 x i8], [7 x i8]* @constant_string_array, i64 0, i64 0), align 8
    @non_constant_string_array = local_unnamed_addr global [7 x i8] c"string\00", align 1
    @non_constant_string_array2 = local_unnamed_addr global [7 x i8] c"string\00", align 1
    @constant_string_array = local_unnamed_addr constant [7 x i8] c"string\00", align 1
    @constant_string_array2 = local_unnamed_addr constant [7 x i8] c"string\00", align 1

If they're static though, the compiler can see that nothing takes the
address (local_unnamed_addr == unnamed_addr if it's internal) so it
doesn't need separate variables anyway:

    static char *const constant_string_constant = "string";
    static char *const constant_string_constant2 = "string";

    char *foo() {
      return constant_string_constant;
    }

    char *bar() {
      return constant_string_constant2;
    }

Becomes (with optimization):

@.str = private unnamed_addr constant [7 x i8] c"string\00", align 1

; Function Attrs: norecurse nounwind readnone uwtable
define i8* @foo() local_unnamed_addr #0 {
  ret i8* getelementptr inbounds ([7 x i8], [7 x i8]* @.str, i64 0, i64 0)
}

; Function Attrs: norecurse nounwind readnone uwtable
define i8* @bar() local_unnamed_addr #0 {
  ret i8* getelementptr inbounds ([7 x i8], [7 x i8]* @.str, i64 0, i64 0)
}

So for statics, I think `static const char *` wins due to allowing
merging (although it doesn't matter here). For non-statics, you end up
with extra pointer constants. Those could get removed, but Linux doesn't
have -fvisibility=hidden and I'm not sure how clever linkers are. Maybe
setting up -fvisibility=hidden to work with monolithic non-module-
enabled builds could actually be realistic. Expect it'd remove a fair
bit of bloat but not sure how much would need to be marked as non-hidden 
other than the userspace ABI.
Download attachment "signature.asc" of type "application/pgp-signature" (867 bytes)