| 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
| ||
|
Message-ID: <bh5kbxlwoavjgliq2m2fco2ahg2ub25ldl4keojewzfadpocv7@3wdcjneve3iw> Date: Tue, 23 Sep 2025 13:16:58 +0200 From: Jan Kara <jack@...e.cz> To: Mateusz Guzik <mjguzik@...il.com> Cc: brauner@...nel.org, viro@...iv.linux.org.uk, jack@...e.cz, linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org, josef@...icpanda.com, kernel-team@...com, amir73il@...il.com, linux-btrfs@...r.kernel.org, linux-ext4@...r.kernel.org, linux-xfs@...r.kernel.org, ceph-devel@...r.kernel.org, linux-unionfs@...r.kernel.org Subject: Re: [PATCH v6 0/4] hide ->i_state behind accessors On Tue 23-09-25 12:47:06, Mateusz Guzik wrote: > First commit message quoted verbatim with rationable + API: > > [quote] > Open-coded accesses prevent asserting they are done correctly. One > obvious aspect is locking, but significantly more can checked. For > example it can be detected when the code is clearing flags which are > already missing, or is setting flags when it is illegal (e.g., I_FREEING > when ->i_count > 0). > > Given the late stage of the release cycle this patchset only aims to > hide access, it does not provide any of the checks. > > Consumers can be trivially converted. Suppose flags I_A and I_B are to > be handled, then: > > state = inode->i_state => state = inode_state_read(inode) > inode->i_state |= (I_A | I_B) => inode_state_set(inode, I_A | I_B) > inode->i_state &= ~(I_A | I_B) => inode_state_clear(inode, I_A | I_B) > inode->i_state = I_A | I_B => inode_state_assign(inode, I_A | I_B) > [/quote] > > Right now this is one big NOP, except for READ_ONCE/WRITE_ONCE for every access. > > Given this, I decided to not submit any per-fs patches. Instead, the > conversion is done in 2 parts: coccinelle and whatever which was missed. This looks good to me now. Feel free to add: Reviewed-by: Jan Kara <jack@...e.cz> Honza > > Generated against: > https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git/commit/?h=vfs-6.18.inode.refcount.preliminaries > > v6: > - rename routines: > set -> assign; add -> set; del -> clear > - update commentary in patch 3 replacing smp_store/load with smp_wmb/rmb > > v5: > - drop lockdep for the time being > > v4: > https://lore.kernel.org/linux-fsdevel/CAGudoHFViBUZ4TPNuLWC7qyK0v8LRwxbpZd9Mx3rHdh5GW9CrQ@mail.gmail.com/T/#m866b3b5740691de9b4008184a9a3f922dfa8e439 > > > Mateusz Guzik (4): > fs: provide accessors for ->i_state > Convert the kernel to use ->i_state accessors > Manual conversion of ->i_state uses > fs: make plain ->i_state access fail to compile > > Documentation/filesystems/porting.rst | 2 +- > block/bdev.c | 4 +- > drivers/dax/super.c | 2 +- > fs/9p/vfs_inode.c | 2 +- > fs/9p/vfs_inode_dotl.c | 2 +- > fs/affs/inode.c | 2 +- > fs/afs/dynroot.c | 6 +- > fs/afs/inode.c | 6 +- > fs/bcachefs/fs.c | 8 +- > fs/befs/linuxvfs.c | 2 +- > fs/bfs/inode.c | 2 +- > fs/btrfs/inode.c | 10 +-- > fs/buffer.c | 4 +- > fs/ceph/cache.c | 2 +- > fs/ceph/crypto.c | 4 +- > fs/ceph/file.c | 4 +- > fs/ceph/inode.c | 28 +++--- > fs/coda/cnode.c | 4 +- > fs/cramfs/inode.c | 2 +- > fs/crypto/keyring.c | 2 +- > fs/crypto/keysetup.c | 2 +- > fs/dcache.c | 8 +- > fs/drop_caches.c | 2 +- > fs/ecryptfs/inode.c | 6 +- > fs/efs/inode.c | 2 +- > fs/erofs/inode.c | 2 +- > fs/ext2/inode.c | 2 +- > fs/ext4/inode.c | 10 +-- > fs/ext4/orphan.c | 4 +- > fs/f2fs/data.c | 2 +- > fs/f2fs/inode.c | 2 +- > fs/f2fs/namei.c | 4 +- > fs/f2fs/super.c | 2 +- > fs/freevxfs/vxfs_inode.c | 2 +- > fs/fs-writeback.c | 123 +++++++++++++------------- > fs/fuse/inode.c | 4 +- > fs/gfs2/file.c | 2 +- > fs/gfs2/glops.c | 2 +- > fs/gfs2/inode.c | 4 +- > fs/gfs2/ops_fstype.c | 2 +- > fs/hfs/btree.c | 2 +- > fs/hfs/inode.c | 2 +- > fs/hfsplus/super.c | 2 +- > fs/hostfs/hostfs_kern.c | 2 +- > fs/hpfs/dir.c | 2 +- > fs/hpfs/inode.c | 2 +- > fs/inode.c | 100 ++++++++++----------- > fs/isofs/inode.c | 2 +- > fs/jffs2/fs.c | 4 +- > fs/jfs/file.c | 4 +- > fs/jfs/inode.c | 2 +- > fs/jfs/jfs_txnmgr.c | 2 +- > fs/kernfs/inode.c | 2 +- > fs/libfs.c | 6 +- > fs/minix/inode.c | 2 +- > fs/namei.c | 8 +- > fs/netfs/misc.c | 8 +- > fs/netfs/read_single.c | 6 +- > fs/nfs/inode.c | 2 +- > fs/nfs/pnfs.c | 2 +- > fs/nfsd/vfs.c | 2 +- > fs/nilfs2/cpfile.c | 2 +- > fs/nilfs2/dat.c | 2 +- > fs/nilfs2/ifile.c | 2 +- > fs/nilfs2/inode.c | 10 +-- > fs/nilfs2/sufile.c | 2 +- > fs/notify/fsnotify.c | 2 +- > fs/ntfs3/inode.c | 2 +- > fs/ocfs2/dlmglue.c | 2 +- > fs/ocfs2/inode.c | 10 +-- > fs/omfs/inode.c | 2 +- > fs/openpromfs/inode.c | 2 +- > fs/orangefs/inode.c | 2 +- > fs/orangefs/orangefs-utils.c | 6 +- > fs/overlayfs/dir.c | 2 +- > fs/overlayfs/inode.c | 6 +- > fs/overlayfs/util.c | 10 +-- > fs/pipe.c | 2 +- > fs/qnx4/inode.c | 2 +- > fs/qnx6/inode.c | 2 +- > fs/quota/dquot.c | 2 +- > fs/romfs/super.c | 2 +- > fs/smb/client/cifsfs.c | 2 +- > fs/smb/client/inode.c | 14 +-- > fs/squashfs/inode.c | 2 +- > fs/sync.c | 2 +- > fs/ubifs/file.c | 2 +- > fs/ubifs/super.c | 2 +- > fs/udf/inode.c | 2 +- > fs/ufs/inode.c | 2 +- > fs/xfs/scrub/common.c | 2 +- > fs/xfs/scrub/inode_repair.c | 2 +- > fs/xfs/scrub/parent.c | 2 +- > fs/xfs/xfs_bmap_util.c | 2 +- > fs/xfs/xfs_health.c | 4 +- > fs/xfs/xfs_icache.c | 6 +- > fs/xfs/xfs_inode.c | 6 +- > fs/xfs/xfs_inode_item.c | 4 +- > fs/xfs/xfs_iops.c | 2 +- > fs/xfs/xfs_reflink.h | 2 +- > fs/zonefs/super.c | 4 +- > include/linux/backing-dev.h | 7 +- > include/linux/fs.h | 42 ++++++++- > include/linux/writeback.h | 4 +- > include/trace/events/writeback.h | 8 +- > mm/backing-dev.c | 2 +- > security/landlock/fs.c | 2 +- > 107 files changed, 345 insertions(+), 307 deletions(-) > > -- > 2.43.0 > -- Jan Kara <jack@...e.com> SUSE Labs, CR
Powered by blists - more mailing lists