Make naked functions incompatible with certain attributes by bstrie · Pull Request #93809 · rust-lang/rust (original) (raw)

How do we know that new feature authors have done this due diligence regarding naked functions?

It is assumed that the lang team will hold feature authors to this standard before allowing a feature to be stabilized. It's not impossible for things to slip through--accidents happen--but this is the acceptable risk that comes with stabilizing any feature.

Even worse, they might not fail but instead silently succeed with security implications (i.e. the stack is corrupted).

The threat in question involves a future attribute #[foo] that is silently incompatible with naked functions. The proposed mitigation is to prevent all but a hand-selected list of attributes on naked functions. However, for this threat to be realized the author of the naked function would have to manually add #[foo] to the naked function; every other existing user of naked functions cannot possibly be affected (at least not by any mechanism that the allowlist proposal would be capable of preventing). Thus, it would be impossible to realize such a threat merely by upgrading one's toolchain; you would have to first upgrade your toolchain, and then consciously adopt #[foo] into your codebase in order to be affected. Finally, #[foo] would have to be an attribute that affects codegen somehow,

I agree up until this point.

and (correct me if I'm wrong) it seems reasonable to assume that any security-minded user will have integration tests in place that would detect a failure on the PR that adds #[foo] to their naked functions. Thus IMO the benefit here is small.

This is a very bad assumption. All users are impacted by these problems regardless if they are security-minded or not. And the interactions between these attributes are non-obvious.

Meanwhile, the cost is very large.

Given that the number of users that consumes Rust features is exponentially larger than the number of Rust feature developers, the total cost is significantly smaller to the first group than to the second.

I find it eminently reasonable to assume that a feature author that adds an attribute that affects codegen will perform their due diligence in remembering to deny its use on #[naked] (especially since they will be forced to update the reference page on codegen attributes, which will by then contain the documentation on attributes that may not be used with naked).

I would hope so indeed. But when this fails, it fails spectacularly. Trying to bisect this will be very painful. OTOH, being able to do a git blame against an allowlist gives us the relevant commit(s) immediately.

I also find it reasonable to assume that feature authors that add an attribute that does not affect codegen will reasonably assume that they do not need to arbitrarily allow their attribute on naked functions; I cannot think of any other precedent for this in the language.

This is because for other features that are this dangerous, like asm, you don't just automatically get integration. For example, if you want to pass a new kind of value to asm you have to explicitly allow support for it in the asm functionality. No built-in hurdle like this exists for naked functions. The problem here is, I think, unique because we have something potentially very dangerous that exists precisely at the junction of an integration point (function attributes) where the number of combinations is high and difficult to reason about.

What seems unreasonable to me would be forcing these latter authors to remember to manually allow their attribute to be used with naked functions; in practice I suspect they would simply not, and users of naked functions would be frustrated by the inability to do things that should obviously be allowed.

But... doing this is safe. And fixing the oversight is trivial if there are no safety concerns. If you stabilize your feature and realize you've forgotten to add it to the allowlist, this should be a trivial patch that can be merged within the next release window. It isn't like you have to go back to the drawing board.

It seems to me like you're arguing that it is reasonable to expect feature developers to remember to consider all the subtle interactions between codegen attributes but not reasonable to expect the same feature developers to remember to add one item to an allowlist. The former is far more work than the latter.

Between the burden on feature authors and the burden on users who would be prevented from doing things for no reason (especially when considering that this policy would also prevent third-party proc macros from being used with naked functions), I feel that the cost outweighs the benefit, IMO.

It seems to me like you're optimizing for the burden of the feature authors rather than for the safety of the consumers of this feature.