linux-kernel - Re: linux-next: Tree for December 9

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: <18752.16273.604044.142078@notabene.brown>
Date:	Thu, 11 Dec 2008 09:15:45 +1100
From:	Neil Brown <neilb@...e.de>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	kay.sievers@...y.org, sfr@...b.auug.org.au,
	linux-next@...r.kernel.org, linux-kernel@...r.kernel.org,
	adobriyan@...il.com, greg@...ah.com
Subject: Re: linux-next: Tree for December 9

On Wednesday December 10, akpm@...ux-foundation.org wrote:
> On Wed, 10 Dec 2008 12:42:58 +1100 (EST)
> "NeilBrown" <neilb@...e.de> wrote:
> 
> > On Wed, December 10, 2008 12:30 pm, Andrew Morton wrote:
> > > On Wed, 10 Dec 2008 01:31:13 +0100 "Kay Sievers" <kay.sievers@...y.org>
> > > wrote:
> > >
> > >> > akpm2:/home/akpm# udevmonitor
> > >> > udevmonitor prints the received event from the kernel [UEVENT]
> > >> > and the event which udev sends out after rule processing [UDEV]
> > >> >
> > >> > UDEV  [1228867146.103334] add@...ass/bdi/9:0
> > >> > UDEV  [1228867146.107566] remove@...ock/md0
> > >> > UDEV  [1228867146.111969] remove@...ass/bdi/9:0
> > >> > UEVENT[1228867146.119889] add@...ock/md0
> > >> > UEVENT[1228867146.119964] add@...ass/bdi/9:0
> > >> > UEVENT[1228867146.120162] remove@...ass/bdi/9:0
> > >> > UEVENT[1228867146.120205] remove@...ock/md0
> > >> > UDEV  [1228867146.122839] add@...ock/md0
> > >> > UDEV  [1228867146.129125] add@...ass/bdi/9:0
> > >> > UDEV  [1228867146.133459] remove@...ock/md0
> > >> > UDEV  [1228867146.137813] remove@...ass/bdi/9:0
> > >> > UEVENT[1228867146.145652] add@...ock/md0
> > >>
> > >> Weird loop, something is accessing /dev/md0, i guess, which creates
> > >> the kernel device, and the event, which accesses /dev/md0 again and it
> > >> goes crazy. Maybe caused by changes Neil did.
> > >
> > > I knew an Australian was to blame - it's just a matter of determining
> > > which one.
> > 
> > Let's not count our chickens .... :-)
> > 
> > Can you get me
> >    tar czvf - /lib/udev /etc/udev | mail neilb
> > 
> > so I can see exactly what FC6 is likely to try to do when an
> > md device appears or disappears?
> 
> Is at http://userweb.kernel.org/~akpm/udev.tar.gz, thanks.

Thanks.
That udev code does no special handling of md device, and runs vol-id
on every device that appears to see what it contains.

So if an md device is ever opened and closed immediately - as
typically happens at boot to sent the AUTO_RUN ioctl to do in-kernel
autodetection - then it will start a loop:

loop:
  The open causes an ADD event to udev.
  By the time udev handles it the device is closed, so when
   udev opens the device (via vol-id), the device is re-created
   triggering another ADD event.
  vol-id then closes the device and it disappears.
  Goto loop

So it isn't safe to destroy the md device as soon as it is closed.
Next best option is to destroy it after close providing at least one
ioctl has been made (or similar change via writing to sysfs).
This means that in most cases the device will disappear when not
needed, but in the particular case of some code which doesn't really
understand 'md' opening the device, it will hang around to avoid
causing confusion.

I have just pushed out an updated for-next branch.
The incremental patch is below.

Thanks,
NeilBrown


diff --git a/drivers/md/md.c b/drivers/md/md.c
index 89700b5..bef91b8 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -262,7 +262,7 @@ static mddev_t * mddev_find(dev_t unit)
 		if (new) {
 			list_add(&new->all_mddevs, &all_mddevs);
 			spin_unlock(&all_mddevs_lock);
-			new->hold_active = UNTIL_CLOSE;
+			new->hold_active = UNTIL_IOCTL;
 			return new;
 		}
 	} else if (new) {
@@ -3487,6 +3487,8 @@ md_attr_store(struct kobject *kobj, struct attribute *attr,
 	if (!capable(CAP_SYS_ADMIN))
 		return -EACCES;
 	rv = mddev_lock(mddev);
+	if (mddev->hold_active == UNTIL_IOCTL)
+		mddev->hold_active = 0;
 	if (!rv) {
 		rv = entry->store(mddev, page, length);
 		mddev_unlock(mddev);
@@ -5160,6 +5162,9 @@ static int md_ioctl(struct block_device *bdev, fmode_t mode,
 
 done_unlock:
 abort_unlock:
+	if (mddev->hold_active == UNTIL_IOCTL &&
+	    err != -EINVAL)
+		mddev->hold_active = 0;
 	mddev_unlock(mddev);
 
 	return err;
@@ -5209,8 +5214,6 @@ static int md_release(struct gendisk *disk, fmode_t mode)
 
 	BUG_ON(!mddev);
 	atomic_dec(&mddev->openers);
-	if (mddev->hold_active == UNTIL_CLOSE)
-		mddev->hold_active = 0;
 	mddev_put(mddev);
 
 	return 0;
diff --git a/include/linux/raid/md_k.h b/include/linux/raid/md_k.h
index 19756d7..dac4217 100644
--- a/include/linux/raid/md_k.h
+++ b/include/linux/raid/md_k.h
@@ -138,7 +138,7 @@ struct mddev_s
 
 	struct kobject			kobj;
 	int				hold_active;
-#define	UNTIL_CLOSE	1
+#define	UNTIL_IOCTL	1
 #define	UNTIL_STOP	2
 
 	/* Superblock information */
--
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/