linux-kernel - Re: [PATCH] fs: add the FIGETFROZEN ioctl call

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: <1460809099.28221.2.camel@margaine.com>
Date:	Sat, 16 Apr 2016 14:18:19 +0200
From:	Florian Margaine <florian@...gaine.com>
To:	Dave Chinner <david@...morbit.com>
Cc:	Alexander Viro <viro@...iv.linux.org.uk>,
	Jeff Layton <jlayton@...chiereds.net>,
	"J. Bruce Fields" <bfields@...ldses.org>,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] fs: add the FIGETFROZEN ioctl call

On Fri, 2016-04-15 at 12:17 +1000, Dave Chinner wrote:
> On Thu, Apr 14, 2016 at 09:57:07AM +0200, Florian Margaine wrote:
> > This lets userland get the filesystem freezing status, aka whether
> > the
> > filesystem is frozen or not. This is so that an application can
> > know if
> > it should freeze the filesystem or if it isn't necessary when
> > taking a
> > snapshot.
> 
> freezing nests, so there is no reason for avoiding a freeze when
> doing a snapshot. Indeed, if you don't wrap freeze/thaw around a
> snapshot, then if the fs is thawed while the snapshot is in progress
> then you are going to get a corrupt snapshot....
> 
> And, besides, polling for frozenness from userspace is inherently
> racy - by the time the syscall returns, the information may be
> incorrect, so you can't rely on it for decision making purposes in
> userspace.

The example I have is mostly about unfreezing. If I freeze, make a
snapshot, then unfreeze. If my program crashes after the
snapshot/before unfreezing, I'll be left with a frozen filesystem,
which is undesirable, at best.

One way to mitigate this is to keep the state of the snapshot, but it
seems silly to keep this information separately (e.g. in a database)
when the kernel has it available.

> 
> > +static int ioctl_fsgetfrozen(struct file *filp)
> > +{
> > + struct super_block *sb = file_inode(filp)->i_sb;
> > +
> > + if (!capable(CAP_SYS_ADMIN))
> > + return -EPERM;
> > +
> > + return sb->s_writers.frozen;
> 
> This makes the internal freeze implementation states part of the
> userspace ABI.  This needs an API that is separate from the internal
> implementation...

I'm not sure I understand, do you mean it should be e.g.:

return sb->s_writers.frozen ? 1 : 0;

?

> 
> Cheers,
> 
> Dave.

Regards,
Florian