linux-kernel - Re: [PATCH v2 20/24] xfs: disallow broken ioctls without compat-32-bit-time

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:   Mon, 16 Dec 2019 08:52:49 -0800
From:   "Darrick J. Wong" <darrick.wong@...cle.com>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     y2038 Mailman List <y2038@...ts.linaro.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        linux-xfs <linux-xfs@...r.kernel.org>,
        Brian Foster <bfoster@...hat.com>,
        Dave Chinner <dchinner@...hat.com>,
        Allison Collins <allison.henderson@...cle.com>,
        Jan Kara <jack@...e.cz>, Eric Sandeen <sandeen@...deen.net>
Subject: Re: [PATCH v2 20/24] xfs: disallow broken ioctls without
 compat-32-bit-time

On Mon, Dec 16, 2019 at 05:45:29PM +0100, Arnd Bergmann wrote:
> On Fri, Dec 13, 2019 at 10:05 PM Darrick J. Wong
> <darrick.wong@...cle.com> wrote:
> >
> > On Fri, Dec 13, 2019 at 09:53:48PM +0100, Arnd Bergmann wrote:
> > > 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.
> >
> > Note that current xfs doesn't support > 32-bit timestamps at all, so for
> > now the old bulkstat/swapext ioctls will never overflow.
> 
> Right, this patch originally came after my version of the 40-bit
> timestamps that I dropped from the series now.
> 
> I've added "... once the extended times are supported." above now.
> 
> > Granted, I melded everyone's suggestions into a more fully formed
> > 'bigtime' feature patchset that I'll dump out soon as part of my usual
> > end of year carpetbombing of the mailing list, so we likely still need
> > most of this patch anyway...
> 
> What is the timeline for that work now? I'm mainly interested in
> getting the removal of 'time_t/timeval/timespec' and 'get_seconds()'
> from the kernel done for v5.6, but it would be good to also have
> this patch and the extended timestamps in the same version
> just so we can claim that "all known y2038 issues" are addressed
> in that release (I'm sure we will run into bugs we don't know yet).

Personally, I think you should push this whenever it's ready.  Are you
aiming to send all 24 patches as a treewide pull request directly to
Linus, or would you rather the 2-3 xfs patches go through the xfs tree?

The y2038 format changes are going to take a while to push through
review.  If somehow it all gets through review for 5.6 I can always
apply both and fix the merge damage, but more likely y2038 timestamps is
a <cough> 5.8 EXPERIMENTAL thing.

Or later, given that Dave and I both have years worth of unreviewed
patch backlog. :(

> > > @@ -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)
> >
> > The v5 bulkstat ioctls follow an entirely separate path through
> > xfs_ioctl.c, so I think you don't need the @cmd parameter.
> 
> The check is there to not forbid XFS_IOC_FSINUMBERS at
> the moment, since that is not affected.

Aha.

> > > @@ -1815,6 +1836,11 @@ xfs_ioc_swapext(
> > >       struct fd       f, tmp;
> > >       int             error = 0;
> > >
> > > +     if (xfs_have_compat_bstat_time32(XFS_IOC_SWAPEXT)) {
> >
> > if (!xfs_have...()) ?
> 
> Right, fixed now.

<nod>

--D

>        Arnd