| Message ID | 20191218163954.296726-2-arnd@arndb.de |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] xfs: rename compat_time_t to old_time32_t | expand |
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.
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c index 7b35d62ede9f..d43582e933a0 100644 --- a/fs/xfs/xfs_ioctl.c +++ b/fs/xfs/xfs_ioctl.c @@ -36,6 +36,7 @@ #include "xfs_reflink.h" #include "xfs_ioctl.h" +#include <linux/compat.h> #include <linux/mount.h> #include <linux/namei.h> @@ -617,6 +618,23 @@ xfs_fsinumbers_fmt( return xfs_ibulk_advance(breq, sizeof(struct xfs_inogrp)); } +/* 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; +} + 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; + 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; + } + /* Pull information for the target fd */ f = fdget((int)sxp->sx_fdtarget); if (!f.file) {
When building a kernel that disables support for 32-bit time_t system calls, it also makes sense to disable the old xfs_bstat ioctls completely, as they truncate the timestamps to 32-bit values once the extended times are supported. Any application using these needs to be updated to use the v5 interfaces. Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- fs/xfs/xfs_ioctl.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) -- 2.20.0