MIME-Version: 1.0
References: <20230321122514.1743889-1-mark.rutland@arm.com> <20230321122514.1743889-2-mark.rutland@arm.com>
In-Reply-To: <20230321122514.1743889-2-mark.rutland@arm.com>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Tue, 21 Mar 2023 11:04:26 -0700
Message-ID: <CAHk-=wgknoR11b+mX=AP8TcHP+gsFGdhPk7sJPROaQBBsqdubw@mail.gmail.com>
Subject: Re: [PATCH v2 1/4] lib: test copy_{to,from}_user()
To:     Mark Rutland <mark.rutland@arm.com>
Cc:     linux-kernel@vger.kernel.org, agordeev@linux.ibm.com,
        aou@eecs.berkeley.edu, bp@alien8.de, catalin.marinas@arm.com,
        dave.hansen@linux.intel.com, davem@davemloft.net,
        gor@linux.ibm.com, hca@linux.ibm.com, linux-arch@vger.kernel.org,
        linux@armlinux.org.uk, mingo@redhat.com, palmer@dabbelt.com,
        paul.walmsley@sifive.com, robin.murphy@arm.com, tglx@linutronix.de,
        viro@zeniv.linux.org.uk, will@kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Tue, Mar 21, 2023 at 5:25=E2=80=AFAM Mark Rutland <mark.rutland@arm.com>=
 wrote:
>
> * arm64's copy_to_user() under-reports the number of bytes copied in
>   some cases, e.g.

So I think this is the ok case.

> * arm's copy_to_user() under-reports the number of bytes copied in some
>   cases, and both copy_to_user() and copy_from_user() don't guarantee
>   that at least a single byte is copied when a partial copy is possible,

Again, this is ok historically.

> * i386's copy_from_user does not guarantee that at least a single byte
>   is copied when a partial copit is possible, e.g.
>
>   | too few bytes consumed (offset=3D4093, size=3D8, ret=3D8)

And here's the real example of "we've always done this optimization".
The exact details have differed, but the i386 case is the really
really traditional one: it does word-at-a-time copies, and does *not*
try to fall back to byte-wise copies. Never has.

> * riscv's copy_to_user() and copy_from_user() don't guarantee that at
>   least a single byte is copied when a partial copy is possible, e.g.
>
>   | too few bytes consumed (offset=3D4095, size=3D2, ret=3D2)

Yup. This is all the same "we've never forced byte-at-a-time copies"

> * s390 passes all tests
>
> * sparc's copy_from_user() over-reports the number of bbytes copied in
>   some caes, e.g.

So this case I think this is wrong, and an outright bug. That can
cause people to think that uninitialized data is initialized, and leak
sensitive information.

> * x86_64 passes all tests

I suspect your testing is flawed due to being too limited, and x86-64
having multiple different copying routines.

Yes, at some point we made everything be quite careful with
"handle_tail" etc, but we end up still having things that fail early,
and fail hard.

At a minimum, at least unsafe_copy_to_user() will fault and not do the
"fill to the very last byte" case. Of course, that doesn't return a
partial length (it only has a "fail" case), but it's an example of
this whole thing where we haven't really been byte-exact when doing
copies.

So again, I get the feeling that these rules may make sense from a
validation standpoint, but I'm not 100% sure we should generally have
to be this careful.

                 Linus