framebuffer: fix cfb_copyarea

The function cfb_copyarea is buggy when the copy operation is not aligned on
long boundary (4 bytes on 32-bit machines, 8 bytes on 64-bit machines).

How to reproduce:
- use x86-64 machine
- use a framebuffer driver without acceleration (for example uvesafb)
- set the framebuffer to 8-bit depth
	(for example fbset -a 1024x768-60 -depth 8)
- load a font with character width that is not a multiple of 8 pixels
	note: the console-tools package cannot load a font that has
	width different from 8 pixels. You need to install the packages
	"kbd" and "console-terminus" and use the program "setfont" to
	set font width (for example: setfont Uni2-Terminus20x10)
- move some text left and right on the bash command line and you get a
	screen corruption

To expose more bugs, put this line to the end of uvesafb_init_info:
info->flags |= FBINFO_HWACCEL_COPYAREA | FBINFO_READS_FAST;
- Now framebuffer console will use cfb_copyarea for console scrolling.
You get a screen corruption when console is scrolled.

This patch is a rewrite of cfb_copyarea. It fixes the bugs, with this
patch, console scrolling in 8-bit depth with a font width that is not a
multiple of 8 pixels works fine.

The cfb_copyarea code was very buggy and it looks like it was written
and never tried with non-8-pixel font.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Cc: stable@vger.kernel.org
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
This commit is contained in:
Mikulas Patocka 2014-01-23 14:39:29 -05:00 committed by Tomi Valkeinen
parent a772d47366
commit 00a9d699bc

View File

