Fedora Merge Review: crash http://cvs.fedora.redhat.com/viewcvs/devel/crash/ Initial Owner: anderson
Created attachment 328610 [details] revised spec according to fedora guidelines I've taken a look at the current version (http://people.redhat.com/anderson/crash-4.0-7.6.src.rpm) and did a local mock rebuild. I am attaching a slightly revised spec which fixes part of the problems. The major issues (I have not tried to fix any of these because I am not familiar with the requirements of this special package) are that 1. the package uses it's own versions of several utilities/libs (at least gdb and readline) which contradicts the guidelines. However, according to http://people.redhat.com/anderson/crash_whitepaper/ this is intended. 2.the package builds a static binary. Once again, this is intended 3.fedora compile flags are completely ignored. This also seems to be intended, each of the libraries is built with a different set of flags. Easily fixable stuff (not done): - SMP_FLAGS are not used (I did not know if it's OK to use them ) - the source files (included in -debuginfo) have the exec bit set, which makes rpmlint unhappy - license seems to be GPLv2+. A lot of files are GPL+, some are GPLv2+, some have no license at all. A cleanup of those would be nice Other problems (fixed) - usage of forbidden tags (packager, vendor) or incorrect values (buildroot, source0) - some aesthetic warnings from rpmlint - missing license file from the binary rpm - there was no changelog. I have added one but it should be properly populated - timestamps (for docs/man) were not preserved - defattr was missing in %files
(In reply to comment #1) > - license seems to be GPLv2+. A lot of files are GPL+, some are GPLv2+, some > have no license at all. A cleanup of those would be nice Certain files (xen_hyper*) use GPLv2 (only), spot already fixed this in CVS. > Other problems (fixed) There are yet more: - You use "Revision" tag to mark upstream release, which is wrong. It is meant to be used to version the SPEC file. Given you (package owner, "crash" group, seem to be upstream, you can definitely fix this by changing the versioning scheme. (e.g use 4.0.8 instead of 4.0-8)) - The bundled gdb is old and has issues. It is likely that some of older GDB security problems affect it: http://web.nvd.nist.gov/view/vuln/detail?vulnId=CVE-2005-1704 http://web.nvd.nist.gov/view/vuln/detail?vulnId=CVE-2005-1705 http://web.nvd.nist.gov/view/vuln/detail?vulnId=CVE-2006-4146 Please address those, if they are relevant. Notify your SRT that you bundle GDB code and communicate with GDB upstream (or people involved in Archer, your colleagues) to avoid having to bundle GDB in longer run.
I'm not sure why the review was made of the crash.spec file from the upstream package instead of the Fedora version? When updating the Fedora package, the upstream spec file is not pulled into Fedora. In any case, I'm presuming that the relevant issues brought up re: the upstream spec file should be addressed in the Fedora version if they haven't already. The Fedora crash utility NVR scheme has (up until now) simply mirrored the upstream version upon which it was based, but I can see the validity in encapsulating the upstream NVR into the Fedora version. It's been an annoyance in the past when somebody has come in (unbeknownst to me) and bumped up the release number, which in turn screwed up the upstream NVR relationship. With respect to the CVE's, both 1704 and 4146 are highly unlikely to be issues given that the crash utility does a number of checks on the vmlinux object file prior to the embedded gdb module ever being invoked. (i.e., using the supplied sample hand-carved object files, they get rejected by the crash code as not being legitimate vmlinux files) Nonetheless, the patches for both of those issues are reasonable and safe to add, and I have done so. I've also incorporated the patch for the .gdbinit-related 1705 issue. When I update the upstream version, I will follow up with a Fedora update -- after I figure out how to check into my account given that I haven't been there since the Fedora break-in. Thanks, Dave Anderson
(In reply to comment #3) I reviewed the upstream version because by definition so to say Fedora wants to have the latest version of the software, and the content in the cvs is very old. I assumed it will be easier for you to maintain a single or at least very similar spec for both places. I apologize if I made an error by this assumption. Let me know what is the URL of the files that I should have reviewed and I will do it. And yes, the relevant issues brought up re: the upstream spec file must be addressed in the Fedora version. And no, I did not touch the spec exactly because I knew that there is an active maintainer who might have a well defined agenda. As of relying on a bundled application, this is not usually permitted in Fedora and therefore they should be discussed.
I meant here "relying on a bundled version of an existing application"
> As of relying on a bundled application, this is not usually permitted in Fedora > and therefore they should be discussed. The crash utility package is essentially built around, on-top-of, wrapped-around, or however you want to put it, and it's designed that way on purpose. There's no way it's ever going to be "unbundled".
> I reviewed the upstream version because by definition so to say Fedora wants to > have the latest version of the software, and the content in the cvs is very > old. I assumed it will be easier for you to maintain a single or at least very > similar spec for both places. Not really. The upstream src.rpm (and its minimal .spec file) was added as a convenience to those users who prefer that format to the tar.gz file. > I apologize if I made an error by this assumption. Let me know what is > the URL of the files that I should have reviewed and I will do it. The "content in the cvs" (Fedora) is currently based upon the upstream 4.0-6.3 release from last April, and its .spec file updated when Tom Calloway rebuilt it as 4.0-7 this past August. Anyway, the .spec file in the devel branch was -- as I recall -- carried over from a RHEL package version some years ago. Anyway, that's the only .spec file I've ever touched w/respect to Fedora, and that's the one that I would have presumed would be the one that would be "review-able".
It would have been nice if a mention of the existence of a new version in the CVS + a link to the new spec and src.rpm would have been provided here. Back where I come from this is called courtesy. Current issues, as of version crash-4_0-8_7_2_fc11 1. $ rpmlint *rpm crash.src: E: summary-too-long Kernel analysis utility for live systems, netdump, diskdump, kdump, LKCD or mcore dumpfiles crash.x86_64: W: spurious-executable-perm /usr/share/doc/crash-4.0/COPYING crash.x86_64: E: summary-too-long Kernel analysis utility for live systems, netdump, diskdump, kdump, LKCD or mcore dumpfiles crash-devel.x86_64: W: no-documentation crash-devel.x86_64: W: summary-not-capitalized kernel crash analysis utility for live systems, netdump, diskdump, kdump, LKCD or mcore dumpfiles crash-devel.x86_64: E: summary-too-long kernel crash analysis utility for live systems, netdump, diskdump, kdump, LKCD or mcore dumpfiles 3 packages and 0 specfiles checked; 3 errors, 3 warnings. The no documentation for -devel is ignorable. The error messages are easy fixable by shortening the summary lines. 2. Source should be "http://people.redhat.com/anderson/crash-4.0-7.7.tar.gz" (full URL to the source should be provided, not just a filename) 3. ppc is excluded from the list of architectures. However, ppc64 is included (and koji builds happily for it) so I have to ask: is excluding ppc really intended or just an oversight? 4. The package uses its private copies of several programs (gdb, binutils, readline) without providing any reason for that. At least a note should be included in the spec, explaining the reason. And I think that an exception must be requested from the FPC. 5. The package ignores the compiler flags which are mandatory according to the guidelines. Once again, no reason is provided. On top of that, the flags are not consistently used (gdb gets " -c -DHAVE_CONFIG_H -g -O2 -I. -I./../include -W -Wall -Wtraditional -pedantic", readline gets " -c -DHAVE_CONFIG_H -I. -I. -DRL_LIBRARY_VERSION='"4.3"' -g -O2", while crash itself is built using "gcc -c -g -O2 -I. -I. -I./config -DLOCALEDIR="\"/usr/local/share/locale\"" -DCRASH_MERGE -DHAVE_CONFIG_H -I./../include/opcode -I./../readline/.. -I../bfd -I./../bfd -I./../include -I../intl -I./../intl -DMI_OUT=1 -DTUI=1 -Wimplicit -Wreturn-type -Wcomment -Wtrigraphs -Wformat -Wparentheses -Wpointer-arith -Wuninitialized -Wformat-nonliteral -Wunused-label -Wunused-function " 6. Parallel make (https://fedoraproject.org/wiki/Packaging/Guidelines#Parallel_make ) is not used. Once again, no reason provided.
I am releasing the review. Due to $JOB issues I no longer have the time and the willingness to watch over the shoulder of an unresponsive maintainer.
Mass reassigning all merge reviews to their component. For more details, see this FESCO ticket: https://fedorahosted.org/fesco/ticket/1269 If you don't know what merge reviews are about, please see: https://fedoraproject.org/wiki/Merge_Reviews How to handle this bug is left to the discretion of the package maintainer.
This bug appears to have been reported against 'rawhide' during the Fedora 23 development cycle. Changing version to '23'. (As we did not run this process for some time, it could affect also pre-Fedora 23 development cycle bugs. We are very sorry. It will help us with cleanup during Fedora 23 End Of Life. Thank you.) More information and reason for this action is here: https://fedoraproject.org/wiki/BugZappers/HouseKeeping/Fedora23
This message is a reminder that Fedora 23 is nearing its end of life. Approximately 4 (four) weeks from now Fedora will stop maintaining and issuing updates for Fedora 23. It is Fedora's policy to close all bug reports from releases that are no longer maintained. At that time this bug will be closed as EOL if it remains open with a Fedora 'version' of '23'. Package Maintainer: If you wish for this bug to remain open because you plan to fix it in a currently maintained version, simply change the 'version' to a later Fedora version. Thank you for reporting this issue and we are sorry that we were not able to fix it before Fedora 23 is end of life. If you would still like to see this bug fixed and are able to reproduce it against a later version of Fedora, you are encouraged change the 'version' to a later Fedora version prior this bug is closed as described in the policy above. Although we aim to fix as many bugs as possible during every release's lifetime, sometimes those efforts are overtaken by events. Often a more recent Fedora release includes newer upstream software that fixes bugs or makes them obsolete.
Fedora 23 changed to end-of-life (EOL) status on 2016-12-20. Fedora 23 is no longer maintained, which means that it will not receive any further security or bug fix updates. As a result we are closing this bug. If you can reproduce this bug against a currently maintained version of Fedora please feel free to reopen this bug against that version. If you are unable to reopen this bug, please file a new report against the current release. If you experience problems, please add a comment to this bug. Thank you for reporting this bug and we are sorry it could not be fixed.