From mboxrd@z Thu Jan  1 00:00:00 1970
From: Aleksa Sarai <cyphar@cyphar.com>
Subject: Re: [PATCH v1 1/4] lib: introduce copy_struct_from_user() helper
Date: Wed, 25 Sep 2019 19:20:49 +0200
Message-ID: <20190925172049.skm6ohnnxpofdkzv@yavin>
References: <20190925165915.8135-1-cyphar@cyphar.com>
 <20190925165915.8135-2-cyphar@cyphar.com>
 <CAHk-=wjFeNjhtUxQ8npmXORz5RLQU7B_3wD=45eug1+MXnuYvA@mail.gmail.com>
Mime-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="d7zsgn6gfbe7cd4l"
Return-path: <linux-kernel-owner@vger.kernel.org>
Content-Disposition: inline
In-Reply-To: <CAHk-=wjFeNjhtUxQ8npmXORz5RLQU7B_3wD=45eug1+MXnuYvA@mail.gmail.com>
Sender: linux-kernel-owner@vger.kernel.org
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Ingo Molnar <mingo@redhat.com>, Peter Zijlstra <peterz@infradead.org>, Alexander Shishkin <alexander.shishkin@linux.intel.com>, Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>, Christian Brauner <christian@brauner.io>, Rasmus Villemoes <linux@rasmusvillemoes.dk>, Al Viro <viro@zeniv.linux.org.uk>, GNU C Library <libc-alpha@sourceware.org>, Linux API <linux-api@vger.kernel.org>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
List-Id: linux-api@vger.kernel.org


--d7zsgn6gfbe7cd4l
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On 2019-09-25, Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Wed, Sep 25, 2019 at 10:00 AM Aleksa Sarai <cyphar@cyphar.com> wrote:
> >
> > +int is_zeroed_user(const void __user *from, size_t size)
>=20
> I like how you've done this, but it's buggy and only works on 64-bit.
>=20
> All the "u64" and "8" cases need to be "unsigned long" and
> "sizeof(unsigned long)".
>=20
> Part of that requirement is:
>=20
> > +               unsafe_get_user(val, (u64 __user *) from, err_fault);
>=20
> This part works fine - although 64-bit accesses migth be much more
> expensive and the win of unrolling might not be sufficient - but:
>=20
> > +               if (align) {
> > +                       /* @from is unaligned. */
> > +                       val &=3D ~aligned_byte_mask(align);
> > +                       align =3D 0;
> > +               }
>=20
> This part fundamentally only works on 'unsigned long'.

Just to make sure I understand, the following diff would this solve the
problem? If so, I'll apply it, and re-send in a few hours.

--8<-----------------------------------------------------------------------=
---

 int is_zeroed_user(const void __user *from, size_t size)
 {
-       u64 val;
-       uintptr_t align =3D (uintptr_t) from % 8;
+       unsigned long val;
+       uintptr_t align =3D (uintptr_t) from % sizeof(unsigned long);
=20
        if (unlikely(!size))
                return true;
@@ -150,8 +150,8 @@ int is_zeroed_user(const void __user *from, size_t size)
        if (!user_access_begin(from, size))
                return -EFAULT;
=20
-       while (size >=3D 8) {
-               unsafe_get_user(val, (u64 __user *) from, err_fault);
+       while (size >=3D sizeof(unsigned long)) {
+               unsafe_get_user(val, (unsigned long __user *) from, err_fau=
lt);
                if (align) {
                        /* @from is unaligned. */
                        val &=3D ~aligned_byte_mask(align);
@@ -159,12 +159,12 @@ int is_zeroed_user(const void __user *from, size_t si=
ze)
                }
                if (val)
                        goto done;
-               from +=3D 8;
-               size -=3D 8;
+               from +=3D sizeof(unsigned long);
+               size -=3D sizeof(unsigned long);
        }
        if (size) {
                /* (@from + @size) is unaligned. */
-               unsafe_get_user(val, (u64 __user *) from, err_fault);
+               unsafe_get_user(val, (unsigned long __user *) from, err_fau=
lt);
                val &=3D aligned_byte_mask(size);
        }

--=20
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>

--d7zsgn6gfbe7cd4l
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYIAB0WIQSxZm6dtfE8gxLLfYqdlLljIbnQEgUCXYuhzgAKCRCdlLljIbnQ
EoTkAP4y5leltK1ihQabd2Qeo0xNwWS4A1sSxNc6gZyek3TIbgD/ZTWvBM6ezLu7
TNEGtnXD7IhbdLW9QNtPAVYBWJhK6AI=
=DQmi
-----END PGP SIGNATURE-----

--d7zsgn6gfbe7cd4l--