diff mbox series

[1/1] video: hyperv_fb: Fix validation of screen resolution

Message ID 1642360711-2335-1-git-send-email-mikelley@microsoft.com (mailing list archive)
State New, archived
Headers show
Series [1/1] video: hyperv_fb: Fix validation of screen resolution | expand

Commit Message

Michael Kelley (LINUX) Jan. 16, 2022, 7:18 p.m. UTC
In the WIN10 version of the Synthetic Video protocol with Hyper-V,
Hyper-V reports a list of supported resolutions as part of the protocol
negotiation. The driver calculates the maximum width and height from
the list of resolutions, and uses those maximums to validate any screen
resolution specified in the video= option on the kernel boot line.

This method of validation is incorrect. For example, the list of
supported resolutions could contain 1600x1200 and 1920x1080, both of
which fit in an 8 Mbyte frame buffer.  But calculating the max width
and height yields 1920 and 1200, and 1920x1200 resolution does not fit
in an 8 Mbyte frame buffer.  Unfortunately, this resolution is accepted,
causing a kernel fault when the driver accesses memory outside the
frame buffer.

Instead, validate the specified screen resolution by calculating
its size, and comparing against the frame buffer size.  Delete the
code for calculating the max width and height from the list of
resolutions, since these max values have no use.  Also add the
frame buffer size to the info message to aid in understanding why
a resolution might be rejected.

Fixes: 67e7cdb4829d ("video: hyperv: hyperv_fb: Obtain screen resolution from Hyper-V host")
Signed-off-by: Michael Kelley <mikelley@microsoft.com>
---
 drivers/video/fbdev/hyperv_fb.c | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

Comments

