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:   "Tian, Kevin" <kevin.tian@intel.com>
To:     Stefan Hajnoczi <stefanha@redhat.com>,
        Alex Williamson <alex.williamson@redhat.com>
CC:     "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "kvm@vger.kernel.org" <kvm@vger.kernel.org>
Subject: RE: [PATCH] vfio/type1: fix cap_migration information leak
Thread-Topic: [PATCH] vfio/type1: fix cap_migration information leak
Thread-Index: AQHZxJCN0AlhA9yfwU2jGe88ugDMa6/YCjWg
Date:   Thu, 3 Aug 2023 05:19:04 +0000
Message-ID: <BN9PR11MB5276CEA18E764411F6E755D98C08A@BN9PR11MB5276.namprd11.prod.outlook.com>
References: <20230801155352.1391945-1-stefanha@redhat.com>
In-Reply-To: <20230801155352.1391945-1-stefanha@redhat.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?1fXp3OIIBAlMzigIjTjO9mdOmDUzt+bcj+DAPNc9BBW8+FhOEtpubR3B08TI?=
 =?us-ascii?Q?hIhvqLL2chNNpUZ8AUpNnwNjZ3tW07B78XRvQoBLnxo1wn3WiawG4iwA31ZY?=
 =?us-ascii?Q?1vNHXJAxYUwZf1Ewl3WdKYYz+hjJcyx0dfztYC7iLXzpLiowTTWtAT3g11TP?=
 =?us-ascii?Q?fjL3Hz60RstU8VHE5aVarJSgk/QHJtPDCeA4U9hkIAf2m6l8CpXKyx5o7PAn?=
 =?us-ascii?Q?wYCmX5UuuDsq4Yv1jdQvJoBJbf1i4nxj+bbCwWK9kCIW8luHvL8xX/txjYYM?=
 =?us-ascii?Q?M76E8r3CL3CSfTCLgS5sw0nE7sw7EodMJKMBzZgdmsQcEgYNHePw99zk8B4i?=
 =?us-ascii?Q?QGOuP+7ZxJCJlllsILWeZkHXXSenhE7XwpRuXXiWe1m7nie7JfJqtYVGYEfd?=
 =?us-ascii?Q?hLxOj6MCq6QY8gmDNvQR5+IyRwSDmwPOy3D5DfMD96ZnU8P1Y6jzePlNzDwA?=
 =?us-ascii?Q?a7ZYtXUOg+012t/PlqiQVEuBQWoSNBxNuFav5vL/x4FzcTU9G/b9gXQV57fR?=
 =?us-ascii?Q?XIff+5ZSuYhL1nzbUxWOjgymNTCBetecIx6g5SF4ZYyK9ksMK7IPL1aGFBqu?=
 =?us-ascii?Q?dVcJnholUWHiFgWck8ryj069EaKA0IjBw4+DiRfVxANL6YBLiNP15fIkgwZJ?=
 =?us-ascii?Q?0UAXoDmNkRFKgea8RE1Gy18w6kXsm9sT3luWLCdA0o/jpeFCQEk6Ts0ePZjq?=
 =?us-ascii?Q?82oGzBJWxLmgot1i494vq1H6k27WNNgnoZDsds4edUAGGSvPSstZkbpyjcpJ?=
 =?us-ascii?Q?NU7LAexSjwioEeMNAtDyU4KTUPau7/GCRfEIDdXF+RfRawcRf1V2ax50haQY?=
 =?us-ascii?Q?lVFCaXGoeXGZcjVmG8Q15AURmg9KIlv9AjvgwRi/18DY1tdK37SWmwV/5/rX?=
 =?us-ascii?Q?2FrQrA/FOxhbfBMkzoSCkjgoZmBxfVQWJMMt53CjXae2h4+yy/W/jGtJuyGs?=
 =?us-ascii?Q?FGn43P2/vU8zj8GH3AUVk7kB0CuYE9ReeoIgVG+fL/JxKjReHmp8Gg4+0e08?=
 =?us-ascii?Q?RPLTWZNmFxQuBLq3owdkqz3gn99duSOUBc9+rUhxYnC2WARXMxR0V3Ru55FI?=
 =?us-ascii?Q?g1sgCkRIeFVmjU0PMyvMdyUU3sw0YowQbG/4DwLyp9RKVkz2Z/lJK9XScMTJ?=
 =?us-ascii?Q?XOoEWjzPnI5cE3SA1lKzIbJSQeb9Vc04AhJifkXW43EqOp06PSxDGKM4fbAi?=
 =?us-ascii?Q?AmM0uzlD0Vyg3fdgNXwfNRB9p43L4wkVZVKs9XSrIfhOXu8cbigqtyvjgEJl?=
 =?us-ascii?Q?upgVOJb0DKOrAarNWGfzKKTTUkLKj/cHKlxcHnrn116fUbyz8YmAZJITWpLp?=
 =?us-ascii?Q?SfJrOCB900IvWTAyMr+PH46rUtxAwHLtzZzGquVEPt7rprYT4+kfjZ3Y62Cc?=
 =?us-ascii?Q?WIQHjUi0TNsCWW9zaQ+czuTrToZnyYjdry38B4a+A+1SMUjZ7tYeebKi/Xh3?=
 =?us-ascii?Q?+GZUH2LugMNrS3umAF930hQVIgWebPRcqTBSe6Ikk2Q3xsBFaTaes4u+/5OP?=
 =?us-ascii?Q?ysGqodlZ1wsjA4wRU5SVNjUqoAMzRmjfET8c9nT7xRJ49DeF6Kf/Py50Xp00?=
 =?us-ascii?Q?PQxVrc6N1+G3gFMcq3LWBsJD1RUE71y7qjP0F9PG?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BN9PR11MB5276.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c3563da8-c010-4f11-4c65-08db93e127cc
