On 12/9/2022 4:01 PM, Alex Williamson wrote: > On Fri, 9 Dec 2022 14:52:49 -0500 > Steven Sistare <steven.sistare@xxxxxxxxxx> wrote: >> On 12/9/2022 2:42 PM, Alex Williamson wrote: >>> On Fri, 9 Dec 2022 13:40:29 -0500 >>> Steven Sistare <steven.sistare@xxxxxxxxxx> wrote: >>> >>>> On 12/8/2022 11:40 AM, Alex Williamson wrote: >>>>> On Thu, 8 Dec 2022 07:56:30 +0000 >>>>> "Tian, Kevin" <kevin.tian@xxxxxxxxx> wrote: >>>>> >>>>>>> From: Alex Williamson <alex.williamson@xxxxxxxxxx> >>>>>>> Sent: Thursday, December 8, 2022 5:45 AM >>>>>>> >>>>>>> Fix several loose ends relative to reverting support for vaddr removal >>>>>>> and update. Mark feature and ioctl flags as deprecated, restore local >>>>>>> variable scope in pin pages, remove remaining support in the mapping >>>>>>> code. >>>>>>> >>>>>>> Signed-off-by: Alex Williamson <alex.williamson@xxxxxxxxxx> >>>>>>> --- >>>>>>> >>>>>>> This applies on top of Steve's patch[1] to fully remove and deprecate >>>>>>> this feature in the short term, following the same methodology we used >>>>>>> for the v1 migration interface removal. The intention would be to pick >>>>>>> Steve's patch and this follow-on for v6.2 given that existing support >>>>>>> exposes vulnerabilities and no known upstream userspaces make use of >>>>>>> this feature. >>>>>>> >>>>>>> [1]https://lore.kernel.org/all/1670363753-249738-2-git-send-email- >>>>>>> steven.sistare@xxxxxxxxxx/ >>>>>>> >>>>>> >>>>>> Reviewed-by: Kevin Tian <kevin.tian@xxxxxxxxx> >>>>>> >>>>>> btw given the exposure and no known upstream usage should this be >>>>>> also pushed to stable kernels? >>>>> >>>>> I'll add to both: >>>>> >>>>> Cc: stable@xxxxxxxxxxxxxxx # v5.12+ >>>> >>>> We maintain and use a version of qemu that contains the live update patches, >>>> and requires these kernel interfaces. Other companies are also experimenting >>>> with it. Please do not remove it from stable. >>> >>> The interface has been determined to have vulnerabilities and the >>> proposal to resolve those vulnerabilities is to implement a new API. >>> If we think it's worthwhile to remove the existing, vulnerable interface >>> in the current kernel, what makes it safe to keep it for stable kernels? >> >> I do not think it's worth while, but I have stopped fighting for 6.2. >> >>> Existing users that could choose not to accept the revert in their >>> downstream kernel and allowing users evaluating the interface more time >>> before they know it's been removed upstream, are not terribly >>> compelling reasons to keep it in upstream stable kernels. Thanks, >> >> The compelling reason is that stable is supposed to be stable and maintain >> existing interfaces, and now I will need to re-merge the interfaces at >> regular intervals when we update UEK from stable. Oracle is a current user >> of these interfaces in our business. Do we count? > > These are the rules for stable from > Documentation/process/stable-kernel-rules.rst: > > - It must be obviously correct and tested. > > (check) > > - It cannot be bigger than 100 lines, with context. > > (We're pushing this a bit, but we could certainly disable w/o removing > the interface in far fewer lines. We're close enough that I think a > direct backport is preferable) > > - It must fix only one thing. > > (check) > > - It must fix a real bug that bothers people (not a, "This could be a > problem..." type thing). > > (This is a point where you could present an objection) > > - It must fix a problem that causes a build error (but not for things > marked CONFIG_BROKEN), an oops, a hang, data corruption, a real > security issue, or some "oh, that's not good" issue. In short, something > critical. > > (This as well) > > - Serious issues as reported by a user of a distribution kernel may also > be considered if they fix a notable performance or interactivity issue. > As these fixes are not as obvious and have a higher risk of a subtle > regression they should only be submitted by a distribution kernel > maintainer and include an addendum linking to a bugzilla entry if it > exists and additional information on the user-visible impact. > > (N/A, but note the mention of a user visible impact) > > - New device IDs and quirks are also accepted. > > (N/A) > > - No "theoretical race condition" issues, unless an explanation of how the > race can be exploited is also provided. > > (AIUI, the vulnerabilities here may not have exploits, but they are real) > > - It cannot contain any "trivial" fixes in it (spelling changes, > whitespace cleanups, etc). > > (N/A) > > - It must follow the > :ref:`Documentation/process/submitting-patches.rst <submittingpatches>` > rules. > > (Of course) > > - It or an equivalent fix must already exist in Linus' tree (upstream). > > This last bullet is really the crux of what brings us to this point, if > you're not willing to defend the vulnerabilities to maintain the > interface in the mainline kernel, why should the upstream community > maintain them in the stable kernels? > > The question is not about who is using the interface, it's the fact that > the resolution to the existing vulnerabilities is to remove the > interface and nobody is making a case around the validity or > exploit-ability of those vulnerabilities to carry along the interface > in the interim. > > If the revert does go into mainline, but were to skip stable, that only > delays your re-merging burden briefly, while continuing to expose stable > kernels to the vulnerabilities, and risks further users adopting an > interface that no longer exists. Thanks, Thank you for your thoughtful response. Rather than debate the degree of of vulnerability, I propose an alternate solution. The technical crux of the matter is support for mediated devices. So, let's exclude them when these legacy interfaces are used, and allow them for native iommufd. The fix is small and simple: if there is no iommu capable domain in the container, then return false for the VFIO_UPDATE_VADDR extension. And to prevent locked_mm underflow, add to the new mm's locked_vm in VFIO_DMA_MAP_FLAG_VADDR. Two small patches, which I can submit on monday, for 6.x and stable. I can also submit a patch for iommufd to use these interfaces with no mdev in vfio compat mode. I am still committed to new interfaces for native iommufd, and am making good progress with Jason's patch. - Steve
