4 ms·
This might be a bit off topic but I wonder why the vulnerability has been patched this way: if (!access_ok(VERIFY_WRITE, infop, sizeof(*infop))) go
by Matt3o12_ 9y ago
This might be a bit off topic but I wonder why the vulnerability has been patched this way:
if (!access_ok(VERIFY_WRITE, infop, sizeof(*infop)))
goto Efault;
Why doesn’t the if use curly brackets? I thought it has been established that it is best practices to always use curly brackets even if they are explicit, especially after Apple's infamous goto bug[1].
Secondly, why does it use goto at all? I thought it has also been established not to use goto unless it is the only performant solution (and performance is important in that case). Sure Efault with probably kill the program but wouldn’t it still be better to use a function call considering that the desired resolution should be the same?
[1]: https://www.imperialviolet.org/2014/02/22/applebug.html https://www.imperialviolet.org/2014/02/22/applebug.html
- chowyuncat 9y agoThe kernel style guide mandates no curly brackets in this case. https://www.kernel.org/doc/html/v4.10/process/coding-style.html https://www.kernel.org/doc/html/v4.10/process/coding-style.h...
- deleted 9y ago[deleted]
- heinrichhartman 9y ago1. Linux kernel coding style is documented here [1], and contains the line: > Do not unnecessarily use braces where a single statement will do. 2. There is no built-in exception handling in C. `goto ERROR_HANDLING_CODE` is a common and well established pattern to handle exceptions in C, see e.g. [2]. [1] https://www.kernel.org/doc/html/v4.10/process/coding-style.html#placing-braces-and-spaces https://www.kernel.org/doc/html/v4.10/process/coding-style.h... [2] https://news.ycombinator.com/item?id=3883310 https://news.ycombinator.com/item?id=3883310
- deleted 9y ago[deleted]
- umanwizard 9y ago> I thought it has also been established not to use goto unless [...] “Established” by whom? Certainly not by kernel developers — `goto` is very common in all kernels I have looked at (xnu, Linux, bsd)
- deleted 9y ago[deleted]
- drchickensalad 9y agoAnd it has quite a consensus as the best solution to this problem. Goto being considered harmful is a generally true statement. However, this usage is a more specific exception to the rule, with objective benefits vs alternatives.
- user5994461 9y ago"goto considered harmful" is an old meme from 50 years ago, when control blocks like if and for were invented.
- icebraining 9y agoFrom your link: Maybe the coding style contributed to this by allowing ifs without braces, but one can have incorrect indentation with braces too, so that doesn't seem terribly convincing to me. By the way, gcc now has -Wmisleading-indentation, which is activated by default if you enable -Wall: https://news.ycombinator.com/item?id=10875449 https://news.ycombinator.com/item?id=10875449
- marcoperaza 9y agogoto is the best way to do early exit and cleanup from a C function. The alternatives are 1) deeply nested if's, one level for each function call that can return an error code, or 2) repeat the same cleanup code over and over again at every exit point.
- anonfunction 9y agohttps://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/kernel/exit.c?id=1c9fec470b81ca5e89391c20a11ead31a1e9314b https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/lin...
- adam-ff 9y agoNote: The present solution replaced the goto with a return: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/kernel/exit.c?id=1c9fec470b81ca5e89391c20a11ead31a1e9314b https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/lin...
- efficax 9y agogoto is kind of a necessity in any complex C codebase unless you want to duplicate tons of code. Sometimes you need to jump way out of the context, especially to handle errors, and C does not have "exceptions" (although you could do them with setjmp/longjmp)