| Summary: | sys-kernel/kerneloops doesn't respect CFLAGS | ||
|---|---|---|---|
| Product: | Gentoo Linux | Reporter: | Justin Lecher (RETIRED) <jlec> |
| Component: | New packages | Assignee: | Greg Kroah-Hartman (RETIRED) <gregkh> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | gregkh |
| Priority: | High | Keywords: | InVCS |
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Package list: | Runtime testing required: | --- | |
| Bug Depends on: | |||
| Bug Blocks: | 59506 | ||
| Attachments: |
kerneloops-0.12.diff
build.log for 0.12 emerge.info Makefile.diff |
||
|
Description
Justin Lecher (RETIRED)
2008-09-19 13:26:13 UTC
Created attachment 165840 [details, diff]
kerneloops-0.12.diff
Don't know if -fstack-protector and -fno-common should not be added.
Why would we want to allow user's cflags here, we should trust upstream instead, it's already saved us from at least one security issue here :) Just curious, what was the security issue pertaining to CFLAGS? (In reply to comment #2) > Why would we want to allow user's cflags here, we should trust upstream > instead, it's already saved us from at least one security issue here :) > I don't understand that argument. If there are issues with compiling when user CFLAGS are used, we can try to handle that, but stripping march/mtune isn't what gentoo should do! Where is the security point? And why should all the warnings upstream wants, should be necessary for the user? (In reply to comment #3) > Just curious, what was the security issue pertaining to CFLAGS? > We avoided one due to the default CFLAGS of the upstream project protecting a buffer overflow issue :) (In reply to comment #4) > (In reply to comment #2) > > Why would we want to allow user's cflags here, we should trust upstream > > instead, it's already saved us from at least one security issue here :) > > > > I don't understand that argument. If there are issues with compiling when user > CFLAGS are used, we can try to handle that, but stripping march/mtune isn't > what gentoo should do! Where is the security point? And why should all the > warnings upstream wants, should be necessary for the user? nothing should be getting "stripped" out here, I don't see where that happens at all. Could you attach a simple patch to show your suggested changes to the ebuild, the one you currently have here does a number of other things as well, which are not relevant to the CFLAGS issue. > src_compile() {
- emake kerneloops || die "Compile deamon failed"
- emake kerneloops-applet || die "Compile applet failed"
+
+ append-flags -fstack-protector -D_FORTIFY_SOURCE=2 -fno-common
+
+ emake \
+ CFLAGS="${CFLAGS}" \
+ CC="$(tc-getCC)" \
+ kerneloops || die "Compile deamon failed"
+
+ emake \
+ CFLAGS="${CFLAGS}" \
+ CC="$(tc-getCC)" \
+ kerneloops-applet || die "Compile applet failed"
}
append-flags -fstack-protector -D_FORTIFY_SOURCE=2 -fno-common
adding the upstream suggested flags, which aren't optimization or warning flags, to the users *FLAGS. This would make, what you want to do by "trusting" upstreams decisions. Additionally you are right, that might rises security to have those flags, but this app only does log watching.
emake \
+ CFLAGS="${CFLAGS}" \
+ CC="$(tc-getCC)" \
Make the compiler use the user's choice of compiler and CFLAGS, which is original the "gentoo way". The only thing this would drop from the upstreams "wanted" are all the warning flags and second let the degree of optimization be a choice of the user.
In summary this would full fill the spirit of gentoo, by giving control over the way the compiler acts to the user, would still have the same security advantages like upstreams solution and stops showing alot noise, which no one needs.
If you still aren't happy, there are function like
strip-flags Removes all non-safe flags from CFLAGS and CXXFLAGS
strip-unsupported-flags Removes any flags in CFLAGS and CXXFLAGS
which are not supported by the active compiler
to give you control over the users CFLAGS.
This is from the devmanual http://devmanual.gentoo.org/general-concepts/user-environment/index.html#not-filtering-variables justin: the kerneloops ebuild is not violating the Gentoo policy. Upstream for kernel utils like this tend to provide for compiling on unusual arches -- user-specified CFLAGS are _not_ being dropped on the floor here.
I tested it on my own system; my optimization flags etc are being preserved.
Here is how it works:
The Makefile only appends upstream flags to the user's flags, e.g.
CFLAGS += -Dlinux \
-Wall \
-Wno-conversion \
-Waggregate-return \
-Wstrict-prototypes \
-Wmissing-prototypes \
-DINSTALL_PREFIX="\"$(INSTALL_PREFIX)\"" \
-DCROSS="\"$(CROSS)\"" \
$(DEBUG)
The default src_compile() will initialize CFLAGS as requested in make.conf, and then the Makefile will tack on these additional flags. So, you end up with both the system-wide flags and the ones needed for this package.
If you think I'm wrong about whether CFLAGS are being respected, please post excerpt from your build log and make.conf to demonstrate flags being killed...
Created attachment 166814 [details]
build.log for 0.12
Created attachment 166816 [details]
emerge.info
I think the Makefile redefines the CFLAGS:
CFLAGS := -O2 \
-g \
-fstack-protector \
-D_FORTIFY_SOURCE=2 \
-Wall \
-W \
-Wstrict-prototypes \
-Wundef \
-fno-common \
-Werror-implicit-function-declaration \
-Wdeclaration-after-statement
I am really no specialist for such things, but I think ":=" is an assoziation and "+=" like you wrote is an addition. So in your case it would be like you are saying, but I think in this case it is really a redefinition. If I am wrong, please show it to me, I am still learning.
Justin, I apologize for my totally mistaken comment. That should teach me to post stuff past my normal bedtime... was looking at totally the wrong Makefile and build log. Your excerpt is from the correct Makefile, so you can resume your conversation with Greg about how having upstream flags appended to user CFLAGS can be a good thing. Maybe you could propose a Makefile patch that uses '+=' instead of ':=' to set the important flags -- try for something that Greg could push upstream to make kerneloops more packager-friendly, rather than adding hacks to the ebuild (copying flags from the upstream Makefile into the ebuild classifies as a hack...) Hi Gregkh, as we now learned how to do it right, here the short nice makefile.patch. In a hand build it is working. Created attachment 166929 [details, diff]
Makefile.diff
|