Michael Tokarev <mjt@tls.msk.ru> (08/04/2013): > I'm pinging this bug, as we're getting seriously out of time. Well, if you had Cc'd the right people in the first place… > > Let me start from scratch please. I wasn't aware of this > > bugreport/discussion, and I made a mistake by not filing a proper > > unblock request when I uploaded the busybox package with all the > > fixes. So here are descriptions of every change. I'm not sure a whole essay was needed… > > So, any bug in busybox which does not affect basic initramfs or > > d-i functionality directly can't be "important enough" for whole > > debian system, so to say. Indeed. > > On the other hand, sometimes, since busybox is almost always > > installed and available on any debian system, it gets used by > > users in various much less restricted environments, which leads to > > situations where the bugs are actually gets hit. This is minority > > of users (again, so to say). > > > > So this is about whenever we really care about this minority > > or not, _plus_ about _possible_ issues in d-i or initramfs. Gotcha! If it isn't triggering a bug in d-i which landed on our radar, better not touch busybox. > > As has been said before, this is a "feature fix". It is not. The > > problem initially has been raized in some other package which had > > initramfs hook which didn't work. I don't remember which package > > it was, the context was that it tried to find some CPU feature by > > grepping /proc/cpuinfo with -w flag to grep, used to remove false > > positives, and it didn't work. (/proc/cpuinfo is just an example, > > I don't really remember what it was). This is one of these bugs > > which makes other, unrelated components fails in unexpected and > > difficult to debug ways. If you can't find which component it was, surely that isn't as important as you pretend it to be. Worst case, that component can be adjusted. Especially when logic gets rewritten, and when the fix is “somewhat large”, and when test cases aren't run. > > The fix is somewhat large, because it had to deal with logic of > > operations in grep applet. On the plus side, it comes with > > additional testcases which checks for this issue, and there are > > lots of other grep-related testcases already present. > > Unfortunatly busybox debian package still does not run any > > testcases during build (this is on my TODO list of first things to > > do for jessie), but having in mind the situation (deep freeze) and > > importance of the applet I ran provided and a few more testcases > > against the resulting grep on x86, ppc and mips platforms (on > > debian porterboxes) manually to ensure the fix does not break > > anything else and actually fixes the bug. I can't wait to see tests enabled. :) > > If you ask me, this is about the same importance as #686502, but > > it is less obvious. This grep-w bug does not lead to data loss > > directly, the problem is that we can't rely on grep doing the > > right thing when we use it in a familiar, natural way - like when > > we try to fix a false positive by adding -w _somewhere else_ (in > > some other package that is), and it stops working. > > > > That's why it isn't a "feature fix", busybox claimed to support > > grep-w but it does not work. That's exactly what I'd call a “feature fix”. > > What we're fixing here mostly is about _further_ d-i or initramfs > > changes (including possibility to have these changes in wheezy > > too) which looks completely correct and obvious but don't work in > > practice. Plus some random rare end-users usage of busybox grep, > > of these are exists. What we risk here is almost nothing, at > > least according to my tests on several platforms. Possible, possibility, random, rare. → Not enough to fix at this point. > > * xz-support-concatenated-xz-streams.patch (Closes: #686502) > > > > This is the "main" RC bug currently filed against busybox. The > > rationale for it being RC is because it causes _silent_ data loss > > when decompressing certain kinds of XZ streams. > > > > Again, the severuty can be argued for sure, due to whole nature of > > busybox as a "second-kind sitizen" as mentioned at the beginning > > of this mail. First, not everyone is using busybox unxz (which is > > used in busybox tar and other archivers too), second, concatenaed > > xz streams aren't frequent either (but this becomes more and more > > of a problem, because such streams are produced by parallel xz, > > and this already seen "in the wild" - in particular, recent kernel > > sources on kernel.org are of this kind). > > > > We're unlikely to met all the conditions for this bug in d-i or > > initramfs during wheezy lifetime, -- _provided_ that some future > > _wheeze_ update will not contain such concatenated xz streams > > produced by, say, an improved version of dpkg (which utilizes > > multiple cores for compression), -- but this is an unlikely > > situation. Agreed, and unlikely means I'm not going to approve of such a chance at this point. > > What we _also_ fixing here is a possible silent data loss for > > end-users who may use buzybox to uncompress their xz files > > somewhere (note this may include rescue CDs based on d-i too). Such end-users can get a fixed busybox if they need it. Debian doesn't, for wheezy, AFAICT. > > The fix is also somewhat large but it is stright-forward and easy > > to read/understand. I ran a few tests on it too to ensure it > > works as expected, -- just a few, not a feeding it a corpus of > > various test xz streams with various corner cases, which I don't > > have. But it appears to work. > > > > Again, if you ask me, I'd say it about of the same importance > > as grep-w fix, but it is much more about end-user expirience > > than about internal debian (initramfs and d-i). Same comment as above. > > * lineedit-initialize-delptr.patch - fix segfault in line editing > > facility (Closes: #701959) > > > > Purely end-user fix - for users who actually use busybox shell for > > something, e.g. rescue CD. I've hit it myself at least once when > > evaluated shell vi line-editing mode. A fun mode I'd say, and it > > isn't an important fix really, but it is so stright- forward and > > obvious and does not affect anything else... So I thought I'd > > include it anyway, as it does no harm at all. Not a serious bug → not considering. > > * mdev-fix-mode-of-dir1-in-=dir1-dir2-file-rule.patch - make intermediate > > dirs in /dev to be of mode 0755 not 0777 (Closes: #701965) > > > > This one got a CVE# assigned to it, but mdev applet is not used in > > Debian at all, only if a user does that manually (ie, without any > > debian-provided package). FWIW, mdev is a trivial "replacement" > > for udev. > > > > It is one of "oh noes this has a CVE# assigned so let's fix it > > right away" bugs as KiBi pointed out, exactly, but this is > > definitely not an RC bug, and I never said it is one. It is more > > about "since this is a very specific applet which does not affect > > anything else and there may be some (strange) users who are > > affected, let's just fix it so we wont have these security tags > > anymore". For one, it actually hits a few of my systems, where I > > actually use mdev instead of udev, but I'm not sure if there's > > anyone who also uses it like me ;) Again, not a serious issue, or even an important one from a security point of view → shrug. > > At any rate, mdev is not included into d-i version of busybox > > (.udeb), so this change does not affect d-i at all, and initramfs > > does not use this applet. So there's very low risk assotiated > > with this change. > > > > > > * fix unaligned access macros (Closes: #701968) > > - fix-move_to_unaligned16.patch > > - xz-fix-put_unaligned_e32.patch > > > > This one is a potential bug, I don't know how probably to hit it > > in practice. It may only happen on some platforms (where > > unaligned access generates a trap) and only in some corner cases > > (in xz case when we're at the end of a buffer which, in turn is at > > the very end of our address space, -- so we access the next byte > > right after our address space. I can only guess that if we _ever_ > > hit it, it'll be difficult to debug and even reproduce, but it > > hopefully should be visible at least (not silent as in #686502). > > > > On the other hand we studied all users of the functions/macros > > we're fixing (that's actually how the second bug was found), and > > ensured the users behaves correctly. Again a potential bug. We can't produce or make sure it disappears with the patches. Not taking a chance. > > Overall, the fixes are ordered the way how I think their > > importance to be in wheezy is (yes with grep-w being more > > important than xz truncation). But as I said in the beginning, > > busybox isn't a first-class sitizen and _all_ this can easily be > > just ignored for wheezy, since all this may only impact very few > > users who uses their systems in "non-standard" way, or has (low) > > potential to break _further_ changes to other packages (tpu), > > again only for wheezy. > > > > I especially picked only really important and _safe_ changes and > > carefully verified the result works in a sane way. My take is: until we hit real bugs in real situations, we keep busybox as it is. If release managers want to cherry-pick a few fixes, I won't stop them from requesting so. But as far as I'm concerned, I'd really like to see practical issues before getting busybox updated. > > And finally, I understand that busybox is a "second-class" package > > (as I already mentioned several times), and I didn't expect it > > will cause so much time-consuming discussion, -- it should be > > quieter than most other packages. Unfortunately this is not the > > case, and I really hope to fix this somehow ;) For reference, since you use it frequently, the proper wording is: “second-class citizen”. Mraw, KiBi.
Attachment:
signature.asc
Description: Digital signature