| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 11 Jan 2017 15:50:43 +0100
From: Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>
To: Takashi Iwai <tiwai@...e.de>
Cc: Tomi Valkeinen <tomi.valkeinen@...com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Jiri Slaby <jslaby@...e.cz>, linux-fbdev@...r.kernel.org,
linux-kernel@...r.kernel.org,
Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] fbcon: Fix vc attr at deinit
Hi,
On Tuesday, January 10, 2017 05:09:43 PM Bartlomiej Zolnierkiewicz wrote:
>
> Hi,
>
> On Tuesday, January 10, 2017 04:47:30 PM Takashi Iwai wrote:
> > On Thu, 05 Jan 2017 13:26:07 +0100,
> > Tomi Valkeinen wrote:
> > >
> > > On 04/01/17 15:50, Takashi Iwai wrote:
> > > > On Tue, 03 Jan 2017 16:03:22 +0100,
> > > > Takashi Iwai wrote:
> > > >>
> > > >> fbcon can deal with vc_hi_font_mask (the upper 256 chars) and adjust
> > > >> the vc attrs dynamically when vc_hi_font_mask is changed at
> > > >> fbcon_init(). When the vc_hi_font_mask is set, it remaps the attrs in
> > > >> the existing console buffer with one bit shift up (for 9 bits), while
> > > >> it remaps with one bit shift down (for 8 bits) when the value is
> > > >> cleared. It works fine as long as the font gets updated after fbcon
> > > >> was initialized.
> > > >>
> > > >> However, we hit a bizarre problem when the console is switched to
> > > >> another fb driver (typically from vesafb or efifb to drmfb). At
> > > >> switching to the new fb driver, we temporarily rebind the console to
> > > >> the dummy console, then rebind to the new driver. During the
> > > >> switching, we leave the modified attrs as is. Thus, the new fbcon
> > > >> takes over the old buffer as if it were to contain 8 bits chars
> > > >> (although the attrs are still shifted for 9 bits), and effectively
> > > >> this results in the yellow color texts instead of the original white
> > > >> color, as found in the bugzilla entry below.
> > > >>
> > > >> An easy fix for this is to re-adjust the attrs before leaving the
> > > >> fbcon at con_deinit callback. Since the code to adjust the attrs is
> > > >> already present in the current fbcon code, in this patch, we simply
> > > >> factor out the relevant code, and call it from fbcon_deinit().
> > > >>
> > > >> Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1000619
> > > >> Signed-off-by: Takashi Iwai <tiwai@...e.de>
> > > >
> > > > Actually not only checkpatch but also I can't find the proper
> > > > maintainer for this...
> > > >
> > > > Tomi, could you check and take if it's OK?
> > > >
> > > > Ideally, this kind of stuff should have been in rather vt side, I
> > > > suppose. But since the code is already present in fbcon, it's easier
> > > > to reuse it as a fix for now.
> > >
> > > I'm not fbdev maintainer anymore. Added Bartlomiej (and Andrew).
> >
> > Bartlomiej (or Andrew), could you check the patch? In case you missed
> > it, I attach it below again.
>
> Thanks, it is on TODO (should be processed till the end of the week).
Patch queued for 4.11, thanks.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
>
> > thanks,
> >
> > Takashi
> >
> > -- 8< --
> > From: Takashi Iwai <tiwai@...e.de>
> > Subject: [PATCH] fbcon: Fix vc attr at deinit
> >
> > fbcon can deal with vc_hi_font_mask (the upper 256 chars) and adjust
> > the vc attrs dynamically when vc_hi_font_mask is changed at
> > fbcon_init(). When the vc_hi_font_mask is set, it remaps the attrs in
> > the existing console buffer with one bit shift up (for 9 bits), while
> > it remaps with one bit shift down (for 8 bits) when the value is
> > cleared. It works fine as long as the font gets updated after fbcon
> > was initialized.
> >
> > However, we hit a bizarre problem when the console is switched to
> > another fb driver (typically from vesafb or efifb to drmfb). At
> > switching to the new fb driver, we temporarily rebind the console to
> > the dummy console, then rebind to the new driver. During the
> > switching, we leave the modified attrs as is. Thus, the new fbcon
> > takes over the old buffer as if it were to contain 8 bits chars
> > (although the attrs are still shifted for 9 bits), and effectively
> > this results in the yellow color texts instead of the original white
> > color, as found in the bugzilla entry below.
> >
> > An easy fix for this is to re-adjust the attrs before leaving the
> > fbcon at con_deinit callback. Since the code to adjust the attrs is
> > already present in the current fbcon code, in this patch, we simply
> > factor out the relevant code, and call it from fbcon_deinit().
> >
> > Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1000619
> > Signed-off-by: Takashi Iwai <tiwai@...e.de>
> > ---
> > drivers/video/console/fbcon.c | 67 ++++++++++++++++++++++++++-----------------
> > 1 file changed, 40 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
> > index a44f5627b82a..f4daadff8a6c 100644
> > --- a/drivers/video/console/fbcon.c
> > +++ b/drivers/video/console/fbcon.c
> > @@ -1165,6 +1165,8 @@ static void fbcon_free_font(struct display *p, bool freefont)
> > p->userfont = 0;
> > }
> >
> > +static void set_vc_hi_font(struct vc_data *vc, bool set);
> > +
> > static void fbcon_deinit(struct vc_data *vc)
> > {
> > struct display *p = &fb_display[vc->vc_num];
> > @@ -1200,6 +1202,9 @@ static void fbcon_deinit(struct vc_data *vc)
> > if (free_font)
> > vc->vc_font.data = NULL;
> >
> > + if (vc->vc_hi_font_mask)
> > + set_vc_hi_font(vc, false);
> > +
> > if (!con_is_bound(&fb_con))
> > fbcon_exit();
> >
> > @@ -2436,32 +2441,10 @@ static int fbcon_get_font(struct vc_data *vc, struct console_font *font)
> > return 0;
> > }
> >
> > -static int fbcon_do_set_font(struct vc_data *vc, int w, int h,
> > - const u8 * data, int userfont)
> > +/* set/clear vc_hi_font_mask and update vc attrs accordingly */
> > +static void set_vc_hi_font(struct vc_data *vc, bool set)
> > {
> > - struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
> > - struct fbcon_ops *ops = info->fbcon_par;
> > - struct display *p = &fb_display[vc->vc_num];
> > - int resize;
> > - int cnt;
> > - char *old_data = NULL;
> > -
> > - if (con_is_visible(vc) && softback_lines)
> > - fbcon_set_origin(vc);
> > -
> > - resize = (w != vc->vc_font.width) || (h != vc->vc_font.height);
> > - if (p->userfont)
> > - old_data = vc->vc_font.data;
> > - if (userfont)
> > - cnt = FNTCHARCNT(data);
> > - else
> > - cnt = 256;
> > - vc->vc_font.data = (void *)(p->fontdata = data);
> > - if ((p->userfont = userfont))
> > - REFCOUNT(data)++;
> > - vc->vc_font.width = w;
> > - vc->vc_font.height = h;
> > - if (vc->vc_hi_font_mask && cnt == 256) {
> > + if (!set) {
> > vc->vc_hi_font_mask = 0;
> > if (vc->vc_can_do_color) {
> > vc->vc_complement_mask >>= 1;
> > @@ -2484,7 +2467,7 @@ static int fbcon_do_set_font(struct vc_data *vc, int w, int h,
> > ((c & 0xfe00) >> 1) | (c & 0xff);
> > vc->vc_attr >>= 1;
> > }
> > - } else if (!vc->vc_hi_font_mask && cnt == 512) {
> > + } else {
> > vc->vc_hi_font_mask = 0x100;
> > if (vc->vc_can_do_color) {
> > vc->vc_complement_mask <<= 1;
> > @@ -2516,8 +2499,38 @@ static int fbcon_do_set_font(struct vc_data *vc, int w, int h,
> > } else
> > vc->vc_video_erase_char = c & ~0x100;
> > }
> > -
> > }
> > +}
> > +
> > +static int fbcon_do_set_font(struct vc_data *vc, int w, int h,
> > + const u8 * data, int userfont)
> > +{
> > + struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
> > + struct fbcon_ops *ops = info->fbcon_par;
> > + struct display *p = &fb_display[vc->vc_num];
> > + int resize;
> > + int cnt;
> > + char *old_data = NULL;
> > +
> > + if (con_is_visible(vc) && softback_lines)
> > + fbcon_set_origin(vc);
> > +
> > + resize = (w != vc->vc_font.width) || (h != vc->vc_font.height);
> > + if (p->userfont)
> > + old_data = vc->vc_font.data;
> > + if (userfont)
> > + cnt = FNTCHARCNT(data);
> > + else
> > + cnt = 256;
> > + vc->vc_font.data = (void *)(p->fontdata = data);
> > + if ((p->userfont = userfont))
> > + REFCOUNT(data)++;
> > + vc->vc_font.width = w;
> > + vc->vc_font.height = h;
> > + if (vc->vc_hi_font_mask && cnt == 256)
> > + set_vc_hi_font(vc, false);
> > + else if (!vc->vc_hi_font_mask && cnt == 512)
> > + set_vc_hi_font(vc, true);
> >
> > if (resize) {
> > int cols, rows;
Powered by blists - more mailing lists