Hacker News Reader

Comment by blinkingled | Hacker News Reader

The offending commit was authored by Christoph Hellwig and possibly reviewed by Al Viro both of whom combined are close to 100% of Linux filesystems and VFS knowledge. Point being with the level of complexity you're just going to live the fact that they'll always be bugs.

VFS/Page Cache/FS layers represent incredible complexity and cross dependencies - but the good news is code is very mature by now and should not see changes like this too often.

View on HN

And the reason for the commit was to have 'nicer code'. The code was working perfectly fine before someone decided it was not nice enough?

Your post sounds like it's a bad thing, but "nicer" code is easier to maintain, i.e. there will be fewer bugs (and fewer vulnerabilities). This bug is an exception of the rule - shit happens. But refactoring code to be "nicer" prevents more bugs than it causes. Two patches were involved in making this bug happen, and minus the bug, I value both of them (and their authors).

I might have sounded harsh but I think shit happens is not the way to look at this. Don't claim I'm a better developer, but I always try to shy away from making things look nicer.

Experience have thought me, deal with problems when it is a problem. Dealing with could be problems can be a deep, very deep rabbit hole.

The commit message gave me the feeling that we should have just trust the author.

https://github.com/torvalds/linux/commit/f6dd975583bd8ce0884...

There's no bug in that commit, the commit is correct, it only makes the bug exploitable. The buggy commit is older, it's https://github.com/torvalds/linux/commit/241699cd72a8489c944... but not exploitable.

> I always try to shy away from making things look nicer

That's understandable, though from my experience, lots of old bugs can be found while refactoring code, even at the (small) risk of introducing new bugs.

While true, it's important to ensure there is adequate test coverage before trying to refactor, in case you miss something.

Also, try to avoid small commits / changes; churn in code should be avoided, especially in kernel code. IIRC the Linux project and a lot of open source projects do not accept 'refactoring' pull requests, among other things for this exact reason.

Agree, but even 100% test coverage can't catch this kind of bug. I don't know of any systematic testing method which would be able to catch it. Maybe something like valgrind which detects accesses to uninitialized memory, but then you'd still have to execute very special code paths (which is "more" than 100% coverage).

Valgrind cannot be used for/in the kernel. However, the kernel has an almost-equivalent use-of-uninitialized-memory detector; https://www.kernel.org/doc/html/v4.14/dev-tools/kmemcheck.ht...

> try to avoid small commits / changes

Not sure what you mean by that

As (almost) always, the expert's answer is: "It depends". How risky is the change, how big the consequences, how un-nice is the code before, how easy is it to test that the code still works afterwards, etc...

