linux-kernel - Re: [BUG] mm: bdi: export BDI attributes in sysfs

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20080515204417.GT23649@ajones-laptop.nbttech.com>
Date:	Thu, 15 May 2008 13:44:17 -0700
From:	Arthur Jones <ajones@...erbed.com>
To:	Miklos Szeredi <miklos@...redi.hu>
CC:	"a.p.zijlstra@...llo.nl" <a.p.zijlstra@...llo.nl>,
	"mszeredi@...e.cz" <mszeredi@...e.cz>,
	"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
	"kay.sievers@...y.org" <kay.sievers@...y.org>,
	"greg@...ah.com" <greg@...ah.com>,
	"trond.myklebust@....uio.no" <trond.myklebust@....uio.no>,
	"torvalds@...ux-foundation.org" <torvalds@...ux-foundation.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [BUG] mm: bdi: export BDI attributes in sysfs

Hi Miklos,  ...

On Thu, May 15, 2008 at 11:53:02AM -0700, Miklos Szeredi wrote:
> > About 1 in 4 times, when turning the power back
> > on for a drive, I get the BUG reported below.  The
> > drives are not mounted, just sitting there getting
> > power cycled (the test looks to make sure they are
> > being seen by the scsi subsystem and udev).
> 
> > > BUG: unable to handle kernel NULL pointer dereference at
> > > 0000000000000000
> > > IP: [<ffffffff802694e3>] read_ahead_kb_show+0x1b/0x2d
> 
> I suspect this is because there's a race between the show/store
> functions and dev_set_drvdata() in bdi_register().
> 
> Could you try this patch to verify that this is indeed the problem?

Well, the patch had more conflicts than Obama at an
RNC fundraiser (spaces/tabs?) and didn't compile at first
-- but w/ some minor tweaks it finally succumbed and after
many power cycles I no longer see the problem.

Thanks for your help...

Arthur

> Index: linux-2.6/mm/backing-dev.c
> ===================================================================
> --- linux-2.6.orig/mm/backing-dev.c     2008-05-14 12:49:33.000000000 +0200
> +++ linux-2.6/mm/backing-dev.c  2008-05-15 20:37:55.000000000 +0200
> @@ -15,6 +15,7 @@ static struct class *bdi_class;
>  #include <linux/seq_file.h>
> 
>  static struct dentry *bdi_debug_root;
> +static DEFINE_MUTEX(bdi_dev_mutex);
> 
>  static void bdi_debug_init(void)
>  {
> @@ -84,11 +85,19 @@ static inline void bdi_debug_unregister(
>  }
>  #endif
> 
> +static struct backing_dev_info *dev_get_bdi(struct device *dev)
> +{
> +       mutex_lock(&bdi_dev_mutex);
> +       mutex_unlock(&bdi_dev_mutex);
> +
> +       return dev_get_drvdata(dev);
> +}
> +
>  static ssize_t read_ahead_kb_store(struct device *dev,
>                                   struct device_attribute *attr,
>                                   const char *buf, size_t count)
>  {
> -       struct backing_dev_info *bdi = dev_get_drvdata(dev);
> +       struct backing_dev_info *bdi = dev_get_bdi(dev);
>         char *end;
>         unsigned long read_ahead_kb;
>         ssize_t ret = -EINVAL;
> @@ -107,7 +116,7 @@ static ssize_t read_ahead_kb_store(struc
>  static ssize_t name##_show(struct device *dev,                         \
>                            struct device_attribute *attr, char *page)   \
>  {                                                                      \
> -       struct backing_dev_info *bdi = dev_get_drvdata(dev);            \
> +       struct backing_dev_info *bdi = dev_get_bdi(dev);                \
>                                                                         \
>         return snprintf(page, PAGE_SIZE-1, "%lld\n", (long long)expr);  \
>  }
> @@ -117,7 +126,7 @@ BDI_SHOW(read_ahead_kb, K(bdi->ra_pages)
>  static ssize_t min_ratio_store(struct device *dev,
>                 struct device_attribute *attr, const char *buf, size_t count)
>  {
> -       struct backing_dev_info *bdi = dev_get_drvdata(dev);
> +       struct backing_dev_info *bdi = dev_get_bdi(dev);
>         char *end;
>         unsigned int ratio;
>         ssize_t ret = -EINVAL;
> @@ -135,7 +144,7 @@ BDI_SHOW(min_ratio, bdi->min_ratio)
>  static ssize_t max_ratio_store(struct device *dev,
>                 struct device_attribute *attr, const char *buf, size_t count)
>  {
> -       struct backing_dev_info *bdi = dev_get_drvdata(dev);
> +       struct backing_dev_info *bdi = dev_get_bdi(dev);
>         char *end;
>         unsigned int ratio;
>         ssize_t ret = -EINVAL;
> @@ -184,6 +193,7 @@ int bdi_register(struct backing_dev_info
>         if (!name)
>                 return -ENOMEM;
> 
> +       mutex_lock(&bdi_dev_mutex);
>         dev = device_create(bdi_class, parent, MKDEV(0, 0), name);
>         if (IS_ERR(dev)) {
>                 ret = PTR_ERR(dev);
> @@ -195,6 +205,7 @@ int bdi_register(struct backing_dev_info
>         bdi_debug_register(bdi, name);
> 
>  exit:
> +       mutex_unlock(&bdi_dev_mutex);
>         kfree(name);
>         return ret;
>  }
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/