5 ms·

Unfortunately, having this kind of function doesn't fix the real problem, which is those certain kinds of programmers who won't let a single machine cycle be wa

by dezgeg 12y ago

Unfortunately, having this kind of function doesn't fix the real problem, which is those certain kinds of programmers who won't let a single machine cycle be wasted. Here's just a completely random example from the Linux kernel: https://lkml.org/lkml/2014/8/19/194 https://lkml.org/lkml/2014/8/19/194 > You are implying that such an overflow could arise in this code. > This is false. [...] > bNumEndpoints is an unsigned 8-bit integer. So instead of changing the code code to be obviously correct (wrt. multiplication overflow), we'll instead keep the old code whose correctness becomes clear only after the programmer has looked up the types of variables involved. This mindset is exactly why vulnerabilities in C code are still common. It really needs to stop.

btown 12y ago
Won't the wrapper be inlined and the condition branch-predicted? In that case, you literally aren't letting a single machine cycle be wasted.
- gizmo686 12y ago
  Well, you might waste a cycle on the first run (or any-run after the branch has been flushed from the branch prediction table), as well as wasting cycles elsewhere from clobbering the table. One solution to both of these problems I have thought about, but do not work at a low enough level to know if it is already a thing, is an opcode for unlikely branches. Sementiclly, this would behave like a normal jmp instruction, but the CPU branch predictor would always assume that the jump is not taken, and not bother keeping track of it. In this particular case, it would also involve a way of signalling to the compiler (or the compiler inferring), but for loops it could be usefull. Of course, in this case it is also possible that the compiler can infer that the branch can never be taken and remove it, so you really do not loose anything.
  - dezgeg 12y ago
    > One solution to both of these problems I have thought about, but do not work at a low enough level to know if it is already a thing, is an opcode for unlikely branches. x86 actually does have jump instructions with branch prediction hints, but surprisingly modern CPUs will completely ignore those hints. Perhaps they never really caused large enough performance improvements to be worthwhile, or something. > In this particular case, it would also involve a way of signalling to the compiler (or the compiler inferring), [...] This functionality does exist, and Linux does make use of them. See the macros likely() and unlikely() at http://lxr.free-electrons.com/source/include/linux/compiler.h#L131 http://lxr.free-electrons.com/source/include/linux/compiler....
  - bri3d 12y ago
    This solution was often part of http://en.wikipedia.org/wiki/Very_long_instruction_word http://en.wikipedia.org/wiki/Very_long_instruction_word architectures. Itanium had the exact instructions you propose - look up ".dpxx" and ".spxx" for more.
  - deleted 12y ago
    [deleted]
smutticus 12y ago
One of the main criticisms I hear about OpenBSD is that it's performance isn't as good as Linux. I've looked at a couple articles quoting benchmark comparisons, so I'll assume for the sake of argument that it's true. The thing is that I don't care. I'm still an OpenBSD advocate and run it whenever I get the chance. I don't actually care about performance at that level. What matters more to me is consistency, and good man pages. I imagine more people are like me, but the thing about performance measurements is they're quantifiable. They can be easily measured. We all probably over optimize to some degree, whether we're C developers or not. There's just something satisfying about getting a quantifiably better score. This is why we get code that sacrifices almost everything for runtime speed.
- talideon 12y ago
  The slowness isn't a generally impacted much by stuff like this. It's more things like OpenBSD's poor multicore performance that are the issue. Just compare the performance of FreeBSD's multithreaded PF to OpenBSD's singlethreaded PF. Still, none of this matters, because there's one major guarantee that the OpenBSD devs try to fulfil: correctness.
  - imanaccount247 12y ago
    The thing is, the "slowness" in question is different every time someone wants to talk about it. Because people don't benchmark it, don't actually know what is slower and by how much, and just go on the "common knowledge" that openbsd is slow. The openbsd is slow meme was going strong in the 1990s, when it didn't support SMP at all and 99% of the people talking about it were using a single CPU.
    - wahern 12y ago
      It's different because the slowness is across the board. FreeBSD and, especially, Linux have optimizations all over the place. A call to time(2) or gettimeofday(2) is hundreds of times faster in Linux than on OpenBSD. In most scenarios the difference doesn't matter. But it does matter in, e.g., an asynchronous network service with lots of events with timeouts being constantly registered. I prefer OpenBSD, and I like that they focus on correctness, bugs, and more high-level features. I run all my personal services on OpenBSD. But commercially and at scale I run Linux, and treat each box as a throw-away in terms of security.