Haiyang Zhang Jan. 16, 2022, 9:53 p.m. UTC | #1
> -----Original Message-----
> From: Michael Kelley (LINUX) <mikelley@microsoft.com>
> Sent: Sunday, January 16, 2022 2:19 PM
> To: KY Srinivasan <kys@microsoft.com>; Haiyang Zhang <haiyangz@microsoft.com>; Stephen
> Hemminger <sthemmin@microsoft.com>; wei.liu@kernel.org; Wei Hu <weh@microsoft.com>; Dexuan
> Cui <decui@microsoft.com>; drawat.floss@gmail.com; hhei <hhei@redhat.com>; linux-
> kernel@vger.kernel.org; linux-hyperv@vger.kernel.org; linux-fbdev@vger.kernel.org; dri-
> devel@lists.freedesktop.org
> Cc: Michael Kelley (LINUX) <mikelley@microsoft.com>
> Subject: [PATCH 1/1] video: hyperv_fb: Fix validation of screen resolution
> 
> In the WIN10 version of the Synthetic Video protocol with Hyper-V,
> Hyper-V reports a list of supported resolutions as part of the protocol
> negotiation. The driver calculates the maximum width and height from
> the list of resolutions, and uses those maximums to validate any screen
> resolution specified in the video= option on the kernel boot line.
> 
> This method of validation is incorrect. For example, the list of
> supported resolutions could contain 1600x1200 and 1920x1080, both of
> which fit in an 8 Mbyte frame buffer.  But calculating the max width
> and height yields 1920 and 1200, and 1920x1200 resolution does not fit
> in an 8 Mbyte frame buffer.  Unfortunately, this resolution is accepted,
> causing a kernel fault when the driver accesses memory outside the
> frame buffer.
> 
> Instead, validate the specified screen resolution by calculating
> its size, and comparing against the frame buffer size.  Delete the
> code for calculating the max width and height from the list of
> resolutions, since these max values have no use.  Also add the
> frame buffer size to the info message to aid in understanding why
> a resolution might be rejected.
> 
> Fixes: 67e7cdb4829d ("video: hyperv: hyperv_fb: Obtain screen resolution from Hyper-V
> host")
> Signed-off-by: Michael Kelley <mikelley@microsoft.com>
> ---
>  drivers/video/fbdev/hyperv_fb.c | 16 +++-------------
>  1 file changed, 3 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
> index 23999df..c8e0ea2 100644
> --- a/drivers/video/fbdev/hyperv_fb.c
> +++ b/drivers/video/fbdev/hyperv_fb.c
> @@ -287,8 +287,6 @@ struct hvfb_par {
> 
>  static uint screen_width = HVFB_WIDTH;
>  static uint screen_height = HVFB_HEIGHT;
> -static uint screen_width_max = HVFB_WIDTH;
> -static uint screen_height_max = HVFB_HEIGHT;
>  static uint screen_depth;
>  static uint screen_fb_size;
>  static uint dio_fb_size; /* FB size for deferred IO */
> @@ -582,7 +580,6 @@ static int synthvid_get_supported_resolution(struct hv_device *hdev)
>  	int ret = 0;
>  	unsigned long t;
>  	u8 index;
> -	int i;
> 
>  	memset(msg, 0, sizeof(struct synthvid_msg));
>  	msg->vid_hdr.type = SYNTHVID_RESOLUTION_REQUEST;
> @@ -613,13 +610,6 @@ static int synthvid_get_supported_resolution(struct hv_device *hdev)
>  		goto out;
>  	}
> 
> -	for (i = 0; i < msg->resolution_resp.resolution_count; i++) {
> -		screen_width_max = max_t(unsigned int, screen_width_max,
> -		    msg->resolution_resp.supported_resolution[i].width);
> -		screen_height_max = max_t(unsigned int, screen_height_max,
> -		    msg->resolution_resp.supported_resolution[i].height);
> -	}
> -
>  	screen_width =
>  		msg->resolution_resp.supported_resolution[index].width;
>  	screen_height =
> @@ -941,7 +931,7 @@ static void hvfb_get_option(struct fb_info *info)
> 
>  	if (x < HVFB_WIDTH_MIN || y < HVFB_HEIGHT_MIN ||
>  	    (synthvid_ver_ge(par->synthvid_version, SYNTHVID_VERSION_WIN10) &&
> -	    (x > screen_width_max || y > screen_height_max)) ||
> +	    (x * y * screen_depth / 8 > screen_fb_size)) ||
>  	    (par->synthvid_version == SYNTHVID_VERSION_WIN8 &&
>  	     x * y * screen_depth / 8 > SYNTHVID_FB_SIZE_WIN8) ||
>  	    (par->synthvid_version == SYNTHVID_VERSION_WIN7 &&
> @@ -1194,8 +1184,8 @@ static int hvfb_probe(struct hv_device *hdev,
>  	}
> 
>  	hvfb_get_option(info);
> -	pr_info("Screen resolution: %dx%d, Color depth: %d\n",
> -		screen_width, screen_height, screen_depth);
> +	pr_info("Screen resolution: %dx%d, Color depth: %d, Frame buffer size: %d\n",
> +		screen_width, screen_height, screen_depth, screen_fb_size);
> 
>  	ret = hvfb_getmem(hdev, info);
>  	if (ret) {

Thank you!

Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
Wei Liu Jan. 23, 2022, 9:56 p.m. UTC | #2
On Sun, Jan 16, 2022 at 09:53:06PM +0000, Haiyang Zhang wrote:
> 
> 
> > -----Original Message-----
> > From: Michael Kelley (LINUX) <mikelley@microsoft.com>
> > Sent: Sunday, January 16, 2022 2:19 PM
> > To: KY Srinivasan <kys@microsoft.com>; Haiyang Zhang <haiyangz@microsoft.com>; Stephen
> > Hemminger <sthemmin@microsoft.com>; wei.liu@kernel.org; Wei Hu <weh@microsoft.com>; Dexuan
> > Cui <decui@microsoft.com>; drawat.floss@gmail.com; hhei <hhei@redhat.com>; linux-
> > kernel@vger.kernel.org; linux-hyperv@vger.kernel.org; linux-fbdev@vger.kernel.org; dri-
> > devel@lists.freedesktop.org
> > Cc: Michael Kelley (LINUX) <mikelley@microsoft.com>
> > Subject: [PATCH 1/1] video: hyperv_fb: Fix validation of screen resolution
> > 
> > In the WIN10 version of the Synthetic Video protocol with Hyper-V,
> > Hyper-V reports a list of supported resolutions as part of the protocol
> > negotiation. The driver calculates the maximum width and height from
> > the list of resolutions, and uses those maximums to validate any screen
> > resolution specified in the video= option on the kernel boot line.
> > 
> > This method of validation is incorrect. For example, the list of
> > supported resolutions could contain 1600x1200 and 1920x1080, both of
> > which fit in an 8 Mbyte frame buffer.  But calculating the max width
> > and height yields 1920 and 1200, and 1920x1200 resolution does not fit
> > in an 8 Mbyte frame buffer.  Unfortunately, this resolution is accepted,
> > causing a kernel fault when the driver accesses memory outside the
> > frame buffer.
> > 
> > Instead, validate the specified screen resolution by calculating
> > its size, and comparing against the frame buffer size.  Delete the
> > code for calculating the max width and height from the list of
> > resolutions, since these max values have no use.  Also add the
> > frame buffer size to the info message to aid in understanding why
> > a resolution might be rejected.
> > 
> > Fixes: 67e7cdb4829d ("video: hyperv: hyperv_fb: Obtain screen resolution from Hyper-V
> > host")
> > Signed-off-by: Michael Kelley <mikelley@microsoft.com>
[...]
> 
> Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> 

Applied to hyperv-fixes. Thanks.
Michael Kelley (LINUX) Jan. 23, 2022, 10:27 p.m. UTC | #3
From: Wei Liu <wei.liu@kernel.org> Sent: Sunday, January 23, 2022 1:56 PM
> 
> On Sun, Jan 16, 2022 at 09:53:06PM +0000, Haiyang Zhang wrote:
> >
> >
> > > -----Original Message-----
> > > From: Michael Kelley (LINUX) <mikelley@microsoft.com>
> > > Sent: Sunday, January 16, 2022 2:19 PM
> > > To: KY Srinivasan <kys@microsoft.com>; Haiyang Zhang
> <haiyangz@microsoft.com>; Stephen
> > > Hemminger <sthemmin@microsoft.com>; wei.liu@kernel.org; Wei Hu
> <weh@microsoft.com>; Dexuan
> > > Cui <decui@microsoft.com>; drawat.floss@gmail.com; hhei <hhei@redhat.com>;
> linux-
> > > kernel@vger.kernel.org; linux-hyperv@vger.kernel.org; linux-
> fbdev@vger.kernel.org; dri-
> > > devel@lists.freedesktop.org
> > > Cc: Michael Kelley (LINUX) <mikelley@microsoft.com>
> > > Subject: [PATCH 1/1] video: hyperv_fb: Fix validation of screen resolution
> > >
> > > In the WIN10 version of the Synthetic Video protocol with Hyper-V,
> > > Hyper-V reports a list of supported resolutions as part of the protocol
> > > negotiation. The driver calculates the maximum width and height from
> > > the list of resolutions, and uses those maximums to validate any screen
> > > resolution specified in the video= option on the kernel boot line.
> > >
> > > This method of validation is incorrect. For example, the list of
> > > supported resolutions could contain 1600x1200 and 1920x1080, both of
> > > which fit in an 8 Mbyte frame buffer.  But calculating the max width
> > > and height yields 1920 and 1200, and 1920x1200 resolution does not fit
> > > in an 8 Mbyte frame buffer.  Unfortunately, this resolution is accepted,
> > > causing a kernel fault when the driver accesses memory outside the
> > > frame buffer.
> > >
> > > Instead, validate the specified screen resolution by calculating
> > > its size, and comparing against the frame buffer size.  Delete the
> > > code for calculating the max width and height from the list of
> > > resolutions, since these max values have no use.  Also add the
> > > frame buffer size to the info message to aid in understanding why
> > > a resolution might be rejected.
> > >
> > > Fixes: 67e7cdb4829d ("video: hyperv: hyperv_fb: Obtain screen resolution from Hyper-V
> > > host")
> > > Signed-off-by: Michael Kelley <mikelley@microsoft.com>
> [...]
> >
> > Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> >
> 
> Applied to hyperv-fixes. Thanks.

This fix got pulled into the fbdev/for-next tree by a new maintainer, Helge Deller.
See https://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev.git/commit/?h=for-next&id=bcc48f8d980b12e66a3d59dfa1041667db971d86

Michael
Wei Liu Jan. 23, 2022, 10:30 p.m. UTC | #4
On Sun, Jan 23, 2022 at 10:27:56PM +0000, Michael Kelley (LINUX) wrote:
> From: Wei Liu <wei.liu@kernel.org> Sent: Sunday, January 23, 2022 1:56 PM
> > 
> > On Sun, Jan 16, 2022 at 09:53:06PM +0000, Haiyang Zhang wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Michael Kelley (LINUX) <mikelley@microsoft.com>
> > > > Sent: Sunday, January 16, 2022 2:19 PM
> > > > To: KY Srinivasan <kys@microsoft.com>; Haiyang Zhang
> > <haiyangz@microsoft.com>; Stephen
> > > > Hemminger <sthemmin@microsoft.com>; wei.liu@kernel.org; Wei Hu
> > <weh@microsoft.com>; Dexuan
> > > > Cui <decui@microsoft.com>; drawat.floss@gmail.com; hhei <hhei@redhat.com>;
> > linux-
> > > > kernel@vger.kernel.org; linux-hyperv@vger.kernel.org; linux-
> > fbdev@vger.kernel.org; dri-
> > > > devel@lists.freedesktop.org
> > > > Cc: Michael Kelley (LINUX) <mikelley@microsoft.com>
> > > > Subject: [PATCH 1/1] video: hyperv_fb: Fix validation of screen resolution
> > > >
> > > > In the WIN10 version of the Synthetic Video protocol with Hyper-V,
> > > > Hyper-V reports a list of supported resolutions as part of the protocol
> > > > negotiation. The driver calculates the maximum width and height from
> > > > the list of resolutions, and uses those maximums to validate any screen
> > > > resolution specified in the video= option on the kernel boot line.
> > > >
> > > > This method of validation is incorrect. For example, the list of
> > > > supported resolutions could contain 1600x1200 and 1920x1080, both of
> > > > which fit in an 8 Mbyte frame buffer.  But calculating the max width
> > > > and height yields 1920 and 1200, and 1920x1200 resolution does not fit
> > > > in an 8 Mbyte frame buffer.  Unfortunately, this resolution is accepted,
> > > > causing a kernel fault when the driver accesses memory outside the
> > > > frame buffer.
> > > >
> > > > Instead, validate the specified screen resolution by calculating
> > > > its size, and comparing against the frame buffer size.  Delete the
> > > > code for calculating the max width and height from the list of
> > > > resolutions, since these max values have no use.  Also add the
> > > > frame buffer size to the info message to aid in understanding why
> > > > a resolution might be rejected.
> > > >
> > > > Fixes: 67e7cdb4829d ("video: hyperv: hyperv_fb: Obtain screen resolution from Hyper-V
> > > > host")
> > > > Signed-off-by: Michael Kelley <mikelley@microsoft.com>
> > [...]
> > >
> > > Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> > >
> > 
> > Applied to hyperv-fixes. Thanks.
> 
> This fix got pulled into the fbdev/for-next tree by a new maintainer, Helge Deller.
> See https://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev.git/commit/?h=for-next&id=bcc48f8d980b12e66a3d59dfa1041667db971d86

OK. I will drop it from hyperv-fixes. Thanks for letting me know!

> 
> Michael
Helge Deller Jan. 24, 2022, 9:52 a.m. UTC | #5
On 1/23/22 23:30, Wei Liu wrote:
> On Sun, Jan 23, 2022 at 10:27:56PM +0000, Michael Kelley (LINUX) wrote:
>> From: Wei Liu <wei.liu@kernel.org> Sent: Sunday, January 23, 2022 1:56 PM
>>>
>>> On Sun, Jan 16, 2022 at 09:53:06PM +0000, Haiyang Zhang wrote:
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Michael Kelley (LINUX) <mikelley@microsoft.com>
>>>>> Sent: Sunday, January 16, 2022 2:19 PM
>>>>> To: KY Srinivasan <kys@microsoft.com>; Haiyang Zhang
>>> <haiyangz@microsoft.com>; Stephen
>>>>> Hemminger <sthemmin@microsoft.com>; wei.liu@kernel.org; Wei Hu
>>> <weh@microsoft.com>; Dexuan
>>>>> Cui <decui@microsoft.com>; drawat.floss@gmail.com; hhei <hhei@redhat.com>;
>>> linux-
>>>>> kernel@vger.kernel.org; linux-hyperv@vger.kernel.org; linux-
>>> fbdev@vger.kernel.org; dri-
>>>>> devel@lists.freedesktop.org
>>>>> Cc: Michael Kelley (LINUX) <mikelley@microsoft.com>
>>>>> Subject: [PATCH 1/1] video: hyperv_fb: Fix validation of screen resolution
>>>>>
>>>>> In the WIN10 version of the Synthetic Video protocol with Hyper-V,
>>>>> Hyper-V reports a list of supported resolutions as part of the protocol
>>>>> negotiation. The driver calculates the maximum width and height from
>>>>> the list of resolutions, and uses those maximums to validate any screen
>>>>> resolution specified in the video= option on the kernel boot line.
>>>>>
>>>>> This method of validation is incorrect. For example, the list of
>>>>> supported resolutions could contain 1600x1200 and 1920x1080, both of
>>>>> which fit in an 8 Mbyte frame buffer.  But calculating the max width
>>>>> and height yields 1920 and 1200, and 1920x1200 resolution does not fit
>>>>> in an 8 Mbyte frame buffer.  Unfortunately, this resolution is accepted,
>>>>> causing a kernel fault when the driver accesses memory outside the
>>>>> frame buffer.
>>>>>
>>>>> Instead, validate the specified screen resolution by calculating
>>>>> its size, and comparing against the frame buffer size.  Delete the
>>>>> code for calculating the max width and height from the list of
>>>>> resolutions, since these max values have no use.  Also add the
>>>>> frame buffer size to the info message to aid in understanding why
>>>>> a resolution might be rejected.
>>>>>
>>>>> Fixes: 67e7cdb4829d ("video: hyperv: hyperv_fb: Obtain screen resolution from Hyper-V
>>>>> host")
>>>>> Signed-off-by: Michael Kelley <mikelley@microsoft.com>
>>> [...]
>>>>
>>>> Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
>>>>
>>>
>>> Applied to hyperv-fixes. Thanks.
>>
>> This fix got pulled into the fbdev/for-next tree by a new maintainer, Helge Deller.
>> See https://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev.git/commit/?h=for-next&id=bcc48f8d980b12e66a3d59dfa1041667db971d86
>
> OK. I will drop it from hyperv-fixes. Thanks for letting me know!

Linus hasn't pulled my tree yet, and he will probably not before the
next merge window. So, if this is an urgent bugfix for you, I can offer
to drop it from the fbdev tree and that you take it through the hyperv-fixes tree.
In that case you may add an Acked-by: Helge Deller <deller@gmx.de>.
Just let me know what you prefer.

Helge
Wei Liu Jan. 24, 2022, 1:31 p.m. UTC | #6
On Mon, Jan 24, 2022 at 10:52:22AM +0100, Helge Deller wrote:
> On 1/23/22 23:30, Wei Liu wrote:
> > On Sun, Jan 23, 2022 at 10:27:56PM +0000, Michael Kelley (LINUX) wrote:
> >> From: Wei Liu <wei.liu@kernel.org> Sent: Sunday, January 23, 2022 1:56 PM
> >>>
> >>> On Sun, Jan 16, 2022 at 09:53:06PM +0000, Haiyang Zhang wrote:
> >>>>
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: Michael Kelley (LINUX) <mikelley@microsoft.com>
> >>>>> Sent: Sunday, January 16, 2022 2:19 PM
> >>>>> To: KY Srinivasan <kys@microsoft.com>; Haiyang Zhang
> >>> <haiyangz@microsoft.com>; Stephen
> >>>>> Hemminger <sthemmin@microsoft.com>; wei.liu@kernel.org; Wei Hu
> >>> <weh@microsoft.com>; Dexuan
> >>>>> Cui <decui@microsoft.com>; drawat.floss@gmail.com; hhei <hhei@redhat.com>;
> >>> linux-
> >>>>> kernel@vger.kernel.org; linux-hyperv@vger.kernel.org; linux-
> >>> fbdev@vger.kernel.org; dri-
> >>>>> devel@lists.freedesktop.org
> >>>>> Cc: Michael Kelley (LINUX) <mikelley@microsoft.com>
> >>>>> Subject: [PATCH 1/1] video: hyperv_fb: Fix validation of screen resolution
> >>>>>
> >>>>> In the WIN10 version of the Synthetic Video protocol with Hyper-V,
> >>>>> Hyper-V reports a list of supported resolutions as part of the protocol
> >>>>> negotiation. The driver calculates the maximum width and height from
> >>>>> the list of resolutions, and uses those maximums to validate any screen
> >>>>> resolution specified in the video= option on the kernel boot line.
> >>>>>
> >>>>> This method of validation is incorrect. For example, the list of
> >>>>> supported resolutions could contain 1600x1200 and 1920x1080, both of
> >>>>> which fit in an 8 Mbyte frame buffer.  But calculating the max width
> >>>>> and height yields 1920 and 1200, and 1920x1200 resolution does not fit
> >>>>> in an 8 Mbyte frame buffer.  Unfortunately, this resolution is accepted,
> >>>>> causing a kernel fault when the driver accesses memory outside the
> >>>>> frame buffer.
> >>>>>
> >>>>> Instead, validate the specified screen resolution by calculating
> >>>>> its size, and comparing against the frame buffer size.  Delete the
> >>>>> code for calculating the max width and height from the list of
> >>>>> resolutions, since these max values have no use.  Also add the
> >>>>> frame buffer size to the info message to aid in understanding why
> >>>>> a resolution might be rejected.
> >>>>>
> >>>>> Fixes: 67e7cdb4829d ("video: hyperv: hyperv_fb: Obtain screen resolution from Hyper-V
> >>>>> host")
> >>>>> Signed-off-by: Michael Kelley <mikelley@microsoft.com>
> >>> [...]
> >>>>
> >>>> Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> >>>>
> >>>
> >>> Applied to hyperv-fixes. Thanks.
> >>
> >> This fix got pulled into the fbdev/for-next tree by a new maintainer, Helge Deller.
> >> See https://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev.git/commit/?h=for-next&id=bcc48f8d980b12e66a3d59dfa1041667db971d86
> >
> > OK. I will drop it from hyperv-fixes. Thanks for letting me know!
> 
> Linus hasn't pulled my tree yet, and he will probably not before the
> next merge window. So, if this is an urgent bugfix for you, I can offer
> to drop it from the fbdev tree and that you take it through the hyperv-fixes tree.
> In that case you may add an Acked-by: Helge Deller <deller@gmx.de>.
> Just let me know what you prefer.

Hi Helge

Yes, I would like to upstream it as soon as possible so that it can
propagate to stable trees and be backported by downstream vendors.

I will pick it up in hyperv-fixes. Please drop it from your for-next
tree.

Thanks,
Wei.

> 
> Helge
Helge Deller Jan. 24, 2022, 1:48 p.m. UTC | #7
On 1/24/22 14:31, Wei Liu wrote:
> On Mon, Jan 24, 2022 at 10:52:22AM +0100, Helge Deller wrote:
>> On 1/23/22 23:30, Wei Liu wrote:
>>> On Sun, Jan 23, 2022 at 10:27:56PM +0000, Michael Kelley (LINUX) wrote:
>>>> From: Wei Liu <wei.liu@kernel.org> Sent: Sunday, January 23, 2022 1:56 PM
>>>>>
>>>>> On Sun, Jan 16, 2022 at 09:53:06PM +0000, Haiyang Zhang wrote:
>>>>>>
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Michael Kelley (LINUX) <mikelley@microsoft.com>
>>>>>>> Sent: Sunday, January 16, 2022 2:19 PM
>>>>>>> To: KY Srinivasan <kys@microsoft.com>; Haiyang Zhang
>>>>> <haiyangz@microsoft.com>; Stephen
>>>>>>> Hemminger <sthemmin@microsoft.com>; wei.liu@kernel.org; Wei Hu
>>>>> <weh@microsoft.com>; Dexuan
>>>>>>> Cui <decui@microsoft.com>; drawat.floss@gmail.com; hhei <hhei@redhat.com>;
>>>>> linux-
>>>>>>> kernel@vger.kernel.org; linux-hyperv@vger.kernel.org; linux-
>>>>> fbdev@vger.kernel.org; dri-
>>>>>>> devel@lists.freedesktop.org
>>>>>>> Cc: Michael Kelley (LINUX) <mikelley@microsoft.com>
>>>>>>> Subject: [PATCH 1/1] video: hyperv_fb: Fix validation of screen resolution
>>>>>>>
>>>>>>> In the WIN10 version of the Synthetic Video protocol with Hyper-V,
>>>>>>> Hyper-V reports a list of supported resolutions as part of the protocol
>>>>>>> negotiation. The driver calculates the maximum width and height from
>>>>>>> the list of resolutions, and uses those maximums to validate any screen
>>>>>>> resolution specified in the video= option on the kernel boot line.
>>>>>>>
>>>>>>> This method of validation is incorrect. For example, the list of
>>>>>>> supported resolutions could contain 1600x1200 and 1920x1080, both of
>>>>>>> which fit in an 8 Mbyte frame buffer.  But calculating the max width
>>>>>>> and height yields 1920 and 1200, and 1920x1200 resolution does not fit
>>>>>>> in an 8 Mbyte frame buffer.  Unfortunately, this resolution is accepted,
>>>>>>> causing a kernel fault when the driver accesses memory outside the
>>>>>>> frame buffer.
>>>>>>>
>>>>>>> Instead, validate the specified screen resolution by calculating
>>>>>>> its size, and comparing against the frame buffer size.  Delete the
>>>>>>> code for calculating the max width and height from the list of
>>>>>>> resolutions, since these max values have no use.  Also add the
>>>>>>> frame buffer size to the info message to aid in understanding why
>>>>>>> a resolution might be rejected.
>>>>>>>
>>>>>>> Fixes: 67e7cdb4829d ("video: hyperv: hyperv_fb: Obtain screen resolution from Hyper-V
>>>>>>> host")
>>>>>>> Signed-off-by: Michael Kelley <mikelley@microsoft.com>
>>>>> [...]
>>>>>>
>>>>>> Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
>>>>>>
>>>>>
>>>>> Applied to hyperv-fixes. Thanks.
>>>>
>>>> This fix got pulled into the fbdev/for-next tree by a new maintainer, Helge Deller.
>>>> See https://git.kernel.org/pub/scm/linux/kernel/git/deller/linux-fbdev.git/commit/?h=for-next&id=bcc48f8d980b12e66a3d59dfa1041667db971d86
>>>
>>> OK. I will drop it from hyperv-fixes. Thanks for letting me know!
>>
>> Linus hasn't pulled my tree yet, and he will probably not before the
>> next merge window. So, if this is an urgent bugfix for you, I can offer
>> to drop it from the fbdev tree and that you take it through the hyperv-fixes tree.
>> In that case you may add an Acked-by: Helge Deller <deller@gmx.de>.
>> Just let me know what you prefer.
>
> Hi Helge
>
> Yes, I would like to upstream it as soon as possible so that it can
> propagate to stable trees and be backported by downstream vendors.
>
> I will pick it up in hyperv-fixes. Please drop it from your for-next
> tree.

Dropped now from fbdev tree.

Thanks!
Helge
Wei Liu Jan. 24, 2022, 2:02 p.m. UTC | #8
On Mon, Jan 24, 2022 at 02:48:57PM +0100, Helge Deller wrote:
> On 1/24/22 14:31, Wei Liu wrote:
[...]
> >>
> >> Linus hasn't pulled my tree yet, and he will probably not before the
> >> next merge window. So, if this is an urgent bugfix for you, I can offer
> >> to drop it from the fbdev tree and that you take it through the hyperv-fixes tree.
> >> In that case you may add an Acked-by: Helge Deller <deller@gmx.de>.
> >> Just let me know what you prefer.
> >
> > Hi Helge
> >
> > Yes, I would like to upstream it as soon as possible so that it can
> > propagate to stable trees and be backported by downstream vendors.
> >
> > I will pick it up in hyperv-fixes. Please drop it from your for-next
> > tree.
> 
> Dropped now from fbdev tree.

Thanks! I added your ack and pushed the patch to hyperv-fixes.

Wei.

> 
> Thanks!
> Helge
diff mbox series

Patch

diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
index 23999df..c8e0ea2 100644
--- a/drivers/video/fbdev/hyperv_fb.c
+++ b/drivers/video/fbdev/hyperv_fb.c
@@ -287,8 +287,6 @@  struct hvfb_par {
 
 static uint screen_width = HVFB_WIDTH;
 static uint screen_height = HVFB_HEIGHT;
-static uint screen_width_max = HVFB_WIDTH;
-static uint screen_height_max = HVFB_HEIGHT;
 static uint screen_depth;
 static uint screen_fb_size;
 static uint dio_fb_size; /* FB size for deferred IO */
@@ -582,7 +580,6 @@  static int synthvid_get_supported_resolution(struct hv_device *hdev)
 	int ret = 0;
 	unsigned long t;
 	u8 index;
-	int i;
 
 	memset(msg, 0, sizeof(struct synthvid_msg));
 	msg->vid_hdr.type = SYNTHVID_RESOLUTION_REQUEST;
@@ -613,13 +610,6 @@  static int synthvid_get_supported_resolution(struct hv_device *hdev)
 		goto out;
 	}
 
-	for (i = 0; i < msg->resolution_resp.resolution_count; i++) {
-		screen_width_max = max_t(unsigned int, screen_width_max,
-		    msg->resolution_resp.supported_resolution[i].width);
-		screen_height_max = max_t(unsigned int, screen_height_max,
-		    msg->resolution_resp.supported_resolution[i].height);
-	}
-
 	screen_width =
 		msg->resolution_resp.supported_resolution[index].width;
 	screen_height =
@@ -941,7 +931,7 @@  static void hvfb_get_option(struct fb_info *info)
 
 	if (x < HVFB_WIDTH_MIN || y < HVFB_HEIGHT_MIN ||
 	    (synthvid_ver_ge(par->synthvid_version, SYNTHVID_VERSION_WIN10) &&
-	    (x > screen_width_max || y > screen_height_max)) ||
+	    (x * y * screen_depth / 8 > screen_fb_size)) ||
 	    (par->synthvid_version == SYNTHVID_VERSION_WIN8 &&
 	     x * y * screen_depth / 8 > SYNTHVID_FB_SIZE_WIN8) ||
 	    (par->synthvid_version == SYNTHVID_VERSION_WIN7 &&
@@ -1194,8 +1184,8 @@  static int hvfb_probe(struct hv_device *hdev,
 	}
 
 	hvfb_get_option(info);
-	pr_info("Screen resolution: %dx%d, Color depth: %d\n",
-		screen_width, screen_height, screen_depth);
+	pr_info("Screen resolution: %dx%d, Color depth: %d, Frame buffer size: %d\n",
+		screen_width, screen_height, screen_depth, screen_fb_size);
 
 	ret = hvfb_getmem(hdev, info);
 	if (ret) {