Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Incorrect memory ordering primitive for publishing lockless state (wmb() vs smp_store_release()) --
commit df7a50134ff93b3213d9bbccb1aca11a71421bee Author: John Groves <[email protected]> famfs: Add daxdev table and dax notify_failure support This commit introduces the daxdev table for famfs, which manages multiple dax devices using a basic structure, primitives, and serialization. It also adds dax_holder_operations and a notify_failure handler required to properly open a famfs-mode daxdev. > diff --git a/fs/famfs/famfs_inode.c b/fs/famfs/famfs_inode.c > index 5a13903da61b7..5735d8d1900bd 100644 > --- a/fs/famfs/famfs_inode.c > +++ b/fs/famfs/famfs_inode.c [ ... ] > @@ -76,6 +76,231 @@ famfs_get_inode( > +int > +famfs_install_daxdev( > + struct famfs_fs_info *fsi, > + struct super_block *sb, > + u64 index, > + dev_t devno, > + const char *name) > +{ [ ... ] > + daxdev->devno = devno; > + if (name) { > + daxdev->name = kstrdup(name, GFP_KERNEL); > + if (!daxdev->name) { > + fs_put_dax(daxdev->devp, sb); > + put_dax(daxdev->devp); > + daxdev->devp = NULL; > + return -ENOMEM; > + } > + } > + > + wmb(); /* All other fields must be visible before valid */ > + daxdev->valid = 1; [Severity: High] Does this code safely publish the daxdev structure to concurrent lockless readers? In famfs_install_daxdev(), the structure is populated and then its valid flag is set to 1 after a naked wmb(). While wmb() provides a write barrier for the writer, it does not enforce pairing on the reader side. If a concurrent lockless reader (such as famfs_daxdev_from_index() added later in this patch series) checks valid using a simple read like if (!dd->valid) without a corresponding read barrier, the CPU could reorder the reads. If the reads are reordered, the reader could observe valid == 1 but see uninitialized data or a NULL devp, which might lead to a NULL pointer dereference. Could this be converted to use smp_store_release(&daxdev->valid, 1) on the writer side, and paired with smp_load_acquire() on the lockless reader side? > + } > + > + return 0; > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/010001a04e801a4e-8eb212cd-b263-4043-ab65-33e480d2d7d4-000...@email.amazonses.com?part=3
