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]
Date:	Mon, 28 Nov 2011 16:04:00 +0100
From:	Jan Kara <jack@...e.cz>
To:	Mikulas Patocka <mpatocka@...hat.com>
Cc:	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	Jan Kara <jack@...e.cz>, dm-devel@...hat.com,
	Valerie Aurora <val@...consulting.com>
Subject: Re: [PATCH] deadlock with suspend and quotas

  Hello,

On Fri 25-11-11 15:25:16, Mikulas Patocka wrote:
> This script causes a kernel deadlock:
> #!/bin/sh
> set -e
> DEVICE=/dev/vg1/linear
> lvchange -ay $DEVICE
> mkfs.ext3 $DEVICE
> mount -t ext3 -o usrquota,grpquota $DEVICE /mnt/test
> quotacheck -gu /mnt/test
> umount /mnt/test
> mount -t ext3 -o usrquota,grpquota $DEVICE /mnt/test
> quotaon /mnt/test
> dmsetup suspend $DEVICE
> setquota -u root 1 2 3 4 /mnt/test &
> sleep 1
> dmsetup resume $DEVICE
> 
> setquota acquired semaphore s_umount for read and then tried to perform
> a transaction (and waits because the device is suspended).
> dmsetup resume tries to acquire s_umount for write before resuming the device
> (and waits for setquota).
> 
> Here are stacktraces:
> setquota:
> [   67.524456]  [<ffffffff810aa84e>] ? get_page_from_freelist+0x31e/0x790
> [   67.524529]  [<ffffffffa0250265>] ? start_this_handle.isra.9+0x265/0x3b0 [jbd]
> [   67.524604]  [<ffffffff8105bc00>] ? add_wait_queue+0x60/0x60
> [   67.524675]  [<ffffffffa02505a1>] ? journal_start+0xc1/0x100 [jbd]
> [   67.524742]  [<ffffffff810e62d6>] ? kmem_cache_alloc+0xf6/0x1b0
> [   67.524808]  [<ffffffffa028018d>] ? ext3_acquire_dquot+0x3d/0x80 [ext3]
> [   67.524872]  [<ffffffff81143749>] ? dqget+0x359/0x3b0
> [   67.524916]  [<ffffffff81143ad4>] ? dquot_get_dqblk+0x14/0x1b0
> [   67.524985]  [<ffffffff81147c34>] ? quota_getquota+0x24/0xd0
> [   67.525048]  [<ffffffff810ff3db>] ? do_path_lookup+0x2b/0x90
> [   67.525082]  [<ffffffff810ff8cd>] ? kern_path+0x1d/0x40
> [   67.525134]  [<ffffffff81148311>] ? do_quotactl+0x421/0x540
> [   67.525191]  [<ffffffff811073f0>] ? dput+0x20/0x230
> [   67.525234]  [<ffffffff81148507>] ? sys_quotactl+0xd7/0x1a0
> [   67.525304]  [<ffffffff8130a03b>] ? system_call_fastpath+0x16/0x1b
> 
> dmsetup resume:
> [   67.525887]  [<ffffffffa0238280>] ? dev_wait+0xc0/0xc0 [dm_mod]
> [   67.525948]  [<ffffffff81309225>] ? rwsem_down_failed_common+0xc5/0x160
> [   67.526013]  [<ffffffff81198a43>] ? call_rwsem_down_write_failed+0x13/0x20
> [   67.526058]  [<ffffffff81308adc>] ? down_write+0x1c/0x1d
> [   67.526103]  [<ffffffff810f3a91>] ? thaw_super+0x21/0xc0
> [   67.526166]  [<ffffffff81124d4d>] ? thaw_bdev+0x6d/0x90
> [   67.526223]  [<ffffffff8105583e>] ? queue_work+0x4e/0x60
> [   67.526269]  [<ffffffffa0230e63>] ? unlock_fs+0x23/0x40 [dm_mod]
> [   67.526341]  [<ffffffffa02336d0>] ? dm_resume+0xb0/0xd0 [dm_mod]
> [   67.526388]  [<ffffffffa0238420>] ? dev_suspend+0x1a0/0x230 [dm_mod]
> [   67.526441]  [<ffffffffa0238a59>] ? ctl_ioctl+0x159/0x2a0 [dm_mod]
> [   67.526510]  [<ffffffff8116c4ee>] ? ipc_addid+0x4e/0xd0
> [   67.526555]  [<ffffffffa0238bae>] ? dm_ctl_ioctl+0xe/0x20 [dm_mod]
> [   67.526620]  [<ffffffff811025de>] ? do_vfs_ioctl+0x8e/0x4e0
> [   67.526670]  [<ffffffff811073f0>] ? dput+0x20/0x230
> [   67.526737]  [<ffffffff810f3112>] ? fput+0x162/0x220
> [   67.526783]  [<ffffffff81102a79>] ? sys_ioctl+0x49/0x90
> [   67.526838]  [<ffffffff8130a03b>] ? system_call_fastpath+0x16/0x1b
> 
> The following patch fixes the deadlock. When the quota subsystem takes s_umount,
> it checks if the filesystem is frozen. If it is, we drop s_umount, wait for
> the filesystem to resume and retry.
  Thanks for the patch. I'm aware of the deadlock and Val Henson is working
