Mike Belopuhov wrote:
> Any OKs or objections to this diff? This looks solid to me, FWIW.
Looks good to me as well.
ok stefan@
> On Wed, Jan 27, 2016 at 09:52 +0100, Martin Natano wrote:
> > In ufs, the calculation of i_modrev can produce signed overflow on 32
> > bit architectures (found on i386). The tv.tv_usec * 4294 calculation is
> > designed to move the microseconds part of a struct timeval to the upper
> > bits of an unsigned(!) 32 bit value to make room for simple i_modrev
> > increments, but the calculation is performed signed, causing overflow.
> > The diff below gets rid of the overflow by casting to unsigned first.
> >
> > While there I replaced the union _qcvt/SETHIGH/SETLOW dance with simple
> > bitshift operations.
> >
> > Index: ufs/ext2fs/ext2fs_subr.c
> > ===================================================================
> > RCS file: /cvs/src/sys/ufs/ext2fs/ext2fs_subr.c,v
> > retrieving revision 1.33
> > diff -u -p -u -r1.33 ext2fs_subr.c
> > --- ufs/ext2fs/ext2fs_subr.c 14 Mar 2015 03:38:52 -0000 1.33
> > +++ ufs/ext2fs/ext2fs_subr.c 27 Jan 2016 08:26:05 -0000
> > @@ -49,25 +49,6 @@
> > #include <ufs/ext2fs/ext2fs_extern.h>
> > #include <ufs/ext2fs/ext2fs_extents.h>
> >
> > -union _qcvt {
> > - int64_t qcvt;
> > - int32_t val[2];
> > -};
> > -
> > -#define SETHIGH(q, h) { \
> > - union _qcvt tmp; \
> > - tmp.qcvt = (q); \
> > - tmp.val[_QUAD_HIGHWORD] = (h); \
> > - (q) = tmp.qcvt; \
> > -}
> > -
> > -#define SETLOW(q, l) { \
> > - union _qcvt tmp; \
> > - tmp.qcvt = (q); \
> > - tmp.val[_QUAD_LOWWORD] = (l); \
> > - (q) = tmp.qcvt; \
> > -}
> > -
> > #ifdef _KERNEL
> >
> > /*
> > @@ -220,8 +201,8 @@ ext2fs_vinit(struct mount *mp, struct vo
> >
> > /* Initialize modrev times */
> > getmicrouptime(&tv);
> > - SETHIGH(ip->i_modrev, tv.tv_sec);
> > - SETLOW(ip->i_modrev, tv.tv_usec * 4294);
> > + ip->i_modrev = (u_quad_t)tv.tv_sec << 32;
> > + ip->i_modrev |= (u_quad_t)tv.tv_usec * 4294;
> >
> > *vpp = vp;
> >
> > Index: ufs/ufs/ufs_vnops.c
> > ===================================================================
> > RCS file: /cvs/src/sys/ufs/ufs/ufs_vnops.c,v
> > retrieving revision 1.123
> > diff -u -p -u -r1.123 ufs_vnops.c
> > --- ufs/ufs/ufs_vnops.c 8 Dec 2015 15:31:01 -0000 1.123
> > +++ ufs/ufs/ufs_vnops.c 27 Jan 2016 08:26:10 -0000
> > @@ -78,24 +78,6 @@ int filt_ufswrite(struct knote *, long);
> > int filt_ufsvnode(struct knote *, long);
> > void filt_ufsdetach(struct knote *);
> >
> > -union _qcvt {
> > - int64_t qcvt;
> > - int32_t val[2];
> > -};
> > -
> > -#define SETHIGH(q, h) { \
> > - union _qcvt tmp; \
> > - tmp.qcvt = (q); \
> > - tmp.val[_QUAD_HIGHWORD] = (h); \
> > - (q) = tmp.qcvt; \
> > -}
> > -#define SETLOW(q, l) { \
> > - union _qcvt tmp; \
> > - tmp.qcvt = (q); \
> > - tmp.val[_QUAD_LOWWORD] = (l); \
> > - (q) = tmp.qcvt; \
> > -}
> > -
> > /*
> > * A virgin directory (no blushing please).
> > */
> > @@ -1879,8 +1861,8 @@ ufs_vinit(struct mount *mntp, struct vop
> > * Initialize modrev times
> > */
> > getmicrouptime(&mtv);
> > - SETHIGH(ip->i_modrev, mtv.tv_sec);
> > - SETLOW(ip->i_modrev, mtv.tv_usec * 4294);
> > + ip->i_modrev = (u_quad_t)mtv.tv_sec << 32;
> > + ip->i_modrev |= (u_quad_t)mtv.tv_usec * 4294;
> > *vpp = vp;
> > return (0);
> > }
> >
> > cheers,
> > natano
> >
>
