Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Oleksandr Natalenko <oleksandr@redhat.com>
To:     linux-kernel@vger.kernel.org
Cc:     linux-scsi@vger.kernel.org, Saurav Kashyap <skashyap@marvell.com>,
        Johannes Thumshirn <Johannes.Thumshirn@wdc.com>,
        GR-QLogic-Storage-Upstream@marvell.com,
        "James E.J. Bottomley" <jejb@linux.ibm.com>,
        "Martin K. Petersen" <martin.petersen@oracle.com>,
        Jozef Bacik <jobacik@redhat.com>,
        Laurence Oberman <loberman@redhat.com>,
        Rob Evers <revers@redhat.com>
Subject: Re: [PATCH 0/3] scsi: qedf: sanitise uaccess
Date:   Fri, 28 Jul 2023 09:05:07 +0200
Message-ID: <3240893.aeNJFYEL58@redhat.com>
Organization: Red Hat
In-Reply-To: <20230728065819.139694-1-oleksandr@redhat.com>
References: <20230728065819.139694-1-oleksandr@redhat.com>
MIME-Version: 1.0
Content-Type: multipart/signed; boundary="nextPart3247109.44csPzL39Z";
 micalg="pgp-sha256"; protocol="application/pgp-signature"
Precedence: bulk

--nextPart3247109.44csPzL39Z
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset="UTF-8"; protected-headers="v1"
From: Oleksandr Natalenko <oleksandr@redhat.com>
To: linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/3] scsi: qedf: sanitise uaccess
Date: Fri, 28 Jul 2023 09:05:07 +0200
Message-ID: <3240893.aeNJFYEL58@redhat.com>
Organization: Red Hat
In-Reply-To: <20230728065819.139694-1-oleksandr@redhat.com>
References: <20230728065819.139694-1-oleksandr@redhat.com>
MIME-Version: 1.0

On p=C3=A1tek 28. =C4=8Dervence 2023 8:58:16 CEST Oleksandr Natalenko wrote:
> qedf driver, debugfs part of it specifically, touches __user pointers
> directly for printing out info to userspace via sprintf(), which may
> cause crash like this:
>=20
> BUG: unable to handle kernel paging request at 00007ffd1d6b43a0
> IP: [<ffffffffaa7a882a>] string.isra.7+0x6a/0xf0
> Oops: 0003 [#1] SMP
> Call Trace:
>  [<ffffffffaa7a9f31>] vsnprintf+0x201/0x6a0
>  [<ffffffffaa7aa556>] sprintf+0x56/0x80
>  [<ffffffffc04227ed>] qedf_dbg_stop_io_on_error_cmd_read+0x6d/0x90 [qedf]
>  [<ffffffffaa65bb2f>] vfs_read+0x9f/0x170
>  [<ffffffffaa65cb82>] SyS_pread64+0x92/0xc0
>=20
> Avoid this by preparing the info in a kernel buffer first, either
> allocated on stack for small printouts, or via vmalloc() for big ones,
> and then copying it to the userspace properly.
>=20
> Previous submission is an RFC: [1]. There are no code changes since
> then. The RFC prefix is dropped. The Tested-by tag from Laurence is
> added.
>=20
> There's similar submission from Saurav [2], but we agreed I could nack
> it and proceed with my one.
>=20
> [1] https://lore.kernel.org/linux-scsi/20230724120241.40495-1-oleksandr@r=
edhat.com/
> [2] https://lore.kernel.org/linux-scsi/20230726101236.11922-1-skashyap@ma=
rvell.com/
>=20
> Oleksandr Natalenko (3):
>   scsi: qedf: do not touch __user pointer in
>     qedf_dbg_stop_io_on_error_cmd_read() directly
>   scsi: qedf: do not touch __user pointer in qedf_dbg_debug_cmd_read()
>     directly
>   scsi: qedf: do not touch __user pointer in qedf_dbg_fp_int_cmd_read()
>     directly
>=20
>  drivers/scsi/qedf/qedf_dbg.h     |  2 ++
>  drivers/scsi/qedf/qedf_debugfs.c | 35 +++++++++++++++++++-------------
>  2 files changed, 23 insertions(+), 14 deletions(-)
>=20
>=20

Oops, I forgot to add:

Reviewed-by: Laurence Oberman <loberman@redhat.com>

as per [1].

My ask to the maintainer to add it if the submission is accepted, or let me=
 know if I should do a v2 instead.

[1] https://lore.kernel.org/linux-scsi/4f35b02968a18e636e1689c9d52729ef63a4=
38f9.camel@redhat.com/

=2D-=20
Oleksandr Natalenko (post-factum)
Principal Software Maintenance Engineer
--nextPart3247109.44csPzL39Z
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: This is a digitally signed message part.
Content-Transfer-Encoding: 7Bit

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

iQIzBAABCAAdFiEENQb0bxzeq+SMr0+vn464/xkP+AUFAmTDaKMACgkQn464/xkP
+AVqDxAAmwO7k1gqDBcgpEupflAyGWPImXduaqsyM574mMzLHI017uhOy+iCAl0W
CiCfoIs5EGu1YdQc0RFB3qvCczLdBlb9d8K8KHD3Fx8v2Bl7acl38wJwfCqiV7V7
akS02IPqsD4liT4/U9KYogXqejYp3PXI7u3uGorSG0FGLrZhTRNKO4H1/myAm8PX
6+p7leJIJLdNRrqdif2p8zImQtRjbGFLJQhmLyy1L257w6To7tTESawChWkUmNpL
hppdTgVzzddSx2XSZvHm/sdyoj1y5xhBqAcGIaANDEDiJWS3dwhGMBV4sX5zI/PU
9E4hTq/Yq3fdSIphjlh8dV75HhFoGCpLr4sjvI/7lxVzQqksjkN1etSsU+e5ZXB3
v9/6Quf3bu1UwAFpJ4HvYC6rCwrLgC+reMWUKSlBsaYQzIY/Q2N0+geTpqCS99Mx
4+sM7/ks2ncU8AQkALL/cYq+NSLJS49Hni8kVnGDBx5KPJrzdBGIS353+IR6qa5q
4VBbDLDsMVQp/WUqJafvrymmnzqV7nCIyMWDxyrjCqeTBMM081je/uFrnRhrfK/E
0+NGsiGcSay03Ier6s7yZ/lwce7uzXQVVfyXowCgYmtguLdrCTZiqITEISt33JN3
rb8iLswNA6Kfb4gAcIKGFN3IJhKG0PdUz8j1HAk0w1XWsTm15ME=
=7C6A
-----END PGP SIGNATURE-----

--nextPart3247109.44csPzL39Z--