linux-ext4 - Re: [PATCH 09/20] ext4: Initialize timestamps limits

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 1 Aug 2019 12:18:28 -0700
From:   Deepa Dinamani <deepa.kernel@...il.com>
To:     "Darrick J. Wong" <darrick.wong@...cle.com>
Cc:     Alexander Viro <viro@...iv.linux.org.uk>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux FS-devel Mailing List <linux-fsdevel@...r.kernel.org>,
        Arnd Bergmann <arnd@...db.de>,
        y2038 Mailman List <y2038@...ts.linaro.org>,
        "Theodore Ts'o" <tytso@....edu>,
        Andreas Dilger <adilger.kernel@...ger.ca>,
        linux-ext4@...r.kernel.org
Subject: Re: [PATCH 09/20] ext4: Initialize timestamps limits

On Wed, Jul 31, 2019 at 8:26 AM Darrick J. Wong <darrick.wong@...cle.com> wrote:
>
> On Mon, Jul 29, 2019 at 06:49:13PM -0700, Deepa Dinamani wrote:
> > ext4 has different overflow limits for max filesystem
> > timestamps based on the extra bytes available.
> >
> > The timestamp limits are calculated according to the
> > encoding table in
> > a4dad1ae24f85i(ext4: Fix handling of extended tv_sec):
> >
> > * extra  msb of                         adjust for signed
> > * epoch  32-bit                         32-bit tv_sec to
> > * bits   time    decoded 64-bit tv_sec  64-bit tv_sec      valid time range
> > * 0 0    1    -0x80000000..-0x00000001  0x000000000   1901-12-13..1969-12-31
> > * 0 0    0    0x000000000..0x07fffffff  0x000000000   1970-01-01..2038-01-19
> > * 0 1    1    0x080000000..0x0ffffffff  0x100000000   2038-01-19..2106-02-07
> > * 0 1    0    0x100000000..0x17fffffff  0x100000000   2106-02-07..2174-02-25
> > * 1 0    1    0x180000000..0x1ffffffff  0x200000000   2174-02-25..2242-03-16
> > * 1 0    0    0x200000000..0x27fffffff  0x200000000   2242-03-16..2310-04-04
> > * 1 1    1    0x280000000..0x2ffffffff  0x300000000   2310-04-04..2378-04-22
> > * 1 1    0    0x300000000..0x37fffffff  0x300000000   2378-04-22..2446-05-10
>
> My recollection of ext4 has gotten rusty, so this could be a bogus
> question:
>
> Say you have a filesystem with s_inode_size > 128 where not all of the
> ondisk inodes have been upgraded to i_extra_isize > 0 and therefore
> don't support nanoseconds or times beyond 2038.  I think this happens on
> ext3 filesystems that reserved extra space for inode attrs that are
> subsequently converted to ext4?
>
> In any case, that means that you have some inodes that support 34-bit
> tv_sec and some inodes that only support 32-bit tv_sec.  For the inodes
> with 32-bit tv_sec, I think all that happens is that a large timestamp
> will be truncated further, right?
>
> And no mount time warning because at least /some/ of the inodes are
> ready to go for the next 30 years?

I'm confused about ext3 being converted to ext4. If the converted
inodes have extra space, then ext4_iget() will start using the extra
space when it modifies the on disk inode, won't it?

But, if there are 32 bit tv_sec and 34 bit tv_sec in a superblock then
from this macro below, if an inode has space for extra bits in
timestamps, it uses it. Otherwise, only the first 32 bits are copied
to the on disk timestamp. This matches the behavior today for 32 bit
tv_sec. But, the 34 bit tv_sec has a better behavior after the series
because of the clamping and warning.

#define EXT4_INODE_SET_XTIME(xtime, inode, raw_inode)                \
do {                                        \
    (raw_inode)->xtime = cpu_to_le32((inode)->xtime.tv_sec);        \
    if (EXT4_FITS_IN_INODE(raw_inode, EXT4_I(inode), xtime ## _extra))     {\
        (raw_inode)->xtime ## _extra =                    \
                ext4_encode_extra_time(&(inode)->xtime);    \
        }                                \
} while (0)

I'm not sure if this corner case if important. Maybe the maintainers
can help me here. If this is important, then the inode time updates
for an ext4 inode should always be through ext4_setattr() and we can
clamp the timestamps there as a special case. And, this patch can be
added separately?

Thanks,
Deepa