On Tue, 15 Oct 2024 at 09:52, <marcandre.lur...@redhat.com> wrote:
>
> From: Roman Penyaev <r.peni...@gmail.com>
>
> With bitset management now it becomes feasible to implement
> the logic of detaching frontends from multiplexer.
>
> Signed-off-by: Roman Penyaev <r.peni...@gmail.com>
> Cc: "Marc-André Lureau" <marcandre.lur...@redhat.com>
> Cc: qemu-devel@nongnu.org
> Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> Message-ID: <20241014152408.427700-8-r.peni...@gmail.com>
Hi; Coverity reports an issue with this patch. I think
it's probably a bit confused, but on the other hand
I read the code and am also a bit confused so we could
probably adjust it to be clearer...
> +bool mux_chr_detach_frontend(MuxChardev *d, unsigned int tag)
> +{
> + unsigned int bit;
> +
> + bit = find_next_bit(&d->mux_bitset, MAX_MUX, tag);
Why are we calling find_next_bit() here? Coverity
points out that it can return MAX_MUX, which means that
if the caller passed in MAX_MUX then we will try to
dereference d->backends[MAX_MUX] which is off the
end of the array.
What I was expecting this code to do was to check
"is the bit actually currently set?", which would be
if (!(d->mux_bitset & (1 << bit))) {
...
}
Why do we want to find the next bit set after the
'tag' bit ?
> + if (bit != tag) {
> + return false;
> + }
> +
> + d->mux_bitset &= ~(1 << bit);
> + d->backends[bit] = NULL;
> +
> + return true;
> +}
(This is CID 1563776.)
thanks
-- PMM
