linux-kernel - [BUG] drivers/video/sis: deadlock introduced by "fbdev: add mutex for fb

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <1246785112.14240.34.camel@falcon>
Date:	Sun, 05 Jul 2009 17:11:52 +0800
From:	Wu Zhangjin <wuzhangjin@...il.com>
To:	linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-mips@...ux-mips.org
Cc:	Krzysztof Helt <krzysztof.h1@...pl>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	"Rafael J. Wysocki" <rjw@...k.pl>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Ralf Baechle <ralf@...ux-mips.org>,
	晏华 <yanh@...ote.com>,
	zhangfx <zhangfx@...ote.com>
Subject: [BUG] drivers/video/sis: deadlock introduced by "fbdev: add mutex
 for fb_mmap locking"

Hi, all

I just tested the sis315 driver in the latest master branch of
linux-mips git repository on fuloong2f(loongson-based machine, loongson
is mips comptable):

Device Drivers  --->
  Graphics support  --->
    <*> Support for frame buffer devices  --->
      <*>   SiS/XGI display support
	[*]     SiS 315/330/340 series and XGI support

The kernel stopped at(output via serial port):

...
sisfb: Detected SiS301C video bridge
sisfb: CRT1 DDC probing failed
sisfb: Default mode is 800x600x8 (60HZ)

no more output, but the Num,Caps light of keyboard works.

and because sis 315 of the latest linux-2.6.30-stable works well on
fuloong2f, so, I tried to check the latest commits who modified sis
relative source code via "git whatchanged", at last I found this: 

commit 537a1bf059fa312355696fa6db80726e655e7f17
Author: Krzysztof Helt <krzysztof.h1@...pl>
Date:   Tue Jun 30 11:41:29 2009 -0700

    fbdev: add mutex for fb_mmap locking
    
    Add a mutex to avoid a circular locking problem between the mm layer
    semaphore and fbdev ioctl mutex through the fb_mmap() call.
    
    Also, add mutex to all places where smem_start and smem_len fields
change
    so the mutex inside the fb_mmap() is actually used.  Changing of
these
    fields before calling the framebuffer_register() are not mutexed.
    
    This is 2.6.31 material.  It removes one lockdep (fb_mmap() and
    register_framebuffer()) but there is still another one (fb_release()
and
    register_framebuffer()).  It also cleans up handling of the
smem_start and
    smem_len fields used by mutexed section of the fb_mmap().
    
    Signed-off-by: Krzysztof Helt <krzysztof.h1@...pl>
    Cc: Peter Zijlstra <a.p.zijlstra@...llo.nl>
    Cc: "Rafael J. Wysocki" <rjw@...k.pl>
    Cc: <stable@...nel.org>
    Signed-off-by: Andrew Morton <akpm@...ux-foundation.org>
    Signed-off-by: Linus Torvalds <torvalds@...ux-foundation.org>

and then, I tried to revert the modification in
drivers/video/sis/sis_main.c:

diff --git a/drivers/video/sis/sis_main.c b/drivers/video/sis/sis_main.c
index fd33455..7072d19 100644
--- a/drivers/video/sis/sis_main.c
+++ b/drivers/video/sis/sis_main.c
@@ -1847,10 +1847,8 @@ sisfb_get_fix(struct fb_fix_screeninfo *fix, int
con, struct fb_info *info)
 
        strcpy(fix->id, ivideo->myid);
 
-       mutex_lock(&info->mm_lock);
        fix->smem_start  = ivideo->video_base + ivideo->video_offset;
        fix->smem_len    = ivideo->sisfb_mem;
-       mutex_unlock(&info->mm_lock);
        fix->type        = FB_TYPE_PACKED_PIXELS;
        fix->type_aux    = 0;
        fix->visual      = (ivideo->video_bpp == 8) ?
FB_VISUAL_PSEUDOCOLOR : FB_VISUAL_TRUECOLOR;	

then it works! so, I guess there is a deadlock introduced by the above
commit.

I attached my config file, hope it can give you more information about
this bug.

Regards,
Wu Zhangjin

View attachment "sis315-deadlock.config" of type "text/plain" (43372 bytes)