kernel_optimize_test/fs/sysfs/bin.c

280 lines
5.9 KiB
C
Raw Normal View History

/*
* fs/sysfs/bin.c - sysfs binary file implementation
*
* Copyright (c) 2003 Patrick Mochel
* Copyright (c) 2003 Matthew Wilcox
* Copyright (c) 2004 Silicon Graphics, Inc.
* Copyright (c) 2007 SUSE Linux Products GmbH
* Copyright (c) 2007 Tejun Heo <teheo@suse.de>
*
* This file is released under the GPLv2.
*
* Please see Documentation/filesystems/sysfs.txt for more information.
*/
#undef DEBUG
#include <linux/errno.h>
#include <linux/fs.h>
#include <linux/kernel.h>
#include <linux/kobject.h>
#include <linux/module.h>
#include <linux/slab.h>
#include <linux/mutex.h>
#include <asm/uaccess.h>
#include "sysfs.h"
struct bin_buffer {
struct mutex mutex;
void *buffer;
sysfs: implement sysfs_dirent active reference and immediate disconnect sysfs: implement sysfs_dirent active reference and immediate disconnect Opening a sysfs node references its associated kobject, so userland can arbitrarily prolong lifetime of a kobject which complicates lifetime rules in drivers. This patch implements active reference and makes the association between kobject and sysfs immediately breakable. Now each sysfs_dirent has two reference counts - s_count and s_active. s_count is a regular reference count which guarantees that the containing sysfs_dirent is accessible. As long as s_count reference is held, all sysfs internal fields in sysfs_dirent are accessible including s_parent and s_name. The newly added s_active is active reference count. This is acquired by invoking sysfs_get_active() and it's the caller's responsibility to ensure sysfs_dirent itself is accessible (should be holding s_count one way or the other). Dereferencing sysfs_dirent to access objects out of sysfs proper requires active reference. This includes access to the associated kobjects, attributes and ops. The active references can be drained and denied by calling sysfs_deactivate(). All active sysfs_dirents must be deactivated after deletion but before the default reference is dropped. This enables immediate disconnect of sysfs nodes. Once a sysfs_dirent is deleted, it won't access any entity external to sysfs proper. Because attr/bin_attr ops access both the node itself and its parent for kobject, they need to hold active references to both. sysfs_get/put_active_two() helpers are provided to help grabbing both references. Parent's is acquired first and released last. Unlike other operations, mmapped area lingers on after mmap() is finished and the module implement implementing it and kobj need to stay referenced till all the mapped pages are gone. This is accomplished by holding one set of active references to the bin_attr and its parent if there have been any mmap during lifetime of an openfile. The references are dropped when the openfile is released. This change makes sysfs lifetime rules independent from both kobject's and module's. It not only fixes several race conditions caused by sysfs not holding onto the proper module when referencing kobject, but also helps fixing and simplifying lifetime management in driver model and drivers by taking sysfs out of the equation. Please read the following message for more info. http://article.gmane.org/gmane.linux.kernel/510293 Signed-off-by: Tejun Heo <htejun@gmail.com> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2007-06-14 02:45:16 +08:00
int mmapped;
};
static int
fill_read(struct dentry *dentry, char *buffer, loff_t off, size_t count)
{
struct sysfs_dirent *attr_sd = dentry->d_fsdata;
struct bin_attribute *attr = attr_sd->s_bin_attr.bin_attr;
struct kobject *kobj = attr_sd->s_parent->s_dir.kobj;
sysfs: implement sysfs_dirent active reference and immediate disconnect sysfs: implement sysfs_dirent active reference and immediate disconnect Opening a sysfs node references its associated kobject, so userland can arbitrarily prolong lifetime of a kobject which complicates lifetime rules in drivers. This patch implements active reference and makes the association between kobject and sysfs immediately breakable. Now each sysfs_dirent has two reference counts - s_count and s_active. s_count is a regular reference count which guarantees that the containing sysfs_dirent is accessible. As long as s_count reference is held, all sysfs internal fields in sysfs_dirent are accessible including s_parent and s_name. The newly added s_active is active reference count. This is acquired by invoking sysfs_get_active() and it's the caller's responsibility to ensure sysfs_dirent itself is accessible (should be holding s_count one way or the other). Dereferencing sysfs_dirent to access objects out of sysfs proper requires active reference. This includes access to the associated kobjects, attributes and ops. The active references can be drained and denied by calling sysfs_deactivate(). All active sysfs_dirents must be deactivated after deletion but before the default reference is dropped. This enables immediate disconnect of sysfs nodes. Once a sysfs_dirent is deleted, it won't access any entity external to sysfs proper. Because attr/bin_attr ops access both the node itself and its parent for kobject, they need to hold active references to both. sysfs_get/put_active_two() helpers are provided to help grabbing both references. Parent's is acquired first and released last. Unlike other operations, mmapped area lingers on after mmap() is finished and the module implement implementing it and kobj need to stay referenced till all the mapped pages are gone. This is accomplished by holding one set of active references to the bin_attr and its parent if there have been any mmap during lifetime of an openfile. The references are dropped when the openfile is released. This change makes sysfs lifetime rules independent from both kobject's and module's. It not only fixes several race conditions caused by sysfs not holding onto the proper module when referencing kobject, but also helps fixing and simplifying lifetime management in driver model and drivers by taking sysfs out of the equation. Please read the following message for more info. http://article.gmane.org/gmane.linux.kernel/510293 Signed-off-by: Tejun Heo <htejun@gmail.com> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2007-06-14 02:45:16 +08:00
int rc;
/* need attr_sd for attr, its parent for kobj */
if (!sysfs_get_active_two(attr_sd))
return -ENODEV;
sysfs: implement sysfs_dirent active reference and immediate disconnect sysfs: implement sysfs_dirent active reference and immediate disconnect Opening a sysfs node references its associated kobject, so userland can arbitrarily prolong lifetime of a kobject which complicates lifetime rules in drivers. This patch implements active reference and makes the association between kobject and sysfs immediately breakable. Now each sysfs_dirent has two reference counts - s_count and s_active. s_count is a regular reference count which guarantees that the containing sysfs_dirent is accessible. As long as s_count reference is held, all sysfs internal fields in sysfs_dirent are accessible including s_parent and s_name. The newly added s_active is active reference count. This is acquired by invoking sysfs_get_active() and it's the caller's responsibility to ensure sysfs_dirent itself is accessible (should be holding s_count one way or the other). Dereferencing sysfs_dirent to access objects out of sysfs proper requires active reference. This includes access to the associated kobjects, attributes and ops. The active references can be drained and denied by calling sysfs_deactivate(). All active sysfs_dirents must be deactivated after deletion but before the default reference is dropped. This enables immediate disconnect of sysfs nodes. Once a sysfs_dirent is deleted, it won't access any entity external to sysfs proper. Because attr/bin_attr ops access both the node itself and its parent for kobject, they need to hold active references to both. sysfs_get/put_active_two() helpers are provided to help grabbing both references. Parent's is acquired first and released last. Unlike other operations, mmapped area lingers on after mmap() is finished and the module implement implementing it and kobj need to stay referenced till all the mapped pages are gone. This is accomplished by holding one set of active references to the bin_attr and its parent if there have been any mmap during lifetime of an openfile. The references are dropped when the openfile is released. This change makes sysfs lifetime rules independent from both kobject's and module's. It not only fixes several race conditions caused by sysfs not holding onto the proper module when referencing kobject, but also helps fixing and simplifying lifetime management in driver model and drivers by taking sysfs out of the equation. Please read the following message for more info. http://article.gmane.org/gmane.linux.kernel/510293 Signed-off-by: Tejun Heo <htejun@gmail.com> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2007-06-14 02:45:16 +08:00
rc = -EIO;
if (attr->read)
sysfs: add parameter "struct bin_attribute *" in .read/.write methods for sysfs binary attributes Well, first of all, I don't want to change so many files either. What I do: Adding a new parameter "struct bin_attribute *" in the .read/.write methods for the sysfs binary attributes. In fact, only the four lines change in fs/sysfs/bin.c and include/linux/sysfs.h do the real work. But I have to update all the files that use binary attributes to make them compatible with the new .read and .write methods. I'm not sure if I missed any. :( Why I do this: For a sysfs attribute, we can get a pointer pointing to the struct attribute in the .show/.store method, while we can't do this for the binary attributes. I don't know why this is different, but this does make it not so handy to use the binary attributes as the regular ones. So I think this patch is reasonable. :) Who benefits from it: The patch that exposes ACPI tables in sysfs requires such an improvement. All the table binary attributes share the same .read method. Parameter "struct bin_attribute *" is used to get the table signature and instance number which are used to distinguish different ACPI table binary attributes. Without this parameter, we need to offer different .read methods for different ACPI table binary attributes. This is impossible as there are various ACPI tables on different platforms, and we don't know what they are until they are loaded. Signed-off-by: Zhang Rui <rui.zhang@intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2007-06-09 13:57:22 +08:00
rc = attr->read(kobj, attr, buffer, off, count);
sysfs: implement sysfs_dirent active reference and immediate disconnect sysfs: implement sysfs_dirent active reference and immediate disconnect Opening a sysfs node references its associated kobject, so userland can arbitrarily prolong lifetime of a kobject which complicates lifetime rules in drivers. This patch implements active reference and makes the association between kobject and sysfs immediately breakable. Now each sysfs_dirent has two reference counts - s_count and s_active. s_count is a regular reference count which guarantees that the containing sysfs_dirent is accessible. As long as s_count reference is held, all sysfs internal fields in sysfs_dirent are accessible including s_parent and s_name. The newly added s_active is active reference count. This is acquired by invoking sysfs_get_active() and it's the caller's responsibility to ensure sysfs_dirent itself is accessible (should be holding s_count one way or the other). Dereferencing sysfs_dirent to access objects out of sysfs proper requires active reference. This includes access to the associated kobjects, attributes and ops. The active references can be drained and denied by calling sysfs_deactivate(). All active sysfs_dirents must be deactivated after deletion but before the default reference is dropped. This enables immediate disconnect of sysfs nodes. Once a sysfs_dirent is deleted, it won't access any entity external to sysfs proper. Because attr/bin_attr ops access both the node itself and its parent for kobject, they need to hold active references to both. sysfs_get/put_active_two() helpers are provided to help grabbing both references. Parent's is acquired first and released last. Unlike other operations, mmapped area lingers on after mmap() is finished and the module implement implementing it and kobj need to stay referenced till all the mapped pages are gone. This is accomplished by holding one set of active references to the bin_attr and its parent if there have been any mmap during lifetime of an openfile. The references are dropped when the openfile is released. This change makes sysfs lifetime rules independent from both kobject's and module's. It not only fixes several race conditions caused by sysfs not holding onto the proper module when referencing kobject, but also helps fixing and simplifying lifetime management in driver model and drivers by taking sysfs out of the equation. Please read the following message for more info. http://article.gmane.org/gmane.linux.kernel/510293 Signed-off-by: Tejun Heo <htejun@gmail.com> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2007-06-14 02:45:16 +08:00
sysfs_put_active_two(attr_sd);
return rc;
}
static ssize_t
read(struct file *file, char __user *userbuf, size_t bytes, loff_t *off)
{
struct bin_buffer *bb = file->private_data;
struct dentry *dentry = file->f_path.dentry;
int size = dentry->d_inode->i_size;
loff_t offs = *off;
int count = min_t(size_t, bytes, PAGE_SIZE);
sysfs: fix deadlock On Thu, Sep 11, 2008 at 10:27:10AM +0200, Ingo Molnar wrote: > and it's working fine on most boxes. One testbox found this new locking > scenario: > > PM: Adding info for No Bus:vcsa7 > EDAC DEBUG: MC0: i82860_check() > > ======================================================= > [ INFO: possible circular locking dependency detected ] > 2.6.27-rc6-tip #1 > ------------------------------------------------------- > X/4873 is trying to acquire lock: > (&bb->mutex){--..}, at: [<c020ba20>] mmap+0x40/0xa0 > > but task is already holding lock: > (&mm->mmap_sem){----}, at: [<c0125a1e>] sys_mmap2+0x8e/0xc0 > > which lock already depends on the new lock. > > > the existing dependency chain (in reverse order) is: > > -> #1 (&mm->mmap_sem){----}: > [<c017dc96>] validate_chain+0xa96/0xf50 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c01aa8fb>] might_fault+0x6b/0x90 > [<c040b618>] copy_to_user+0x38/0x60 > [<c020bcfb>] read+0xfb/0x170 > [<c01c09a5>] vfs_read+0x95/0x110 > [<c01c1443>] sys_pread64+0x63/0x80 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<ffffffff>] 0xffffffff > > -> #0 (&bb->mutex){--..}: > [<c017d8b7>] validate_chain+0x6b7/0xf50 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c0d6f2ab>] __mutex_lock_common+0xab/0x3c0 > [<c0d6f698>] mutex_lock_nested+0x38/0x50 > [<c020ba20>] mmap+0x40/0xa0 > [<c01b111e>] mmap_region+0x14e/0x450 > [<c01b170f>] do_mmap_pgoff+0x2ef/0x310 > [<c0125a3d>] sys_mmap2+0xad/0xc0 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<ffffffff>] 0xffffffff > > other info that might help us debug this: > > 1 lock held by X/4873: > #0: (&mm->mmap_sem){----}, at: [<c0125a1e>] sys_mmap2+0x8e/0xc0 > > stack backtrace: > Pid: 4873, comm: X Not tainted 2.6.27-rc6-tip #1 > [<c017cd09>] print_circular_bug_tail+0x79/0xc0 > [<c017d8b7>] validate_chain+0x6b7/0xf50 > [<c017a5b5>] ? trace_hardirqs_off_caller+0x15/0xb0 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c0d6f2ab>] __mutex_lock_common+0xab/0x3c0 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c0d6f698>] mutex_lock_nested+0x38/0x50 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c020ba20>] mmap+0x40/0xa0 > [<c01b111e>] mmap_region+0x14e/0x450 > [<c01afb88>] ? arch_get_unmapped_area_topdown+0xf8/0x160 > [<c01b170f>] do_mmap_pgoff+0x2ef/0x310 > [<c0125a3d>] sys_mmap2+0xad/0xc0 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<c0120000>] ? __switch_to+0x130/0x220 > ======================= > evbug.c: Event. Dev: input3, Type: 20, Code: 0, Value: 500 > warning: `sudo' uses deprecated v2 capabilities in a way that may be insecure. > > i've attached the config. > > at first sight it looks like a genuine bug in fs/sysfs/bin.c? Yes, it is a real bug by the looks. bin.c takes bb->mutex under mmap_sem when it is mmapped, and then does its copy_*_user under bb->mutex too. Here is a basic fix for the sysfs lor. From: Nick Piggin <npiggin@suse.de> Signed-off-by: Ingo Molnar <mingo@elte.hu> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2008-09-12 17:24:11 +08:00
char *temp;
if (size) {
if (offs > size)
return 0;
if (offs + count > size)
count = size - offs;
}
sysfs: fix deadlock On Thu, Sep 11, 2008 at 10:27:10AM +0200, Ingo Molnar wrote: > and it's working fine on most boxes. One testbox found this new locking > scenario: > > PM: Adding info for No Bus:vcsa7 > EDAC DEBUG: MC0: i82860_check() > > ======================================================= > [ INFO: possible circular locking dependency detected ] > 2.6.27-rc6-tip #1 > ------------------------------------------------------- > X/4873 is trying to acquire lock: > (&bb->mutex){--..}, at: [<c020ba20>] mmap+0x40/0xa0 > > but task is already holding lock: > (&mm->mmap_sem){----}, at: [<c0125a1e>] sys_mmap2+0x8e/0xc0 > > which lock already depends on the new lock. > > > the existing dependency chain (in reverse order) is: > > -> #1 (&mm->mmap_sem){----}: > [<c017dc96>] validate_chain+0xa96/0xf50 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c01aa8fb>] might_fault+0x6b/0x90 > [<c040b618>] copy_to_user+0x38/0x60 > [<c020bcfb>] read+0xfb/0x170 > [<c01c09a5>] vfs_read+0x95/0x110 > [<c01c1443>] sys_pread64+0x63/0x80 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<ffffffff>] 0xffffffff > > -> #0 (&bb->mutex){--..}: > [<c017d8b7>] validate_chain+0x6b7/0xf50 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c0d6f2ab>] __mutex_lock_common+0xab/0x3c0 > [<c0d6f698>] mutex_lock_nested+0x38/0x50 > [<c020ba20>] mmap+0x40/0xa0 > [<c01b111e>] mmap_region+0x14e/0x450 > [<c01b170f>] do_mmap_pgoff+0x2ef/0x310 > [<c0125a3d>] sys_mmap2+0xad/0xc0 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<ffffffff>] 0xffffffff > > other info that might help us debug this: > > 1 lock held by X/4873: > #0: (&mm->mmap_sem){----}, at: [<c0125a1e>] sys_mmap2+0x8e/0xc0 > > stack backtrace: > Pid: 4873, comm: X Not tainted 2.6.27-rc6-tip #1 > [<c017cd09>] print_circular_bug_tail+0x79/0xc0 > [<c017d8b7>] validate_chain+0x6b7/0xf50 > [<c017a5b5>] ? trace_hardirqs_off_caller+0x15/0xb0 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c0d6f2ab>] __mutex_lock_common+0xab/0x3c0 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c0d6f698>] mutex_lock_nested+0x38/0x50 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c020ba20>] mmap+0x40/0xa0 > [<c01b111e>] mmap_region+0x14e/0x450 > [<c01afb88>] ? arch_get_unmapped_area_topdown+0xf8/0x160 > [<c01b170f>] do_mmap_pgoff+0x2ef/0x310 > [<c0125a3d>] sys_mmap2+0xad/0xc0 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<c0120000>] ? __switch_to+0x130/0x220 > ======================= > evbug.c: Event. Dev: input3, Type: 20, Code: 0, Value: 500 > warning: `sudo' uses deprecated v2 capabilities in a way that may be insecure. > > i've attached the config. > > at first sight it looks like a genuine bug in fs/sysfs/bin.c? Yes, it is a real bug by the looks. bin.c takes bb->mutex under mmap_sem when it is mmapped, and then does its copy_*_user under bb->mutex too. Here is a basic fix for the sysfs lor. From: Nick Piggin <npiggin@suse.de> Signed-off-by: Ingo Molnar <mingo@elte.hu> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2008-09-12 17:24:11 +08:00
temp = kmalloc(count, GFP_KERNEL);
if (!temp)
return -ENOMEM;
mutex_lock(&bb->mutex);
count = fill_read(dentry, bb->buffer, offs, count);
sysfs: fix deadlock On Thu, Sep 11, 2008 at 10:27:10AM +0200, Ingo Molnar wrote: > and it's working fine on most boxes. One testbox found this new locking > scenario: > > PM: Adding info for No Bus:vcsa7 > EDAC DEBUG: MC0: i82860_check() > > ======================================================= > [ INFO: possible circular locking dependency detected ] > 2.6.27-rc6-tip #1 > ------------------------------------------------------- > X/4873 is trying to acquire lock: > (&bb->mutex){--..}, at: [<c020ba20>] mmap+0x40/0xa0 > > but task is already holding lock: > (&mm->mmap_sem){----}, at: [<c0125a1e>] sys_mmap2+0x8e/0xc0 > > which lock already depends on the new lock. > > > the existing dependency chain (in reverse order) is: > > -> #1 (&mm->mmap_sem){----}: > [<c017dc96>] validate_chain+0xa96/0xf50 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c01aa8fb>] might_fault+0x6b/0x90 > [<c040b618>] copy_to_user+0x38/0x60 > [<c020bcfb>] read+0xfb/0x170 > [<c01c09a5>] vfs_read+0x95/0x110 > [<c01c1443>] sys_pread64+0x63/0x80 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<ffffffff>] 0xffffffff > > -> #0 (&bb->mutex){--..}: > [<c017d8b7>] validate_chain+0x6b7/0xf50 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c0d6f2ab>] __mutex_lock_common+0xab/0x3c0 > [<c0d6f698>] mutex_lock_nested+0x38/0x50 > [<c020ba20>] mmap+0x40/0xa0 > [<c01b111e>] mmap_region+0x14e/0x450 > [<c01b170f>] do_mmap_pgoff+0x2ef/0x310 > [<c0125a3d>] sys_mmap2+0xad/0xc0 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<ffffffff>] 0xffffffff > > other info that might help us debug this: > > 1 lock held by X/4873: > #0: (&mm->mmap_sem){----}, at: [<c0125a1e>] sys_mmap2+0x8e/0xc0 > > stack backtrace: > Pid: 4873, comm: X Not tainted 2.6.27-rc6-tip #1 > [<c017cd09>] print_circular_bug_tail+0x79/0xc0 > [<c017d8b7>] validate_chain+0x6b7/0xf50 > [<c017a5b5>] ? trace_hardirqs_off_caller+0x15/0xb0 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c0d6f2ab>] __mutex_lock_common+0xab/0x3c0 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c0d6f698>] mutex_lock_nested+0x38/0x50 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c020ba20>] mmap+0x40/0xa0 > [<c01b111e>] mmap_region+0x14e/0x450 > [<c01afb88>] ? arch_get_unmapped_area_topdown+0xf8/0x160 > [<c01b170f>] do_mmap_pgoff+0x2ef/0x310 > [<c0125a3d>] sys_mmap2+0xad/0xc0 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<c0120000>] ? __switch_to+0x130/0x220 > ======================= > evbug.c: Event. Dev: input3, Type: 20, Code: 0, Value: 500 > warning: `sudo' uses deprecated v2 capabilities in a way that may be insecure. > > i've attached the config. > > at first sight it looks like a genuine bug in fs/sysfs/bin.c? Yes, it is a real bug by the looks. bin.c takes bb->mutex under mmap_sem when it is mmapped, and then does its copy_*_user under bb->mutex too. Here is a basic fix for the sysfs lor. From: Nick Piggin <npiggin@suse.de> Signed-off-by: Ingo Molnar <mingo@elte.hu> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2008-09-12 17:24:11 +08:00
if (count < 0) {
mutex_unlock(&bb->mutex);
goto out_free;
}
sysfs: fix deadlock On Thu, Sep 11, 2008 at 10:27:10AM +0200, Ingo Molnar wrote: > and it's working fine on most boxes. One testbox found this new locking > scenario: > > PM: Adding info for No Bus:vcsa7 > EDAC DEBUG: MC0: i82860_check() > > ======================================================= > [ INFO: possible circular locking dependency detected ] > 2.6.27-rc6-tip #1 > ------------------------------------------------------- > X/4873 is trying to acquire lock: > (&bb->mutex){--..}, at: [<c020ba20>] mmap+0x40/0xa0 > > but task is already holding lock: > (&mm->mmap_sem){----}, at: [<c0125a1e>] sys_mmap2+0x8e/0xc0 > > which lock already depends on the new lock. > > > the existing dependency chain (in reverse order) is: > > -> #1 (&mm->mmap_sem){----}: > [<c017dc96>] validate_chain+0xa96/0xf50 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c01aa8fb>] might_fault+0x6b/0x90 > [<c040b618>] copy_to_user+0x38/0x60 > [<c020bcfb>] read+0xfb/0x170 > [<c01c09a5>] vfs_read+0x95/0x110 > [<c01c1443>] sys_pread64+0x63/0x80 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<ffffffff>] 0xffffffff > > -> #0 (&bb->mutex){--..}: > [<c017d8b7>] validate_chain+0x6b7/0xf50 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c0d6f2ab>] __mutex_lock_common+0xab/0x3c0 > [<c0d6f698>] mutex_lock_nested+0x38/0x50 > [<c020ba20>] mmap+0x40/0xa0 > [<c01b111e>] mmap_region+0x14e/0x450 > [<c01b170f>] do_mmap_pgoff+0x2ef/0x310 > [<c0125a3d>] sys_mmap2+0xad/0xc0 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<ffffffff>] 0xffffffff > > other info that might help us debug this: > > 1 lock held by X/4873: > #0: (&mm->mmap_sem){----}, at: [<c0125a1e>] sys_mmap2+0x8e/0xc0 > > stack backtrace: > Pid: 4873, comm: X Not tainted 2.6.27-rc6-tip #1 > [<c017cd09>] print_circular_bug_tail+0x79/0xc0 > [<c017d8b7>] validate_chain+0x6b7/0xf50 > [<c017a5b5>] ? trace_hardirqs_off_caller+0x15/0xb0 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c0d6f2ab>] __mutex_lock_common+0xab/0x3c0 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c0d6f698>] mutex_lock_nested+0x38/0x50 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c020ba20>] mmap+0x40/0xa0 > [<c01b111e>] mmap_region+0x14e/0x450 > [<c01afb88>] ? arch_get_unmapped_area_topdown+0xf8/0x160 > [<c01b170f>] do_mmap_pgoff+0x2ef/0x310 > [<c0125a3d>] sys_mmap2+0xad/0xc0 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<c0120000>] ? __switch_to+0x130/0x220 > ======================= > evbug.c: Event. Dev: input3, Type: 20, Code: 0, Value: 500 > warning: `sudo' uses deprecated v2 capabilities in a way that may be insecure. > > i've attached the config. > > at first sight it looks like a genuine bug in fs/sysfs/bin.c? Yes, it is a real bug by the looks. bin.c takes bb->mutex under mmap_sem when it is mmapped, and then does its copy_*_user under bb->mutex too. Here is a basic fix for the sysfs lor. From: Nick Piggin <npiggin@suse.de> Signed-off-by: Ingo Molnar <mingo@elte.hu> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2008-09-12 17:24:11 +08:00
memcpy(temp, bb->buffer, count);
mutex_unlock(&bb->mutex);
if (copy_to_user(userbuf, temp, count)) {
count = -EFAULT;
sysfs: fix deadlock On Thu, Sep 11, 2008 at 10:27:10AM +0200, Ingo Molnar wrote: > and it's working fine on most boxes. One testbox found this new locking > scenario: > > PM: Adding info for No Bus:vcsa7 > EDAC DEBUG: MC0: i82860_check() > > ======================================================= > [ INFO: possible circular locking dependency detected ] > 2.6.27-rc6-tip #1 > ------------------------------------------------------- > X/4873 is trying to acquire lock: > (&bb->mutex){--..}, at: [<c020ba20>] mmap+0x40/0xa0 > > but task is already holding lock: > (&mm->mmap_sem){----}, at: [<c0125a1e>] sys_mmap2+0x8e/0xc0 > > which lock already depends on the new lock. > > > the existing dependency chain (in reverse order) is: > > -> #1 (&mm->mmap_sem){----}: > [<c017dc96>] validate_chain+0xa96/0xf50 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c01aa8fb>] might_fault+0x6b/0x90 > [<c040b618>] copy_to_user+0x38/0x60 > [<c020bcfb>] read+0xfb/0x170 > [<c01c09a5>] vfs_read+0x95/0x110 > [<c01c1443>] sys_pread64+0x63/0x80 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<ffffffff>] 0xffffffff > > -> #0 (&bb->mutex){--..}: > [<c017d8b7>] validate_chain+0x6b7/0xf50 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c0d6f2ab>] __mutex_lock_common+0xab/0x3c0 > [<c0d6f698>] mutex_lock_nested+0x38/0x50 > [<c020ba20>] mmap+0x40/0xa0 > [<c01b111e>] mmap_region+0x14e/0x450 > [<c01b170f>] do_mmap_pgoff+0x2ef/0x310 > [<c0125a3d>] sys_mmap2+0xad/0xc0 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<ffffffff>] 0xffffffff > > other info that might help us debug this: > > 1 lock held by X/4873: > #0: (&mm->mmap_sem){----}, at: [<c0125a1e>] sys_mmap2+0x8e/0xc0 > > stack backtrace: > Pid: 4873, comm: X Not tainted 2.6.27-rc6-tip #1 > [<c017cd09>] print_circular_bug_tail+0x79/0xc0 > [<c017d8b7>] validate_chain+0x6b7/0xf50 > [<c017a5b5>] ? trace_hardirqs_off_caller+0x15/0xb0 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c0d6f2ab>] __mutex_lock_common+0xab/0x3c0 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c0d6f698>] mutex_lock_nested+0x38/0x50 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c020ba20>] mmap+0x40/0xa0 > [<c01b111e>] mmap_region+0x14e/0x450 > [<c01afb88>] ? arch_get_unmapped_area_topdown+0xf8/0x160 > [<c01b170f>] do_mmap_pgoff+0x2ef/0x310 > [<c0125a3d>] sys_mmap2+0xad/0xc0 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<c0120000>] ? __switch_to+0x130/0x220 > ======================= > evbug.c: Event. Dev: input3, Type: 20, Code: 0, Value: 500 > warning: `sudo' uses deprecated v2 capabilities in a way that may be insecure. > > i've attached the config. > > at first sight it looks like a genuine bug in fs/sysfs/bin.c? Yes, it is a real bug by the looks. bin.c takes bb->mutex under mmap_sem when it is mmapped, and then does its copy_*_user under bb->mutex too. Here is a basic fix for the sysfs lor. From: Nick Piggin <npiggin@suse.de> Signed-off-by: Ingo Molnar <mingo@elte.hu> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2008-09-12 17:24:11 +08:00
goto out_free;
}
pr_debug("offs = %lld, *off = %lld, count = %d\n", offs, *off, count);
*off = offs + count;
sysfs: fix deadlock On Thu, Sep 11, 2008 at 10:27:10AM +0200, Ingo Molnar wrote: > and it's working fine on most boxes. One testbox found this new locking > scenario: > > PM: Adding info for No Bus:vcsa7 > EDAC DEBUG: MC0: i82860_check() > > ======================================================= > [ INFO: possible circular locking dependency detected ] > 2.6.27-rc6-tip #1 > ------------------------------------------------------- > X/4873 is trying to acquire lock: > (&bb->mutex){--..}, at: [<c020ba20>] mmap+0x40/0xa0 > > but task is already holding lock: > (&mm->mmap_sem){----}, at: [<c0125a1e>] sys_mmap2+0x8e/0xc0 > > which lock already depends on the new lock. > > > the existing dependency chain (in reverse order) is: > > -> #1 (&mm->mmap_sem){----}: > [<c017dc96>] validate_chain+0xa96/0xf50 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c01aa8fb>] might_fault+0x6b/0x90 > [<c040b618>] copy_to_user+0x38/0x60 > [<c020bcfb>] read+0xfb/0x170 > [<c01c09a5>] vfs_read+0x95/0x110 > [<c01c1443>] sys_pread64+0x63/0x80 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<ffffffff>] 0xffffffff > > -> #0 (&bb->mutex){--..}: > [<c017d8b7>] validate_chain+0x6b7/0xf50 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c0d6f2ab>] __mutex_lock_common+0xab/0x3c0 > [<c0d6f698>] mutex_lock_nested+0x38/0x50 > [<c020ba20>] mmap+0x40/0xa0 > [<c01b111e>] mmap_region+0x14e/0x450 > [<c01b170f>] do_mmap_pgoff+0x2ef/0x310 > [<c0125a3d>] sys_mmap2+0xad/0xc0 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<ffffffff>] 0xffffffff > > other info that might help us debug this: > > 1 lock held by X/4873: > #0: (&mm->mmap_sem){----}, at: [<c0125a1e>] sys_mmap2+0x8e/0xc0 > > stack backtrace: > Pid: 4873, comm: X Not tainted 2.6.27-rc6-tip #1 > [<c017cd09>] print_circular_bug_tail+0x79/0xc0 > [<c017d8b7>] validate_chain+0x6b7/0xf50 > [<c017a5b5>] ? trace_hardirqs_off_caller+0x15/0xb0 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c0d6f2ab>] __mutex_lock_common+0xab/0x3c0 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c0d6f698>] mutex_lock_nested+0x38/0x50 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c020ba20>] mmap+0x40/0xa0 > [<c01b111e>] mmap_region+0x14e/0x450 > [<c01afb88>] ? arch_get_unmapped_area_topdown+0xf8/0x160 > [<c01b170f>] do_mmap_pgoff+0x2ef/0x310 > [<c0125a3d>] sys_mmap2+0xad/0xc0 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<c0120000>] ? __switch_to+0x130/0x220 > ======================= > evbug.c: Event. Dev: input3, Type: 20, Code: 0, Value: 500 > warning: `sudo' uses deprecated v2 capabilities in a way that may be insecure. > > i've attached the config. > > at first sight it looks like a genuine bug in fs/sysfs/bin.c? Yes, it is a real bug by the looks. bin.c takes bb->mutex under mmap_sem when it is mmapped, and then does its copy_*_user under bb->mutex too. Here is a basic fix for the sysfs lor. From: Nick Piggin <npiggin@suse.de> Signed-off-by: Ingo Molnar <mingo@elte.hu> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2008-09-12 17:24:11 +08:00
out_free:
kfree(temp);
return count;
}
static int
flush_write(struct dentry *dentry, char *buffer, loff_t offset, size_t count)
{
struct sysfs_dirent *attr_sd = dentry->d_fsdata;
struct bin_attribute *attr = attr_sd->s_bin_attr.bin_attr;
struct kobject *kobj = attr_sd->s_parent->s_dir.kobj;
sysfs: implement sysfs_dirent active reference and immediate disconnect sysfs: implement sysfs_dirent active reference and immediate disconnect Opening a sysfs node references its associated kobject, so userland can arbitrarily prolong lifetime of a kobject which complicates lifetime rules in drivers. This patch implements active reference and makes the association between kobject and sysfs immediately breakable. Now each sysfs_dirent has two reference counts - s_count and s_active. s_count is a regular reference count which guarantees that the containing sysfs_dirent is accessible. As long as s_count reference is held, all sysfs internal fields in sysfs_dirent are accessible including s_parent and s_name. The newly added s_active is active reference count. This is acquired by invoking sysfs_get_active() and it's the caller's responsibility to ensure sysfs_dirent itself is accessible (should be holding s_count one way or the other). Dereferencing sysfs_dirent to access objects out of sysfs proper requires active reference. This includes access to the associated kobjects, attributes and ops. The active references can be drained and denied by calling sysfs_deactivate(). All active sysfs_dirents must be deactivated after deletion but before the default reference is dropped. This enables immediate disconnect of sysfs nodes. Once a sysfs_dirent is deleted, it won't access any entity external to sysfs proper. Because attr/bin_attr ops access both the node itself and its parent for kobject, they need to hold active references to both. sysfs_get/put_active_two() helpers are provided to help grabbing both references. Parent's is acquired first and released last. Unlike other operations, mmapped area lingers on after mmap() is finished and the module implement implementing it and kobj need to stay referenced till all the mapped pages are gone. This is accomplished by holding one set of active references to the bin_attr and its parent if there have been any mmap during lifetime of an openfile. The references are dropped when the openfile is released. This change makes sysfs lifetime rules independent from both kobject's and module's. It not only fixes several race conditions caused by sysfs not holding onto the proper module when referencing kobject, but also helps fixing and simplifying lifetime management in driver model and drivers by taking sysfs out of the equation. Please read the following message for more info. http://article.gmane.org/gmane.linux.kernel/510293 Signed-off-by: Tejun Heo <htejun@gmail.com> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2007-06-14 02:45:16 +08:00
int rc;
/* need attr_sd for attr, its parent for kobj */
if (!sysfs_get_active_two(attr_sd))
return -ENODEV;
sysfs: implement sysfs_dirent active reference and immediate disconnect sysfs: implement sysfs_dirent active reference and immediate disconnect Opening a sysfs node references its associated kobject, so userland can arbitrarily prolong lifetime of a kobject which complicates lifetime rules in drivers. This patch implements active reference and makes the association between kobject and sysfs immediately breakable. Now each sysfs_dirent has two reference counts - s_count and s_active. s_count is a regular reference count which guarantees that the containing sysfs_dirent is accessible. As long as s_count reference is held, all sysfs internal fields in sysfs_dirent are accessible including s_parent and s_name. The newly added s_active is active reference count. This is acquired by invoking sysfs_get_active() and it's the caller's responsibility to ensure sysfs_dirent itself is accessible (should be holding s_count one way or the other). Dereferencing sysfs_dirent to access objects out of sysfs proper requires active reference. This includes access to the associated kobjects, attributes and ops. The active references can be drained and denied by calling sysfs_deactivate(). All active sysfs_dirents must be deactivated after deletion but before the default reference is dropped. This enables immediate disconnect of sysfs nodes. Once a sysfs_dirent is deleted, it won't access any entity external to sysfs proper. Because attr/bin_attr ops access both the node itself and its parent for kobject, they need to hold active references to both. sysfs_get/put_active_two() helpers are provided to help grabbing both references. Parent's is acquired first and released last. Unlike other operations, mmapped area lingers on after mmap() is finished and the module implement implementing it and kobj need to stay referenced till all the mapped pages are gone. This is accomplished by holding one set of active references to the bin_attr and its parent if there have been any mmap during lifetime of an openfile. The references are dropped when the openfile is released. This change makes sysfs lifetime rules independent from both kobject's and module's. It not only fixes several race conditions caused by sysfs not holding onto the proper module when referencing kobject, but also helps fixing and simplifying lifetime management in driver model and drivers by taking sysfs out of the equation. Please read the following message for more info. http://article.gmane.org/gmane.linux.kernel/510293 Signed-off-by: Tejun Heo <htejun@gmail.com> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2007-06-14 02:45:16 +08:00
rc = -EIO;
if (attr->write)
sysfs: add parameter "struct bin_attribute *" in .read/.write methods for sysfs binary attributes Well, first of all, I don't want to change so many files either. What I do: Adding a new parameter "struct bin_attribute *" in the .read/.write methods for the sysfs binary attributes. In fact, only the four lines change in fs/sysfs/bin.c and include/linux/sysfs.h do the real work. But I have to update all the files that use binary attributes to make them compatible with the new .read and .write methods. I'm not sure if I missed any. :( Why I do this: For a sysfs attribute, we can get a pointer pointing to the struct attribute in the .show/.store method, while we can't do this for the binary attributes. I don't know why this is different, but this does make it not so handy to use the binary attributes as the regular ones. So I think this patch is reasonable. :) Who benefits from it: The patch that exposes ACPI tables in sysfs requires such an improvement. All the table binary attributes share the same .read method. Parameter "struct bin_attribute *" is used to get the table signature and instance number which are used to distinguish different ACPI table binary attributes. Without this parameter, we need to offer different .read methods for different ACPI table binary attributes. This is impossible as there are various ACPI tables on different platforms, and we don't know what they are until they are loaded. Signed-off-by: Zhang Rui <rui.zhang@intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2007-06-09 13:57:22 +08:00
rc = attr->write(kobj, attr, buffer, offset, count);
sysfs: implement sysfs_dirent active reference and immediate disconnect sysfs: implement sysfs_dirent active reference and immediate disconnect Opening a sysfs node references its associated kobject, so userland can arbitrarily prolong lifetime of a kobject which complicates lifetime rules in drivers. This patch implements active reference and makes the association between kobject and sysfs immediately breakable. Now each sysfs_dirent has two reference counts - s_count and s_active. s_count is a regular reference count which guarantees that the containing sysfs_dirent is accessible. As long as s_count reference is held, all sysfs internal fields in sysfs_dirent are accessible including s_parent and s_name. The newly added s_active is active reference count. This is acquired by invoking sysfs_get_active() and it's the caller's responsibility to ensure sysfs_dirent itself is accessible (should be holding s_count one way or the other). Dereferencing sysfs_dirent to access objects out of sysfs proper requires active reference. This includes access to the associated kobjects, attributes and ops. The active references can be drained and denied by calling sysfs_deactivate(). All active sysfs_dirents must be deactivated after deletion but before the default reference is dropped. This enables immediate disconnect of sysfs nodes. Once a sysfs_dirent is deleted, it won't access any entity external to sysfs proper. Because attr/bin_attr ops access both the node itself and its parent for kobject, they need to hold active references to both. sysfs_get/put_active_two() helpers are provided to help grabbing both references. Parent's is acquired first and released last. Unlike other operations, mmapped area lingers on after mmap() is finished and the module implement implementing it and kobj need to stay referenced till all the mapped pages are gone. This is accomplished by holding one set of active references to the bin_attr and its parent if there have been any mmap during lifetime of an openfile. The references are dropped when the openfile is released. This change makes sysfs lifetime rules independent from both kobject's and module's. It not only fixes several race conditions caused by sysfs not holding onto the proper module when referencing kobject, but also helps fixing and simplifying lifetime management in driver model and drivers by taking sysfs out of the equation. Please read the following message for more info. http://article.gmane.org/gmane.linux.kernel/510293 Signed-off-by: Tejun Heo <htejun@gmail.com> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2007-06-14 02:45:16 +08:00
sysfs_put_active_two(attr_sd);
return rc;
}
static ssize_t write(struct file *file, const char __user *userbuf,
size_t bytes, loff_t *off)
{
struct bin_buffer *bb = file->private_data;
struct dentry *dentry = file->f_path.dentry;
int size = dentry->d_inode->i_size;
loff_t offs = *off;
int count = min_t(size_t, bytes, PAGE_SIZE);
sysfs: fix deadlock On Thu, Sep 11, 2008 at 10:27:10AM +0200, Ingo Molnar wrote: > and it's working fine on most boxes. One testbox found this new locking > scenario: > > PM: Adding info for No Bus:vcsa7 > EDAC DEBUG: MC0: i82860_check() > > ======================================================= > [ INFO: possible circular locking dependency detected ] > 2.6.27-rc6-tip #1 > ------------------------------------------------------- > X/4873 is trying to acquire lock: > (&bb->mutex){--..}, at: [<c020ba20>] mmap+0x40/0xa0 > > but task is already holding lock: > (&mm->mmap_sem){----}, at: [<c0125a1e>] sys_mmap2+0x8e/0xc0 > > which lock already depends on the new lock. > > > the existing dependency chain (in reverse order) is: > > -> #1 (&mm->mmap_sem){----}: > [<c017dc96>] validate_chain+0xa96/0xf50 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c01aa8fb>] might_fault+0x6b/0x90 > [<c040b618>] copy_to_user+0x38/0x60 > [<c020bcfb>] read+0xfb/0x170 > [<c01c09a5>] vfs_read+0x95/0x110 > [<c01c1443>] sys_pread64+0x63/0x80 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<ffffffff>] 0xffffffff > > -> #0 (&bb->mutex){--..}: > [<c017d8b7>] validate_chain+0x6b7/0xf50 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c0d6f2ab>] __mutex_lock_common+0xab/0x3c0 > [<c0d6f698>] mutex_lock_nested+0x38/0x50 > [<c020ba20>] mmap+0x40/0xa0 > [<c01b111e>] mmap_region+0x14e/0x450 > [<c01b170f>] do_mmap_pgoff+0x2ef/0x310 > [<c0125a3d>] sys_mmap2+0xad/0xc0 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<ffffffff>] 0xffffffff > > other info that might help us debug this: > > 1 lock held by X/4873: > #0: (&mm->mmap_sem){----}, at: [<c0125a1e>] sys_mmap2+0x8e/0xc0 > > stack backtrace: > Pid: 4873, comm: X Not tainted 2.6.27-rc6-tip #1 > [<c017cd09>] print_circular_bug_tail+0x79/0xc0 > [<c017d8b7>] validate_chain+0x6b7/0xf50 > [<c017a5b5>] ? trace_hardirqs_off_caller+0x15/0xb0 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c0d6f2ab>] __mutex_lock_common+0xab/0x3c0 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c0d6f698>] mutex_lock_nested+0x38/0x50 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c020ba20>] mmap+0x40/0xa0 > [<c01b111e>] mmap_region+0x14e/0x450 > [<c01afb88>] ? arch_get_unmapped_area_topdown+0xf8/0x160 > [<c01b170f>] do_mmap_pgoff+0x2ef/0x310 > [<c0125a3d>] sys_mmap2+0xad/0xc0 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<c0120000>] ? __switch_to+0x130/0x220 > ======================= > evbug.c: Event. Dev: input3, Type: 20, Code: 0, Value: 500 > warning: `sudo' uses deprecated v2 capabilities in a way that may be insecure. > > i've attached the config. > > at first sight it looks like a genuine bug in fs/sysfs/bin.c? Yes, it is a real bug by the looks. bin.c takes bb->mutex under mmap_sem when it is mmapped, and then does its copy_*_user under bb->mutex too. Here is a basic fix for the sysfs lor. From: Nick Piggin <npiggin@suse.de> Signed-off-by: Ingo Molnar <mingo@elte.hu> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2008-09-12 17:24:11 +08:00
char *temp;
if (size) {
if (offs > size)
return 0;
if (offs + count > size)
count = size - offs;
}
sysfs: fix deadlock On Thu, Sep 11, 2008 at 10:27:10AM +0200, Ingo Molnar wrote: > and it's working fine on most boxes. One testbox found this new locking > scenario: > > PM: Adding info for No Bus:vcsa7 > EDAC DEBUG: MC0: i82860_check() > > ======================================================= > [ INFO: possible circular locking dependency detected ] > 2.6.27-rc6-tip #1 > ------------------------------------------------------- > X/4873 is trying to acquire lock: > (&bb->mutex){--..}, at: [<c020ba20>] mmap+0x40/0xa0 > > but task is already holding lock: > (&mm->mmap_sem){----}, at: [<c0125a1e>] sys_mmap2+0x8e/0xc0 > > which lock already depends on the new lock. > > > the existing dependency chain (in reverse order) is: > > -> #1 (&mm->mmap_sem){----}: > [<c017dc96>] validate_chain+0xa96/0xf50 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c01aa8fb>] might_fault+0x6b/0x90 > [<c040b618>] copy_to_user+0x38/0x60 > [<c020bcfb>] read+0xfb/0x170 > [<c01c09a5>] vfs_read+0x95/0x110 > [<c01c1443>] sys_pread64+0x63/0x80 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<ffffffff>] 0xffffffff > > -> #0 (&bb->mutex){--..}: > [<c017d8b7>] validate_chain+0x6b7/0xf50 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c0d6f2ab>] __mutex_lock_common+0xab/0x3c0 > [<c0d6f698>] mutex_lock_nested+0x38/0x50 > [<c020ba20>] mmap+0x40/0xa0 > [<c01b111e>] mmap_region+0x14e/0x450 > [<c01b170f>] do_mmap_pgoff+0x2ef/0x310 > [<c0125a3d>] sys_mmap2+0xad/0xc0 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<ffffffff>] 0xffffffff > > other info that might help us debug this: > > 1 lock held by X/4873: > #0: (&mm->mmap_sem){----}, at: [<c0125a1e>] sys_mmap2+0x8e/0xc0 > > stack backtrace: > Pid: 4873, comm: X Not tainted 2.6.27-rc6-tip #1 > [<c017cd09>] print_circular_bug_tail+0x79/0xc0 > [<c017d8b7>] validate_chain+0x6b7/0xf50 > [<c017a5b5>] ? trace_hardirqs_off_caller+0x15/0xb0 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c0d6f2ab>] __mutex_lock_common+0xab/0x3c0 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c0d6f698>] mutex_lock_nested+0x38/0x50 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c020ba20>] mmap+0x40/0xa0 > [<c01b111e>] mmap_region+0x14e/0x450 > [<c01afb88>] ? arch_get_unmapped_area_topdown+0xf8/0x160 > [<c01b170f>] do_mmap_pgoff+0x2ef/0x310 > [<c0125a3d>] sys_mmap2+0xad/0xc0 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<c0120000>] ? __switch_to+0x130/0x220 > ======================= > evbug.c: Event. Dev: input3, Type: 20, Code: 0, Value: 500 > warning: `sudo' uses deprecated v2 capabilities in a way that may be insecure. > > i've attached the config. > > at first sight it looks like a genuine bug in fs/sysfs/bin.c? Yes, it is a real bug by the looks. bin.c takes bb->mutex under mmap_sem when it is mmapped, and then does its copy_*_user under bb->mutex too. Here is a basic fix for the sysfs lor. From: Nick Piggin <npiggin@suse.de> Signed-off-by: Ingo Molnar <mingo@elte.hu> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2008-09-12 17:24:11 +08:00
temp = kmalloc(count, GFP_KERNEL);
if (!temp)
return -ENOMEM;
sysfs: fix deadlock On Thu, Sep 11, 2008 at 10:27:10AM +0200, Ingo Molnar wrote: > and it's working fine on most boxes. One testbox found this new locking > scenario: > > PM: Adding info for No Bus:vcsa7 > EDAC DEBUG: MC0: i82860_check() > > ======================================================= > [ INFO: possible circular locking dependency detected ] > 2.6.27-rc6-tip #1 > ------------------------------------------------------- > X/4873 is trying to acquire lock: > (&bb->mutex){--..}, at: [<c020ba20>] mmap+0x40/0xa0 > > but task is already holding lock: > (&mm->mmap_sem){----}, at: [<c0125a1e>] sys_mmap2+0x8e/0xc0 > > which lock already depends on the new lock. > > > the existing dependency chain (in reverse order) is: > > -> #1 (&mm->mmap_sem){----}: > [<c017dc96>] validate_chain+0xa96/0xf50 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c01aa8fb>] might_fault+0x6b/0x90 > [<c040b618>] copy_to_user+0x38/0x60 > [<c020bcfb>] read+0xfb/0x170 > [<c01c09a5>] vfs_read+0x95/0x110 > [<c01c1443>] sys_pread64+0x63/0x80 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<ffffffff>] 0xffffffff > > -> #0 (&bb->mutex){--..}: > [<c017d8b7>] validate_chain+0x6b7/0xf50 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c0d6f2ab>] __mutex_lock_common+0xab/0x3c0 > [<c0d6f698>] mutex_lock_nested+0x38/0x50 > [<c020ba20>] mmap+0x40/0xa0 > [<c01b111e>] mmap_region+0x14e/0x450 > [<c01b170f>] do_mmap_pgoff+0x2ef/0x310 > [<c0125a3d>] sys_mmap2+0xad/0xc0 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<ffffffff>] 0xffffffff > > other info that might help us debug this: > > 1 lock held by X/4873: > #0: (&mm->mmap_sem){----}, at: [<c0125a1e>] sys_mmap2+0x8e/0xc0 > > stack backtrace: > Pid: 4873, comm: X Not tainted 2.6.27-rc6-tip #1 > [<c017cd09>] print_circular_bug_tail+0x79/0xc0 > [<c017d8b7>] validate_chain+0x6b7/0xf50 > [<c017a5b5>] ? trace_hardirqs_off_caller+0x15/0xb0 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c0d6f2ab>] __mutex_lock_common+0xab/0x3c0 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c0d6f698>] mutex_lock_nested+0x38/0x50 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c020ba20>] mmap+0x40/0xa0 > [<c01b111e>] mmap_region+0x14e/0x450 > [<c01afb88>] ? arch_get_unmapped_area_topdown+0xf8/0x160 > [<c01b170f>] do_mmap_pgoff+0x2ef/0x310 > [<c0125a3d>] sys_mmap2+0xad/0xc0 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<c0120000>] ? __switch_to+0x130/0x220 > ======================= > evbug.c: Event. Dev: input3, Type: 20, Code: 0, Value: 500 > warning: `sudo' uses deprecated v2 capabilities in a way that may be insecure. > > i've attached the config. > > at first sight it looks like a genuine bug in fs/sysfs/bin.c? Yes, it is a real bug by the looks. bin.c takes bb->mutex under mmap_sem when it is mmapped, and then does its copy_*_user under bb->mutex too. Here is a basic fix for the sysfs lor. From: Nick Piggin <npiggin@suse.de> Signed-off-by: Ingo Molnar <mingo@elte.hu> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2008-09-12 17:24:11 +08:00
if (copy_from_user(temp, userbuf, count)) {
count = -EFAULT;
sysfs: fix deadlock On Thu, Sep 11, 2008 at 10:27:10AM +0200, Ingo Molnar wrote: > and it's working fine on most boxes. One testbox found this new locking > scenario: > > PM: Adding info for No Bus:vcsa7 > EDAC DEBUG: MC0: i82860_check() > > ======================================================= > [ INFO: possible circular locking dependency detected ] > 2.6.27-rc6-tip #1 > ------------------------------------------------------- > X/4873 is trying to acquire lock: > (&bb->mutex){--..}, at: [<c020ba20>] mmap+0x40/0xa0 > > but task is already holding lock: > (&mm->mmap_sem){----}, at: [<c0125a1e>] sys_mmap2+0x8e/0xc0 > > which lock already depends on the new lock. > > > the existing dependency chain (in reverse order) is: > > -> #1 (&mm->mmap_sem){----}: > [<c017dc96>] validate_chain+0xa96/0xf50 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c01aa8fb>] might_fault+0x6b/0x90 > [<c040b618>] copy_to_user+0x38/0x60 > [<c020bcfb>] read+0xfb/0x170 > [<c01c09a5>] vfs_read+0x95/0x110 > [<c01c1443>] sys_pread64+0x63/0x80 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<ffffffff>] 0xffffffff > > -> #0 (&bb->mutex){--..}: > [<c017d8b7>] validate_chain+0x6b7/0xf50 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c0d6f2ab>] __mutex_lock_common+0xab/0x3c0 > [<c0d6f698>] mutex_lock_nested+0x38/0x50 > [<c020ba20>] mmap+0x40/0xa0 > [<c01b111e>] mmap_region+0x14e/0x450 > [<c01b170f>] do_mmap_pgoff+0x2ef/0x310 > [<c0125a3d>] sys_mmap2+0xad/0xc0 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<ffffffff>] 0xffffffff > > other info that might help us debug this: > > 1 lock held by X/4873: > #0: (&mm->mmap_sem){----}, at: [<c0125a1e>] sys_mmap2+0x8e/0xc0 > > stack backtrace: > Pid: 4873, comm: X Not tainted 2.6.27-rc6-tip #1 > [<c017cd09>] print_circular_bug_tail+0x79/0xc0 > [<c017d8b7>] validate_chain+0x6b7/0xf50 > [<c017a5b5>] ? trace_hardirqs_off_caller+0x15/0xb0 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c0d6f2ab>] __mutex_lock_common+0xab/0x3c0 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c0d6f698>] mutex_lock_nested+0x38/0x50 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c020ba20>] mmap+0x40/0xa0 > [<c01b111e>] mmap_region+0x14e/0x450 > [<c01afb88>] ? arch_get_unmapped_area_topdown+0xf8/0x160 > [<c01b170f>] do_mmap_pgoff+0x2ef/0x310 > [<c0125a3d>] sys_mmap2+0xad/0xc0 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<c0120000>] ? __switch_to+0x130/0x220 > ======================= > evbug.c: Event. Dev: input3, Type: 20, Code: 0, Value: 500 > warning: `sudo' uses deprecated v2 capabilities in a way that may be insecure. > > i've attached the config. > > at first sight it looks like a genuine bug in fs/sysfs/bin.c? Yes, it is a real bug by the looks. bin.c takes bb->mutex under mmap_sem when it is mmapped, and then does its copy_*_user under bb->mutex too. Here is a basic fix for the sysfs lor. From: Nick Piggin <npiggin@suse.de> Signed-off-by: Ingo Molnar <mingo@elte.hu> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2008-09-12 17:24:11 +08:00
goto out_free;
}
sysfs: fix deadlock On Thu, Sep 11, 2008 at 10:27:10AM +0200, Ingo Molnar wrote: > and it's working fine on most boxes. One testbox found this new locking > scenario: > > PM: Adding info for No Bus:vcsa7 > EDAC DEBUG: MC0: i82860_check() > > ======================================================= > [ INFO: possible circular locking dependency detected ] > 2.6.27-rc6-tip #1 > ------------------------------------------------------- > X/4873 is trying to acquire lock: > (&bb->mutex){--..}, at: [<c020ba20>] mmap+0x40/0xa0 > > but task is already holding lock: > (&mm->mmap_sem){----}, at: [<c0125a1e>] sys_mmap2+0x8e/0xc0 > > which lock already depends on the new lock. > > > the existing dependency chain (in reverse order) is: > > -> #1 (&mm->mmap_sem){----}: > [<c017dc96>] validate_chain+0xa96/0xf50 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c01aa8fb>] might_fault+0x6b/0x90 > [<c040b618>] copy_to_user+0x38/0x60 > [<c020bcfb>] read+0xfb/0x170 > [<c01c09a5>] vfs_read+0x95/0x110 > [<c01c1443>] sys_pread64+0x63/0x80 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<ffffffff>] 0xffffffff > > -> #0 (&bb->mutex){--..}: > [<c017d8b7>] validate_chain+0x6b7/0xf50 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c0d6f2ab>] __mutex_lock_common+0xab/0x3c0 > [<c0d6f698>] mutex_lock_nested+0x38/0x50 > [<c020ba20>] mmap+0x40/0xa0 > [<c01b111e>] mmap_region+0x14e/0x450 > [<c01b170f>] do_mmap_pgoff+0x2ef/0x310 > [<c0125a3d>] sys_mmap2+0xad/0xc0 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<ffffffff>] 0xffffffff > > other info that might help us debug this: > > 1 lock held by X/4873: > #0: (&mm->mmap_sem){----}, at: [<c0125a1e>] sys_mmap2+0x8e/0xc0 > > stack backtrace: > Pid: 4873, comm: X Not tainted 2.6.27-rc6-tip #1 > [<c017cd09>] print_circular_bug_tail+0x79/0xc0 > [<c017d8b7>] validate_chain+0x6b7/0xf50 > [<c017a5b5>] ? trace_hardirqs_off_caller+0x15/0xb0 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c0d6f2ab>] __mutex_lock_common+0xab/0x3c0 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c0d6f698>] mutex_lock_nested+0x38/0x50 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c020ba20>] mmap+0x40/0xa0 > [<c01b111e>] mmap_region+0x14e/0x450 > [<c01afb88>] ? arch_get_unmapped_area_topdown+0xf8/0x160 > [<c01b170f>] do_mmap_pgoff+0x2ef/0x310 > [<c0125a3d>] sys_mmap2+0xad/0xc0 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<c0120000>] ? __switch_to+0x130/0x220 > ======================= > evbug.c: Event. Dev: input3, Type: 20, Code: 0, Value: 500 > warning: `sudo' uses deprecated v2 capabilities in a way that may be insecure. > > i've attached the config. > > at first sight it looks like a genuine bug in fs/sysfs/bin.c? Yes, it is a real bug by the looks. bin.c takes bb->mutex under mmap_sem when it is mmapped, and then does its copy_*_user under bb->mutex too. Here is a basic fix for the sysfs lor. From: Nick Piggin <npiggin@suse.de> Signed-off-by: Ingo Molnar <mingo@elte.hu> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2008-09-12 17:24:11 +08:00
mutex_lock(&bb->mutex);
memcpy(bb->buffer, temp, count);
count = flush_write(dentry, bb->buffer, offs, count);
sysfs: fix deadlock On Thu, Sep 11, 2008 at 10:27:10AM +0200, Ingo Molnar wrote: > and it's working fine on most boxes. One testbox found this new locking > scenario: > > PM: Adding info for No Bus:vcsa7 > EDAC DEBUG: MC0: i82860_check() > > ======================================================= > [ INFO: possible circular locking dependency detected ] > 2.6.27-rc6-tip #1 > ------------------------------------------------------- > X/4873 is trying to acquire lock: > (&bb->mutex){--..}, at: [<c020ba20>] mmap+0x40/0xa0 > > but task is already holding lock: > (&mm->mmap_sem){----}, at: [<c0125a1e>] sys_mmap2+0x8e/0xc0 > > which lock already depends on the new lock. > > > the existing dependency chain (in reverse order) is: > > -> #1 (&mm->mmap_sem){----}: > [<c017dc96>] validate_chain+0xa96/0xf50 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c01aa8fb>] might_fault+0x6b/0x90 > [<c040b618>] copy_to_user+0x38/0x60 > [<c020bcfb>] read+0xfb/0x170 > [<c01c09a5>] vfs_read+0x95/0x110 > [<c01c1443>] sys_pread64+0x63/0x80 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<ffffffff>] 0xffffffff > > -> #0 (&bb->mutex){--..}: > [<c017d8b7>] validate_chain+0x6b7/0xf50 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c0d6f2ab>] __mutex_lock_common+0xab/0x3c0 > [<c0d6f698>] mutex_lock_nested+0x38/0x50 > [<c020ba20>] mmap+0x40/0xa0 > [<c01b111e>] mmap_region+0x14e/0x450 > [<c01b170f>] do_mmap_pgoff+0x2ef/0x310 > [<c0125a3d>] sys_mmap2+0xad/0xc0 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<ffffffff>] 0xffffffff > > other info that might help us debug this: > > 1 lock held by X/4873: > #0: (&mm->mmap_sem){----}, at: [<c0125a1e>] sys_mmap2+0x8e/0xc0 > > stack backtrace: > Pid: 4873, comm: X Not tainted 2.6.27-rc6-tip #1 > [<c017cd09>] print_circular_bug_tail+0x79/0xc0 > [<c017d8b7>] validate_chain+0x6b7/0xf50 > [<c017a5b5>] ? trace_hardirqs_off_caller+0x15/0xb0 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c0d6f2ab>] __mutex_lock_common+0xab/0x3c0 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c0d6f698>] mutex_lock_nested+0x38/0x50 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c020ba20>] mmap+0x40/0xa0 > [<c01b111e>] mmap_region+0x14e/0x450 > [<c01afb88>] ? arch_get_unmapped_area_topdown+0xf8/0x160 > [<c01b170f>] do_mmap_pgoff+0x2ef/0x310 > [<c0125a3d>] sys_mmap2+0xad/0xc0 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<c0120000>] ? __switch_to+0x130/0x220 > ======================= > evbug.c: Event. Dev: input3, Type: 20, Code: 0, Value: 500 > warning: `sudo' uses deprecated v2 capabilities in a way that may be insecure. > > i've attached the config. > > at first sight it looks like a genuine bug in fs/sysfs/bin.c? Yes, it is a real bug by the looks. bin.c takes bb->mutex under mmap_sem when it is mmapped, and then does its copy_*_user under bb->mutex too. Here is a basic fix for the sysfs lor. From: Nick Piggin <npiggin@suse.de> Signed-off-by: Ingo Molnar <mingo@elte.hu> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2008-09-12 17:24:11 +08:00
mutex_unlock(&bb->mutex);
if (count > 0)
*off = offs + count;
sysfs: fix deadlock On Thu, Sep 11, 2008 at 10:27:10AM +0200, Ingo Molnar wrote: > and it's working fine on most boxes. One testbox found this new locking > scenario: > > PM: Adding info for No Bus:vcsa7 > EDAC DEBUG: MC0: i82860_check() > > ======================================================= > [ INFO: possible circular locking dependency detected ] > 2.6.27-rc6-tip #1 > ------------------------------------------------------- > X/4873 is trying to acquire lock: > (&bb->mutex){--..}, at: [<c020ba20>] mmap+0x40/0xa0 > > but task is already holding lock: > (&mm->mmap_sem){----}, at: [<c0125a1e>] sys_mmap2+0x8e/0xc0 > > which lock already depends on the new lock. > > > the existing dependency chain (in reverse order) is: > > -> #1 (&mm->mmap_sem){----}: > [<c017dc96>] validate_chain+0xa96/0xf50 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c01aa8fb>] might_fault+0x6b/0x90 > [<c040b618>] copy_to_user+0x38/0x60 > [<c020bcfb>] read+0xfb/0x170 > [<c01c09a5>] vfs_read+0x95/0x110 > [<c01c1443>] sys_pread64+0x63/0x80 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<ffffffff>] 0xffffffff > > -> #0 (&bb->mutex){--..}: > [<c017d8b7>] validate_chain+0x6b7/0xf50 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c0d6f2ab>] __mutex_lock_common+0xab/0x3c0 > [<c0d6f698>] mutex_lock_nested+0x38/0x50 > [<c020ba20>] mmap+0x40/0xa0 > [<c01b111e>] mmap_region+0x14e/0x450 > [<c01b170f>] do_mmap_pgoff+0x2ef/0x310 > [<c0125a3d>] sys_mmap2+0xad/0xc0 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<ffffffff>] 0xffffffff > > other info that might help us debug this: > > 1 lock held by X/4873: > #0: (&mm->mmap_sem){----}, at: [<c0125a1e>] sys_mmap2+0x8e/0xc0 > > stack backtrace: > Pid: 4873, comm: X Not tainted 2.6.27-rc6-tip #1 > [<c017cd09>] print_circular_bug_tail+0x79/0xc0 > [<c017d8b7>] validate_chain+0x6b7/0xf50 > [<c017a5b5>] ? trace_hardirqs_off_caller+0x15/0xb0 > [<c017ef2b>] __lock_acquire+0x2cb/0x5b0 > [<c017f299>] lock_acquire+0x89/0xc0 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c0d6f2ab>] __mutex_lock_common+0xab/0x3c0 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c0d6f698>] mutex_lock_nested+0x38/0x50 > [<c020ba20>] ? mmap+0x40/0xa0 > [<c020ba20>] mmap+0x40/0xa0 > [<c01b111e>] mmap_region+0x14e/0x450 > [<c01afb88>] ? arch_get_unmapped_area_topdown+0xf8/0x160 > [<c01b170f>] do_mmap_pgoff+0x2ef/0x310 > [<c0125a3d>] sys_mmap2+0xad/0xc0 > [<c012146f>] sysenter_do_call+0x12/0x43 > [<c0120000>] ? __switch_to+0x130/0x220 > ======================= > evbug.c: Event. Dev: input3, Type: 20, Code: 0, Value: 500 > warning: `sudo' uses deprecated v2 capabilities in a way that may be insecure. > > i've attached the config. > > at first sight it looks like a genuine bug in fs/sysfs/bin.c? Yes, it is a real bug by the looks. bin.c takes bb->mutex under mmap_sem when it is mmapped, and then does its copy_*_user under bb->mutex too. Here is a basic fix for the sysfs lor. From: Nick Piggin <npiggin@suse.de> Signed-off-by: Ingo Molnar <mingo@elte.hu> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2008-09-12 17:24:11 +08:00
out_free:
kfree(temp);
return count;
}
static int mmap(struct file *file, struct vm_area_struct *vma)
{
struct bin_buffer *bb = file->private_data;
struct sysfs_dirent *attr_sd = file->f_path.dentry->d_fsdata;
struct bin_attribute *attr = attr_sd->s_bin_attr.bin_attr;
struct kobject *kobj = attr_sd->s_parent->s_dir.kobj;
int rc;
mutex_lock(&bb->mutex);
sysfs: implement sysfs_dirent active reference and immediate disconnect sysfs: implement sysfs_dirent active reference and immediate disconnect Opening a sysfs node references its associated kobject, so userland can arbitrarily prolong lifetime of a kobject which complicates lifetime rules in drivers. This patch implements active reference and makes the association between kobject and sysfs immediately breakable. Now each sysfs_dirent has two reference counts - s_count and s_active. s_count is a regular reference count which guarantees that the containing sysfs_dirent is accessible. As long as s_count reference is held, all sysfs internal fields in sysfs_dirent are accessible including s_parent and s_name. The newly added s_active is active reference count. This is acquired by invoking sysfs_get_active() and it's the caller's responsibility to ensure sysfs_dirent itself is accessible (should be holding s_count one way or the other). Dereferencing sysfs_dirent to access objects out of sysfs proper requires active reference. This includes access to the associated kobjects, attributes and ops. The active references can be drained and denied by calling sysfs_deactivate(). All active sysfs_dirents must be deactivated after deletion but before the default reference is dropped. This enables immediate disconnect of sysfs nodes. Once a sysfs_dirent is deleted, it won't access any entity external to sysfs proper. Because attr/bin_attr ops access both the node itself and its parent for kobject, they need to hold active references to both. sysfs_get/put_active_two() helpers are provided to help grabbing both references. Parent's is acquired first and released last. Unlike other operations, mmapped area lingers on after mmap() is finished and the module implement implementing it and kobj need to stay referenced till all the mapped pages are gone. This is accomplished by holding one set of active references to the bin_attr and its parent if there have been any mmap during lifetime of an openfile. The references are dropped when the openfile is released. This change makes sysfs lifetime rules independent from both kobject's and module's. It not only fixes several race conditions caused by sysfs not holding onto the proper module when referencing kobject, but also helps fixing and simplifying lifetime management in driver model and drivers by taking sysfs out of the equation. Please read the following message for more info. http://article.gmane.org/gmane.linux.kernel/510293 Signed-off-by: Tejun Heo <htejun@gmail.com> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2007-06-14 02:45:16 +08:00
/* need attr_sd for attr, its parent for kobj */
if (!sysfs_get_active_two(attr_sd))
return -ENODEV;
rc = -EINVAL;
if (attr->mmap)
rc = attr->mmap(kobj, attr, vma);
if (rc == 0 && !bb->mmapped)
bb->mmapped = 1;
else
sysfs_put_active_two(attr_sd);
mutex_unlock(&bb->mutex);
return rc;
}
static int open(struct inode * inode, struct file * file)
{
struct sysfs_dirent *attr_sd = file->f_path.dentry->d_fsdata;
struct bin_attribute *attr = attr_sd->s_bin_attr.bin_attr;
struct bin_buffer *bb = NULL;
sysfs: implement sysfs_dirent active reference and immediate disconnect sysfs: implement sysfs_dirent active reference and immediate disconnect Opening a sysfs node references its associated kobject, so userland can arbitrarily prolong lifetime of a kobject which complicates lifetime rules in drivers. This patch implements active reference and makes the association between kobject and sysfs immediately breakable. Now each sysfs_dirent has two reference counts - s_count and s_active. s_count is a regular reference count which guarantees that the containing sysfs_dirent is accessible. As long as s_count reference is held, all sysfs internal fields in sysfs_dirent are accessible including s_parent and s_name. The newly added s_active is active reference count. This is acquired by invoking sysfs_get_active() and it's the caller's responsibility to ensure sysfs_dirent itself is accessible (should be holding s_count one way or the other). Dereferencing sysfs_dirent to access objects out of sysfs proper requires active reference. This includes access to the associated kobjects, attributes and ops. The active references can be drained and denied by calling sysfs_deactivate(). All active sysfs_dirents must be deactivated after deletion but before the default reference is dropped. This enables immediate disconnect of sysfs nodes. Once a sysfs_dirent is deleted, it won't access any entity external to sysfs proper. Because attr/bin_attr ops access both the node itself and its parent for kobject, they need to hold active references to both. sysfs_get/put_active_two() helpers are provided to help grabbing both references. Parent's is acquired first and released last. Unlike other operations, mmapped area lingers on after mmap() is finished and the module implement implementing it and kobj need to stay referenced till all the mapped pages are gone. This is accomplished by holding one set of active references to the bin_attr and its parent if there have been any mmap during lifetime of an openfile. The references are dropped when the openfile is released. This change makes sysfs lifetime rules independent from both kobject's and module's. It not only fixes several race conditions caused by sysfs not holding onto the proper module when referencing kobject, but also helps fixing and simplifying lifetime management in driver model and drivers by taking sysfs out of the equation. Please read the following message for more info. http://article.gmane.org/gmane.linux.kernel/510293 Signed-off-by: Tejun Heo <htejun@gmail.com> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2007-06-14 02:45:16 +08:00
int error;
/* binary file operations requires both @sd and its parent */
if (!sysfs_get_active_two(attr_sd))
sysfs: implement sysfs_dirent active reference and immediate disconnect sysfs: implement sysfs_dirent active reference and immediate disconnect Opening a sysfs node references its associated kobject, so userland can arbitrarily prolong lifetime of a kobject which complicates lifetime rules in drivers. This patch implements active reference and makes the association between kobject and sysfs immediately breakable. Now each sysfs_dirent has two reference counts - s_count and s_active. s_count is a regular reference count which guarantees that the containing sysfs_dirent is accessible. As long as s_count reference is held, all sysfs internal fields in sysfs_dirent are accessible including s_parent and s_name. The newly added s_active is active reference count. This is acquired by invoking sysfs_get_active() and it's the caller's responsibility to ensure sysfs_dirent itself is accessible (should be holding s_count one way or the other). Dereferencing sysfs_dirent to access objects out of sysfs proper requires active reference. This includes access to the associated kobjects, attributes and ops. The active references can be drained and denied by calling sysfs_deactivate(). All active sysfs_dirents must be deactivated after deletion but before the default reference is dropped. This enables immediate disconnect of sysfs nodes. Once a sysfs_dirent is deleted, it won't access any entity external to sysfs proper. Because attr/bin_attr ops access both the node itself and its parent for kobject, they need to hold active references to both. sysfs_get/put_active_two() helpers are provided to help grabbing both references. Parent's is acquired first and released last. Unlike other operations, mmapped area lingers on after mmap() is finished and the module implement implementing it and kobj need to stay referenced till all the mapped pages are gone. This is accomplished by holding one set of active references to the bin_attr and its parent if there have been any mmap during lifetime of an openfile. The references are dropped when the openfile is released. This change makes sysfs lifetime rules independent from both kobject's and module's. It not only fixes several race conditions caused by sysfs not holding onto the proper module when referencing kobject, but also helps fixing and simplifying lifetime management in driver model and drivers by taking sysfs out of the equation. Please read the following message for more info. http://article.gmane.org/gmane.linux.kernel/510293 Signed-off-by: Tejun Heo <htejun@gmail.com> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2007-06-14 02:45:16 +08:00
return -ENODEV;
error = -EACCES;
if ((file->f_mode & FMODE_WRITE) && !(attr->write || attr->mmap))
goto err_out;
if ((file->f_mode & FMODE_READ) && !(attr->read || attr->mmap))
goto err_out;
error = -ENOMEM;
bb = kzalloc(sizeof(*bb), GFP_KERNEL);
if (!bb)
goto err_out;
bb->buffer = kmalloc(PAGE_SIZE, GFP_KERNEL);
if (!bb->buffer)
goto err_out;
mutex_init(&bb->mutex);
file->private_data = bb;
/* open succeeded, put active references */
sysfs_put_active_two(attr_sd);
sysfs: implement sysfs_dirent active reference and immediate disconnect sysfs: implement sysfs_dirent active reference and immediate disconnect Opening a sysfs node references its associated kobject, so userland can arbitrarily prolong lifetime of a kobject which complicates lifetime rules in drivers. This patch implements active reference and makes the association between kobject and sysfs immediately breakable. Now each sysfs_dirent has two reference counts - s_count and s_active. s_count is a regular reference count which guarantees that the containing sysfs_dirent is accessible. As long as s_count reference is held, all sysfs internal fields in sysfs_dirent are accessible including s_parent and s_name. The newly added s_active is active reference count. This is acquired by invoking sysfs_get_active() and it's the caller's responsibility to ensure sysfs_dirent itself is accessible (should be holding s_count one way or the other). Dereferencing sysfs_dirent to access objects out of sysfs proper requires active reference. This includes access to the associated kobjects, attributes and ops. The active references can be drained and denied by calling sysfs_deactivate(). All active sysfs_dirents must be deactivated after deletion but before the default reference is dropped. This enables immediate disconnect of sysfs nodes. Once a sysfs_dirent is deleted, it won't access any entity external to sysfs proper. Because attr/bin_attr ops access both the node itself and its parent for kobject, they need to hold active references to both. sysfs_get/put_active_two() helpers are provided to help grabbing both references. Parent's is acquired first and released last. Unlike other operations, mmapped area lingers on after mmap() is finished and the module implement implementing it and kobj need to stay referenced till all the mapped pages are gone. This is accomplished by holding one set of active references to the bin_attr and its parent if there have been any mmap during lifetime of an openfile. The references are dropped when the openfile is released. This change makes sysfs lifetime rules independent from both kobject's and module's. It not only fixes several race conditions caused by sysfs not holding onto the proper module when referencing kobject, but also helps fixing and simplifying lifetime management in driver model and drivers by taking sysfs out of the equation. Please read the following message for more info. http://article.gmane.org/gmane.linux.kernel/510293 Signed-off-by: Tejun Heo <htejun@gmail.com> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2007-06-14 02:45:16 +08:00
return 0;
err_out:
sysfs_put_active_two(attr_sd);
sysfs: implement sysfs_dirent active reference and immediate disconnect sysfs: implement sysfs_dirent active reference and immediate disconnect Opening a sysfs node references its associated kobject, so userland can arbitrarily prolong lifetime of a kobject which complicates lifetime rules in drivers. This patch implements active reference and makes the association between kobject and sysfs immediately breakable. Now each sysfs_dirent has two reference counts - s_count and s_active. s_count is a regular reference count which guarantees that the containing sysfs_dirent is accessible. As long as s_count reference is held, all sysfs internal fields in sysfs_dirent are accessible including s_parent and s_name. The newly added s_active is active reference count. This is acquired by invoking sysfs_get_active() and it's the caller's responsibility to ensure sysfs_dirent itself is accessible (should be holding s_count one way or the other). Dereferencing sysfs_dirent to access objects out of sysfs proper requires active reference. This includes access to the associated kobjects, attributes and ops. The active references can be drained and denied by calling sysfs_deactivate(). All active sysfs_dirents must be deactivated after deletion but before the default reference is dropped. This enables immediate disconnect of sysfs nodes. Once a sysfs_dirent is deleted, it won't access any entity external to sysfs proper. Because attr/bin_attr ops access both the node itself and its parent for kobject, they need to hold active references to both. sysfs_get/put_active_two() helpers are provided to help grabbing both references. Parent's is acquired first and released last. Unlike other operations, mmapped area lingers on after mmap() is finished and the module implement implementing it and kobj need to stay referenced till all the mapped pages are gone. This is accomplished by holding one set of active references to the bin_attr and its parent if there have been any mmap during lifetime of an openfile. The references are dropped when the openfile is released. This change makes sysfs lifetime rules independent from both kobject's and module's. It not only fixes several race conditions caused by sysfs not holding onto the proper module when referencing kobject, but also helps fixing and simplifying lifetime management in driver model and drivers by taking sysfs out of the equation. Please read the following message for more info. http://article.gmane.org/gmane.linux.kernel/510293 Signed-off-by: Tejun Heo <htejun@gmail.com> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2007-06-14 02:45:16 +08:00
kfree(bb);
return error;
}
static int release(struct inode * inode, struct file * file)
{
struct sysfs_dirent *attr_sd = file->f_path.dentry->d_fsdata;
struct bin_buffer *bb = file->private_data;
sysfs: implement sysfs_dirent active reference and immediate disconnect sysfs: implement sysfs_dirent active reference and immediate disconnect Opening a sysfs node references its associated kobject, so userland can arbitrarily prolong lifetime of a kobject which complicates lifetime rules in drivers. This patch implements active reference and makes the association between kobject and sysfs immediately breakable. Now each sysfs_dirent has two reference counts - s_count and s_active. s_count is a regular reference count which guarantees that the containing sysfs_dirent is accessible. As long as s_count reference is held, all sysfs internal fields in sysfs_dirent are accessible including s_parent and s_name. The newly added s_active is active reference count. This is acquired by invoking sysfs_get_active() and it's the caller's responsibility to ensure sysfs_dirent itself is accessible (should be holding s_count one way or the other). Dereferencing sysfs_dirent to access objects out of sysfs proper requires active reference. This includes access to the associated kobjects, attributes and ops. The active references can be drained and denied by calling sysfs_deactivate(). All active sysfs_dirents must be deactivated after deletion but before the default reference is dropped. This enables immediate disconnect of sysfs nodes. Once a sysfs_dirent is deleted, it won't access any entity external to sysfs proper. Because attr/bin_attr ops access both the node itself and its parent for kobject, they need to hold active references to both. sysfs_get/put_active_two() helpers are provided to help grabbing both references. Parent's is acquired first and released last. Unlike other operations, mmapped area lingers on after mmap() is finished and the module implement implementing it and kobj need to stay referenced till all the mapped pages are gone. This is accomplished by holding one set of active references to the bin_attr and its parent if there have been any mmap during lifetime of an openfile. The references are dropped when the openfile is released. This change makes sysfs lifetime rules independent from both kobject's and module's. It not only fixes several race conditions caused by sysfs not holding onto the proper module when referencing kobject, but also helps fixing and simplifying lifetime management in driver model and drivers by taking sysfs out of the equation. Please read the following message for more info. http://article.gmane.org/gmane.linux.kernel/510293 Signed-off-by: Tejun Heo <htejun@gmail.com> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
2007-06-14 02:45:16 +08:00
if (bb->mmapped)
sysfs_put_active_two(attr_sd);
kfree(bb->buffer);
kfree(bb);
return 0;
}
const struct file_operations bin_fops = {
.read = read,
.write = write,
.mmap = mmap,
.llseek = generic_file_llseek,
.open = open,
.release = release,
};
/**
* sysfs_create_bin_file - create binary file for object.
* @kobj: object.
* @attr: attribute descriptor.
*/
int sysfs_create_bin_file(struct kobject * kobj, struct bin_attribute * attr)
{
BUG_ON(!kobj || !kobj->sd || !attr);
return sysfs_add_file(kobj->sd, &attr->attr, SYSFS_KOBJ_BIN_ATTR);
}
/**
* sysfs_remove_bin_file - remove binary file for object.
* @kobj: object.
* @attr: attribute descriptor.
*/
void sysfs_remove_bin_file(struct kobject * kobj, struct bin_attribute * attr)
{
sysfs_hash_and_remove(kobj->sd, attr->attr.name);
}
EXPORT_SYMBOL_GPL(sysfs_create_bin_file);
EXPORT_SYMBOL_GPL(sysfs_remove_bin_file);