Re: [PATCH 1/5] WIP: Add syscall unlinkat_s (currently x86* only)

[Lukáš Czerner]

On Wed, 4 Feb 2015, Alexander Holler wrote:

> Date: Wed, 04 Feb 2015 11:19:01 +0100
> From: Alexander Holler <holler@xxxxxxxxxxxxx>
> To: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
> Cc: Theodore Ts'o <tytso@xxxxxxx>, linux-fsdevel@xxxxxxxxxxxxxxx,
>     linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH 1/5] WIP: Add syscall unlinkat_s (currently x86* only)
> 
> Am 04.02.2015 um 00:33 schrieb Al Viro:
> > On Tue, Feb 03, 2015 at 07:01:50PM +0100, Alexander Holler wrote:
> > 
> > > Yeah, as I've already admitted in the bug, I never should have use
> > > the word secure, because everyone nowadays seems to end up in panic
> > > when reading that word.
> > > 
> > > So, if I would be able to use sed on my mails, I would replace
> > > unlinkat_s() with unlinkat_w() (for wipe) or would say that _s does
> > > stand for 'shred' in the means of shred(1).
> > 
> > TBH, I suspect that the saner API would be something like
> > EXT2_IOC_[SG[ETFLAGS,
> > allowing to set and query that along with other flags (append-only, etc.).
> > 
> > Forget about unlink; first of all, whatever API you use should only _mark_
> > the inode as "zero freed blocks" (or trim, for that matter).  You can't
> > force freeing of an inode, so either you make sure that subsequent freeing
> > of inode, whenever it happens, will do that work, or your API is hopelessly
> > racy.  Moreover, when link has been removed it's too late to report that
> > fs has no way to e.g. trim those blocks, so you really want to have it done
> > _before_ the actual link removal.  And if the file contents is that
> > sensitive,
> > you'd better extend the same protection to all operations that free its
> > blocks, including truncate(), fallocate() hole-punching, whatever.  What's
> > more, if you divorce that from link removal, you probably don't want it as
> > in-core-only flag - have it stored in inode, if fs supports that.
> > 
> > Alternatively, you might want to represent it as xattr - as much as I hate
> > those, it might turn out to be the best fit in this case, if we end up
> > with several variants for freed blocks disposal.  Not sure...
> > 
> > But whichever way we represent that state, IMO
> > 	a) operation should be similar to chmod/chattr/setfattr - modifying
> > inode metadata.
> > 	b) it should affect _all_ operations freeing blocks of that file
> > from that point on
> > 	c) it should be able to fail, telling you that you can't do that for
> > this backing store.
> 
> My intention to use unlinkat() or unlinkat_s() was the following:
> 
> - It can be supported by most filesystems (see my fat patch)
> 
> - It doesn't really make any promises it can't, like deleting leftovers of an
> already modified file. That's where a much more complicated solution like the
> 's' attribute would appropriate. It just should try to wipe the current
> contents of a file.
> 
> The second reason was also the reason why I've crafted the subject of the RFC
> very carefully: "Offer a way for userspace to request real deletion of files".
> 
> I did that to avoid the nitpickers. It doesn't say how the request is or has
> to be handled. I was aware of all the problems which arise if one tries to
> fullfill what the 's' flag promises. The final result of trying to get a 100
> percent solution is just what we have now: nothing at all.
> 
> It wasn't the first time I've posted a patch to LKML, I know that maintainers
> like to request high towers from ordinary people and therefor very often nice
> dog houses were refused. There might be a legitimate reason to request a high
> tower from a big company, but that's something totally different.
> 
> And I refuse to try to understand why maintainers request high towers. ;)
> 
> And because hope never dies, I was again silly enough to post a simple patch.
> ;)

It really comes down to what your intentions and expectations are.
If you just wanted to have a discussion about this feature, you have
it and that's fine.

If your intention was to propose the actual solution, the actual
feature that would be merged to kernel, then your expectation should
have been exactly this, discussion, improvement propositions, cases
to take care of and so on.

For example the interface Al proposed is much better than the
solution in the patch. And he is right about the other block freeing
operations.

No one told you not to proceed with the implementation AFAIK. Some
are skeptical and that's fine, but you have all means to proof them
wrong.

The fact is that the current patches are useless for anything other
than proof-of-concept. Now you know more that needs to be done or
thought about, but if you're not willing to do the work, then please
stop complaining about "high towers". I am not a maintainer and I
thinks that the feedback you've got is entirely reasonable. Take it
as you will.

One more thing, can I ask you what were your expectations when
posting those patches ?

Regards,
-Lukas

> 
> Regards,
> 
> Alexander Hpller
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/