Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Liam Howlett <liam.howlett@oracle.com>
To:     Michel Lespinasse <walken@google.com>
CC:     Richard Henderson <rth@twiddle.net>,
        Ivan Kokshaysky <ink@jurassic.park.msu.ru>,
        Matt Turner <mattst88@gmail.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        "linux-alpha@vger.kernel.org" <linux-alpha@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Dmitry Safonov <0x7f454c46@gmail.com>,
        "Gustavo A . R . Silva" <gustavoars@kernel.org>
Subject: Re: [PATCH] alpha/kernel/traps: Use find_vma_intersection() in traps
 for setting si_code
Thread-Topic: [PATCH] alpha/kernel/traps: Use find_vma_intersection() in traps
 for setting si_code
Thread-Index: AQHXJzBzAo0LIIQ3IkiycclLKpCw2qqgG+qAgAEXVgA=
Date:   Fri, 2 Apr 2021 13:05:19 +0000
Message-ID: <20210402130512.h7vkbxdwn7errxmh@revolver>
References: <20210401195138.2895285-1-Liam.Howlett@Oracle.com>
 <CANN689GC8jXbzFb_HL-vpoBxuD4-By029=PMsi4Vs9ufik4RhQ@mail.gmail.com>
In-Reply-To: <CANN689GC8jXbzFb_HL-vpoBxuD4-By029=PMsi4Vs9ufik4RhQ@mail.gmail.com>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-ID: <E3BBC585AEB34A42B7676F9713D36C54@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 5599f55c-fccd-490c-2e8c-08d8f5d7f82a
X-MS-Exchange-CrossTenant-originalarrivaltime: 02 Apr 2021 13:05:20.1603
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: w2z1rDcWeEOJdsTMOLLcnTGSJoiWxgQAHYWQTNBplVZa/xsw7SGJy2AC1zISWJZv7Zv6gnn8f+kMzs5avQZ0UQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR10MB1405
X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9941 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 phishscore=0 spamscore=0
 mlxscore=0 mlxlogscore=999 bulkscore=0 adultscore=0 suspectscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2103310000
 definitions=main-2104020094
X-Proofpoint-GUID: iACzCPP-LU7RORRBX_ox3vHk-DNRICO8
X-Proofpoint-ORIG-GUID: iACzCPP-LU7RORRBX_ox3vHk-DNRICO8
X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9941 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 mlxlogscore=999 mlxscore=0
 lowpriorityscore=0 suspectscore=0 priorityscore=1501 phishscore=0
 clxscore=1015 impostorscore=0 malwarescore=0 bulkscore=0 adultscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2103310000
 definitions=main-2104020095
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

* Michel Lespinasse <walken@google.com> [210401 16:25]:
> You are correct that find_vma is insufficient for what's intended
> here, and that find_vma_intersection fixes it.
>=20
> I'll let the arch maintainers speak of what the consequences of the
> changed si_code would be - the bug has been here so long, that I would
> worry some userspace might have come to depend on it (the old "common
> law feature" issue).

Fair point.  Is this a valid concern given the result will vary,
although uncommon, based on the address passed in?  A user would see
different behaviour based on where the address lands in the virtual
address space.

>  Just a concern I have, with 0 evidence behind it, so I hope it turns
> out not to be an actual issue.
>=20
> Acked-by: Michel Lespinasse <walken@google.com>
>=20
> On Thu, Apr 1, 2021 at 12:51 PM Liam Howlett <liam.howlett@oracle.com> wr=
ote:
> >
> > find_vma() will continue to search upwards until the end of the virtual
> > memory space.  This means the si_code would almost never be set to
> > SEGV_MAPERR even when the address falls outside of any VMA.
> >
> > Using find_vma_intersection() allows for what is intended by only
> > returning a VMA if it falls within the range provided, in this case a
> > window of 1.
> >
> > Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com>
> > ---
> >  arch/alpha/kernel/traps.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/alpha/kernel/traps.c b/arch/alpha/kernel/traps.c
> > index 921d4b6e4d95..7f51386c06d0 100644
> > --- a/arch/alpha/kernel/traps.c
> > +++ b/arch/alpha/kernel/traps.c
> > @@ -957,8 +957,10 @@ do_entUnaUser(void __user * va, unsigned long opco=
de,
> >                 si_code =3D SEGV_ACCERR;
> >         else {
> >                 struct mm_struct *mm =3D current->mm;
> > +               unsigned long addr =3D (unsigned long)va;
> > +
> >                 mmap_read_lock(mm);
> > -               if (find_vma(mm, (unsigned long)va))
> > +               if (find_vma_intersection(mm, addr, addr + 1))
> >                         si_code =3D SEGV_ACCERR;
> >                 else
> >                         si_code =3D SEGV_MAPERR;
> > --
> > 2.30.0
>=20
>=20
>=20
> --=20
> Michel "Walken" Lespinasse
> A program is never fully debugged until the last user dies.=