6 ms·
My safe language doesn't have "null", more or less. What it has is Option<T>, and I cannot turn that into a T without handling the failure case: there is liter
by deathanatos 4y ago
My safe language doesn't have "null", more or less.
What it has is Option<T>, and I cannot turn that into a T without handling the failure case: there is literally no way to construct the code otherwise¹. One must handle the failure path. (That might be way of explicit panic/abort/oops, but it's then right there in the code: that branch will panic … and safely.)
¹(this example is using safe Rust. There's unsafe Rust too and there I can chase the null pointer all I want with that, but the parent's point is that we should be sticking to safe interfaces for stuff like this. And I'm using Rust as an example, but Option is hardly unique to Rust, heck, Rust stole the idea from its predecessors.)
- tedunangst 4y agoWhat happens when I call unwrap?
- SpaghettiCthulu 4y agoYou shouldn't be allowed to in the kernel
- deathanatos 4y agoThat's the explicit handling of the None case I mentioned in the comment: it causes an explicit, and safe, abort. By "explicit", I mean the .unwrap() call will be right there, in the method that needs to turn an Option<T> into a T, and visible to a code reviewer. In the larger context here of kernel code, it should raise the eyebrow on the reviewer: "wait, this function shouldn't abort, it needs to handle the edge cases!". (But for some userland app, aborting might be acceptable. The kernel is in a bit of a bind, since an abort — a kernel panic — means the user loses computer until they reboot, and the work along with it.) Vs. a C pointer … all uses are more or less equally suspect; any given use, you hope the code has done it's homework for ensuring they're not NULL, and if they are, the consequence is UB. (And in Rust, and in the languages Rust steals the idea of Option from, you're only using/passing Options where "None"/null/nil is a possibility. If it's not, or you've verified or handled that at some outer stack frame, then you just pass a reference to a T, which is statically guaranteed to point to a valid object¹.) ¹again, barring buggy code using unsafe Rust, in the example of Rust, or calling into C code that fails to maintain its invariants, etc. Take the example in the article, where the code does, priv->mm->mmap->vm_start while trying to generate the output for smaps_rollup. That's compilable, but buggy, C, because mmap can be null, but we failed to check for it. Vs., if mmap were an Option<T>, where T is whatever type that pointer points to. Let's say our coder attempts to write, priv->mm->mmap->vm_start (In some imaginary language, because C doesn't have Option, AFAIK.) The compiler would say, no, you can't "->vm_start", because "mmap" could be None (whatever you call the "nothing here" value/variant; I'm going to call it None, to distinguish it from the null pointer). In the case of unwrap, the coder could do something like (this is psuedo-code) (priv->mm->mmap).unwrap().vm_start It would then be obvious there is an abort there. Their reviewer would not be pleased with that, I suspect: we don't want kernel panics or oops or aborts while generating a file in /proc. And likely our imaginary coder would know this too, and when the compiler errored the first time, saying, "hey, mmap is an Option", they'd raise an eyebrow, say something like, "wait, it is? When would mmap be None?" and then proceed to properly handle that case. (E.g., by treating it as if it where the empty list.)
- tedunangst 4y ago[flagged]
- Dylan16807 4y agoIf an abort function still triggers cleanup, then yes it is better. C doesn't have such a thing, so your sarcasm about 'telling them' is unwarranted. If an abort function doesn't trigger cleanup, then you can block it at compile time to prevent this kind of bug. But before you can even think about doing that, you need to split pointers into nullable and non-nullable. And the kernel devs already know about that idea, and how hard it is to implement in C. Nobody is naively suggesting "hey kernel devs do this thing!" as if there isn't decades of momentum behind the current codebase. It's just a look at how C is bad at this particular kind of bug.
- loeg 4y agoThese juvenile and facile retorts do not elevate the discourse, nor are they a good look. As a long-time OpenBSD developer you have a lot of smart things to say about technical subjects, including kernels specifically, and I wish you would leave the dumb snarky comments unwritten.
- chc4 4y agoThe root cause of the bug isn't the kernel dereferencing a NULL and causing UB. It's the kernel doing error handling and attempting to kill the oopsing task and continue. If the semantics of Rust panics also did a kernel oops, unwrap() would trigger the exact same bug described in the blog post with regards to reference count rollover (if Rust in the kernel doesn't do stack unwinding)
- jeffbee 4y agoYes, but fundamentally the kernel's inability to handle exceptional cases is all in deference to performance. Non-performance-critical sections, which in a fair analysis would be 99.9% of the kernel at least, should be written in a language and style that provides structured unwinding, not just jumping to the error case and whoops I accidentally jumped beyond all the unlocks and reference count decrements and deallocations. That's the issue.
- monocasa 4y agoA panic there absolutely could bail out to the read call at the root of this kernel stack with an EIO or some such.
- btown 4y agoOne of the missing pieces IMO is that your language needs the right kind of shorthand to make it easy to say "I'm calling things that return Options or Results, I myself return an Option or Result, any time I unwrap let's pipeline any null values or failures into returning a failure immediately." In Rust this is the ? operator: https://doc.rust-lang.org/reference/expressions/operator-expr.html#the-question-mark-operator https://doc.rust-lang.org/reference/expressions/operator-exp... The whole idea is that there should never be a way to call unwrap() if you as the caller cannot handle it gracefully. And if you do this at every step up until the UI layer, which can handle any failure as an error to be displayed in the UI, then the job is complete!
- Y_Y 4y agoThe Maybe Monad
- btown 4y agoYep, and monads without syntactic support are a pain to work with in practice! For instance, the best possible way in a language like Python is to break down what would otherwise be a set of imperative statements into a whole bunch of lambda functions: https://returns.readthedocs.io/en/latest/#id1 https://returns.readthedocs.io/en/latest/#id1
- loeg 4y agoIn a C++ code base I work on, we use a macro that is essentially Rust’s ?. It’s obviously less convenient than ?, but it’s not bad (even without syntax support). In the past in systems C code, I’ve manually written every error check and return.
- aseipp 4y agoThe thread panics because the value is None, and it stops execution before the attacker can take control of EIP. Was that supposed to be a hard question?
- avgcorrection 4y agoIt shouldn’t be a question that tedunangst should even have to ask, given that he has had a chip on his shoulder about Rust and its safety guarantees for years and thus should have learned that much about it by now.
- lmm 4y agoOK, and what does your kernel actually do when a kernel thread panics? It "stops execution", sure, so it will... oops? Causing exactly the problem given in the article?
- roguebantha 4y agoThe real issue with this error is not the panic of course - it's the fact that during a panic, C doesn't provide a canonical way of unwinding whatever actions have been performed so far. Rust (or even C++) do provide a bit more robustness in regards to handling errors in an unwindable way, but Rust and C++ probably aren't tenable solutions for the kernel in this case. It's far easier to add an oops limit and kiss this technique goodbye (hopefully!).
- deleted 4y ago[deleted]
- Gigachad 4y agoThe CI rejects your change for failing the lint.