linux-kernel - Re: [PATCH] Introduce freeze_super and thaw

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:	Tue, 23 Mar 2010 14:28:44 +0000
From:	Al Viro <viro@...IV.linux.org.uk>
To:	Josef Bacik <josef@...hat.com>
Cc:	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
	chris.mason@...cle.com, hch@....de
Subject: Re: [PATCH] Introduce freeze_super and thaw_super for the fsfreeze
 ioctl

On Tue, Mar 23, 2010 at 10:22:00AM -0400, Josef Bacik wrote:
> Currently the way we do freezing is by passing sb>s_bdev to freeze_bdev and then
> letting it do all the work.  But freezing is more of an fs thing, and doesn't
> really have much to do with the bdev at all, all the work gets done with the
> super.  In btrfs we do not populate s_bdev, since we can have multiple bdev's
> for one fs and setting s_bdev makes removing devices from a pool kind of tricky.
> This means that freezing a btrfs filesystem fails, which causes us to corrupt
> with things like tux-on-ice which use the fsfreeze mechanism.  So instead of
> populating sb->s_bdev with a random bdev in our pool, I've broken the actual fs
> freezing stuff into freeze_super and thaw_super.  These just take the
> super_block that we're freezing and does the appropriate work.  It's basically
> just copy and pasted from freeze_bdev.  I've then converted freeze_bdev over to
> use the new super helpers.  I've tested this with ext4 and btrfs and verified
> everything continues to work the same as before.
> 
> The only new gotcha is multiple calls to the fsfreeze ioctl will return EBUSY if
> the fs is already frozen.  I thought this was a better solution than adding a
> freeze counter to the super_block, but if everybody hates this idea I'm open to
> suggestions.  Thanks,

Locking is all wrong there.  We don't need to worry about umount; we *already*
have an active reference.  And leaving a kernel object with semaphore held
when ioctl returns is completely wrong.
--
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/