| Message ID | 7-v5-4001c2997bd0+30c-iommufd_jgg@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | IOMMUFD Generic interface | expand |
On 11/16/22 22:00, Jason Gunthorpe wrote: > Following the pattern of io_uring, perf, skb, and bpf iommfd will use > user->locked_vm for accounting pinned pages. Ensure the value is included > in the struct and export free_uid() as iommufd is modular. > > user->locked_vm is the good accounting to use for ulimit because it is > per-user, and the security sandboxing of locked pages is not supposed to > be per-process. Other places (vfio, vdpa and infiniband) have used > mm->pinned_vm and/or mm->locked_vm for accounting pinned pages, but this > is only per-process and inconsistent with the new FOLL_LONGTERM users in > the kernel. > > Concurrent work is underway to try to put this in a cgroup, so everything > can be consistent and the kernel can provide a FOLL_LONGTERM limit that > actually provides security. > > Tested-by: Nicolin Chen <nicolinc@nvidia.com> > Tested-by: Yi Liu <yi.l.liu@intel.com> > Tested-by: Lixiao Yang <lixiao.yang@intel.com> > Tested-by: Matthew Rosato <mjrosato@linux.ibm.com> > Reviewed-by: Kevin Tian <kevin.tian@intel.com> > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> Reviewed-by: Eric Auger <eric.auger@redhat.com> Eric > --- > include/linux/sched/user.h | 2 +- > kernel/user.c | 1 + > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/include/linux/sched/user.h b/include/linux/sched/user.h > index f054d0360a7533..4cc52698e214e2 100644 > --- a/include/linux/sched/user.h > +++ b/include/linux/sched/user.h > @@ -25,7 +25,7 @@ struct user_struct { > > #if defined(CONFIG_PERF_EVENTS) || defined(CONFIG_BPF_SYSCALL) || \ > defined(CONFIG_NET) || defined(CONFIG_IO_URING) || \ > - defined(CONFIG_VFIO_PCI_ZDEV_KVM) > + defined(CONFIG_VFIO_PCI_ZDEV_KVM) || IS_ENABLED(CONFIG_IOMMUFD) > atomic_long_t locked_vm; > #endif > #ifdef CONFIG_WATCH_QUEUE > diff --git a/kernel/user.c b/kernel/user.c > index e2cf8c22b539a7..d667debeafd609 100644 > --- a/kernel/user.c > +++ b/kernel/user.c > @@ -185,6 +185,7 @@ void free_uid(struct user_struct *up) > if (refcount_dec_and_lock_irqsave(&up->__count, &uidhash_lock, &flags)) > free_user(up, flags); > } > +EXPORT_SYMBOL_GPL(free_uid); > > struct user_struct *alloc_uid(kuid_t uid) > {
Hi Jason, On 11/16/22 22:00, Jason Gunthorpe wrote: > Following the pattern of io_uring, perf, skb, and bpf iommfd will use > user->locked_vm for accounting pinned pages. Ensure the value is included > in the struct and export free_uid() as iommufd is modular. > > user->locked_vm is the good accounting to use for ulimit because it is > per-user, and the security sandboxing of locked pages is not supposed to > be per-process. Other places (vfio, vdpa and infiniband) have used > mm->pinned_vm and/or mm->locked_vm for accounting pinned pages, but this > is only per-process and inconsistent with the new FOLL_LONGTERM users in > the kernel. > > Concurrent work is underway to try to put this in a cgroup, so everything > can be consistent and the kernel can provide a FOLL_LONGTERM limit that > actually provides security. > > Tested-by: Nicolin Chen <nicolinc@nvidia.com> > Tested-by: Yi Liu <yi.l.liu@intel.com> > Tested-by: Lixiao Yang <lixiao.yang@intel.com> > Tested-by: Matthew Rosato <mjrosato@linux.ibm.com> > Reviewed-by: Kevin Tian <kevin.tian@intel.com> > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> Reviewed-by: Eric Auger <eric.auger@redhat.com> Eric > --- > include/linux/sched/user.h | 2 +- > kernel/user.c | 1 + > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/include/linux/sched/user.h b/include/linux/sched/user.h > index f054d0360a7533..4cc52698e214e2 100644 > --- a/include/linux/sched/user.h > +++ b/include/linux/sched/user.h > @@ -25,7 +25,7 @@ struct user_struct { > > #if defined(CONFIG_PERF_EVENTS) || defined(CONFIG_BPF_SYSCALL) || \ > defined(CONFIG_NET) || defined(CONFIG_IO_URING) || \ > - defined(CONFIG_VFIO_PCI_ZDEV_KVM) > + defined(CONFIG_VFIO_PCI_ZDEV_KVM) || IS_ENABLED(CONFIG_IOMMUFD) > atomic_long_t locked_vm; > #endif > #ifdef CONFIG_WATCH_QUEUE > diff --git a/kernel/user.c b/kernel/user.c > index e2cf8c22b539a7..d667debeafd609 100644 > --- a/kernel/user.c > +++ b/kernel/user.c > @@ -185,6 +185,7 @@ void free_uid(struct user_struct *up) > if (refcount_dec_and_lock_irqsave(&up->__count, &uidhash_lock, &flags)) > free_user(up, flags); > } > +EXPORT_SYMBOL_GPL(free_uid); > > struct user_struct *alloc_uid(kuid_t uid) > {
On 11/16/22 22:00, Jason Gunthorpe wrote: > Following the pattern of io_uring, perf, skb, and bpf iommfd will use > user->locked_vm for accounting pinned pages. Ensure the value is included > in the struct and export free_uid() as iommufd is modular. nit: s/bpf/bpf, in case you have to respinned > > user->locked_vm is the good accounting to use for ulimit because it is > per-user, and the security sandboxing of locked pages is not supposed to > be per-process. Other places (vfio, vdpa and infiniband) have used > mm->pinned_vm and/or mm->locked_vm for accounting pinned pages, but this > is only per-process and inconsistent with the new FOLL_LONGTERM users in > the kernel. > > Concurrent work is underway to try to put this in a cgroup, so everything > can be consistent and the kernel can provide a FOLL_LONGTERM limit that > actually provides security. > > Tested-by: Nicolin Chen <nicolinc@nvidia.com> > Tested-by: Yi Liu <yi.l.liu@intel.com> > Tested-by: Lixiao Yang <lixiao.yang@intel.com> > Tested-by: Matthew Rosato <mjrosato@linux.ibm.com> > Reviewed-by: Kevin Tian <kevin.tian@intel.com> > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> > --- > include/linux/sched/user.h | 2 +- > kernel/user.c | 1 + > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/include/linux/sched/user.h b/include/linux/sched/user.h > index f054d0360a7533..4cc52698e214e2 100644 > --- a/include/linux/sched/user.h > +++ b/include/linux/sched/user.h > @@ -25,7 +25,7 @@ struct user_struct { > > #if defined(CONFIG_PERF_EVENTS) || defined(CONFIG_BPF_SYSCALL) || \ > defined(CONFIG_NET) || defined(CONFIG_IO_URING) || \ > - defined(CONFIG_VFIO_PCI_ZDEV_KVM) > + defined(CONFIG_VFIO_PCI_ZDEV_KVM) || IS_ENABLED(CONFIG_IOMMUFD) > atomic_long_t locked_vm; > #endif > #ifdef CONFIG_WATCH_QUEUE > diff --git a/kernel/user.c b/kernel/user.c > index e2cf8c22b539a7..d667debeafd609 100644 > --- a/kernel/user.c > +++ b/kernel/user.c > @@ -185,6 +185,7 @@ void free_uid(struct user_struct *up) > if (refcount_dec_and_lock_irqsave(&up->__count, &uidhash_lock, &flags)) > free_user(up, flags); > } > +EXPORT_SYMBOL_GPL(free_uid); > > struct user_struct *alloc_uid(kuid_t uid) > {
On Fri, Nov 18, 2022 at 05:28:19PM +0100, Eric Auger wrote: > > > On 11/16/22 22:00, Jason Gunthorpe wrote: > > Following the pattern of io_uring, perf, skb, and bpf iommfd will use > > user->locked_vm for accounting pinned pages. Ensure the value is included > > in the struct and export free_uid() as iommufd is modular. > nit: s/bpf/bpf, > in case you have to respinned Ok Jason
diff --git a/include/linux/sched/user.h b/include/linux/sched/user.h index f054d0360a7533..4cc52698e214e2 100644 --- a/include/linux/sched/user.h +++ b/include/linux/sched/user.h @@ -25,7 +25,7 @@ struct user_struct { #if defined(CONFIG_PERF_EVENTS) || defined(CONFIG_BPF_SYSCALL) || \ defined(CONFIG_NET) || defined(CONFIG_IO_URING) || \ - defined(CONFIG_VFIO_PCI_ZDEV_KVM) + defined(CONFIG_VFIO_PCI_ZDEV_KVM) || IS_ENABLED(CONFIG_IOMMUFD) atomic_long_t locked_vm; #endif #ifdef CONFIG_WATCH_QUEUE diff --git a/kernel/user.c b/kernel/user.c index e2cf8c22b539a7..d667debeafd609 100644 --- a/kernel/user.c +++ b/kernel/user.c @@ -185,6 +185,7 @@ void free_uid(struct user_struct *up) if (refcount_dec_and_lock_irqsave(&up->__count, &uidhash_lock, &flags)) free_user(up, flags); } +EXPORT_SYMBOL_GPL(free_uid); struct user_struct *alloc_uid(kuid_t uid) {