Nothing should ever be attempting to allocate an out of range vector
like this, but bugs do happen. Ensure interrupt_sources doesn't
overflow even in presence of bugs.
Details
- Reviewers
- None
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 41509 Build 38398: arc lint + arc unit
Event Timeline
It shouldn't be possible to trigger, but part of D30743 fixes such an overflow. Actually more elegant to detect it at this layer and pass that back.
Actually on a second quick look, this is also an issue here.
The one question is whether it might instead be appropriate to panic() or printf() here.
@imp thoughts on this, or could you suggest reviewers? Perhaps in release builds the code elsewhere is supposed to ensure this doesn't happen, yet the one case it did get rather close. Even if the case remains a KASSERT(), vector being signed is a bug.
I'm unsure whether I've got the right reviewers/subscribers for this...
In theory this should be well-protected by code outside of this function. Yet despite that I found something which with a suitable push might have triggered this. As such I do like having an extra check in the core.
I can see arguments for this being a KASSERT() or panic(). I can also see an argument for including a printf() to flag trouble.
I would probably keep this as a KASSERT(). pic_vector itself returns an int rather than a u_int. vector should match that, so I would say either explicitly check for vector < 0' in the KASSERT, or change pic_vector` and the various implementations of it to return a u_int in addition to changing vector. (Calling it pic_first_irq while at it might be even better)
Making it fully unsigned seems a reasonable approach. How about D32070 then? I still think testing this in regular builds may be worthwhile.