RE: [EXTERNAL] Re: [PATCH] Drivers: hv: Introduce mshv_vtl driver

[Saurabh Singh Sengar <ssengar@microsoft.com>]

> -----Original Message-----
> From: Wei Liu <wei.liu@kernel.org>
> Sent: 08 May 2025 23:33
> To: Roman Kisel <romank@linux.microsoft.com>
> Cc: Saurabh Singh Sengar <ssengar@microsoft.com>; Naman Jain
> <namjain@linux.microsoft.com>; KY Srinivasan <kys@microsoft.com>;
> Haiyang Zhang <haiyangz@microsoft.com>; Wei Liu <wei.liu@kernel.org>;
> Dexuan Cui <decui@microsoft.com>; Anirudh Rayabharam
> <anrayabh@linux.microsoft.com>; Saurabh Sengar
> <ssengar@linux.microsoft.com>; Stanislav Kinsburskii
> <skinsburskii@linux.microsoft.com>; Nuno Das Neves
> <nunodasneves@linux.microsoft.com>; linux-kernel@vger.kernel.org; linux-
> hyperv@vger.kernel.org
> Subject: [EXTERNAL] Re: [PATCH] Drivers: hv: Introduce mshv_vtl driver
> 
> On Wed, May 07, 2025 at 12:20:36PM -0700, Roman Kisel wrote:
> >
> >
> > On 5/7/2025 6:02 AM, Saurabh Singh Sengar wrote:
> > >
> > [..]
> >
> > > > +	}
> > > > +
> > > > +	local_irq_save(flags);
> > > > +	in = *this_cpu_ptr(hyperv_pcpu_input_arg);
> > > > +	out = *this_cpu_ptr(hyperv_pcpu_output_arg);
> > > > +
> > > > +	if (copy_from_user(in, (void __user *)hvcall.input_ptr,
> > > > hvcall.input_size)) {
> > >
> > > Here is an issue related to usage of user copy functions when interrupt are
> disabled.
> > > It was reported by Michael K here:
> > > https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi
> > > thub.com%2Fmicrosoft%2FOHCL-Linux-
> Kernel%2Fissues%2F33&data=05%7C02%
> > >
> 7Cssengar%40microsoft.com%7C3a21fc17545e4bafb86e08dd8e5aa427%7C72
> f98
> > >
> 8bf86f141af91ab2d7cd011db47%7C1%7C0%7C638823242185564641%7CUnk
> nown%7
> > >
> CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJ
> XaW4
> > >
> zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=pdQ90a
> aKZsy
> > > 67qkPZkV4BfpwM6cM6scUbTeH9lT7s6s%3D&reserved=0
> >
> > From the practical point of view, that memory will be touched by the
> > user mode by virtue of Rust requiring initialization so the a possible
> > page fault would be resolved before the IOCTL. OpenHCL runs without
> > swap so the the memory will not be paged out to require page faults to
> > be brought in back.
> >
> > I do agree that might be turned into a footgun by the user land if
> > they malloc a page w/o prefaulting (so it's just a VA range, not
> > backed with the physical page), and then send its address straight
> > over here right after w/o writing any data to it. Perhaps likelier
> > with the output data. Anyway, yes, relying on the user land doing sane
> > things isn't the best approach to the kernel programming.
> 
> Yep. We don't rely on user land software doing sane things to maintain
> correctness in kernel, so this needs to be fixed.
> 
> Thanks,
> Wei.


How about fixing this for normal x86 for now and put a TODO for CVM to be fixed later, when we bring in CVM support ?

Regards,
Saurabh