On 14/10/25 14:59, Peter Maydell wrote:
On Tue, 14 Oct 2025 at 09:36, Peter Maydell <[email protected]> wrote:
On Tue, 29 Jul 2025 at 12:14, Markus Armbruster <[email protected]> wrote:
xenfb_mouse_event() has a switch statement whose controlling
expression move->axis is an enum InputAxis. The enum values are
INPUT_AXIS_X and INPUT_AXIS_Y, encoded as 0 and 1. The switch has a
case for both axes. In addition, it has an unreachable default label.
This convinces Coverity that move->axis can be greater than 1. It
duly reports a buffer overrun when it is used to subscript an array
with two elements.
I think also that Coverity gets confused by QAPI's convention
in generated code of defining enumerations like this:
typedef enum InputAxis {
INPUT_AXIS_X,
INPUT_AXIS_Y,
INPUT_AXIS__MAX,
} InputAxis;
Coverity thinks that INPUT_AXIS__MAX might be a valid
value it can see in move->axis, because we defined the
enum that way.
In theory I suppose that since the __MAX value is only
there to be an array or enumeration bound that we could
emit code that #defines it rather than making it part
of the enum.
Also, there's an argument that this function should
ignore unknown input-axis enum values. If we ever in future
extend this to support a Z-axis, it would be better to
ignore the events we can't send, the same way we already
do for unknown INPUT_EVENT_KIND_BTN button types, rather
than aborting. But it's not very important, so the
g_assert_not_reached() will do.
(In some other languages, we'd get a compile failure for
adding a new value to the enum that we didn't handle.
But not in C :-))
See this thread where it was discussed (until I gave up...):
https://lore.kernel.org/qemu-devel/[email protected]/
