Messages in this thread | | | Subject | Re: [kernel-hardening] [PATCH 3/4] Make static usermode helper binaries constant | From | Daniel Micay <> | Date | Thu, 15 Dec 2016 19:05:20 -0500 |
| |
> 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.[unhandled content-type:application/pgp-signature] | |