[PATCH] vgacon, x86/boot: Prevent faulty bootparams.screeninfo from causing harm

[PATCH] vgacon, x86/boot: Prevent faulty bootparams.screeninfo from causing harm

[Jan H. Schönherr]

If a zero for the number of colums or rows manages to slip through,
gotoxy() will underflow vc->vc_pos, causing the next action on the
referenced memory to end with a page fault.

Make the check in vgacon more pessimistic to prevent that. Also change
a similar check in the x86 boot code with a similar problem.

Signed-off-by: Jan H. Schönherr <jschoenh@amazon.de>
---
Let me know, if I should split this into two patches.

 arch/x86/boot/compressed/misc.c | 3 +--
 drivers/video/console/vgacon.c  | 5 ++---
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index a0838ab..c14217c 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -116,8 +116,7 @@ void __putstr(const char *s)
 		}
 	}
 
-	if (boot_params->screen_info.orig_video_mode = 0 &&
-	    lines = 0 && cols = 0)
+	if (lines = 0 || cols = 0)
 		return;
 
 	x = boot_params->screen_info.orig_x;
diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
index dc06cb6..445b1dc 100644
--- a/drivers/video/console/vgacon.c
+++ b/drivers/video/console/vgacon.c
@@ -398,9 +398,8 @@ static const char *vgacon_startup(void)
 #endif
 	}
 
-	/* boot_params.screen_info initialized? */
-	if ((screen_info.orig_video_mode  = 0) &&
-	    (screen_info.orig_video_lines = 0) &&
+	/* boot_params.screen_info reasonably initialized? */
+	if ((screen_info.orig_video_lines = 0) ||
 	    (screen_info.orig_video_cols  = 0))
 		goto no_vga;
 
-- 
2.3.1.dirty

Re: [PATCH] vgacon, x86/boot: Prevent faulty bootparams.screeninfo from causing harm

[Greg Kroah-Hartman]

On Sun, Aug 27, 2017 at 03:56:46PM +0200, Jan H. Schönherr wrote:
> If a zero for the number of colums or rows manages to slip through,
> gotoxy() will underflow vc->vc_pos, causing the next action on the
> referenced memory to end with a page fault.
> 
> Make the check in vgacon more pessimistic to prevent that. Also change
> a similar check in the x86 boot code with a similar problem.
> 
> Signed-off-by: Jan H. Schönherr <jschoenh@amazon.de>
> ---
> Let me know, if I should split this into two patches.
> 
>  arch/x86/boot/compressed/misc.c | 3 +--
>  drivers/video/console/vgacon.c  | 5 ++---
>  2 files changed, 3 insertions(+), 5 deletions(-)

Why send this patch to me?

And yes, of course you have to split this up, these two files have
nothing to do with each other...

greg k-h

Re: [PATCH] vgacon, x86/boot: Prevent faulty bootparams.screeninfo from causing harm

[Jan H. Schönherr]

On 08/27/2017 05:05 PM, Greg Kroah-Hartman wrote:
> On Sun, Aug 27, 2017 at 03:56:46PM +0200, Jan H. Schönherr wrote:
>> If a zero for the number of colums or rows manages to slip through,
>> gotoxy() will underflow vc->vc_pos, causing the next action on the
>> referenced memory to end with a page fault.
>>
>> Make the check in vgacon more pessimistic to prevent that. Also change
>> a similar check in the x86 boot code with a similar problem.
>>
>> Signed-off-by: Jan H. Schönherr <jschoenh@amazon.de>
>> ---
>> Let me know, if I should split this into two patches.
>>
>>  arch/x86/boot/compressed/misc.c | 3 +--
>>  drivers/video/console/vgacon.c  | 5 ++---
>>  2 files changed, 3 insertions(+), 5 deletions(-)
>
> Why send this patch to me?

Because there isn't a clear maintainer: The drivers/video catch-all in MAINTAINERS
is captioned as "Framebuffer layer",  but all recent patches to vgacon.c went through
your hands.

> And yes, of course you have to split this up, these two files have
> nothing to do with each other...

Okay, then ignore this patch, and I'll split this down. I'll send the vgacon.c part
through Bartlomiej Zolnierkiewicz, then.

Regards
Jan