Besides, it's not as if Linux local privilege escalations are incredibly rare, and it would be surprising if anyone with enough money, time and expertise couldn't uncover them at a rate sufficient to make this kind of skulduggery unnecessary.

The flow was always BitKeeper => CVS, not CVS => BitKeeper.

The only way this change would have made it into BK is if one of the users of CVS tree modified the same area of code and included that change in a patch sent to Linus or one of the other BK users. The chances of that happening and not being caught in review were, in my opinion, zero. Ziltch. Just not possible, any reasonable programmer would have seen this.

To this day I believe this was a script kiddy who broke in and wanted to see if he could stick a trojan in there and get root on machines that installed this code. Seems dicey at best because while some people used the CVS stuff at the time, all the releases were done from BK at that time so that code would have never hit mainstream distros.

Someone who breaks into computers by merely executing the scripts and hacking tools written by others, without any coding on their own, and without any deep knowledge about that attack.

See also: https://en.wikipedia.org/wiki/Script_kiddie

In spite of all the shit I've taken for the choices I have made, I'd do it all over again to help Linus. That guy is unique. I know you guys like him but I think few people really realize how unique he is. He's a big deal, I am not.

On the face of it, it appears that the parens are to ensure correct operator precedence because a bitwise OR is used (a common idiom because bitwise operators have the wrong precedence versus comparison in C).

But that's not what the parens are really for! They are there because gcc complains about assignments inside of conditionals unless you put parens around it.

  if ((options == (__WCLONE|__WALL)) && (current->uid = 0))
        retval = -EINVAL;

Wait... Wait... A casual reading by any expert should result in the question "why is this code setting current->uid to 0?"

After all, in case said expert has forgotten the difference between = and ==, this little bit of code all by itself has two correct reminders about what those operators actually do.

http://www.emacswiki.org/emacs/CWarnMode

http://en.wikipedia.org/wiki/MicroEMACS

http://web.archive.org/web/20061124122032/http://www.stifflo...

    It would be spotted right away by anyone
    remotely competent who glanced over it.

Another test it fails: even if it were == as they are trying to make people think, why would this return EINVAL only if the user is root? There aren't a lot of real world examples where you would want that. It pops out right away as a bogus check that makes no sense, and surprise surprise, it also sets the uid to 0, my guess is in a place where it makes no sense to even look at uid, and very superficially hiding as a "rookie mistake"...

It's easy to thump your chest and assume that you're smarter than those developers (which is almost certainly wrong) but that misses the underlying lesson that humans will make mistakes and the likelihood goes up with the complexity of the task. That can be switching to a language which bans problematic behaviour, automatic use of linters and other tools, rigorous code reviews, etc. but in each case you're making environment safer rather than trying to find perfect developers.

See also my reply to cbr where I make essentially your same point: the way to get this past a code review is make it part of a large diff.

That said, I do disagree that this is easily detected – I would bet that a significant percentage of C developers would not notice this if it wasn't mentioned in the context of a security problem. Linux kernel developers are [hopefully] well above average but … there's a reason why so many style guidelines feature this one prominently and it's not because few people have made this mistake.

The author of the code if malicious must have realised that it would not pass any kind of code review.

PS: I tend to discount 'tone' when reading blog pages as getting the 'tone' right is the kind of thing professional authors do.

In any case, this is something a competent lint tool or static code analyzer would warn about, which makes a good case for people using such tools as part of their normal process.

Ctrl-F the above phrase in

http://www.mit.edu/hacker/part1.html

Security concerns always seem to involve low level code in device drivers or firmware. Indeed, one character in the wrong place and we have problems

Something like that:

  do {
    switch (bleh) {
      case MEH:
        if (derp) {
          break; /* supposed exit the do..while loop early...
                    except it only exits the switch statement */
        }
        [...]
        break; /* usually there's a break at the end of each
                  case statement, but the programmer might
                  not have seen it if it's far away */
    [...]
    }
    fun_stuff(); /* not supposed to be called when
                    bleh == MEH and derp is non-null */
  } while (quux);

Well, you have compilers that displays you warnings about it. If you don't catch the error you have static analyzers, if not you have Valgrind and other memory tools(linux have some jewels here), and if not you have your own parsing code for automatic checking of anything suspicious like becoming superuser.

So is not that disturbing as this example is something that even gcc with Waring=all catch. LLVM is better in this respect.

I'm not an NSA expert though, I declined to get clearance, so salt my opinion a bit.

Taking advantage of assignment-as-an-expression is idiomatic in a fair amount of code I've seen, at least.

Stuff like:

    /* check_something returns non-zero on error */
    if (need_to_check && (status = check_something())) {
        handle_error(status);
    }

Definitely. I personally find that style bad because it's concealed a far number of bugs in the past (either things like the =/== confusion or simply logic errors when the line became long enough that scanning it is non-trivial) but the only way that could fly for a compiler would be as a flag or pragma allowing you to opt-in for valid-but-discouraged style warnings and that's served well enough by existing linters that standardization isn't worth the cost of moving it into a compiler.

Many people say, calm down, its git they cant have inserted backdoors etc without messing up the git history/changelog/hashes/whatever. But what if, git was modified and backdoored previously to hide some objects/changes? How would such an attack work? Lets say you discover a problem in git, which allows you to omit changesets in its output. How would that work to backdoor the kernel?

EDIT: Plus, git is stored in git, so you'd need to backdoor git first... ;)

[1] - https://github.com/git/git/commit/e83c5163316f89bfbde7d9ab23...

BitKeeper wasnt as good as git when it comes to changesets and introducing bugs, still it was founded on more social side of changes needing approvals. Before that it was CVS and for that its almost easy to introduce backdoors.

Until the redesign kernel.org stated: "We will be writing up a report on the incident in the future.".

But since the compromise was more than 2 years ago it seems unlikely.