On Thursday 03 March 2016 09:19:05 Dave Chinner wrote: > On Wed, Mar 02, 2016 at 07:51:34AM -0800, Deepa Dinamani wrote: > > MAX_INVALID_VFS_TIME and MIN_INVALID_VFS_TIME are initialized to S64_MIN > > and S64_MAX respectively so that even if one of the fields is > > uninitialized, it can be detected by using the condition > > max_time < min_time. > > I can't work out what MIN/MAX_INVALID_VFS_TIME is supposed to mean > when I see it in the code. does it mean time that lies between > MIN_INVALID_VFS_TIME > time > MAX_INVALID_VFS_TIME is invalid > (unlikely, but that's the obvious reading :)? > > Or that time < MIN_INVALID_VFS_TIME is invalid? Or is it valid? I > can't tell... > > Normally limits are specified by "min valid" and "max valid" > defines, which are pretty clear in their meaning. Like: Hmm, I had meant to comment on this in my private discussion. I agree this needs some more explanation in the source code, the idea is that once we have modified all file systems to correctly set the actual limits, we can then detect any newly added file system that forgets to set them, so we don't get accidental incorrect limits. > > --- a/include/linux/fs.h > > +++ b/include/linux/fs.h > > @@ -927,6 +927,12 @@ static inline struct file *get_file(struct file *f) > > #define MAX_LFS_FILESIZE ((loff_t)0x7fffffffffffffffLL) > > #endif > > > > +#define MAX_VFS_TIME S64_MAX > > +#define MIN_VFS_TIME S64_MIN > > These. Anything ouside these ranges is invalid. > > As such, I think this is wrong for 32 bit systems as the min/max VFS > times right now are S32_MAX/S32_MIN... I think the file system should always set the actual on-disk format limits in the superblock, regardless of the word size of the CPU architecture. We already support 32-bit time_t in compat tasks on 64-bit architectures, and we will support 64-bit time_t in normal tasks on 32-bit architectures in the future, and there is no need to range-check the timestamps written to disk. The one range-check that we probably want for current 32-bit tasks is in the vfs_fstatat/vfs_getattr code, so a kernel that internally handles post-2038 timestamps can limit them to S32_MAX instead of wrapping. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
