On Tue, Oct 14, 2025 at 2:49 PM Alex Deucher <[email protected]> wrote:
> On Tue, Oct 14, 2025 at 2:10 PM Marek Olšák <[email protected]> wrote: > > > > On Tue, Oct 14, 2025, 12:37 Alex Deucher <[email protected]> wrote: > >> > >> On Tue, Oct 14, 2025 at 11:11 AM Marek Olšák <[email protected]> wrote: > >> > > >> > On Tue, Oct 14, 2025 at 10:12 AM Alex Deucher <[email protected]> > wrote: > >> >> > >> >> On Tue, Oct 14, 2025 at 2:49 AM Marek Olšák <[email protected]> > wrote: > >> >> > > >> >> > On Mon, Oct 13, 2025 at 3:11 PM Alex Deucher < > [email protected]> wrote: > >> >> >> > >> >> >> On Mon, Oct 13, 2025 at 10:21 AM Liang, Prike < > [email protected]> wrote: > >> >> >> > > >> >> >> > [Public] > >> >> >> > > >> >> >> > Regards, > >> >> >> > Prike > >> >> >> > > >> >> >> > > -----Original Message----- > >> >> >> > > From: Alex Deucher <[email protected]> > >> >> >> > > Sent: Monday, October 13, 2025 9:44 PM > >> >> >> > > To: Liang, Prike <[email protected]> > >> >> >> > > Cc: Deucher, Alexander <[email protected]>; amd- > >> >> >> > > [email protected] > >> >> >> > > Subject: Re: [PATCH 3/7] drm/amdgpu/gfx: add eop size and > alignment to shadow > >> >> >> > > info > >> >> >> > > > >> >> >> > > On Mon, Oct 13, 2025 at 4:54 AM Liang, Prike < > [email protected]> wrote: > >> >> >> > > > > >> >> >> > > > [Public] > >> >> >> > > > > >> >> >> > > > We may need to update the userspace EOP buffer request; > otherwise, the EOP > >> >> >> > > buffer validation may fail. > >> >> >> > > > >> >> >> > > Existing userspace should be ok. It currently uses PAGE_SIZE > which is larger than > >> >> >> > > 2048. > >> >> >> > The mesa uses the EOP size as max_t(u32, PAGE_SIZE, > AMDGPU_GPU_PAGE_SIZE) which is sees larger than 2048, so the kernel > validates the eop buffer can be successful at this point. > >> >> >> > > >> >> >> > But the mesa may need to use the shadow_info->eop_size as well > in the future? > >> >> >> > >> >> >> Ideally mesa would query the kernel to get the proper minimum > size. > >> >> >> Yogesh will be looking at this. > >> >> >> > >> >> >> Alex > >> >> > > >> >> > > >> >> > Does the EOP buffer store privileged information? What is its > content? > >> >> > >> >> It stores end of pipe events for the compute queue generated from > >> >> things like RELEASE_MEM or AQL packets. They are specific to each > >> >> user queue. In theory corrupting or messing with the data in the > >> >> buffer should only affect that queue. > >> > > >> > > >> > RELEASE_MEM has a hidden implicit VMID parameter. That's why it's > important to know whether it's stored in the EOP buffer that can be > overwritten by userspace. > >> > >> My understanding is that that is only relevant for kernel queues where > >> the vmid comes from the IB for each job. For user queues, the vmid is > >> determined by the HQD so that is unused in the user queue case. > > > > > > This is NAK'd until a proof is given that the EOP buffer can't be used > to change the VMID of EOP fence writes. > > The EOP buffer allocation is already in use. It's used for compute > user queues for both ROCm and KGD. All this patch does is allow > userspace to query what the size requirements are. If we find a > problem in the EOP handling in firmware we should fix it in firmware, > but the software side is already in use. > So are you going to just brush it off? Aren't you interested in at least verifying whether it's possible to do privilege escalation with it even if it's already used? Do you really just want to merge this quickly without verifying whether it allows userspace to write any physical address because "userspace already uses it"? Do you really not want to know what is possible to do with the EOP buffer? Marek