FWIW, I tend to err on the side of "do it", and I usually do it. But I have been in a situation where a customer asked for the risk level, I answered to the best of my knowledge (quite low but it's hard to be 100% sure), and they declined the change. The consequences of a bug would have been pretty horrible, too. Hundreds of thousands of (things) shipped with buggy software that is somewhat cumbersome to update.

Caveat - I know this doesn't directly apply to the vulnerability at hand, but is a discussion of a tangential view.

> Experience have thought me, deal with problems when it is a problem.

Experience has taught me that disparate ways of doing the same thing tend to have bugs in one or more of the implementations. Then trying to figure out if a specific bug exists other places requires digging into those other places.

Make it work. Make it good. Make it faster (as necessary) is the way my long-lived code tends to evolve.

> I always try to shy away from making things look nicer

Anyone who doesn't, hasn't been burnt enough so far, but will be burnt in the future.

Nonsense. It's just easy to blame refactoring when it breaks something. "You fool! Why did you change things? It was perfectly fine before.". Much harder to say "Why has this bug been here for 10 years? Why did nobody refactor the code?" even when it would have helped.

Not refactoring code also sacrifices long term issues in return for short term risk reduction. Look at all of the government systems stuck on COBOL. I guarantee there was someone in the 90s offering to rewrite it in Java, and someone else saying "no it's too risky!". Then your ancient system crashes in 2022 and nobody knows how it works let alone how to fix it.

A lot of times, this is just shifting the problem to the future and making life harder.

We have a team like this -- their processes often failing, and their error reporting is lacking in important details. But they are not willing to improve reporting / make errors nicer (=with relevant details), instead they have to manually dig into the logs to see what happens. They waste a lot of time because they "shy away from making things look nicer."

"There are two ways of constructing a software design: One way is to make it so simple that there are obviously no deficiencies and the other way is to make it so complicated that there are no obvious deficiencies."

        — C.A.R. Hoare, The 1980 ACM Turing Award Lecture

Unless the code is very well covered by unit tests, any refactoring can introduce bugs. If the code is well established and no longer changing, there is no ease of maintenance to be gained. There is only downside to changing it.

If the code is causing more work to maintenance and new development, sure it may make sense to refactor it. Otherwise, like the human appendix, just leave it alone until it causes a problem.

Very good point. Often developers talk in cargo-cult terminology like "beautiful" or "nice" or "elegant" code, but there is no definition of what that even means or whether it empirically leads to better (or worse) outcomes. We know people like it more, but that doesn't mean we should be doing it. A true science would provide hypothesis, experiment, repeated evidence, rather than anecdotes.

(from the downvotes it seems like some people don't want software to be a science)

Easy: simpler is better. What is considered simple wildly varies based on ones experiences though.

While scientific approach would be nice, it is hard to do, and even harder to do correctly in the way applicable to the specific situation. And in the absence of the research, all we have is intuition and anecdotes.

And they work both ways -- there are anecdotes that making code beautiful leads to better outcomes, and there are anecdotes that having ugly code leads to better outcomes.

This means you cannot use lack of scientific research to give weight to your personal opinions. After all, that argument works in either direction ("There is no evidence that leaving duplicate code in the tree leaves to worse outcomes... A true science would provide...")

I've the impression that most maintainers and project founders care about the project and the source. Contrary to what in industry happens often, where other things are more important {sales, features, marketing, blingbling}.

One of the prevailing features of well driven open-sources project is - you're encouraged to improve the code i.e. make it better {readable, maintainable, faster, hard}. You're not encouraged to change it for the sake of change i.e impress people.

I've the feeling it is the first case because it reduced the number of lines and kept source readable. Aside from that, I don't think good developers want to impress others.

My reading of the write up was that the new code didn’t introduce the bug, but merely exposed a latent uninitialised memory bug?

if it ain't broken, fix it til it is

> Point being with the level of complexity you're just going to live the fact that they'll always be bugs.

I'd like to add, for the less tenured developers around: "with the level of experience you're just going to live the fact that there'll always be bugs."

Do you have an example in mind, for complex software that is bug free?

It’s possible that TeX is some of the most bug free code around and bugs are still being found (very edge cases, to be sure).

This will be worst over time until "more planned obsolescence than anything else" code is committed into the linux kernel. Many parts of the linux kernel are "done", but you will have always some ppl which will manage to commit stuff in order to force people to upgrade. This is very accute with "backdoor injectors for current and futur CPUs", aka compilers: you should be able to compile git linux git with gcc 4.7.4 (the last C gcc which has beyond than enough extensions to write a kernel), and if someting needs to be done in linux code closely related to compiler support, it should be _removing_ stuff without breaking such compiler support, _NOT_ adding stuff which makes linux code compile only with a very recent gcc/clang. For instance, in the network stack, tons of switch/case and initializer statements don't use constant expressions. Fixing this in the network stack was refused, I tried. Lately, you can see some linux devs pouring code using the toxic "_Generic" c11 keyword, instead of using type explicit code, or new _mandadory_ builtins did pop up (I did detect them is 5.16 while upgrading from 5.13) which are available only in recent gcc/clang. When you look at the pertinence of those changes, those are more "planned obsolescence 101" than anything else. It is really disappointing.

>you should be able to compile git linux git with gcc 4.7.4 (the last C gcc which has beyond than enough extensions to write a kernel)

By this logic why not write the entire kernel in assembly? Tools evolve and improve over time and it makes sense to migrate to better tools over time. We shouldn't have to live in the past because you refuse to update your compiler.

That's obviously not their logic at all. Trying to diminish this to "OP refuses to update compiler" is frankly disrespectful of them & their actual point.

Their claim is "you should be able to compile git linux git with gcc 4.7.4" which is a completely arbitrary requirement.

It's not completely arbitrary. Notice that they said "the last C GCC". After that version, GCC started using C++ in the compiler itself. I can see why some people would see that as a complexity line that must not be crossed, as it makes bootstrapping harder.

What GCC is written in only matters if you intent to write your own compiler to compile it - which as you have no compiler yet would likely have to be written in assembly.

Otherwise you need to download a prebuilt compiler anyway, and whether that is C11 or C++11 is rather unimportant.

To me their logic is that their old tool works just fine so they shouldn't have to upgrade it. He essentially said that having a plan to upgrade to a newer version of the language or to a more up to date toolchain is planned obsolescence. He seems to want to be able to use his specific version of his compiler to the end of time. To me I don't quite get the justifications of this perspective as GCC is free software and it is simple to upgrade.

Thank you, that's a great reply to his comment. My first impression of his comment was that the kernel project shouldn't chase the latest-and-best compiler releases -- or similarly the most recent C language changes; rather, a boring-technology approach is sensible for such a foundational project as Linux. I see your point, though, that GCC is simple to upgrade. (If I were making the tech decision here, I'd want to ensure that newer GCC's didn't introduce features that I thought were too risky for my project, or at least that I could disable/restrict those features with flags.)

GCC 5.1 (released in 2015) is hardly latest-and-best, though: moving the version bar up only very slowly and with an eye to what distros are using as their compiler version is a pretty solid boring-technology approach, in my view.

This kind of argument is hypocritical: You want to use newer versions of the Linux kernel yourself (otherwise you could just stick to whatever builds with your toolchain!), but say that the Linux kernel must not use newer versions of things.

The GCC version requirement is 5.1 (which is 7 years old). Before that, it was 4.9, 4.8, 4.6 and 3.2. It has never been 4.7.

Use of newer versions of C than C89 which provides solutions to actual issues is perfectly fine. C11 was picked because it does not require an increase in minimum GCC version to use it, making your entire argument pointless.

The Linux kernel is already pretty lenient, as many alternatives have their a compiler on the tree and target only that.

Reply on news.ycombinator.com