diff mbox series

[12/20] fs: fat: Initialize filesystem timestamp ranges

Message ID 20190730014924.2193-13-deepa.kernel@gmail.com (mailing list archive)
State New, archived
Headers show
Series vfs: Add support for timestamp limits | expand

Commit Message

Deepa Dinamani July 30, 2019, 1:49 a.m. UTC
Fill in the appropriate limits to avoid inconsistencies
in the vfs cached inode times when timestamps are
outside the permitted range.

Some FAT variants indicate that the years after 2099 are not supported.
Since commit 7decd1cb0305 ("fat: Fix and cleanup timestamp conversion"),
we support the full range of years that can be represented, up to 2107.

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
Cc: hirofumi@mail.parknet.co.jp
---
 fs/fat/inode.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

OGAWA Hirofumi July 30, 2019, 9:31 a.m. UTC | #1
Deepa Dinamani <deepa.kernel@gmail.com> writes:

> +/* DOS dates from 1980/1/1 through 2107/12/31 */
> +#define FAT_DATE_MIN (0<<9 | 1<<5 | 1)
> +#define FAT_DATE_MAX (127<<9 | 12<<5 | 31)
> +#define FAT_TIME_MAX (23<<11 | 59<<5 | 29)
> +
>  /*
>   * A deserialized copy of the on-disk structure laid out in struct
>   * fat_boot_sector.
> @@ -1605,6 +1610,7 @@ int fat_fill_super(struct super_block *sb, void *data, int silent, int isvfat,
>  	int debug;
>  	long error;
>  	char buf[50];
> +	struct timespec64 ts;
>  
>  	/*
>  	 * GFP_KERNEL is ok here, because while we do hold the
> @@ -1698,6 +1704,12 @@ int fat_fill_super(struct super_block *sb, void *data, int silent, int isvfat,
>  	sbi->free_clus_valid = 0;
>  	sbi->prev_free = FAT_START_ENT;
>  	sb->s_maxbytes = 0xffffffff;
> +	fat_time_fat2unix(sbi, &ts, 0, cpu_to_le16(FAT_DATE_MIN), 0);
> +	sb->s_time_min = ts.tv_sec;
> +
> +	fat_time_fat2unix(sbi, &ts, cpu_to_le16(FAT_TIME_MAX),
> +			  cpu_to_le16(FAT_DATE_MAX), 0);
> +	sb->s_time_max = ts.tv_sec;

At least, it is wrong to call fat_time_fat2unix() before setup parameters
in sbi.

And please move those timestamp stuff to fat/misc.c like other fat
timestamp helpers. (Maybe, provide fat_time_{min,max}() from fat/misc.c,
or fat_init_time() such?).

Thanks.
Deepa Dinamani July 30, 2019, 5:39 p.m. UTC | #2
On Tue, Jul 30, 2019 at 2:31 AM OGAWA Hirofumi
<hirofumi@mail.parknet.co.jp> wrote:
>
> Deepa Dinamani <deepa.kernel@gmail.com> writes:
>
> > +/* DOS dates from 1980/1/1 through 2107/12/31 */
> > +#define FAT_DATE_MIN (0<<9 | 1<<5 | 1)
> > +#define FAT_DATE_MAX (127<<9 | 12<<5 | 31)
> > +#define FAT_TIME_MAX (23<<11 | 59<<5 | 29)
> > +
> >  /*
> >   * A deserialized copy of the on-disk structure laid out in struct
> >   * fat_boot_sector.
> > @@ -1605,6 +1610,7 @@ int fat_fill_super(struct super_block *sb, void *data, int silent, int isvfat,
> >       int debug;
> >       long error;
> >       char buf[50];
> > +     struct timespec64 ts;
> >
> >       /*
> >        * GFP_KERNEL is ok here, because while we do hold the
> > @@ -1698,6 +1704,12 @@ int fat_fill_super(struct super_block *sb, void *data, int silent, int isvfat,
> >       sbi->free_clus_valid = 0;
> >       sbi->prev_free = FAT_START_ENT;
> >       sb->s_maxbytes = 0xffffffff;
> > +     fat_time_fat2unix(sbi, &ts, 0, cpu_to_le16(FAT_DATE_MIN), 0);
> > +     sb->s_time_min = ts.tv_sec;
> > +
> > +     fat_time_fat2unix(sbi, &ts, cpu_to_le16(FAT_TIME_MAX),
> > +                       cpu_to_le16(FAT_DATE_MAX), 0);
> > +     sb->s_time_max = ts.tv_sec;
>
> At least, it is wrong to call fat_time_fat2unix() before setup parameters
> in sbi.

All the parameters that fat_time_fat2unix() cares in sbi is accessed through

static inline int fat_tz_offset(struct msdos_sb_info *sbi)
{
    return (sbi->options.tz_set ?
           -sbi->options.time_offset :
           sys_tz.tz_minuteswest) * SECS_PER_MIN;
}

Both the sbi fields sbi->options.tz_set and sbi->options.time_offset
are set by the call to parse_options(). And, parse_options() is called
before the calls to fat_time_fat2unix().:

int fat_fill_super(struct super_block *sb, void *data, int silent, int isvfat,
           void (*setup)(struct super_block *))
{
     <snip>

    error = parse_options(sb, data, isvfat, silent, &debug, &sbi->options);
    if (error)
        goto out_fail;

   <snip>

    sbi->prev_free = FAT_START_ENT;
    sb->s_maxbytes = 0xffffffff;
    fat_time_fat2unix(sbi, &ts, 0, cpu_to_le16(FAT_DATE_MIN), 0);
    sb->s_time_min = ts.tv_sec;

    fat_time_fat2unix(sbi, &ts, cpu_to_le16(FAT_TIME_MAX),
              cpu_to_le16(FAT_DATE_MAX), 0);
    sb->s_time_max = ts.tv_sec;

   <snip>
}

I do not see what the problem is.

-Deepa
OGAWA Hirofumi July 31, 2019, 12:48 a.m. UTC | #3
Deepa Dinamani <deepa.kernel@gmail.com> writes:

>> At least, it is wrong to call fat_time_fat2unix() before setup parameters
>> in sbi.
>
> All the parameters that fat_time_fat2unix() cares in sbi is accessed through
>
> static inline int fat_tz_offset(struct msdos_sb_info *sbi)
> {
>     return (sbi->options.tz_set ?
>            -sbi->options.time_offset :
>            sys_tz.tz_minuteswest) * SECS_PER_MIN;
> }
>
> Both the sbi fields sbi->options.tz_set and sbi->options.time_offset
> are set by the call to parse_options(). And, parse_options() is called
> before the calls to fat_time_fat2unix().:
>
> int fat_fill_super(struct super_block *sb, void *data, int silent, int isvfat,
>            void (*setup)(struct super_block *))
> {
>      <snip>
>
>     error = parse_options(sb, data, isvfat, silent, &debug, &sbi->options);
>     if (error)
>         goto out_fail;
>
>    <snip>
>
>     sbi->prev_free = FAT_START_ENT;
>     sb->s_maxbytes = 0xffffffff;
>     fat_time_fat2unix(sbi, &ts, 0, cpu_to_le16(FAT_DATE_MIN), 0);
>     sb->s_time_min = ts.tv_sec;
>
>     fat_time_fat2unix(sbi, &ts, cpu_to_le16(FAT_TIME_MAX),
>               cpu_to_le16(FAT_DATE_MAX), 0);
>     sb->s_time_max = ts.tv_sec;
>
>    <snip>
> }
>
> I do not see what the problem is.

Ouch, you are right. I was reading that patch wrongly, sorry.

Thanks.
diff mbox series

Patch

diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index 05689198f5af..5f04c5c810fb 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -31,6 +31,11 @@ 
 
 #define KB_IN_SECTORS 2
 
+/* DOS dates from 1980/1/1 through 2107/12/31 */
+#define FAT_DATE_MIN (0<<9 | 1<<5 | 1)
+#define FAT_DATE_MAX (127<<9 | 12<<5 | 31)
+#define FAT_TIME_MAX (23<<11 | 59<<5 | 29)
+
 /*
  * A deserialized copy of the on-disk structure laid out in struct
  * fat_boot_sector.
@@ -1605,6 +1610,7 @@  int fat_fill_super(struct super_block *sb, void *data, int silent, int isvfat,
 	int debug;
 	long error;
 	char buf[50];
+	struct timespec64 ts;
 
 	/*
 	 * GFP_KERNEL is ok here, because while we do hold the
@@ -1698,6 +1704,12 @@  int fat_fill_super(struct super_block *sb, void *data, int silent, int isvfat,
 	sbi->free_clus_valid = 0;
 	sbi->prev_free = FAT_START_ENT;
 	sb->s_maxbytes = 0xffffffff;
+	fat_time_fat2unix(sbi, &ts, 0, cpu_to_le16(FAT_DATE_MIN), 0);
+	sb->s_time_min = ts.tv_sec;
+
+	fat_time_fat2unix(sbi, &ts, cpu_to_le16(FAT_TIME_MAX),
+			  cpu_to_le16(FAT_DATE_MAX), 0);
+	sb->s_time_max = ts.tv_sec;
 
 	if (!sbi->fat_length && bpb.fat32_length) {
 		struct fat_boot_fsinfo *fsinfo;