On Mon, Dec 07, 2020 at 05:31:11PM +0000, Christoph Hellwig wrote: > Btw, while looking at the code before replying to Casey I noticed > something else in this area of code which we should probably fix > if we touch all this. We are really supposed to create the ACLs > and security labels atomically with the actual inode creation. And > I think we have all the infrastructure to do this without too much > pain now for ACLs. Security labels with the weird > security_inode_init_security interface might be a little harder but > not impossible. Yes, that's long been a known problem - it was a problem way back in the day for DMF and the DMAPI attributes that needed to be added at create time. That's where XFS_TRANS_RESERVE originally came from, so that ENOSPC didn't prevent the dmapi xattr from being added to a newly created inode. This atomicity problem is one of the things that Allison's attribute defer-op rework is intended to address. i.e. being able to atomically create xattrs at inode create time and have them fully recoverable by intent replay if the initial inode allocation and directory linkage transaction succeeds. Essential the transaction context would start in xfs_generic_create(), not xfs_create(), and roll through to the xattr creations for ACLs and security contexts... > And I suspect security_inode_init_security might be right thing > to reuse for the helper to figure out what attrs would be set. Problem is that it appears to need an inode to already be allocated and instantiated to work.... > If > security_inode_init_security with an idempotent callback is > idempotent itself we might be able to use it directly, but all the > weird hooking makes it rather hard to read. Yeah, it's like a layer of inscrutible obscurity has been added to a simple ops table... :/ Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx
