[v2,2/4] usertools: add option to change mount point owner
Checks
Commit Message
Per mount(8), the previous owner and mode of the mount point
become invisible as long as this filesystem remains mounted.
Because dpdk-hugepages.py must be run as root,
the new owner would be root.
This is undesirable if the hugepage directory is being set up
by the administrator for an unprivileged user.
HugeTLB filesystem has options to set the mount point owner.
Add --owner/-o option to apply this option when mounting.
The benefit of performing this in dpdk-hugepages.py
is that the user does not need to care about this detail
of mount command operation.
Signed-off-by: Dmitry Kozlyuk <dkozlyuk@nvidia.com>
---
usertools/dpdk-hugepages.py | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
Comments
On Fri, Jun 17, 2022 at 02:25:06PM +0300, Dmitry Kozlyuk wrote:
> Per mount(8), the previous owner and mode of the mount point
> become invisible as long as this filesystem remains mounted.
> Because dpdk-hugepages.py must be run as root,
> the new owner would be root.
> This is undesirable if the hugepage directory is being set up
> by the administrator for an unprivileged user.
> HugeTLB filesystem has options to set the mount point owner.
> Add --owner/-o option to apply this option when mounting.
> The benefit of performing this in dpdk-hugepages.py
> is that the user does not need to care about this detail
> of mount command operation.
>
> Signed-off-by: Dmitry Kozlyuk <dkozlyuk@nvidia.com>
> ---
> usertools/dpdk-hugepages.py | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/usertools/dpdk-hugepages.py b/usertools/dpdk-hugepages.py
> index 8bab086a2f..5120518bcb 100755
> --- a/usertools/dpdk-hugepages.py
> +++ b/usertools/dpdk-hugepages.py
> @@ -170,7 +170,7 @@ def get_mountpoints():
> return mounted
>
>
> -def mount_huge(pagesize, mountpoint):
> +def mount_huge(pagesize, mountpoint, owner):
> '''Mount the huge TLB file system'''
> if mountpoint in get_mountpoints():
> print(mountpoint, "already mounted")
> @@ -178,6 +178,9 @@ def mount_huge(pagesize, mountpoint):
> cmd = "mount -t hugetlbfs"
> if pagesize:
> cmd += ' -o pagesize={}'.format(pagesize * 1024)
> + if owner:
> + uid, gid = owner.split(':', maxsplit=1)
> + cmd += ' -o uid={},gid={}'.format(uid, gid)
I'm not sure about forcing the user to always provide a "user:group" format
parameter. How about:
1. checking initially for the presence of a ":" and if not present just
working with the uid parameter?
2. alternatively, what about adding in separate parameters for user or
group, so they can be specified independently?
/Bruce
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Friday, June 17, 2022 6:53 PM
> [...]
> > + if owner:
> > + uid, gid = owner.split(':', maxsplit=1)
> > + cmd += ' -o uid={},gid={}'.format(uid, gid)
>
> I'm not sure about forcing the user to always provide a "user:group"
> format parameter. How about:
>
> 1. checking initially for the presence of a ":" and if not present
> just working with the uid parameter?
> 2. alternatively, what about adding in separate parameters for user or
> group, so they can be specified independently?
Explicit is better than implicit, especially in security configuration.
If you insist on changing, I prefer option 2
to clearly indicate what is configured and what is not.
@@ -170,7 +170,7 @@ def get_mountpoints():
return mounted
-def mount_huge(pagesize, mountpoint):
+def mount_huge(pagesize, mountpoint, owner):
'''Mount the huge TLB file system'''
if mountpoint in get_mountpoints():
print(mountpoint, "already mounted")
@@ -178,6 +178,9 @@ def mount_huge(pagesize, mountpoint):
cmd = "mount -t hugetlbfs"
if pagesize:
cmd += ' -o pagesize={}'.format(pagesize * 1024)
+ if owner:
+ uid, gid = owner.split(':', maxsplit=1)
+ cmd += ' -o uid={},gid={}'.format(uid, gid)
cmd += ' nodev ' + mountpoint
os.system(cmd)
@@ -234,6 +237,11 @@ def main():
metavar='DIR',
default=HUGE_MOUNT,
help='mount point')
+ parser.add_argument(
+ '--owner',
+ '-o',
+ metavar='USER:GROUP',
+ help='change the mounted directory owner')
parser.add_argument(
'--node', '-n', help='select numa node to reserve pages on')
parser.add_argument(
@@ -279,7 +287,7 @@ def main():
reserve_pages(
int(reserve_kb / pagesize_kb), pagesize_kb, node=args.node)
if args.mount:
- mount_huge(pagesize_kb, args.directory)
+ mount_huge(pagesize_kb, args.directory, args.owner)
if args.show:
show_pages()
print()