linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] test capability for remote task
@ 2016-12-21 16:10 Jike Song
  2016-12-21 16:10 ` [PATCH 1/2] capability: export has_capability Jike Song
  2016-12-21 16:10 ` [PATCH 2/2] vfio iommu type1: fix the testing of capability for remote task Jike Song
  0 siblings, 2 replies; 8+ messages in thread
From: Jike Song @ 2016-12-21 16:10 UTC (permalink / raw)
  To: serge, alex.williamson
  Cc: linux-security-module, kvm, linux-kernel, kwankhede, kraxel, 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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/2] capability: export has_capability
  2016-12-21 16:10 [PATCH 0/2] test capability for remote task Jike Song
@ 2016-12-21 16:10 ` Jike Song
  2017-01-11 18:47   ` Alex Williamson
  2016-12-21 16:10 ` [PATCH 2/2] vfio iommu type1: fix the testing of capability for remote task Jike Song
  1 sibling, 1 reply; 8+ messages in thread
From: Jike Song @ 2016-12-21 16:10 UTC (permalink / raw)
  To: serge, alex.williamson
  Cc: linux-security-module, kvm, linux-kernel, kwankhede, kraxel, 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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 2/2] vfio iommu type1: fix the testing of capability for remote task
  2016-12-21 16:10 [PATCH 0/2] test capability for remote task Jike Song
  2016-12-21 16:10 ` [PATCH 1/2] capability: export has_capability Jike Song
@ 2016-12-21 16:10 ` Jike Song
  2016-12-22 12:20   ` Kirti Wankhede
  1 sibling, 1 reply; 8+ messages in thread
From: Jike Song @ 2016-12-21 16:10 UTC (permalink / raw)
  To: serge, alex.williamson
  Cc: linux-security-module, kvm, linux-kernel, kwankhede, kraxel, 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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] vfio iommu type1: fix the testing of capability for remote task
  2016-12-21 16:10 ` [PATCH 2/2] vfio iommu type1: fix the testing of capability for remote task Jike Song
@ 2016-12-22 12:20   ` Kirti Wankhede
  2016-12-23  2:36     ` Jike Song
  0 siblings, 1 reply; 8+ messages in thread
From: Kirti Wankhede @ 2016-12-22 12:20 UTC (permalink / raw)
  To: Jike Song, serge, alex.williamson
  Cc: linux-security-module, kvm, linux-kernel, kraxel



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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] vfio iommu type1: fix the testing of capability for remote task
  2016-12-22 12:20   ` Kirti Wankhede
@ 2016-12-23  2:36     ` Jike Song
  0 siblings, 0 replies; 8+ messages in thread
From: Jike Song @ 2016-12-23  2:36 UTC (permalink / raw)
  To: Kirti Wankhede
  Cc: serge, alex.williamson, linux-security-module, kvm, linux-kernel, kraxel

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;
>>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] capability: export has_capability
  2016-12-21 16:10 ` [PATCH 1/2] capability: export has_capability Jike Song
@ 2017-01-11 18:47   ` Alex Williamson
  2017-01-12  0:56     ` Jike Song
  2017-01-12  6:45     ` Serge E. Hallyn
  0 siblings, 2 replies; 8+ messages in thread
From: Alex Williamson @ 2017-01-11 18:47 UTC (permalink / raw)
  To: Jike Song
  Cc: serge, linux-security-module, kvm, linux-kernel, kwankhede, kraxel

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] capability: export has_capability
  2017-01-11 18:47   ` Alex Williamson
@ 2017-01-12  0:56     ` Jike Song
  2017-01-12  6:45     ` Serge E. Hallyn
  1 sibling, 0 replies; 8+ messages in thread
From: Jike Song @ 2017-01-12  0:56 UTC (permalink / raw)
  To: Alex Williamson
  Cc: serge, linux-security-module, kvm, linux-kernel, kwankhede, kraxel

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] capability: export has_capability
  2017-01-11 18:47   ` Alex Williamson
  2017-01-12  0:56     ` Jike Song
@ 2017-01-12  6:45     ` Serge E. Hallyn
  1 sibling, 0 replies; 8+ messages in thread
From: Serge E. Hallyn @ 2017-01-12  6:45 UTC (permalink / raw)
  To: Alex Williamson
  Cc: Jike Song, serge, linux-security-module, kvm, linux-kernel,
	kwankhede, kraxel

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>

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-01-12  6:45 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-21 16:10 [PATCH 0/2] test capability for remote task Jike Song
2016-12-21 16:10 ` [PATCH 1/2] capability: export has_capability Jike Song
2017-01-11 18:47   ` Alex Williamson
2017-01-12  0:56     ` Jike Song
2017-01-12  6:45     ` Serge E. Hallyn
2016-12-21 16:10 ` [PATCH 2/2] vfio iommu type1: fix the testing of capability for remote task Jike Song
2016-12-22 12:20   ` Kirti Wankhede
2016-12-23  2:36     ` Jike Song

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).