Hi Thomas, On 2019/05/27, Thomas Hellstrom wrote: > > I think we might be talking past each other, let's take a step back: > > > > - as of previous patch, all of vmwgfx ioctls size is consistently > > handled by the core > > I don't think I follow you here, AFAICT patch 3/5 only affects and > relaxes the execbuf checking (and in fact a little more than I would > like)? > Precisely, it makes execbuf ioctl behave like all other ioctls - both vmwgfx and rest of drm. > > - handling of featue flags, as always, is responsibility of the > > driver > > ifself > > - with this patch, ioctl direction is also handled by core. > > > > Here core ensures we only copy in/out as much data as the kernel > > implementation can handle. > > > > > > Let's consider the following real world example - msm and virtio_gpu. > > > > An in field of an _IOW ioctl becomes in/out aka _IORW ioctl. > > - we add a flag to annotate/request the out, as always invalid flags > > are return -EINVAL > > - we change the ioctl encoding > > > > As currently handled by core DRM, old kernel/new userspace and > > vice-versa works just fine. Sadly, vmwgfx will error out, while it > > could > > be avoided. > > IMO basically we have a tradeoff between strict checking in this case, > and new user-space vs old kernel "hazzle-free" transition in the > relaxed case. > Precisely. If I read the code correctly, ATM new userspace will fail against old kernels. Unless userspace writes two versions of the ioctl - with with each encoding. > > > > As said above, I'll gladly adjust core and/or others, if this relaxed > > approach causes an issue somewhere. A specific use-case, real or > > hypothetical will be appreciated. > > To me there are two important reasons to keep the strict approach. > > 1) Avoid user-space mistakes early in the development cycle. We can't > distinguish between buggy user-space and "new" user-space. This is > important because of [a]) below. > Can you provide a concrete example, please? > 2) Catch a lot of fuzzer combinations and error out early instead of > forwarding them to the ioctl function where they may cause harm. > Struggling to see why this is a problem? At some point the fuzzer will get past this first line of defence, so we want to make the rest of the ioctl is robust. > I think the new user-space vs old kernel can be handled nicely in user- > space with feature flags or API versions. That's the way we've handled > them up to now? > How is a feature flag doing to help if the encoding changes from _IOW to _IORW? Thanks Emil _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
