|
From: Andrew M. <ak...@zi...> - 2002-01-28 22:19:52
|
Andrew Morton wrote:
>
> Daniel Jacobowitz wrote:
> >
> > Frame buffers aren't reliable marked VM_IO when mapped, currently. Ben
> > H. said he was going to push a fix for this at least to the PPC trees
> > today or tomorrow.
>
> They are now, I hope. I fixed that in 2.4.18-pre2.
> drivers/video/fbmem.c:fb_mmap() marks the vma as
> VM_IO for all architectures. But perhaps I missed some;
> an audit is needed in there, which I'll do.
I lied. Linus applied it, but not, it seems, Marcelo.
So. Here's a patch.
It marks all framebuffer mappings as VM_IO. This prevents
kernel deadlocks which can occur when a program which
has a framebuffer mapping attempts to dump core.
It also allows get_user_pages() to detect and skip these
IO mappings, so ptrace, O_DIRECT, etc will not permit
I/O against these mappings.
I've Cc'ed linux-fbdev-devel. Could someone please
review?
--- linux-2.4.18-pre7/drivers/video/acornfb.c Thu Nov 22 23:02:58 2001
+++ linux-akpm/drivers/video/acornfb.c Mon Jan 28 14:00:21 2002
@@ -1139,9 +1139,6 @@ acornfb_mmap(struct fb_info *info, struc
off += start;
vma->vm_pgoff = off >> PAGE_SHIFT;
- /* This is an IO map - tell maydump to skip this VMA */
- vma->vm_flags |= VM_IO;
-
#ifdef CONFIG_CPU_32
pgprot_val(vma->vm_page_prot) &= ~L_PTE_CACHEABLE;
#endif
--- linux-2.4.18-pre7/drivers/video/igafb.c Thu Nov 22 23:02:58 2001
+++ linux-akpm/drivers/video/igafb.c Mon Jan 28 14:02:10 2002
@@ -293,8 +293,6 @@ static int igafb_mmap(struct fb_info *in
if (!map_size)
return -EINVAL;
- vma->vm_flags |= VM_IO;
-
if (!fb->mmaped) {
int lastconsole = 0;
--- linux-2.4.18-pre7/drivers/video/sgivwfb.c Thu Nov 22 23:02:58 2001
+++ linux-akpm/drivers/video/sgivwfb.c Mon Jan 28 14:02:49 2002
@@ -846,7 +846,6 @@ static int sgivwfb_mmap(struct fb_info *
return -EINVAL;
offset += sgivwfb_mem_phys;
pgprot_val(vma->vm_page_prot) = pgprot_val(vma->vm_page_prot) | _PAGE_PCD;
- vma->vm_flags |= VM_IO;
if (remap_page_range(vma->vm_start, offset, size, vma->vm_page_prot))
return -EAGAIN;
vma->vm_file = file;
--- linux-2.4.18-pre7/drivers/video/fbmem.c Fri Dec 21 11:19:14 2001
+++ linux-akpm/drivers/video/fbmem.c Mon Jan 28 14:07:08 2002
@@ -543,6 +543,8 @@ fb_mmap(struct file *file, struct vm_are
lock_kernel();
res = fb->fb_mmap(info, file, vma);
unlock_kernel();
+ /* This is an IO map - tell maydump to skip this VMA */
+ vma->vm_flags |= VM_IO;
return res;
}
@@ -576,12 +578,13 @@ fb_mmap(struct file *file, struct vm_are
return -EINVAL;
off += start;
vma->vm_pgoff = off >> PAGE_SHIFT;
+ /* This is an IO map - tell maydump to skip this VMA */
+ vma->vm_flags |= VM_IO;
#if defined(__sparc_v9__)
vma->vm_flags |= (VM_SHM | VM_LOCKED);
if (io_remap_page_range(vma->vm_start, off,
vma->vm_end - vma->vm_start, vma->vm_page_prot, 0))
return -EAGAIN;
- vma->vm_flags |= VM_IO;
#else
#if defined(__mc68000__)
#if defined(CONFIG_SUN3)
@@ -607,8 +610,6 @@ fb_mmap(struct file *file, struct vm_are
pgprot_val(vma->vm_page_prot) |= _CACHE_UNCACHED;
#elif defined(__arm__)
vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
- /* This is an IO map - tell maydump to skip this VMA */
- vma->vm_flags |= VM_IO;
#elif defined(__sh__)
pgprot_val(vma->vm_page_prot) &= ~_PAGE_CACHABLE;
#else
|
|
From: James S. <jsi...@tr...> - 2002-01-29 22:59:52
|
> It marks all framebuffer mappings as VM_IO. This prevents > kernel deadlocks which can occur when a program which > has a framebuffer mapping attempts to dump core. Good this is needed. > I've Cc'ed linux-fbdev-devel. Could someone please > review? > --- linux-2.4.18-pre7/drivers/video/acornfb.c Thu Nov 22 23:02:58 2001 You missed the atyfb driver. Other then that it looks fine. Geert what do you think? |
|
From: Andrew M. <ak...@zi...> - 2002-01-29 23:10:09
|
James Simmons wrote:
>
> > It marks all framebuffer mappings as VM_IO. This prevents
> > kernel deadlocks which can occur when a program which
> > has a framebuffer mapping attempts to dump core.
>
> Good this is needed.
>
> > I've Cc'ed linux-fbdev-devel. Could someone please
> > review?
>
> > --- linux-2.4.18-pre7/drivers/video/acornfb.c Thu Nov 22 23:02:58 2001
>
> You missed the atyfb driver. Other then that it looks fine. Geert what do
> you think?
Ah, thanks. Also I missed sbusfb.c (not that it was fatal - the driver
got it right anyway).
Here's the current patch. After some discussion with Ben
Herrenschmidt, it now sets VM_IO against the vma *before*
calling the subdriver's mmap method, just in case the
driver really does want to clear that flag (presumably, a shadow
buffer in main memory).
--- linux-2.4.18-pre7/drivers/video/acornfb.c Thu Nov 22 23:02:58 2001
+++ linux-akpm/drivers/video/acornfb.c Tue Jan 29 14:57:00 2002
@@ -1139,9 +1139,6 @@ acornfb_mmap(struct fb_info *info, struc
off += start;
vma->vm_pgoff = off >> PAGE_SHIFT;
- /* This is an IO map - tell maydump to skip this VMA */
- vma->vm_flags |= VM_IO;
-
#ifdef CONFIG_CPU_32
pgprot_val(vma->vm_page_prot) &= ~L_PTE_CACHEABLE;
#endif
--- linux-2.4.18-pre7/drivers/video/igafb.c Thu Nov 22 23:02:58 2001
+++ linux-akpm/drivers/video/igafb.c Tue Jan 29 14:57:00 2002
@@ -293,8 +293,6 @@ static int igafb_mmap(struct fb_info *in
if (!map_size)
return -EINVAL;
- vma->vm_flags |= VM_IO;
-
if (!fb->mmaped) {
int lastconsole = 0;
--- linux-2.4.18-pre7/drivers/video/sgivwfb.c Thu Nov 22 23:02:58 2001
+++ linux-akpm/drivers/video/sgivwfb.c Tue Jan 29 14:57:00 2002
@@ -846,7 +846,6 @@ static int sgivwfb_mmap(struct fb_info *
return -EINVAL;
offset += sgivwfb_mem_phys;
pgprot_val(vma->vm_page_prot) = pgprot_val(vma->vm_page_prot) | _PAGE_PCD;
- vma->vm_flags |= VM_IO;
if (remap_page_range(vma->vm_start, offset, size, vma->vm_page_prot))
return -EAGAIN;
vma->vm_file = file;
--- linux-2.4.18-pre7/drivers/video/fbmem.c Fri Dec 21 11:19:14 2001
+++ linux-akpm/drivers/video/fbmem.c Tue Jan 29 14:57:00 2002
@@ -541,6 +541,16 @@ fb_mmap(struct file *file, struct vm_are
if (fb->fb_mmap) {
int res;
lock_kernel();
+ /*
+ * This is an IO map - tell maydump to skip this VMA.
+ * If, for some reason, the sub-driver wishes to allow the
+ * mapping to be dumpable and accessible via ptrace then
+ * it may clear VM_IO later.
+ * (This only makes sense if the buffer is actually
+ * in main memory, and is described by a page struct in
+ * mem_map[])
+ */
+ vma->vm_flags |= VM_IO;
res = fb->fb_mmap(info, file, vma);
unlock_kernel();
return res;
@@ -576,12 +586,13 @@ fb_mmap(struct file *file, struct vm_are
return -EINVAL;
off += start;
vma->vm_pgoff = off >> PAGE_SHIFT;
+ /* This is an IO map - tell maydump to skip this VMA */
+ vma->vm_flags |= VM_IO;
#if defined(__sparc_v9__)
vma->vm_flags |= (VM_SHM | VM_LOCKED);
if (io_remap_page_range(vma->vm_start, off,
vma->vm_end - vma->vm_start, vma->vm_page_prot, 0))
return -EAGAIN;
- vma->vm_flags |= VM_IO;
#else
#if defined(__mc68000__)
#if defined(CONFIG_SUN3)
@@ -607,8 +618,6 @@ fb_mmap(struct file *file, struct vm_are
pgprot_val(vma->vm_page_prot) |= _CACHE_UNCACHED;
#elif defined(__arm__)
vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
- /* This is an IO map - tell maydump to skip this VMA */
- vma->vm_flags |= VM_IO;
#elif defined(__sh__)
pgprot_val(vma->vm_page_prot) &= ~_PAGE_CACHABLE;
#else
--- linux-2.4.18-pre7/drivers/video/sbusfb.c Thu Sep 13 16:04:43 2001
+++ linux-akpm/drivers/video/sbusfb.c Tue Jan 29 14:58:59 2002
@@ -220,7 +220,6 @@ static int sbusfb_mmap(struct fb_info *i
page += map_size;
}
- vma->vm_flags |= VM_IO;
if (!fb->mmaped) {
int lastconsole = 0;
--- linux-2.4.18-pre7/drivers/video/aty/atyfb_base.c Wed Jan 23 15:11:34 2002
+++ linux-akpm/drivers/video/aty/atyfb_base.c Tue Jan 29 14:56:46 2002
@@ -1426,8 +1426,6 @@ static int atyfb_mmap(struct fb_info *in
if (!map_size)
return -EINVAL;
- vma->vm_flags |= VM_IO;
-
if (!fb->mmaped) {
int lastconsole = 0;
|
|
From: James S. <jsi...@tr...> - 2002-01-30 00:13:39
|
> Ah, thanks. Also I missed sbusfb.c (not that it was fatal - the driver
> got it right anyway).
Great.
> Here's the current patch. After some discussion with Ben
> Herrenschmidt, it now sets VM_IO against the vma *before*
> calling the subdriver's mmap method, just in case the
> driver really does want to clear that flag (presumably, a shadow
> buffer in main memory).
I also thought about it as well. Never thought about the issue with shadow
buffers. Is it safe to move it before if (fb->fb_mmap)?
> --- linux-2.4.18-pre7/drivers/video/fbmem.c Fri Dec 21 11:19:14 2001
> +++ linux-akpm/drivers/video/fbmem.c Tue Jan 29 14:57:00 2002
> @@ -541,6 +541,16 @@ fb_mmap(struct file *file, struct vm_are
> if (fb->fb_mmap) {
> int res;
> lock_kernel();
> + /*
> + * This is an IO map - tell maydump to skip this VMA.
> + * If, for some reason, the sub-driver wishes to allow the
> + * mapping to be dumpable and accessible via ptrace then
> + * it may clear VM_IO later.
> + * (This only makes sense if the buffer is actually
> + * in main memory, and is described by a page struct in
> + * mem_map[])
> + */
> + vma->vm_flags |= VM_IO;
> res = fb->fb_mmap(info, file, vma);
> unlock_kernel();
> return res;
|