* [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 related [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 related [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).