Commit 00a9d699 authored by Mikulas Patocka's avatar Mikulas Patocka Committed by Tomi Valkeinen
Browse files

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: default avatarMikulas Patocka <mpatocka@redhat.com>
Cc: stable@vger.kernel.org
Signed-off-by: default avatarTomi Valkeinen <tomi.valkeinen@ti.com>
parent a772d473

drivers/video/cfbcopyarea.c

+78 −75
Original line number Original line Diff line number Diff line
@@ -43,13 +43,22 @@ 
     */

     */





static void

static void

bitcpy(struct fb_info *p, unsigned long __iomem *dst, int dst_idx,

bitcpy(struct fb_info *p, unsigned long __iomem *dst, unsigned dst_idx,

		const unsigned long __iomem *src, int src_idx, int bits,

		const unsigned long __iomem *src, unsigned src_idx, int bits,

		unsigned n, u32 bswapmask)

		unsigned n, u32 bswapmask)

{

{

	unsigned long first, last;

	unsigned long first, last;

	int const shift = dst_idx-src_idx;

	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);

	first = fb_shifted_pixels_mask_long(p, dst_idx, bswapmask);

	last = ~fb_shifted_pixels_mask_long(p, (dst_idx+n) % bits, 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;

		unsigned long d0, d1;

		int m;

		int m;





		right = shift & (bits - 1);

		int const left = shift & (bits - 1);

		left = -shift & (bits - 1);

		int const right = -shift & (bits - 1);

		bswapmask &= shift;





		if (dst_idx+n <= bits) {

		if (dst_idx+n <= bits) {

			// Single destination word

			// 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);

			d0 = fb_rev_pixels_in_long(d0, bswapmask);

			if (shift > 0) {

			if (shift > 0) {

				// Single source word

				// Single source word

				d0 >>= right;

				d0 <<= left;

			} else if (src_idx+n <= bits) {

			} else if (src_idx+n <= bits) {

				// Single source word

				// Single source word

				d0 <<= left;

				d0 >>= right;

			} else {

			} else {

				// 2 source words

				// 2 source words

				d1 = FB_READL(src + 1);

				d1 = FB_READL(src + 1);

				d1 = fb_rev_pixels_in_long(d1, bswapmask);

				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);

			d0 = fb_rev_pixels_in_long(d0, bswapmask);

			FB_WRITEL(comp(d0, FB_READL(dst), first), dst);

			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) {

			if (shift > 0) {

				// Single source word

				// Single source word

				d1 = d0;

				d1 = d0;

				d0 >>= right;

				d0 <<= left;

				dst++;

				n -= bits - dst_idx;

				n -= bits - dst_idx;

			} else {

			} else {

				// 2 source words

				// 2 source words

				d1 = FB_READL(src++);

				d1 = FB_READL(src++);

				d1 = fb_rev_pixels_in_long(d1, bswapmask);

				d1 = fb_rev_pixels_in_long(d1, bswapmask);





				d0 = d0<<left | d1>>right;

				d0 = d0 >> right | d1 << left;

				dst++;

				n -= bits - dst_idx;

				n -= bits - dst_idx;

			}

			}

			d0 = fb_rev_pixels_in_long(d0, bswapmask);

			d0 = fb_rev_pixels_in_long(d0, bswapmask);

			FB_WRITEL(comp(d0, FB_READL(dst), first), dst);

			FB_WRITEL(comp(d0, FB_READL(dst), first), dst);

			d0 = d1;

			d0 = d1;

			dst++;





			// Main chunk

			// Main chunk

			m = n % bits;

			m = n % bits;

			n /= bits;

			n /= bits;

			while ((n >= 4) && !bswapmask) {

			while ((n >= 4) && !bswapmask) {

				d1 = FB_READL(src++);

				d1 = FB_READL(src++);

				FB_WRITEL(d0 << left | d1 >> right, dst++);

				FB_WRITEL(d0 >> right | d1 << left, dst++);

				d0 = d1;

				d0 = d1;

				d1 = FB_READL(src++);

				d1 = FB_READL(src++);

				FB_WRITEL(d0 << left | d1 >> right, dst++);

				FB_WRITEL(d0 >> right | d1 << left, dst++);

				d0 = d1;

				d0 = d1;

				d1 = FB_READL(src++);

				d1 = FB_READL(src++);

				FB_WRITEL(d0 << left | d1 >> right, dst++);

				FB_WRITEL(d0 >> right | d1 << left, dst++);

				d0 = d1;

				d0 = d1;

				d1 = FB_READL(src++);

				d1 = FB_READL(src++);

				FB_WRITEL(d0 << left | d1 >> right, dst++);

				FB_WRITEL(d0 >> right | d1 << left, dst++);

				d0 = d1;

				d0 = d1;

				n -= 4;

				n -= 4;

			}

			}

			while (n--) {

			while (n--) {

				d1 = FB_READL(src++);

				d1 = FB_READL(src++);

				d1 = fb_rev_pixels_in_long(d1, bswapmask);

				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);

				d0 = fb_rev_pixels_in_long(d0, bswapmask);

				FB_WRITEL(d0, dst++);

				FB_WRITEL(d0, dst++);

				d0 = d1;

				d0 = d1;

			}

			}





			// Trailing bits

			// Trailing bits

			if (last) {

			if (m) {

				if (m <= right) {

				if (m <= bits - right) {

					// Single source word

					// Single source word

					d0 <<= left;

					d0 >>= right;

				} else {

				} else {

					// 2 source words

					// 2 source words

					d1 = FB_READL(src);

					d1 = FB_READL(src);

					d1 = fb_rev_pixels_in_long(d1,

					d1 = fb_rev_pixels_in_long(d1,

								bswapmask);

								bswapmask);

					d0 = d0<<left | d1>>right;

					d0 = d0 >> right | d1 << left;

				}

				}

				d0 = fb_rev_pixels_in_long(d0, bswapmask);

				d0 = fb_rev_pixels_in_long(d0, bswapmask);

				FB_WRITEL(comp(d0, FB_READL(dst), last), dst);

				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

static void

bitcpy_rev(struct fb_info *p, unsigned long __iomem *dst, int dst_idx,

bitcpy_rev(struct fb_info *p, unsigned long __iomem *dst, unsigned dst_idx,

		const unsigned long __iomem *src, int src_idx, int bits,

		const unsigned long __iomem *src, unsigned src_idx, int bits,

		unsigned n, u32 bswapmask)

		unsigned n, u32 bswapmask)

{

{

	unsigned long first, last;

	unsigned long first, last;

	int shift;

	int shift;





	dst += (n-1)/bits;

#if 0

	src += (n-1)/bits;

	/*

	if ((n-1) % bits) {

	 * If you suspect bug in this function, compare it with this simple

		dst_idx += (n-1) % bits;

	 * memmove implementation.

		dst += dst_idx >> (ffs(bits) - 1);

	 */

		dst_idx &= bits - 1;

	fb_memmove((char *)dst + ((dst_idx & (bits - 1))) / 8,

		src_idx += (n-1) % bits;

		   (char *)src + ((src_idx & (bits - 1))) / 8, n / 8);

		src += src_idx >> (ffs(bits) - 1);

	return;

		src_idx &= bits - 1;

#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;

	shift = dst_idx-src_idx;





	first = fb_shifted_pixels_mask_long(p, bits - 1 - dst_idx, bswapmask);

	first = ~fb_shifted_pixels_mask_long(p, (dst_idx + 1) % bits, bswapmask);

	last = ~fb_shifted_pixels_mask_long(p, bits - 1 - ((dst_idx-n) % bits),

	last = fb_shifted_pixels_mask_long(p, (bits + dst_idx + 1 - n) % bits, bswapmask);

					    bswapmask);





	if (!shift) {

	if (!shift) {

		// Same alignment for source and dest

		// Same alignment for source and dest





		if ((unsigned long)dst_idx+1 >= n) {

		if ((unsigned long)dst_idx+1 >= n) {

			// Single word

			// Single word

			if (last)

			if (first)

				first &= last;

				last &= first;

			FB_WRITEL( comp( FB_READL(src), FB_READL(dst), first), dst);

			FB_WRITEL( comp( FB_READL(src), FB_READL(dst), last), dst);

		} else {

		} else {

			// Multiple destination words

			// Multiple destination words





			// Leading bits

			// Leading bits

			if (first != ~0UL) {

			if (first) {

				FB_WRITEL( comp( FB_READL(src), FB_READL(dst), first), dst);

				FB_WRITEL( comp( FB_READL(src), FB_READL(dst), first), dst);

				dst--;

				dst--;

				src--;

				src--;
@@ -262,7 +272,7 @@ bitcpy_rev(struct fb_info *p, unsigned long __iomem *dst, int dst_idx, 
				FB_WRITEL(FB_READL(src--), dst--);

				FB_WRITEL(FB_READL(src--), dst--);





			// Trailing bits

			// Trailing bits

			if (last)

			if (last != -1UL)

				FB_WRITEL( comp( FB_READL(src), FB_READL(dst), last), dst);

				FB_WRITEL( comp( FB_READL(src), FB_READL(dst), last), dst);

		}

		}

	} else {

	} else {
@@ -270,29 +280,28 @@ bitcpy_rev(struct fb_info *p, unsigned long __iomem *dst, int dst_idx, 
		unsigned long d0, d1;

		unsigned long d0, d1;

		int m;

		int m;





		int const left = -shift & (bits-1);

		int const left = shift & (bits-1);

		int const right = shift & (bits-1);

		int const right = -shift & (bits-1);

		bswapmask &= shift;





		if ((unsigned long)dst_idx+1 >= n) {

		if ((unsigned long)dst_idx+1 >= n) {

			// Single destination word

			// Single destination word

			if (last)

			if (first)

				first &= last;

				last &= first;

			d0 = FB_READL(src);

			d0 = FB_READL(src);

			if (shift < 0) {

			if (shift < 0) {

				// Single source word

				// Single source word

				d0 <<= left;

				d0 >>= right;

			} else if (1+(unsigned long)src_idx >= n) {

			} else if (1+(unsigned long)src_idx >= n) {

				// Single source word

				// Single source word

				d0 >>= right;

				d0 <<= left;

			} else {

			} else {

				// 2 source words

				// 2 source words

				d1 = FB_READL(src - 1);

				d1 = FB_READL(src - 1);

				d1 = fb_rev_pixels_in_long(d1, bswapmask);

				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);

			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 {

		} else {

			// Multiple destination words

			// Multiple destination words

			/** We must always remember the last value read, because in case

			/** 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) {

			if (shift < 0) {

				// Single source word

				// Single source word

				d1 = d0;

				d1 = d0;

				d0 <<= left;

				d0 >>= right;

			} else {

			} else {

				// 2 source words

				// 2 source words

				d1 = FB_READL(src--);

				d1 = FB_READL(src--);

				d1 = fb_rev_pixels_in_long(d1, bswapmask);

				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);

			d0 = fb_rev_pixels_in_long(d0, bswapmask);

			FB_WRITEL(comp(d0, FB_READL(dst), first), dst);

			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;

			n /= bits;

			while ((n >= 4) && !bswapmask) {

			while ((n >= 4) && !bswapmask) {

				d1 = FB_READL(src--);

				d1 = FB_READL(src--);

				FB_WRITEL(d0 >> right | d1 << left, dst--);

				FB_WRITEL(d0 << left | d1 >> right, dst--);

				d0 = d1;

				d0 = d1;

				d1 = FB_READL(src--);

				d1 = FB_READL(src--);

				FB_WRITEL(d0 >> right | d1 << left, dst--);

				FB_WRITEL(d0 << left | d1 >> right, dst--);

				d0 = d1;

				d0 = d1;

				d1 = FB_READL(src--);

				d1 = FB_READL(src--);

				FB_WRITEL(d0 >> right | d1 << left, dst--);

				FB_WRITEL(d0 << left | d1 >> right, dst--);

				d0 = d1;

				d0 = d1;

				d1 = FB_READL(src--);

				d1 = FB_READL(src--);

				FB_WRITEL(d0 >> right | d1 << left, dst--);

				FB_WRITEL(d0 << left | d1 >> right, dst--);

				d0 = d1;

				d0 = d1;

				n -= 4;

				n -= 4;

			}

			}

			while (n--) {

			while (n--) {

				d1 = FB_READL(src--);

				d1 = FB_READL(src--);

				d1 = fb_rev_pixels_in_long(d1, bswapmask);

				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);

				d0 = fb_rev_pixels_in_long(d0, bswapmask);

				FB_WRITEL(d0, dst--);

				FB_WRITEL(d0, dst--);

				d0 = d1;

				d0 = d1;

			}

			}





			// Trailing bits

			// Trailing bits

			if (last) {

			if (m) {

				if (m <= left) {

				if (m <= bits - left) {

					// Single source word

					// Single source word

					d0 >>= right;

					d0 <<= left;

				} else {

				} else {

					// 2 source words

					// 2 source words

					d1 = FB_READL(src);

					d1 = FB_READL(src);

					d1 = fb_rev_pixels_in_long(d1,

					d1 = fb_rev_pixels_in_long(d1,

								bswapmask);

								bswapmask);

					d0 = d0>>right | d1<<left;

					d0 = d0 << left | d1 >> right;

				}

				}

				d0 = fb_rev_pixels_in_long(d0, bswapmask);

				d0 = fb_rev_pixels_in_long(d0, bswapmask);

				FB_WRITEL(comp(d0, FB_READL(dst), last), dst);

				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 dx = area->dx, dy = area->dy, sx = area->sx, sy = area->sy;

	u32 height = area->height, width = area->width;

	u32 height = area->height, width = area->width;

	unsigned long const bits_per_line = p->fix.line_length*8u;

	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 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);

	u32 bswapmask = fb_compute_bswapmask(p);





	if (p->state != FBINFO_STATE_RUNNING)

	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

	// split the base of the framebuffer into a long-aligned address and the

	// index of the first bit

	// 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));

	dst_idx = src_idx = 8*((unsigned long)p->screen_base & (bytes-1));

	// add offset of source and target area

	// add offset of source and target area

	dst_idx += dy*bits_per_line + dx*p->var.bits_per_pixel;

	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--) {

		while (height--) {

			dst_idx -= bits_per_line;

			dst_idx -= bits_per_line;

			src_idx -= bits_per_line;

			src_idx -= bits_per_line;

			dst += dst_idx >> (ffs(bits) - 1);

			bitcpy_rev(p, base + (dst_idx / bits), dst_idx % bits,

			dst_idx &= (bytes - 1);

				base + (src_idx / bits), src_idx % bits, bits,

			src += src_idx >> (ffs(bits) - 1);

			src_idx &= (bytes - 1);

			bitcpy_rev(p, dst, dst_idx, src, src_idx, bits,

				width*p->var.bits_per_pixel, bswapmask);

				width*p->var.bits_per_pixel, bswapmask);

		}

		}

	} else {

	} else {

		while (height--) {

		while (height--) {

			dst += dst_idx >> (ffs(bits) - 1);

			bitcpy(p, base + (dst_idx / bits), dst_idx % bits,

			dst_idx &= (bytes - 1);

				base + (src_idx / bits), src_idx % bits, bits,

			src += src_idx >> (ffs(bits) - 1);

			src_idx &= (bytes - 1);

			bitcpy(p, dst, dst_idx, src, src_idx, bits,

				width*p->var.bits_per_pixel, bswapmask);

				width*p->var.bits_per_pixel, bswapmask);

			dst_idx += bits_per_line;

			dst_idx += bits_per_line;

			src_idx += bits_per_line;

			src_idx += bits_per_line;