acveilleux 12y ago
My favorite thing with the OpenBSD folks isn't that they add APIs that avoid classes of security bugs but rather that they then ruthlessly update their code base to rely on the safer version.
to3m 12y ago
I suspect nobody playing with the USB code would have to look up the type of bNumEndpoints. The `b' prefix tells you it's a byte (this is a common thing in the USB spec), and you'll probably know it's a byte anyway due to familiarity... Another way of looking at the matter is that you shouldn't change code that's already correct.
- dezgeg 12y ago
  > I suspect nobody playing with the USB code would have to look up the type of bNumEndpoints. I could very well imagine a security auditor (who is not an USB expert) reading this code. > Another way of looking at the matter is that you shouldn't change code that's already correct. I personally find this kind of attitude extremely worrying. Perceptions about what is bad/good code or secure/insecure code have definitely changed since the first days of Unix. We shouldn't let code slip through the cracks just because it's old.
kazinator 12y ago
Overflows can crop up all over the place in C: you have to instrument every single +, - and * operator instance with overflow checks, each one of which needs a sane recovery strategy. Some overflows indicate internal errors that should panic the kernel. Others are configuration or resource issues that can bubble up some errno value to user space. Some can have a fallback strategy: this calculation overflowed, try another one. Some overflows can be handled by "saturation" (a.k.a clamping): common in digital signal processing and supported natively in some DSP machine languages. In other words, a silly patch that fixes a non-problem in one tiny place in a huge codebase doesn't accomplish anything. It's a big, wide-reaching task.
- dezgeg 12y ago
  > In other words, a silly patch that fixes a non-problem in one tiny place in a huge codebase doesn't accomplish anything. I don't understand this line of thinking at all. "reallocarray()/kmalloc_array() doesn't fix all the possible integer overflow issues in C, so why bother using it at all." Several people in this discussion have mentioned exact vulnerabilities that would have been prevented by reallocarray(): meowface: "Here is one of many examples of an integer overflow resulting in an improper size being passed to a memory allocation function further leading to remote code execution: https://docs.google.com/document/d/1tHElG04AJR5OR2Ex-m_Jsmc8S5fAbRB3s4RmTG_PFnw/edit https://docs.google.com/document/d/1tHElG04AJR5OR2Ex-m_Jsmc8... " tedunangst: "You've never heard of the one openssh exploit?"
  - kazinator 12y ago
    The discussion has changed; it is about the pointless patch to some USB code where an 8 bit endpoint count is multiplied by the size of some structure.
  - kazinator 12y ago
    I took realloc_array and applied it to a program I'm working on which contains a bunch of realloc calls. It was completely useless; I had to invent a different interface. It was useless because several calculations take place in every instance to produce the size, whereas realloc_array only checks one multiplication.

by dezgeg 12y ago

btown 12y ago

Won't the wrapper be inlined and the condition branch-predicted? In that case, you literally aren't letting a single machine cycle be wasted.

gizmo686 12y ago
Well, you might waste a cycle on the first run (or any-run after the branch has been flushed from the branch prediction table), as well as wasting cycles elsewhere from clobbering the table. One solution to both of these problems I have thought about, but do not work at a low enough level to know if it is already a thing, is an opcode for unlikely branches. Sementiclly, this would behave like a normal jmp instruction, but the CPU branch predictor would always assume that the jump is not taken, and not bother keeping track of it. In this particular case, it would also involve a way of signalling to the compiler (or the compiler inferring), but for loops it could be usefull. Of course, in this case it is also possible that the compiler can infer that the branch can never be taken and remove it, so you really do not loose anything.
- dezgeg 12y ago
  > One solution to both of these problems I have thought about, but do not work at a low enough level to know if it is already a thing, is an opcode for unlikely branches. x86 actually does have jump instructions with branch prediction hints, but surprisingly modern CPUs will completely ignore those hints. Perhaps they never really caused large enough performance improvements to be worthwhile, or something. > In this particular case, it would also involve a way of signalling to the compiler (or the compiler inferring), [...] This functionality does exist, and Linux does make use of them. See the macros likely() and unlikely() at http://lxr.free-electrons.com/source/include/linux/compiler.h#L131 http://lxr.free-electrons.com/source/include/linux/compiler....
- bri3d 12y ago
  This solution was often part of http://en.wikipedia.org/wiki/Very_long_instruction_word http://en.wikipedia.org/wiki/Very_long_instruction_word architectures. Itanium had the exact instructions you propose - look up ".dpxx" and ".spxx" for more.
- deleted 12y ago
  [deleted]

smutticus 12y ago

One of the main criticisms I hear about OpenBSD is that it's performance isn't as good as Linux. I've looked at a couple articles quoting benchmark comparisons, so I'll assume for the sake of argument that it's true. The thing is that I don't care. I'm still an OpenBSD advocate and run it whenever I get the chance. I don't actually care about performance at that level. What matters more to me is consistency, and good man pages. I imagine more people are like me, but the thing about performance measurements is they're quantifiable. They can be easily measured. We all probably over optimize to some degree, whether we're C developers or not. There's just something satisfying about getting a quantifiably better score. This is why we get code that sacrifices almost everything for runtime speed.

talideon 12y ago
The slowness isn't a generally impacted much by stuff like this. It's more things like OpenBSD's poor multicore performance that are the issue. Just compare the performance of FreeBSD's multithreaded PF to OpenBSD's singlethreaded PF. Still, none of this matters, because there's one major guarantee that the OpenBSD devs try to fulfil: correctness.
- imanaccount247 12y ago
  The thing is, the "slowness" in question is different every time someone wants to talk about it. Because people don't benchmark it, don't actually know what is slower and by how much, and just go on the "common knowledge" that openbsd is slow. The openbsd is slow meme was going strong in the 1990s, when it didn't support SMP at all and 99% of the people talking about it were using a single CPU.
  - wahern 12y ago
    It's different because the slowness is across the board. FreeBSD and, especially, Linux have optimizations all over the place. A call to time(2) or gettimeofday(2) is hundreds of times faster in Linux than on OpenBSD. In most scenarios the difference doesn't matter. But it does matter in, e.g., an asynchronous network service with lots of events with timeouts being constantly registered. I prefer OpenBSD, and I like that they focus on correctness, bugs, and more high-level features. I run all my personal services on OpenBSD. But commercially and at scale I run Linux, and treat each box as a throw-away in terms of security.

acveilleux 12y ago

My favorite thing with the OpenBSD folks isn't that they add APIs that avoid classes of security bugs but rather that they then ruthlessly update their code base to rely on the safer version.

to3m 12y ago

I suspect nobody playing with the USB code would have to look up the type of bNumEndpoints. The `b' prefix tells you it's a byte (this is a common thing in the USB spec), and you'll probably know it's a byte anyway due to familiarity... Another way of looking at the matter is that you shouldn't change code that's already correct.