X-MS-Exchange-CrossTenant-originalarrivaltime: 03 Aug 2023 05:19:04.7442
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: MCEyunSEc10jfzTW3zWvVDGOYdqhW1Q48YQ1Q5fsZ39J8FixYDd5GKeYRMAaE5AWQdBJ37Cg0wh+Pa1NtRw6Bg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR11MB8198
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,
        RCVD_IN_DNSWL_BLOCKED,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> From: Stefan Hajnoczi <stefanha@redhat.com>
> Sent: Tuesday, August 1, 2023 11:54 PM
>=20
> Fix an information leak where an uninitialized hole in struct
> vfio_iommu_type1_info_cap_migration on the stack is exposed to userspace.
>=20
> The definition of struct vfio_iommu_type1_info_cap_migration contains a
> hole as
> shown in this pahole(1) output:
>=20
>   struct vfio_iommu_type1_info_cap_migration {
>           struct vfio_info_cap_header header;              /*     0     8=
 */
>           __u32                      flags;                /*     8     4=
 */
>=20
>           /* XXX 4 bytes hole, try to pack */
>=20
>           __u64                      pgsize_bitmap;        /*    16     8=
 */
>           __u64                      max_dirty_bitmap_size; /*    24     =
8 */
>=20
>           /* size: 32, cachelines: 1, members: 4 */
>           /* sum members: 28, holes: 1, sum holes: 4 */
>           /* last cacheline: 32 bytes */
>   };
>=20
> The cap_mig variable is filled in without initializing the hole:
>=20
>   static int vfio_iommu_migration_build_caps(struct vfio_iommu *iommu,
>                          struct vfio_info_cap *caps)
>   {
>       struct vfio_iommu_type1_info_cap_migration cap_mig;
>=20
>       cap_mig.header.id =3D VFIO_IOMMU_TYPE1_INFO_CAP_MIGRATION;
>       cap_mig.header.version =3D 1;
>=20
>       cap_mig.flags =3D 0;
>       /* support minimum pgsize */
>       cap_mig.pgsize_bitmap =3D (size_t)1 << __ffs(iommu->pgsize_bitmap);
>       cap_mig.max_dirty_bitmap_size =3D DIRTY_BITMAP_SIZE_MAX;
>=20
>       return vfio_info_add_capability(caps, &cap_mig.header, sizeof(cap_m=
ig));
>   }
>=20
> The structure is then copied to a temporary location on the heap. At this
> point
> it's already too late and ioctl(VFIO_IOMMU_GET_INFO) copies it to userspa=
ce
> later:
>=20
>   int vfio_info_add_capability(struct vfio_info_cap *caps,
>                    struct vfio_info_cap_header *cap, size_t size)
>   {
>       struct vfio_info_cap_header *header;
>=20
>       header =3D vfio_info_cap_add(caps, size, cap->id, cap->version);
>       if (IS_ERR(header))
>           return PTR_ERR(header);
>=20
>       memcpy(header + 1, cap + 1, size - sizeof(*header));
>=20
>       return 0;
>   }
>=20
> This issue was found by code inspection.
>=20
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

nit. better also add a 'reserved' field in cap_migration to
mark the hole.

Reviewed-by: Kevin Tian <kevin.tian@intel.com>