|
[AMD Public Use]
The full vbios versioning information consists of the version (numeric), build date and the name... I this this interface exposes only the name.
Should we expose those on different sysfs files or just combine all of them in a single file?
David
From: Kees Cook <keescook@xxxxxxxxxxxx>
Sent: Saturday, May 8, 2021 2:51 AM To: Gu, JiaWei (Will) <JiaWei.Gu@xxxxxxx> Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; StDenis, Tom <Tom.StDenis@xxxxxxx>; Christian König <ckoenig.leichtzumerken@xxxxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>; Nieto, David M <David.Nieto@xxxxxxx>; linux-next@xxxxxxxxxxxxxxx <linux-next@xxxxxxxxxxxxxxx> Subject: Re: [PATCH] drm/amdgpu: Add vbios info ioctl interface On Sat, May 08, 2021 at 06:01:23AM +0000, Gu, JiaWei (Will) wrote:
> [AMD Official Use Only - Internal Distribution Only] > > Thanks for this catching Kees. > > Yes it should be 20, not 16. I was not aware that serial size had been changed from 16 to 20 in struct amdgpu_device. > Will submit a fix soon. You might want to add a BUILD_BUG_ON() to keep those in sync, especially since it's about to be UAPI. -Kees > > Best regards, > Jiawei > > > -----Original Message----- > From: Kees Cook <keescook@xxxxxxxxxxxx> > Sent: Saturday, May 8, 2021 12:28 PM > To: Gu, JiaWei (Will) <JiaWei.Gu@xxxxxxx>; Deucher, Alexander <Alexander.Deucher@xxxxxxx> > Cc: StDenis, Tom <Tom.StDenis@xxxxxxx>; Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Christian König <ckoenig.leichtzumerken@xxxxxxxxx>; Gu, JiaWei (Will) <JiaWei.Gu@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx; Nieto, David M <David.Nieto@xxxxxxx>; linux-next@xxxxxxxxxxxxxxx > Subject: Re: [PATCH] drm/amdgpu: Add vbios info ioctl interface > > Hi! > > This patch needs some fixing. > > On Thu, Apr 22, 2021 at 10:34:48AM +0800, Jiawei Gu wrote: > > + case AMDGPU_INFO_VBIOS_INFO: { > > + struct drm_amdgpu_info_vbios vbios_info = {}; > > + struct atom_context *atom_context; > > + > > + atom_context = adev->mode_info.atom_context; > > + memcpy(vbios_info.name, atom_context->name, sizeof(atom_context->name)); > > + vbios_info.dbdf = PCI_DEVID(adev->pdev->bus->number, adev->pdev->devfn); > > + memcpy(vbios_info.vbios_pn, atom_context->vbios_pn, sizeof(atom_context->vbios_pn)); > > + vbios_info.version = atom_context->version; > > + memcpy(vbios_info.date, atom_context->date, sizeof(atom_context->date)); > > + memcpy(vbios_info.serial, adev->serial, sizeof(adev->serial)); > > This writes beyond the end of vbios_info.serial. > > > + vbios_info.dev_id = adev->pdev->device; > > + vbios_info.rev_id = adev->pdev->revision; > > + vbios_info.sub_dev_id = atom_context->sub_dev_id; > > + vbios_info.sub_ved_id = atom_context->sub_ved_id; > > Though it gets "repaired" by these writes. > > > + > > + return copy_to_user(out, &vbios_info, > > + min((size_t)size, sizeof(vbios_info))) ? -EFAULT : 0; > > + } > > sizeof(adev->serial) != sizeof(vbios_info.serial) > > adev is struct amdgpu_device: > > struct amdgpu_device { > ... > char serial[20]; > > > > +struct drm_amdgpu_info_vbios { > > [...] > > + __u8 serial[16]; > > + __u32 dev_id; > > + __u32 rev_id; > > + __u32 sub_dev_id; > > + __u32 sub_ved_id; > > +}; > > Is there a truncation issue (20 vs 16) and is this intended to be a NUL-terminated string? > > -- > Kees Cook -- Kees Cook |
_______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx
