[PATCH 0/2] test capability for remote task

[PATCH 0/2] test capability for remote task

[Jike Song]

Sometimes vfio iommu type1 needs to pin memory for a remote task other
than current, thereby needs to test the CAP_IPC_LOCK capability for
that task.

The proper routine for this purpose is has_capability(), but it is
not yet exported for modules. None of currently exported capability-
testing symbols allows a specified task. So here has_capability()
is exported then used in the vfio iommu type1 driver.


Jike Song (2):
  capability: export has_capability
  vfio iommu type1: fix the testing of capability for remote task

 drivers/vfio/vfio_iommu_type1.c | 6 ++----
 kernel/capability.c             | 1 +
 2 files changed, 3 insertions(+), 4 deletions(-)

-- 
2.4.4.488.gdf97e5d

[PATCH 1/2] capability: export has_capability

[Jike Song]

has_capability() is sometimes needed by modules to test capability
for specified task other than current, so export it.

Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Kirti Wankhede <kwankhede@nvidia.com>
Signed-off-by: Jike Song <jike.song@intel.com>
---
 kernel/capability.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/capability.c b/kernel/capability.c
index 4984e1f..e2e198c 100644
--- a/kernel/capability.c
+++ b/kernel/capability.c
@@ -318,6 +318,7 @@ bool has_capability(struct task_struct *t, int cap)
 {
 	return has_ns_capability(t, &init_user_ns, cap);
 }
+EXPORT_SYMBOL(has_capability);
 
 /**
  * has_ns_capability_noaudit - Does a task have a capability (unaudited)
-- 
2.4.4.488.gdf97e5d

[PATCH 2/2] vfio iommu type1: fix the testing of capability for remote task

[Jike Song]

Before the mdev enhancement type1 iommu used capable() to test the
capability of current task; in the course of mdev development a
new requirement, testing for another task other than current, was
raised.  ns_capable() was used for this purpose, however it still
tests current, the only difference is, in a specified namespace.

Fix it by using has_capability() instead, which tests the cap for
specified task in init_user_ns, the same namespace as capable().

Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Kirti Wankhede <kwankhede@nvidia.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Jike Song <jike.song@intel.com>
---
 drivers/vfio/vfio_iommu_type1.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index f3726ba..b54aedf 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -394,8 +394,7 @@ static long vfio_pin_pages_remote(struct vfio_dma *dma, unsigned long vaddr,
 				  long npage, unsigned long *pfn_base)
 {
 	unsigned long limit;
-	bool lock_cap = ns_capable(task_active_pid_ns(dma->task)->user_ns,
-				   CAP_IPC_LOCK);
+	bool lock_cap = has_capability(dma->task, CAP_IPC_LOCK);
 	struct mm_struct *mm;
 	long ret, i = 0, lock_acct = 0;
 	bool rsvd;
@@ -491,8 +490,7 @@ static int vfio_pin_page_external(struct vfio_dma *dma, unsigned long vaddr,
 				  unsigned long *pfn_base, bool do_accounting)
 {
 	unsigned long limit;
-	bool lock_cap = ns_capable(task_active_pid_ns(dma->task)->user_ns,
-				   CAP_IPC_LOCK);
+	bool lock_cap = has_capability(dma->task, CAP_IPC_LOCK);
 	struct mm_struct *mm;
 	int ret;
 	bool rsvd;
-- 
2.4.4.488.gdf97e5d

Re: [PATCH 2/2] vfio iommu type1: fix the testing of capability for remote task

[Kirti Wankhede]

On 12/21/2016 9:40 PM, Jike Song wrote:
> Before the mdev enhancement type1 iommu used capable() to test the
> capability of current task; in the course of mdev development a
> new requirement, testing for another task other than current, was
> raised.  ns_capable() was used for this purpose, however it still
> tests current, the only difference is, in a specified namespace.
> 
> Fix it by using has_capability() instead, which tests the cap for
> specified task in init_user_ns, the same namespace as capable().
> 
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Cc: Kirti Wankhede <kwankhede@nvidia.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Jike Song <jike.song@intel.com>
> ---
>  drivers/vfio/vfio_iommu_type1.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index f3726ba..b54aedf 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -394,8 +394,7 @@ static long vfio_pin_pages_remote(struct vfio_dma *dma, unsigned long vaddr,
>  				  long npage, unsigned long *pfn_base)
>  {
>  	unsigned long limit;
> -	bool lock_cap = ns_capable(task_active_pid_ns(dma->task)->user_ns,
> -				   CAP_IPC_LOCK);
> +	bool lock_cap = has_capability(dma->task, CAP_IPC_LOCK);


Hi Jike,

Alex's patch already changes this to capable(), you need to resolve.
https://lkml.org/lkml/2016/12/20/490

You need to do only below change, which looks fine to me.

>  	struct mm_struct *mm;
>  	long ret, i = 0, lock_acct = 0;
>  	bool rsvd;
> @@ -491,8 +490,7 @@ static int vfio_pin_page_external(struct vfio_dma *dma, unsigned long vaddr,
>  				  unsigned long *pfn_base, bool do_accounting)
>  {
>  	unsigned long limit;
> -	bool lock_cap = ns_capable(task_active_pid_ns(dma->task)->user_ns,
> -				   CAP_IPC_LOCK);
> +	bool lock_cap = has_capability(dma->task, CAP_IPC_LOCK);
>  	struct mm_struct *mm;
>  	int ret;
>  	bool rsvd;
> 


Thanks,
Kirti

Re: [PATCH 2/2] vfio iommu type1: fix the testing of capability for remote task

[Jike Song]

On 12/22/2016 08:20 PM, Kirti Wankhede wrote:
> On 12/21/2016 9:40 PM, Jike Song wrote:
>> Before the mdev enhancement type1 iommu used capable() to test the
>> capability of current task; in the course of mdev development a
>> new requirement, testing for another task other than current, was
>> raised.  ns_capable() was used for this purpose, however it still
>> tests current, the only difference is, in a specified namespace.
>>
>> Fix it by using has_capability() instead, which tests the cap for
>> specified task in init_user_ns, the same namespace as capable().
>>
>> Cc: Alex Williamson <alex.williamson@redhat.com>
>> Cc: Kirti Wankhede <kwankhede@nvidia.com>
>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>> Signed-off-by: Jike Song <jike.song@intel.com>
>> ---
>>  drivers/vfio/vfio_iommu_type1.c | 6 ++----
>>  1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
>> index f3726ba..b54aedf 100644
>> --- a/drivers/vfio/vfio_iommu_type1.c
>> +++ b/drivers/vfio/vfio_iommu_type1.c
>> @@ -394,8 +394,7 @@ static long vfio_pin_pages_remote(struct vfio_dma *dma, unsigned long vaddr,
>>  				  long npage, unsigned long *pfn_base)
>>  {
>>  	unsigned long limit;
>> -	bool lock_cap = ns_capable(task_active_pid_ns(dma->task)->user_ns,
>> -				   CAP_IPC_LOCK);
>> +	bool lock_cap = has_capability(dma->task, CAP_IPC_LOCK);
> 
> 
> Hi Jike,
> 
> Alex's patch already changes this to capable(), you need to resolve.
> https://lkml.org/lkml/2016/12/20/490
> 
> You need to do only below change, which looks fine to me.
> 

Thanks for the point, will change it in v2.  However, that will probably be
after patch 1/2 accepted, otherwise we get undefined symbols.

--
Thanks,
Jike

>>  	struct mm_struct *mm;
>>  	long ret, i = 0, lock_acct = 0;
>>  	bool rsvd;
>> @@ -491,8 +490,7 @@ static int vfio_pin_page_external(struct vfio_dma *dma, unsigned long vaddr,
>>  				  unsigned long *pfn_base, bool do_accounting)
>>  {
>>  	unsigned long limit;
>> -	bool lock_cap = ns_capable(task_active_pid_ns(dma->task)->user_ns,
>> -				   CAP_IPC_LOCK);
>> +	bool lock_cap = has_capability(dma->task, CAP_IPC_LOCK);
>>  	struct mm_struct *mm;
>>  	int ret;
>>  	bool rsvd;
>>

Re: [PATCH 1/2] capability: export has_capability

[Alex Williamson]

On Thu, 22 Dec 2016 00:10:15 +0800
Jike Song <jike.song@intel.com> wrote:

> has_capability() is sometimes needed by modules to test capability
> for specified task other than current, so export it.
> 
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Cc: Kirti Wankhede <kwankhede@nvidia.com>
> Signed-off-by: Jike Song <jike.song@intel.com>
> ---
>  kernel/capability.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/kernel/capability.c b/kernel/capability.c
> index 4984e1f..e2e198c 100644
> --- a/kernel/capability.c
> +++ b/kernel/capability.c
> @@ -318,6 +318,7 @@ bool has_capability(struct task_struct *t, int cap)
>  {
>  	return has_ns_capability(t, &init_user_ns, cap);
>  }
> +EXPORT_SYMBOL(has_capability);
>  
>  /**
>   * has_ns_capability_noaudit - Does a task have a capability (unaudited)

Are we using EXPORT_SYMBOL vs EXPORT_SYMBOL_GPL here to match the other
exports in this file?  We could use _GPL to match the expected caller
of this.


Serge,

Do you have any comments on this patch?  I'd be happy to pull it
through the vfio tree with an appropriate Ack.  Thanks,

Alex

Re: [PATCH 1/2] capability: export has_capability

[Jike Song]

On 01/12/2017 02:47 AM, Alex Williamson wrote:
> On Thu, 22 Dec 2016 00:10:15 +0800
> Jike Song <jike.song@intel.com> wrote:
> 
>> has_capability() is sometimes needed by modules to test capability
>> for specified task other than current, so export it.
>>
>> Cc: Alex Williamson <alex.williamson@redhat.com>
>> Cc: Kirti Wankhede <kwankhede@nvidia.com>
>> Signed-off-by: Jike Song <jike.song@intel.com>
>> ---
>>  kernel/capability.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/kernel/capability.c b/kernel/capability.c
>> index 4984e1f..e2e198c 100644
>> --- a/kernel/capability.c
>> +++ b/kernel/capability.c
>> @@ -318,6 +318,7 @@ bool has_capability(struct task_struct *t, int cap)
>>  {
>>  	return has_ns_capability(t, &init_user_ns, cap);
>>  }
>> +EXPORT_SYMBOL(has_capability);
>>  
>>  /**
>>   * has_ns_capability_noaudit - Does a task have a capability (unaudited)
> 
> Are we using EXPORT_SYMBOL vs EXPORT_SYMBOL_GPL here to match the other
> exports in this file?  We could use _GPL to match the expected caller
> of this.
> 

Yes, I chose EXPORT_SYMBOL to match the existing exports in capability.c.
Either is good to me, of course :)

> 
> Serge,
> 
> Do you have any comments on this patch?  I'd be happy to pull it
> through the vfio tree with an appropriate Ack.  Thanks,

Guess Serge still on Xmas vocation? :)

--
Thanks,
Jike

Re: [PATCH 1/2] capability: export has_capability

[Serge E. Hallyn]

On Wed, Jan 11, 2017 at 01:47:01PM -0500, Alex Williamson wrote:
> On Thu, 22 Dec 2016 00:10:15 +0800
> Jike Song <jike.song@intel.com> wrote:
> 
> > has_capability() is sometimes needed by modules to test capability
> > for specified task other than current, so export it.
> > 
> > Cc: Alex Williamson <alex.williamson@redhat.com>
> > Cc: Kirti Wankhede <kwankhede@nvidia.com>
> > Signed-off-by: Jike Song <jike.song@intel.com>
> > ---
> >  kernel/capability.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/kernel/capability.c b/kernel/capability.c
> > index 4984e1f..e2e198c 100644
> > --- a/kernel/capability.c
> > +++ b/kernel/capability.c
> > @@ -318,6 +318,7 @@ bool has_capability(struct task_struct *t, int cap)
> >  {
> >  	return has_ns_capability(t, &init_user_ns, cap);
> >  }
> > +EXPORT_SYMBOL(has_capability);
> >  
> >  /**
> >   * has_ns_capability_noaudit - Does a task have a capability (unaudited)
> 
> Are we using EXPORT_SYMBOL vs EXPORT_SYMBOL_GPL here to match the other
> exports in this file?  We could use _GPL to match the expected caller
> of this.
> 
> 
> Serge,
> 
> Do you have any comments on this patch?  I'd be happy to pull it
> through the vfio tree with an appropriate Ack.  Thanks,

Sure, thanks, looks good to me.

Acked-by: Serge Hallyn <serge@hallyn.com>