on resolving these types of deadlocks more systematically. But since I
haven't heard from her for a while, I guess I'll merge your fix and she'll
update her series to reflect your change since those patches are going to
go in at earliest in the next merge window.
  To sum up: I've merged your patch.

								Honza
> 
> Signed-off-by: Mikulas Patocka <mpatocka@...hat.com>
> CC: stable@...nel.org
> 
> ---
>  fs/quota/quota.c   |   12 ++++++++++++
>  include/linux/fs.h |    1 +
>  2 files changed, 13 insertions(+)
> 
> Index: linux-3.1-fast/fs/quota/quota.c
> ===================================================================
> --- linux-3.1-fast.orig/fs/quota/quota.c	2011-11-25 20:19:14.000000000 +0100
> +++ linux-3.1-fast/fs/quota/quota.c	2011-11-25 21:12:32.000000000 +0100
> @@ -310,7 +310,19 @@ static struct super_block *quotactl_bloc
>  	putname(tmp);
>  	if (IS_ERR(bdev))
>  		return ERR_CAST(bdev);
> +retry_super:
>  	sb = get_super(bdev);
> +	if (sb && sb->s_frozen != SB_UNFROZEN) {
> +		/*
> +		 * When resuming a frozen device, s_umount is taken for write.
> +		 * To avoid deadlock, we drop s_umount if the filesystem
> +		 * is frozen and wait for it to thaw.
> +		 */
> +		up_read(&sb->s_umount);
> +		vfs_check_frozen(sb, SB_FREEZE_WRITE);
> +		put_super(sb);
> +		goto retry_super;
> +	}
>  	bdput(bdev);
>  	if (!sb)
>  		return ERR_PTR(-ENODEV);
> Index: linux-3.1-fast/include/linux/fs.h
> ===================================================================
> --- linux-3.1-fast.orig/include/linux/fs.h	2011-11-25 21:13:56.000000000 +0100
> +++ linux-3.1-fast/include/linux/fs.h	2011-11-25 21:14:03.000000000 +0100
> @@ -2496,6 +2496,7 @@ extern struct file_system_type *get_fs_t
>  extern struct super_block *get_super(struct block_device *);
>  extern struct super_block *get_active_super(struct block_device *bdev);
>  extern struct super_block *user_get_super(dev_t);
> +extern void put_super(struct super_block *sb);
>  extern void drop_super(struct super_block *sb);
>  extern void iterate_supers(void (*)(struct super_block *, void *), void *);
>  extern void iterate_supers_type(struct file_system_type *,
-- 
Jan Kara <jack@...e.cz>
SUSE Labs, CR
--
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