From: Jacob Pan <jacob.jun.pan@linux.intel.com> To: Alex Williamson <alex.williamson@redhat.com> Cc: "Tian, Kevin" <kevin.tian@intel.com>, Christoph Hellwig <hch@infradead.org>, Joerg Roedel <joro@8bytes.org>, Lu Baolu <baolu.lu@linux.intel.com>, "iommu@lists.linux-foundation.org" <iommu@lists.linux-foundation.org>, LKML <linux-kernel@vger.kernel.org>, David Woodhouse <dwmw2@infradead.org>, Jean-Philippe Brucker <jean-philippe@linaro.com>, "Raj, Ashok" <ashok.raj@intel.com>, jacob.jun.pan@linux.intel.com, "eric.auger@redhat.com" <eric.auger@redhat.com> Subject: Re: [PATCH v2 1/3] iommu/uapi: Define uapi version and capabilities Date: Tue, 14 Apr 2020 15:32:26 -0700 [thread overview] Message-ID: <20200414153226.30d8cab3@jacob-builder> (raw) In-Reply-To: <20200414101304.27e587eb@jacob-builder> On Tue, 14 Apr 2020 10:13:04 -0700 Jacob Pan <jacob.jun.pan@linux.intel.com> wrote: > > > > In any of the proposed solutions, the > > > > IOMMU driver is ultimately responsible for validating the user > > > > data, so do we want vfio performing the copy_from_user() to an > > > > object that could later be assumed to be sanitized, or should > > > > vfio just pass a user pointer to make it obvious that the > > > > consumer is responsible for all the user protections? Seems > > > > like the latter. > > > I like the latter as well. > > > On a second thought, I think the former is better. Two reasons: 1. IOMMU API such as page_response is also used in baremetal. So it is not suitable to pass a __user *. https://www.spinics.net/lists/arm-kernel/msg798677.html 2. Some data are in the mandatory (fixed offset, never removed or extended) portion of the uAPI structure. It is simpler for VFIO to extract that and pass it to IOMMU API. For example, the PASID value used for unbind_gpasid(). VFIO also need to sanitize the PASID value to make sure it belongs to the same VM that did the allocation. > > > > That still really > > > > doesn't address what's in that user data blob yet, but the vfio > > > > interface could be: > > > > > > > > struct { > > > > __u32 argsz; > > > > __u32 flags; > > > > __u8 data[]; > > > > } > > > > > > > > Where flags might be partitioned like we do for DEVICE_FEATURE > > > > to indicate the format of data and what vfio should do with it, > > > > and data might simply be defined as a (__u64 __user *). > > > > > > > So, __user * will be passed to IOMMU driver if VFIO checks minsz > > > include flags and they are valid. > > > IOMMU driver can copy the rest based on the mandatory > > > version/minsz and flags in the IOMMU uAPI structs. > > > Does it sound right? This is really choice #2. > > > > Sounds like each IOMMU UAPI struct just needs to have an embedded > > size and flags field, but yes. > > > Yes, an argsz field can be added to each UAPI. There are already flags > or the equivalent. IOMMU driver can process the __user * based on the > argsz, flags, check argsz against offsetofend(iommu_uapi_struct, > last_element), etc.;
WARNING: multiple messages have this Message-ID (diff)
From: Jacob Pan <jacob.jun.pan@linux.intel.com> To: Alex Williamson <alex.williamson@redhat.com> Cc: "Tian, Kevin" <kevin.tian@intel.com>, "Raj, Ashok" <ashok.raj@intel.com>, Jean-Philippe Brucker <jean-philippe@linaro.com>, LKML <linux-kernel@vger.kernel.org>, "iommu@lists.linux-foundation.org" <iommu@lists.linux-foundation.org>, David Woodhouse <dwmw2@infradead.org> Subject: Re: [PATCH v2 1/3] iommu/uapi: Define uapi version and capabilities Date: Tue, 14 Apr 2020 15:32:26 -0700 [thread overview] Message-ID: <20200414153226.30d8cab3@jacob-builder> (raw) In-Reply-To: <20200414101304.27e587eb@jacob-builder> On Tue, 14 Apr 2020 10:13:04 -0700 Jacob Pan <jacob.jun.pan@linux.intel.com> wrote: > > > > In any of the proposed solutions, the > > > > IOMMU driver is ultimately responsible for validating the user > > > > data, so do we want vfio performing the copy_from_user() to an > > > > object that could later be assumed to be sanitized, or should > > > > vfio just pass a user pointer to make it obvious that the > > > > consumer is responsible for all the user protections? Seems > > > > like the latter. > > > I like the latter as well. > > > On a second thought, I think the former is better. Two reasons: 1. IOMMU API such as page_response is also used in baremetal. So it is not suitable to pass a __user *. https://www.spinics.net/lists/arm-kernel/msg798677.html 2. Some data are in the mandatory (fixed offset, never removed or extended) portion of the uAPI structure. It is simpler for VFIO to extract that and pass it to IOMMU API. For example, the PASID value used for unbind_gpasid(). VFIO also need to sanitize the PASID value to make sure it belongs to the same VM that did the allocation. > > > > That still really > > > > doesn't address what's in that user data blob yet, but the vfio > > > > interface could be: > > > > > > > > struct { > > > > __u32 argsz; > > > > __u32 flags; > > > > __u8 data[]; > > > > } > > > > > > > > Where flags might be partitioned like we do for DEVICE_FEATURE > > > > to indicate the format of data and what vfio should do with it, > > > > and data might simply be defined as a (__u64 __user *). > > > > > > > So, __user * will be passed to IOMMU driver if VFIO checks minsz > > > include flags and they are valid. > > > IOMMU driver can copy the rest based on the mandatory > > > version/minsz and flags in the IOMMU uAPI structs. > > > Does it sound right? This is really choice #2. > > > > Sounds like each IOMMU UAPI struct just needs to have an embedded > > size and flags field, but yes. > > > Yes, an argsz field can be added to each UAPI. There are already flags > or the equivalent. IOMMU driver can process the __user * based on the > argsz, flags, check argsz against offsetofend(iommu_uapi_struct, > last_element), etc.; _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
next prev parent reply other threads:[~2020-04-14 22:26 UTC|newest] Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-03-25 23:17 [PATCH v2 0/3] IOMMU user API enhancement Jacob Pan 2020-03-25 23:17 ` Jacob Pan 2020-03-25 23:17 ` [PATCH v2 1/3] iommu/uapi: Define uapi version and capabilities Jacob Pan 2020-03-25 23:17 ` Jacob Pan 2020-03-26 9:23 ` Christoph Hellwig 2020-03-26 9:23 ` Christoph Hellwig 2020-03-26 16:44 ` Jacob Pan 2020-03-26 16:44 ` Jacob Pan 2020-03-27 2:49 ` Tian, Kevin 2020-03-27 2:49 ` Tian, Kevin 2020-03-27 7:47 ` Christoph Hellwig 2020-03-27 7:47 ` Christoph Hellwig 2020-03-27 23:53 ` Jacob Pan 2020-03-27 23:53 ` Jacob Pan 2020-03-30 5:40 ` Tian, Kevin 2020-03-30 5:40 ` Tian, Kevin 2020-03-30 16:07 ` Jacob Pan 2020-03-30 16:07 ` Jacob Pan 2020-03-31 6:06 ` Tian, Kevin 2020-03-31 6:06 ` Tian, Kevin 2020-03-31 15:54 ` Jacob Pan 2020-03-31 15:54 ` Jacob Pan 2020-04-01 5:32 ` Tian, Kevin 2020-04-01 5:32 ` Tian, Kevin 2020-04-02 18:36 ` Jacob Pan 2020-04-02 18:36 ` Jacob Pan 2020-04-13 20:41 ` Jacob Pan 2020-04-13 20:41 ` Jacob Pan 2020-04-13 22:21 ` Alex Williamson 2020-04-13 22:21 ` Alex Williamson 2020-04-14 5:05 ` Jacob Pan 2020-04-14 5:05 ` Jacob Pan 2020-04-14 16:13 ` Alex Williamson 2020-04-14 16:13 ` Alex Williamson 2020-04-14 17:13 ` Jacob Pan 2020-04-14 17:13 ` Jacob Pan 2020-04-14 22:32 ` Jacob Pan [this message] 2020-04-14 22:32 ` Jacob Pan 2020-04-14 23:47 ` Tian, Kevin 2020-04-14 23:47 ` Tian, Kevin 2020-04-15 15:38 ` Jacob Pan 2020-04-15 15:38 ` Jacob Pan 2020-04-16 1:27 ` Tian, Kevin 2020-04-16 1:27 ` Tian, Kevin 2020-04-14 8:15 ` Christoph Hellwig 2020-04-14 8:15 ` Christoph Hellwig 2020-04-14 8:11 ` Christoph Hellwig 2020-04-14 8:11 ` Christoph Hellwig 2020-04-14 16:06 ` Jacob Pan 2020-04-14 16:06 ` Jacob Pan 2020-03-25 23:17 ` [PATCH v2 2/3] iommu/uapi: Use unified UAPI version Jacob Pan 2020-03-25 23:17 ` Jacob Pan 2020-03-25 23:17 ` [PATCH v2 3/3] iommu/uapi: Add helper function for size lookup Jacob Pan 2020-03-25 23:17 ` Jacob Pan
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200414153226.30d8cab3@jacob-builder \ --to=jacob.jun.pan@linux.intel.com \ --cc=alex.williamson@redhat.com \ --cc=ashok.raj@intel.com \ --cc=baolu.lu@linux.intel.com \ --cc=dwmw2@infradead.org \ --cc=eric.auger@redhat.com \ --cc=hch@infradead.org \ --cc=iommu@lists.linux-foundation.org \ --cc=jean-philippe@linaro.com \ --cc=joro@8bytes.org \ --cc=kevin.tian@intel.com \ --cc=linux-kernel@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.