On Thu, Dec 5, 2024 at 7:15 PM Rick Macklem <rick.mack...@gmail.com> wrote: > > On Thu, Dec 5, 2024 at 6:40 PM Jessica Clarke <jrt...@freebsd.org> wrote: > > > > CAUTION: This email originated from outside of the University of Guelph. Do > > not click links or open attachments unless you recognize the sender and > > know the content is safe. If in doubt, forward suspicious emails to > > ith...@uoguelph.ca. > > > > > > On 6 Dec 2024, at 02:06, Rick Macklem <rmack...@freebsd.org> wrote: > > > > > > The branch main has been updated by rmacklem: > > > > > > URL: > > > https://cgit.FreeBSD.org/src/commit/?id=bfc8e3308bee23d0f7836d57f32ed8d47da02627 > > > > > > commit bfc8e3308bee23d0f7836d57f32ed8d47da02627 > > > Author: Rick Macklem <rmack...@freebsd.org> > > > AuthorDate: 2024-12-06 02:05:06 +0000 > > > Commit: Rick Macklem <rmack...@freebsd.org> > > > CommitDate: 2024-12-06 02:05:06 +0000 > > > > > > ext2fs: Fix the size of struct ufid and add a static assert > > > > > > File system specific *fid structures are copied into the generic > > > struct fid defined in sys/mount.h. > > > As such, they cannot be larger than struct fid. > > > > > > This patch packed the structure and checks via a __Static_assert(). > > > > > > Reviewed by: markj > > > MFC after: 2 weeks > > > --- > > > sys/fs/ext2fs/ext2_vnops.c | 2 ++ > > > sys/fs/ext2fs/inode.h | 2 +- > > > 2 files changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/sys/fs/ext2fs/ext2_vnops.c b/sys/fs/ext2fs/ext2_vnops.c > > > index dfbb11f75421..064c10bd18b2 100644 > > > --- a/sys/fs/ext2fs/ext2_vnops.c > > > +++ b/sys/fs/ext2fs/ext2_vnops.c > > > @@ -1889,6 +1889,8 @@ ext2_vptofh(struct vop_vptofh_args *ap) > > > { > > > struct inode *ip; > > > struct ufid *ufhp; > > > + _Static_assert(sizeof(struct ufid) <= sizeof(struct fid), > > > + "struct ufid cannot be larger than struct fid"); > > > > > > ip = VTOI(ap->a_vp); > > > ufhp = (struct ufid *)ap->a_fhp; > > > diff --git a/sys/fs/ext2fs/inode.h b/sys/fs/ext2fs/inode.h > > > index 9ee1b5672da6..63102deb10b0 100644 > > > --- a/sys/fs/ext2fs/inode.h > > > +++ b/sys/fs/ext2fs/inode.h > > > @@ -191,7 +191,7 @@ struct ufid { > > > uint16_t ufid_pad; /* Force 32-bit alignment. */ > > > ino_t ufid_ino; /* File number (ino). */ > > > uint32_t ufid_gen; /* Generation number. */ > > > -}; > > > +} __packed; > > > > Why not just swap ufid_ino and ufid_gen? This will give worse codegen > > on architectures that don’t assume fast unaligned access. > You can commit such a change if you'd like, as far as I am concerned. > > I suspect few (if any) export ext2fs file systems anyhow, so this structure > is unlikely to ever be used. > > There was a discussion on phabricator in D47879 about reordering > entries and the consensus there seemed to be avoid doing that, > although admittedly I think they were referring to the first two fields, > which match the ones in the generic "struct fid". > > rick ps: I used __packed since that was what markj@ used for cd9660, to keep things consistent.
> > > > > Jess > >
