Messages in this thread |  | | Date | Tue, 5 May 2026 11:26:42 +0800 | | Subject | Re: [PATCH v2] iommufd: Use sizeof(*hdr) instead of sizeof(hdr) in veventq read | | From | Baolu Lu <> |
| |
On 5/1/2026 1:56 AM, Kai Aizen wrote: > The bound-check in iommufd_veventq_fops_read() for the normal vEVENT > path uses sizeof(hdr) where the surrounding code uses sizeof(*hdr): > > if (!vevent_for_lost_events_header(cur) && > sizeof(hdr) + cur->data_len > count - done) { > > hdr is declared as struct iommufd_vevent_header *, so sizeof(hdr) > evaluates to the size of the pointer. Surrounding code uses > sizeof(*hdr) consistently: > > if (done >= count || sizeof(*hdr) > count - done) { > ... > if (copy_to_user(buf + done, hdr, sizeof(*hdr))) { > ... > done += sizeof(*hdr); > > struct iommufd_vevent_header is currently 8 bytes (two __u32 fields, > flags and sequence), so on 64-bit (sizeof(void *) == 8) the two > expressions happen to be equal and the check works as intended. > > On 32-bit (sizeof(void *) == 4) the check under-counts the header by > 4 bytes: a vEVENT whose data_len causes 8 + cur->data_len to exceed > count - done while 4 + cur->data_len does not will pass the check, > then the loop will copy_to_user 8 bytes of header followed by data_len > bytes of payload, writing past the user-supplied buffer. > > It is also a latent bug for any future expansion of struct > iommufd_vevent_header beyond sizeof(void *) on 64-bit; the check > should not depend on the type happening to match the host pointer > width. > > Use sizeof(*hdr) to match the rest of the function and the actual > amount that will be copied. > > Fixes: e36ba5ab808e ("iommufd: Add IOMMUFD_OBJ_VEVENTQ and IOMMUFD_CMD_VEVENTQ_ALLOC") > Cc:stable@vger.kernel.org > Reported-by: Kai Aizen<kai.aizen.dev@gmail.com> > Signed-off-by: Kai Aizen<kai.aizen.dev@gmail.com>
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
|  |