dezgeg 12y ago
> I suspect nobody playing with the USB code would have to look up the type of bNumEndpoints. I could very well imagine a security auditor (who is not an USB expert) reading this code. > Another way of looking at the matter is that you shouldn't change code that's already correct. I personally find this kind of attitude extremely worrying. Perceptions about what is bad/good code or secure/insecure code have definitely changed since the first days of Unix. We shouldn't let code slip through the cracks just because it's old.

kazinator 12y ago

Overflows can crop up all over the place in C: you have to instrument every single +, - and * operator instance with overflow checks, each one of which needs a sane recovery strategy. Some overflows indicate internal errors that should panic the kernel. Others are configuration or resource issues that can bubble up some errno value to user space. Some can have a fallback strategy: this calculation overflowed, try another one. Some overflows can be handled by "saturation" (a.k.a clamping): common in digital signal processing and supported natively in some DSP machine languages. In other words, a silly patch that fixes a non-problem in one tiny place in a huge codebase doesn't accomplish anything. It's a big, wide-reaching task.

dezgeg 12y ago
> In other words, a silly patch that fixes a non-problem in one tiny place in a huge codebase doesn't accomplish anything. I don't understand this line of thinking at all. "reallocarray()/kmalloc_array() doesn't fix all the possible integer overflow issues in C, so why bother using it at all." Several people in this discussion have mentioned exact vulnerabilities that would have been prevented by reallocarray(): meowface: "Here is one of many examples of an integer overflow resulting in an improper size being passed to a memory allocation function further leading to remote code execution: https://docs.google.com/document/d/1tHElG04AJR5OR2Ex-m_Jsmc8S5fAbRB3s4RmTG_PFnw/edit https://docs.google.com/document/d/1tHElG04AJR5OR2Ex-m_Jsmc8... " tedunangst: "You've never heard of the one openssh exploit?"
- kazinator 12y ago
  The discussion has changed; it is about the pointless patch to some USB code where an 8 bit endpoint count is multiplied by the size of some structure.
- kazinator 12y ago
  I took realloc_array and applied it to a program I'm working on which contains a bunch of realloc calls. It was completely useless; I had to invent a different interface. It was useless because several calculations take place in every instance to produce the size, whereas realloc_array only checks one multiplication.