On Thu, May 14, 2026 at 05:49:01AM +0000, Anirudh Rayabharam wrote:
> On Wed, May 13, 2026 at 10:39:31AM -0700, Stanislav Kinsburskii wrote:
> > On Wed, May 13, 2026 at 11:12:05AM +0000, Anirudh Rayabharam wrote:
> > > On Thu, May 07, 2026 at 03:44:26PM +0000, Stanislav Kinsburskii wrote:
> > > > handle_pair_message() iterates up to msg->vp_count without verifying it
> > > > against HV_MESSAGE_MAX_PARTITION_VP_PAIR_COUNT. Since vp_count is read
> > > > from untrusted hypervisor data, a malformed message with a large value
> > > > would cause out-of-bounds reads from the partition_ids and vp_indexes
> > > > arrays.
> > > >
> > > > handle_bitset_message() iterates over set bits in valid_bank_mask (up to
> > > > 64) and advances bank_contents for each one. However, the payload buffer
> > > > only has space for 16 bank entries. A valid_bank_mask with more than 16
> > > > bits set causes bank_contents to read beyond the message buffer.
> > > >
> > > > Fix both by adding bounds validation:
> > > > - Clamp vp_count to HV_MESSAGE_MAX_PARTITION_VP_PAIR_COUNT
> > > > - Track banks consumed and stop before exceeding buffer capacity
> > > >
> > > > Fixes: 621191d709b1 ("Drivers: hv: Introduce mshv_root module to expose
> > > > /dev/mshv to VMMs")
> > > > Signed-off-by: Stanislav Kinsburskii <[email protected]>
> > > > ---
> > > > drivers/hv/mshv_synic.c | 20 ++++++++++++++++++--
> > > > 1 file changed, 18 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/hv/mshv_synic.c b/drivers/hv/mshv_synic.c
> > > > index 89207aad7cf1f..5d509299f14d7 100644
> > > > --- a/drivers/hv/mshv_synic.c
> > > > +++ b/drivers/hv/mshv_synic.c
> > > > @@ -190,7 +190,9 @@ static void kick_vp(struct mshv_vp *vp)
> > > > static void
> > > > handle_bitset_message(const struct
> > > > hv_vp_signal_bitset_scheduler_message *msg)
> > > > {
> > > > - int bank_idx, vps_signaled = 0, bank_mask_size;
> > > > + int bank_idx, vps_signaled = 0, bank_mask_size, banks_used = 0;
> > > > + const int max_banks = sizeof(msg->vp_bitset.bitset_buffer) /
> > > > + sizeof(u64) - 2; /* subtract format +
> > > > mask */
> > >
> > > Could this be a constant in the header?
> > >
> >
> > Yes, it could. But it the only place it's used and it's pretty
> > self-explanatory, so I don't think it needs to be.
>
> The "subtract format+mask" part is a bit concerning. We might forget to update
> this code if the struct layout ever changes. Whereas if the constant is
> right next to the definition in the header, it is unlikely to be missed.
>
Fair enough. But I'd suggest sending this a a follow up for the series.
What do you think?
Thanks,
Stanislav
> Thanks,
> Anirudh.
>
