On Wed, Dec 18, 2019 at 05:39:29PM +0100, Arnd Bergmann wrote: > +/* disallow y2038-unsafe ioctls with CONFIG_COMPAT_32BIT_TIME=n */ > +static bool xfs_have_compat_bstat_time32(unsigned int cmd) > +{ > + if (IS_ENABLED(CONFIG_COMPAT_32BIT_TIME)) > + return true; > + > + if (IS_ENABLED(CONFIG_64BIT) && !in_compat_syscall()) > + return true; > + > + if (cmd == XFS_IOC_FSBULKSTAT_SINGLE || > + cmd == XFS_IOC_FSBULKSTAT || > + cmd == XFS_IOC_SWAPEXT) > + return false; > + > + return true;
I think the check for the individual command belongs into the callers, which laves us with: static inline bool have_time32(void) { return IS_ENABLED(CONFIG_COMPAT_32BIT_TIME) || (IS_ENABLED(CONFIG_64BIT) && !in_compat_syscall()); } and that looks like it should be in a generic helper somewhere. > STATIC int > xfs_ioc_fsbulkstat( > xfs_mount_t *mp, > @@ -637,6 +655,9 @@ xfs_ioc_fsbulkstat( > if (!capable(CAP_SYS_ADMIN)) > return -EPERM; > > + if (!xfs_have_compat_bstat_time32(cmd)) > + return -EINVAL; Here we can simply check for cmd != XFS_IOC_FSINUMBERS before the call. > if (XFS_FORCED_SHUTDOWN(mp)) > return -EIO; > > @@ -1815,6 +1836,11 @@ xfs_ioc_swapext( > struct fd f, tmp; > int error = 0; > > + if (!xfs_have_compat_bstat_time32(XFS_IOC_SWAPEXT)) { > + error = -EINVAL; > + goto out; > + } And for this one we just have one cmd anyway. But I actually still disagree with the old_time check for this one entirely, as voiced on one of the last iterations. For swapext the time stamp really is only used as a generation counter, so overflows are entirely harmless. _______________________________________________ Y2038 mailing list Y2038@lists.linaro.org https://lists.linaro.org/mailman/listinfo/y2038