Re: Bug#702278: busybox upload
I'm pinging this bug, as we're getting seriously out of time.
Now, I guess, we should either let the whole thing go (it has
already been unblocked for, only d-i block remains), or mark
the relevant bugs as non-RC for wheezy.
Thanks,
/mjt
25.03.2013 15:38, Michael Tokarev wrote:
> 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.
>
> A sort of a fore-word first. Busybox is kind of unusual package
> in a way that it re-implements functionality of various existing
> packages. Base debian system uses other implementations of the
> same functionality usually. So from this PoV, any busybox bug
> is not "that" important for a user's debian system, since a
> typical user does not use any busybox applets at all.
>
> Busybox _is_ used on almost every end-user system because it
> provides the set of basic *nix utilities for initramfs, and
> it is used in debian-installer. But these are very restricted
> environments with much better defined components and much less
> chance to have a combination which triggers one or another bug.
>
> 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.
>
> 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.
>
> Another source of "unusuality" comes from the fact that busybox
> implements a ton of _independent_ applets, so that a change in
> one does not affect anything else (if we don't count changes in,
> say, memory layout which triggers bugs elsewhere - this has already
> happened at least once during wheezy development cycle, when we
> discovered bug with unaligned memory access which was hidden
> because the objects were actually aligned properly before we
> changed something unrelated in memory).
>
> So, back to the changes.
>
> busybox (1:1.20.0-8) unstable; urgency=low
>
> * grep-fix-grep--Fw-not-respecting-the--w-option.patch - implement
> fgrep -w correctly (Closes: #695862)
>
> 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.
>
> 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.
>
> 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.
>
> 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.
>
>
> * 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.
>
> 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).
>
> 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).
>
>
> * 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.
>
>
> * 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 ;)
>
> 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.
>
>
> 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.
>
>
> Now, we have several options to deal with the situation (in no
> particular order):
>
> o accept whole thing to wheezy (this is what I was aiming for,
> but failed to write the above description properly in time).
>
> o ignore whole thing and keep current wheezy version 1.20.0-7
> in wheezy (and mark #695862 - xz truncation - as non-RC).
> This is the simplest solution.
>
> o make another upload with only some changes (from the whole
> list of 4). If we do that, I propose to include at least
> grep-w and xz-truncation fixes.
>
> If the question is only about d-i, all fixes appears to be at
> least harmless for it, and at least grep-w is important. But
> this is IMHO, just like the rest of this email.
>
>
> 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 ;)
>
> Thanks,
>
> /mjt
>
Reply to: