4 ms·

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 kno

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.

dncornholio 5y ago
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?
- max_k 5y ago
  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).
  - dncornholio 5y ago
    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/f6dd975583bd8ce088400648fd9819e4691c8958 https://github.com/torvalds/linux/commit/f6dd975583bd8ce0884...
    - max_k 5y ago
      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/241699cd72a8489c9446ae3910ddd243e9b9061b 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.
      - Cthulhu_ 5y ago
        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.
        max_k 5y ago
        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).
        aaronmdjones 5y ago
        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.html https://www.kernel.org/doc/html/v4.14/dev-tools/kmemcheck.ht...
        cwilkes 5y ago
        > try to avoid small commits / changes Not sure what you mean by that
      - ahartmetz 5y ago
    - Supermancho 5y ago
      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.
    - Traubenfuchs 5y ago
      > 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.
      - IshKebab 5y ago
        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.
    - theamk 5y ago
      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."
  - Cthulhu_ 5y ago
    "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
  - throwawayboise 5y ago
    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.
- 0xbadcafebee 5y ago
  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)
  - vanviegen 5y ago
    Easy: simpler is better. What is considered simple wildly varies based on ones experiences though.
  - theamk 5y ago
    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...")
- ho_schi 5y ago
  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.
- olliej 5y ago
  My reading of the write up was that the new code didn’t introduce the bug, but merely exposed a latent uninitialised memory bug?
- pengaru 5y ago
  if it ain't broken, fix it til it is
xcambar 5y ago
> 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."
- nomel 5y ago
  Do you have an example in mind, for complex software that is bug free?
  - bombcar 5y ago
    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).
sylware 5y ago
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.
- charcircuit 5y ago
  >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.
  - gmfawcett 5y ago
    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.
    - pjc50 5y ago
      Their claim is "you should be able to compile git linux git with gcc 4.7.4" which is a completely arbitrary requirement.

dncornholio 5y ago

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?

max_k 5y ago
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).
- dncornholio 5y ago
  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/f6dd975583bd8ce088400648fd9819e4691c8958 https://github.com/torvalds/linux/commit/f6dd975583bd8ce0884...
  - max_k 5y ago
    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/241699cd72a8489c9446ae3910ddd243e9b9061b 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.
    - Cthulhu_ 5y ago
      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.
      - max_k 5y ago
        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).
        aaronmdjones 5y ago
        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.html https://www.kernel.org/doc/html/v4.14/dev-tools/kmemcheck.ht...
      - cwilkes 5y ago
        > try to avoid small commits / changes Not sure what you mean by that
    - ahartmetz 5y ago
  - Supermancho 5y ago
    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.
  - Traubenfuchs 5y ago
    > 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.
    - IshKebab 5y ago
      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.
  - theamk 5y ago
    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."
- Cthulhu_ 5y ago
  "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
- throwawayboise 5y ago
  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.
0xbadcafebee 5y ago
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)
- vanviegen 5y ago
  Easy: simpler is better. What is considered simple wildly varies based on ones experiences though.
- theamk 5y ago
  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...")
ho_schi 5y ago
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.
olliej 5y ago
My reading of the write up was that the new code didn’t introduce the bug, but merely exposed a latent uninitialised memory bug?
pengaru 5y ago
if it ain't broken, fix it til it is

xcambar 5y ago

> 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."

nomel 5y ago
Do you have an example in mind, for complex software that is bug free?
- bombcar 5y ago
  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).

sylware 5y ago

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.

charcircuit 5y ago
>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.
- gmfawcett 5y ago
  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.
  - pjc50 5y ago
    Their claim is "you should be able to compile git linux git with gcc 4.7.4" which is a completely arbitrary requirement.