On Tue, Dec 24, 2019 at 9:45 AM Christoph Hellwig hch@infradead.org wrote:
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.
Yes, makes sense.
I was going for something XFS specific here because XFS is unique in the kernel in completely deprecating a set of ioctl commands (replacing the old interface with a v5) rather than allowing the user space to be compiled with 64-bit time_t.
If we add a global helper for this, I'd be tempted to also stick a WARN_RATELIMIT() in there to give users a better indication of what broke after disabling CONFIG_COMPAT_32BIT_TIME.
The same warning would make sense in the system calls, but then we have to decide which combinations we want to allow being configured at runtime or compile-time.
a) unmodified behavior b) just warn but allow c) no warning but disallow d) warn and disallow
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.
Sorry I missed that comment earlier. I've had a fresh look now, but I think we still need to deprecate XFS_IOC_SWAPEXT and add a v5 version of it, since the comparison will fail as soon as the range of the inode timestamps is extended beyond 2038, otherwise the comparison will always be false, or require comparing the truncated time values which would add yet another representation.
Arnd