@ -43,13 +43,22 @@
*/
static void
bitcpy(struct fb_info *p, unsigned long __iomem *dst, int dst_idx,
const unsigned long __iomem *src, int src_idx, int bits,
bitcpy(struct fb_info *p, unsigned long __iomem *dst, unsigned dst_idx,
const unsigned long __iomem *src, unsigned src_idx, int bits,
unsigned n, u32 bswapmask)
{
unsigned long first, last;
int const shift = dst_idx-src_idx;
int left, right;
#if 0
/*
* If you suspect bug in this function, compare it with this simple
* memmove implementation.
*/
fb_memmove((char *)dst + ((dst_idx & (bits - 1))) / 8,
(char *)src + ((src_idx & (bits - 1))) / 8, n / 8);
return;
#endif
first = fb_shifted_pixels_mask_long(p, dst_idx, bswapmask);
last = ~fb_shifted_pixels_mask_long(p, (dst_idx+n) % bits, bswapmask);
@ -98,9 +107,8 @@ bitcpy(struct fb_info *p, unsigned long __iomem *dst, int dst_idx,
unsigned long d0, d1;
int m;
right = shift & (bits - 1);
left = -shift & (bits - 1);
bswapmask &= shift;
int const left = shift & (bits - 1);
int const right = -shift & (bits - 1);
if (dst_idx+n <= bits) {
// Single destination word
@ -110,15 +118,15 @@ bitcpy(struct fb_info *p, unsigned long __iomem *dst, int dst_idx,
d0 = fb_rev_pixels_in_long(d0, bswapmask);
if (shift > 0) {
// Single source word
d0 >>= right;
d0 <<= left;
} else if (src_idx+n <= bits) {
// Single source word
d0 <<= left;
d0 >>= right;
} else {
// 2 source words
d1 = FB_READL(src + 1);
d1 = fb_rev_pixels_in_long(d1, bswapmask);
d0 = d0<<left | d1>>right;
d0 = d0 >> right | d1 << left;
}
d0 = fb_rev_pixels_in_long(d0, bswapmask);
FB_WRITEL(comp(d0, FB_READL(dst), first), dst);
@ -135,60 +143,59 @@ bitcpy(struct fb_info *p, unsigned long __iomem *dst, int dst_idx,
if (shift > 0) {
// Single source word
d1 = d0;
d0 >>= right;
dst++;
d0 <<= left;
n -= bits - dst_idx;
} else {
// 2 source words
d1 = FB_READL(src++);
d1 = fb_rev_pixels_in_long(d1, bswapmask);
d0 = d0<<left | d1>>right;
dst++;
d0 = d0 >> right | d1 << left;
n -= bits - dst_idx;
}
d0 = fb_rev_pixels_in_long(d0, bswapmask);
FB_WRITEL(comp(d0, FB_READL(dst), first), dst);
d0 = d1;
dst++;
// Main chunk
m = n % bits;
n /= bits;
while ((n >= 4) && !bswapmask) {
d1 = FB_READL(src++);
FB_WRITEL(d0 << left | d1 >> right, dst++);
FB_WRITEL(d0 >> right | d1 << left, dst++);
d0 = d1;
d1 = FB_READL(src++);
FB_WRITEL(d0 << left | d1 >> right, dst++);
FB_WRITEL(d0 >> right | d1 << left, dst++);
d0 = d1;
d1 = FB_READL(src++);
FB_WRITEL(d0 << left | d1 >> right, dst++);
FB_WRITEL(d0 >> right | d1 << left, dst++);
d0 = d1;
d1 = FB_READL(src++);
FB_WRITEL(d0 << left | d1 >> right, dst++);
FB_WRITEL(d0 >> right | d1 << left, dst++);
d0 = d1;
n -= 4;
}
while (n--) {
d1 = FB_READL(src++);
d1 = fb_rev_pixels_in_long(d1, bswapmask);
d0 = d0 << left | d1 >> right;
d0 = d0 >> right | d1 << left;
d0 = fb_rev_pixels_in_long(d0, bswapmask);
FB_WRITEL(d0, dst++);
d0 = d1;
}
// Trailing bits
if (last) {
if (m <= right) {
if (m) {
if (m <= bits - right) {
// Single source word
d0 <<= left;
d0 >>= right;
} else {
// 2 source words
d1 = FB_READL(src);
d1 = fb_rev_pixels_in_long(d1,
bswapmask);
d0 = d0<<left | d1>>right;
d0 = d0 >> right | d1 << left;
}
d0 = fb_rev_pixels_in_long(d0, bswapmask);
FB_WRITEL(comp(d0, FB_READL(dst), last), dst);
@ -202,43 +209,46 @@ bitcpy(struct fb_info *p, unsigned long __iomem *dst, int dst_idx,
*/
static void
bitcpy_rev(struct fb_info *p, unsigned long __iomem *dst, int dst_idx,
const unsigned long __iomem *src, int src_idx, int bits,
bitcpy_rev(struct fb_info *p, unsigned long __iomem *dst, unsigned dst_idx,
const unsigned long __iomem *src, unsigned src_idx, int bits,
unsigned n, u32 bswapmask)
{
unsigned long first, last;
int shift;
dst += (n-1)/bits;
src += (n-1)/bits;
if ((n-1) % bits) {
dst_idx += (n-1) % bits;
dst += dst_idx >> (ffs(bits) - 1);
dst_idx &= bits - 1;
src_idx += (n-1) % bits;
src += src_idx >> (ffs(bits) - 1);
src_idx &= bits - 1;
}
#if 0
/*
* If you suspect bug in this function, compare it with this simple
* memmove implementation.
*/
fb_memmove((char *)dst + ((dst_idx & (bits - 1))) / 8,
(char *)src + ((src_idx & (bits - 1))) / 8, n / 8);
return;
#endif
dst += (dst_idx + n - 1) / bits;
src += (src_idx + n - 1) / bits;
dst_idx = (dst_idx + n - 1) % bits;
src_idx = (src_idx + n - 1) % bits;
shift = dst_idx-src_idx;
first = fb_shifted_pixels_mask_long(p, bits - 1 - dst_idx, bswapmask);
last = ~fb_shifted_pixels_mask_long(p, bits - 1 - ((dst_idx-n) % bits),
bswapmask);
first = ~fb_shifted_pixels_mask_long(p, (dst_idx + 1) % bits, bswapmask);
last = fb_shifted_pixels_mask_long(p, (bits + dst_idx + 1 - n) % bits, bswapmask);
if (!shift) {
// Same alignment for source and dest
if ((unsigned long)dst_idx+1 >= n) {
// Single word
if (last)
first &= last;
FB_WRITEL( comp( FB_READL(src), FB_READL(dst), first), dst);
if (first)
last &= first;
FB_WRITEL( comp( FB_READL(src), FB_READL(dst), last), dst);
} else {
// Multiple destination words
// Leading bits
if (first != ~0UL) {
if (first) {
FB_WRITEL( comp( FB_READL(src), FB_READL(dst), first), dst);
dst--;
src--;
@ -262,7 +272,7 @@ bitcpy_rev(struct fb_info *p, unsigned long __iomem *dst, int dst_idx,
FB_WRITEL(FB_READL(src--), dst--);
// Trailing bits
if (last)
if (last != -1UL)
FB_WRITEL( comp( FB_READL(src), FB_READL(dst), last), dst);
}
} else {
@ -270,29 +280,28 @@ bitcpy_rev(struct fb_info *p, unsigned long __iomem *dst, int dst_idx,
unsigned long d0, d1;
int m;
int const left = -shift & (bits-1);
int const right = shift & (bits-1);
bswapmask &= shift;
int const left = shift & (bits-1);
int const right = -shift & (bits-1);
if ((unsigned long)dst_idx+1 >= n) {
// Single destination word
if (last)
first &= last;
if (first)
last &= first;
d0 = FB_READL(src);
if (shift < 0) {
// Single source word
d0 <<= left;
d0 >>= right;
} else if (1+(unsigned long)src_idx >= n) {
// Single source word
d0 >>= right;
d0 <<= left;
} else {
// 2 source words
d1 = FB_READL(src - 1);
d1 = fb_rev_pixels_in_long(d1, bswapmask);
d0 = d0>>right | d1<<left;
d0 = d0 << left | d1 >> right;
}
d0 = fb_rev_pixels_in_long(d0, bswapmask);
FB_WRITEL(comp(d0, FB_READL(dst), first), dst);
FB_WRITEL(comp(d0, FB_READL(dst), last), dst);
} else {
// Multiple destination words
/** We must always remember the last value read, because in case
@ -307,12 +316,12 @@ bitcpy_rev(struct fb_info *p, unsigned long __iomem *dst, int dst_idx,
if (shift < 0) {
// Single source word
d1 = d0;
d0 <<= left;
d0 >>= right;
} else {
// 2 source words
d1 = FB_READL(src--);
d1 = fb_rev_pixels_in_long(d1, bswapmask);
d0 = d0>>right | d1<<left;
d0 = d0 << left | d1 >> right;
}
d0 = fb_rev_pixels_in_long(d0, bswapmask);
FB_WRITEL(comp(d0, FB_READL(dst), first), dst);
@ -325,39 +334,39 @@ bitcpy_rev(struct fb_info *p, unsigned long __iomem *dst, int dst_idx,
n /= bits;
while ((n >= 4) && !bswapmask) {
d1 = FB_READL(src--);
FB_WRITEL(d0 >> right | d1 << left, dst--);
FB_WRITEL(d0 << left | d1 >> right, dst--);
d0 = d1;
d1 = FB_READL(src--);
FB_WRITEL(d0 >> right | d1 << left, dst--);
FB_WRITEL(d0 << left | d1 >> right, dst--);
d0 = d1;
d1 = FB_READL(src--);
FB_WRITEL(d0 >> right | d1 << left, dst--);
FB_WRITEL(d0 << left | d1 >> right, dst--);
d0 = d1;
d1 = FB_READL(src--);
FB_WRITEL(d0 >> right | d1 << left, dst--);
FB_WRITEL(d0 << left | d1 >> right, dst--);
d0 = d1;
n -= 4;
}
while (n--) {
d1 = FB_READL(src--);
d1 = fb_rev_pixels_in_long(d1, bswapmask);
d0 = d0 >> right | d1 << left;
d0 = d0 << left | d1 >> right;
d0 = fb_rev_pixels_in_long(d0, bswapmask);
FB_WRITEL(d0, dst--);
d0 = d1;
}
// Trailing bits
if (last) {
if (m <= left) {
if (m) {
if (m <= bits - left) {
// Single source word
d0 >>= right;
d0 <<= left;
} else {
// 2 source words
d1 = FB_READL(src);
d1 = fb_rev_pixels_in_long(d1,
bswapmask);
d0 = d0>>right | d1<<left;
d0 = d0 << left | d1 >> right;
}
d0 = fb_rev_pixels_in_long(d0, bswapmask);
FB_WRITEL(comp(d0, FB_READL(dst), last), dst);
@ -371,9 +380,9 @@ void cfb_copyarea(struct fb_info *p, const struct fb_copyarea *area)
u32 dx = area->dx, dy = area->dy, sx = area->sx, sy = area->sy;
u32 height = area->height, width = area->width;
unsigned long const bits_per_line = p->fix.line_length*8u;
unsigned long __iomem *dst = NULL, *src = NULL;
unsigned long __iomem *base = NULL;
int bits = BITS_PER_LONG, bytes = bits >> 3;
int dst_idx = 0, src_idx = 0, rev_copy = 0;
unsigned dst_idx = 0, src_idx = 0, rev_copy = 0;
u32 bswapmask = fb_compute_bswapmask(p);
if (p->state != FBINFO_STATE_RUNNING)
@ -389,7 +398,7 @@ void cfb_copyarea(struct fb_info *p, const struct fb_copyarea *area)
// split the base of the framebuffer into a long-aligned address and the
// index of the first bit
dst = src = (unsigned long __iomem *)((unsigned long)p->screen_base & ~(bytes-1));
base = (unsigned long __iomem *)((unsigned long)p->screen_base & ~(bytes-1));
dst_idx = src_idx = 8*((unsigned long)p->screen_base & (bytes-1));
// add offset of source and target area
dst_idx += dy*bits_per_line + dx*p->var.bits_per_pixel;
@ -402,20 +411,14 @@ void cfb_copyarea(struct fb_info *p, const struct fb_copyarea *area)
while (height--) {
dst_idx -= bits_per_line;
src_idx -= bits_per_line;
dst += dst_idx >> (ffs(bits) - 1);
dst_idx &= (bytes - 1);
src += src_idx >> (ffs(bits) - 1);
src_idx &= (bytes - 1);
bitcpy_rev(p, dst, dst_idx, src, src_idx, bits,
bitcpy_rev(p, base + (dst_idx / bits), dst_idx % bits,
base + (src_idx / bits), src_idx % bits, bits,
width*p->var.bits_per_pixel, bswapmask);
}
} else {
while (height--) {
dst += dst_idx >> (ffs(bits) - 1);
dst_idx &= (bytes - 1);
src += src_idx >> (ffs(bits) - 1);
src_idx &= (bytes - 1);
bitcpy(p, dst, dst_idx, src, src_idx, bits,
bitcpy(p, base + (dst_idx / bits), dst_idx % bits,
base + (src_idx / bits), src_idx % bits, bits,
width*p->var.bits_per_pixel, bswapmask);
dst_idx += bits_per_line;
src_idx += bits_per_line;