undefined | Better HN

0 pointswtallis2y ago0 comments

I wouldn't generally expect a use-after-free to result from improper pointer arithmetic; that's the recipe for a buffer overflow. But Rust happens to also be well-known for helping manage object lifetimes, which seems to be what went wrong here.

0 comments

eklitzke2y ago

I'm not sure if your claim here is correct. The patch is to change call sites like

  priv->set->use++;

To look like:

  nf_tables_activate_set(ctx, priv->set);

Where this function is defined as:

  void nf_tables_activate_set(const struct nft_ctx *ctx, struct nft_set *set) {
    if (nft_set_is_anonymous(set))
      nft_clear(ctx->net, set);
    set->use++;
  }

So to me (someone who is not an expert in this code) it looks like the fix is checking if the set has the anonymous flag before changing the reference count. I'm not an expert in this code and I could be mistaken, but I think your claim that this would be fixed by Rust object lifetime checking requires better evidence.

wtallisOP2y ago

I think a Rust-influenced design would have shied away from the manual direct reference count management in the first place and resulted in a fairly different-looking API; but at a minimum I'd expect that the safe wrapper `nf_tables_activate_set` would probably have existed from the beginning, and may have been designed to transfer ownership of the `nft_set` rather than just capture a reference to it.

More generally: doing a line-by-line translation from C to Rust is never going to be the best way to make use of the capabilities Rust has that C lacks.

comex2y ago

One of the parts of Rust’s safety story is to always use smart pointers for reference counting rather than the type of ad-hoc manual reference count management seen in the code you quoted. Combined with lifetime checking, it makes it impossible for some random logic error to cause a use-after-free.

max_k2y ago

> to always use smart pointers for reference counting

Agree - and the Linux kernel is extremely fragile because it is full of ad-hoc manual code like that.

Unfortunately, Rust won't be the rescue, because (in the foreseeable future) Rust will only be available in leaf code due to the many hard problems of transitioning from fragile C APIs to something better. Writing drivers in Rust is useful, but limits the scope of how Rust helps.

Many of Rust's advantages at a tiny fraction of the effort could be had easily with a smooth transition path by switching the compiler from C to C++ mode. The fruit hangs so low, it nearly touches the ground, but a silly Linus rejects C++ for the wrong reasons ("to keep the C++ programmers out", wtf).

Every time I work on the Linux kernel source, I'm horrified by how much pain the kernel developers inflict on themselves. Even with C, it would be possible to install a mandatory coding style that is less fragile.

For example, in the aftermath of the Dirty Pipe vulnerability last year, I submitted a patch to make the code less fragile, a coding style that would have prevented the vulnerability: https://lore.kernel.org/lkml/20220225185431.2617232-4-max.ke... - but my patch went nowhere.

1 more reply

mtlmtlmtlmtl2y ago

Correction: it is impossible in safe Rust that only ever calls safe Rust. The moment you're calling unsafe Rust, the possibility returns.

Not saying Rust isn't an improvement, it's a huge improvement over C, but there's no reason to oversell it. Rust is not going to make these errors magically go away, at least not in a kernel, even if you wrote the kernel from scratch, all in Rust. Unless you managed to write all of it in safe Rust which... good luck with that.

j / k navigate · click thread line to collapse