Re: [PATCH v2] mshv: Fix interrupt state corruption in hv_do_map_pfns error path

[Stanislav Kinsburskii]

On Tue, Apr 28, 2026 at 12:20:35AM +0000, Michael Kelley wrote:
> From: Stanislav Kinsburskii <skinsburskii@xxxxxxxxxxxxxxxxxxx> Sent: Monday, April 27, 2026 7:44 AM
> > 
> > Restore interrupt state before breaking out of the loop on error.
> > 
> > The irq_flags are saved before entering the loop, but the early exit
> > path on error fails to restore them. This leaves interrupts in an
> > inconsistent state and can lead to lockdep warnings or other
> > interrupt-related issues.
> > 
> > Fixes: 621191d709b14 ("Drivers: hv: Introduce mshv_root module to expose /dev/mshv to VMMs")
> > Signed-off-by: Stanislav Kinsburskii <skinsburskii@xxxxxxxxxxxxxxxxxxx>
> > ---
> >  drivers/hv/mshv_root_hv_call.c |    4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/hv/mshv_root_hv_call.c b/drivers/hv/mshv_root_hv_call.c
> > index ab210a7fcb8c3..61291ec6f3468 100644
> > --- a/drivers/hv/mshv_root_hv_call.c
> > +++ b/drivers/hv/mshv_root_hv_call.c
> > @@ -229,8 +229,10 @@ static int hv_do_map_gpa_hcall(u64 partition_id, u64 gfn, u64 page_struct_count,
> >  			} else {
> >  				pfnlist[i] = mmio_spa + done + i;
> >  			}
> > -		if (ret)
> > +		if (ret) {
> > +			local_irq_restore(irq_flags);
> >  			break;
> > +		}
> > 
> 
> This looks good for fixing the immediate bug.
> 
> But I'd note that this error path occurs solely based on the
> if (index >= page_struct_count) test in the preceding 'for' loop. That test is a
> "can't happen" sanity test that never triggers if hv_do_map_gpa_hcall()
> is coded correctly. At the beginning of the function there are validations of
> the input arguments, which is reasonable. But this sanity test isn't based
> on the input arguments, and it adds non-trivial complexity to the code
> because of the nested loops and the need to figure out where the two
> "break" statements go. I'd argue for dropping the sanity test entirely,
> along with this test of 'ret' and the need to restore the interrupt state.
> 

Fair enough. Let me rework this function (and it's unmap peer).

Thanks,
Stanislav

> Michael