| Message ID | 20230403084759.884681-1-cem@kernel.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | shmem: Add user and group quota support for tmpfs | expand |
On Mon, Apr 03, 2023 at 10:47:53AM +0200, cem@kernel.org wrote: > From: Carlos Maiolino <cmaiolino@redhat.com> > > Hi folks. this work has been done originally by Lukas, but he left the company, > so I'm taking over his work from where he left it of. This series is virtually > done, and he had updated it with comments from the last version, but, I'm I've commented on the last version: https://lore.kernel.org/linux-fsdevel/20221129112133.rrpoywlwdw45k3qa@wittgenstein trying to point out that tmpfs can be mounted in user namespaces. Which means that the quota uids and gids need to take the idmapping of the user namespace in which the tmpfs instances is mounted in into account; not the one on the host. See the link above for some details. Before we can merge this it would be very good if we could get tests that verify tmpfs being mounted inside a userns with quotas enabled because I don't think this is covered yet by xfstests. Or you punt on it for now and restricted quotas to tmpfs instances mounted on the host. > initially posting it as a RFC because it's been a while since he posted the > last version. > Most of what I did here was rebase his last work on top of current Linus's tree. > > Honza, there is one patch from you in this series, which I believe you had it > suggested to Lukas on a previous version. > > The original cover-letter follows... > > people have been asking for quota support in tmpfs many times in the past > mostly to avoid one malicious user, or misbehaving user/program to consume > all of the system memory. This has been partially solved with the size > mount option, but some problems still prevail. > > One of the problems is the fact that /dev/shm is still generally unprotected > with this and another is administration overhead of managing multiple tmpfs > mounts and lack of more fine grained control. > > Quota support can solve all these problems in a somewhat standard way > people are already familiar with from regular file systems. It can give us > more fine grained control over how much memory user/groups can consume. > Additionally it can also control number of inodes and with special quota > mount options introduced with a second patch we can set global limits > allowing us to replace the size mount option with quota entirely. > > Currently the standard userspace quota tools (quota, xfs_quota) are only > using quotactl ioctl which is expecting a block device. I patched quota [1] > and xfs_quota [2] to use quotactl_fd in case we want to run the tools on > mount point directory to work nicely with tmpfs. > > The implementation was tested on patched version of xfstests [3]. > > > Jan Kara (1): > quota: Check presence of quota operation structures instead of > ->quota_read and ->quota_write callbacks > > Lukas Czerner (5): > shmem: make shmem_inode_acct_block() return error > shmem: make shmem_get_inode() return ERR_PTR instead of NULL > shmem: prepare shmem quota infrastructure > shmem: quota support > Add default quota limit mount options > > Documentation/filesystems/tmpfs.rst | 28 ++ > fs/Kconfig | 12 + > fs/quota/dquot.c | 2 +- > include/linux/shmem_fs.h | 25 ++ > include/uapi/linux/quota.h | 1 + > mm/Makefile | 2 +- > mm/shmem.c | 452 +++++++++++++++++++++------- > mm/shmem_quota.c | 327 ++++++++++++++++++++ > 8 files changed, 740 insertions(+), 109 deletions(-) > create mode 100644 mm/shmem_quota.c > > -- > 2.30.2 >
Hi Christian. On Wed, Apr 05, 2023 at 10:52:44AM +0200, Christian Brauner wrote: > On Mon, Apr 03, 2023 at 10:47:53AM +0200, cem@kernel.org wrote: > > From: Carlos Maiolino <cmaiolino@redhat.com> > > > > Hi folks. this work has been done originally by Lukas, but he left the company, > > so I'm taking over his work from where he left it of. This series is virtually > > done, and he had updated it with comments from the last version, but, I'm > > I've commented on the last version: > > https://lore.kernel.org/linux-fsdevel/20221129112133.rrpoywlwdw45k3qa@wittgenstein > > trying to point out that tmpfs can be mounted in user namespaces. Which > means that the quota uids and gids need to take the idmapping of the > user namespace in which the tmpfs instances is mounted in into account; > not the one on the host. > > See the link above for some details. Before we can merge this it would > be very good if we could get tests that verify tmpfs being mounted > inside a userns with quotas enabled because I don't think this is > covered yet by xfstests. Or you punt on it for now and restricted quotas > to tmpfs instances mounted on the host. > Thanks for the link, I've read it before, and this is by now a limitation I'd like to keep in this series. I can extend it to be namespace aware later on, but the current goal of this series is to be able tmpfs mounts on the host to limit the amount of memory consumed by users. Being namespace aware is something I plan to work later. Because as you said, it needs more testing coverage, which will only delay the main goal of this series, which again, is to avoid users to consume all memory in the host itself. > > initially posting it as a RFC because it's been a while since he posted the > > last version. > > Most of what I did here was rebase his last work on top of current Linus's tree. > > > > Honza, there is one patch from you in this series, which I believe you had it > > suggested to Lukas on a previous version. > > > > The original cover-letter follows... > > > > people have been asking for quota support in tmpfs many times in the past > > mostly to avoid one malicious user, or misbehaving user/program to consume > > all of the system memory. This has been partially solved with the size > > mount option, but some problems still prevail. > > > > One of the problems is the fact that /dev/shm is still generally unprotected > > with this and another is administration overhead of managing multiple tmpfs > > mounts and lack of more fine grained control. > > > > Quota support can solve all these problems in a somewhat standard way > > people are already familiar with from regular file systems. It can give us > > more fine grained control over how much memory user/groups can consume. > > Additionally it can also control number of inodes and with special quota > > mount options introduced with a second patch we can set global limits > > allowing us to replace the size mount option with quota entirely. > > > > Currently the standard userspace quota tools (quota, xfs_quota) are only > > using quotactl ioctl which is expecting a block device. I patched quota [1] > > and xfs_quota [2] to use quotactl_fd in case we want to run the tools on > > mount point directory to work nicely with tmpfs. > > > > The implementation was tested on patched version of xfstests [3]. > > > > > > Jan Kara (1): > > quota: Check presence of quota operation structures instead of > > ->quota_read and ->quota_write callbacks > > > > Lukas Czerner (5): > > shmem: make shmem_inode_acct_block() return error > > shmem: make shmem_get_inode() return ERR_PTR instead of NULL > > shmem: prepare shmem quota infrastructure > > shmem: quota support > > Add default quota limit mount options > > > > Documentation/filesystems/tmpfs.rst | 28 ++ > > fs/Kconfig | 12 + > > fs/quota/dquot.c | 2 +- > > include/linux/shmem_fs.h | 25 ++ > > include/uapi/linux/quota.h | 1 + > > mm/Makefile | 2 +- > > mm/shmem.c | 452 +++++++++++++++++++++------- > > mm/shmem_quota.c | 327 ++++++++++++++++++++ > > 8 files changed, 740 insertions(+), 109 deletions(-) > > create mode 100644 mm/shmem_quota.c > > > > -- > > 2.30.2 > >
On Wed, Apr 05, 2023 at 12:44:27PM +0200, Carlos Maiolino wrote: > Hi Christian. > > On Wed, Apr 05, 2023 at 10:52:44AM +0200, Christian Brauner wrote: > > On Mon, Apr 03, 2023 at 10:47:53AM +0200, cem@kernel.org wrote: > > > From: Carlos Maiolino <cmaiolino@redhat.com> > > > > > > Hi folks. this work has been done originally by Lukas, but he left the company, > > > so I'm taking over his work from where he left it of. This series is virtually > > > done, and he had updated it with comments from the last version, but, I'm > > > > I've commented on the last version: > > > > https://lore.kernel.org/linux-fsdevel/20221129112133.rrpoywlwdw45k3qa@wittgenstein > > > > trying to point out that tmpfs can be mounted in user namespaces. Which > > means that the quota uids and gids need to take the idmapping of the > > user namespace in which the tmpfs instances is mounted in into account; > > not the one on the host. > > > > See the link above for some details. Before we can merge this it would > > be very good if we could get tests that verify tmpfs being mounted > > inside a userns with quotas enabled because I don't think this is > > covered yet by xfstests. Or you punt on it for now and restricted quotas > > to tmpfs instances mounted on the host. > > > > Thanks for the link, I've read it before, and this is by now a limitation I'd > like to keep in this series. I can extend it to be namespace aware later on, but > the current goal of this series is to be able tmpfs mounts on the host to limit > the amount of memory consumed by users. Being namespace aware is something I This is fine with me. But please point the restriction out in the documentation and in the commit message. This is especially important because the check is hidden in the bowls of dquot_load_quota_sb(). Ideally we'd probably check for fc->user_ns == &init_user_ns directly when parsing the quota mount options instead of waiting until fill_super.
On Wed, Apr 05, 2023 at 03:11:22PM +0200, Christian Brauner wrote: > On Wed, Apr 05, 2023 at 12:44:27PM +0200, Carlos Maiolino wrote: > > Hi Christian. > > > > On Wed, Apr 05, 2023 at 10:52:44AM +0200, Christian Brauner wrote: > > > On Mon, Apr 03, 2023 at 10:47:53AM +0200, cem@kernel.org wrote: > > > > From: Carlos Maiolino <cmaiolino@redhat.com> > > > > > > > > Hi folks. this work has been done originally by Lukas, but he left the company, > > > > so I'm taking over his work from where he left it of. This series is virtually > > > > done, and he had updated it with comments from the last version, but, I'm > > > > > > I've commented on the last version: > > > > > > https://lore.kernel.org/linux-fsdevel/20221129112133.rrpoywlwdw45k3qa@wittgenstein > > > > > > trying to point out that tmpfs can be mounted in user namespaces. Which > > > means that the quota uids and gids need to take the idmapping of the > > > user namespace in which the tmpfs instances is mounted in into account; > > > not the one on the host. > > > > > > See the link above for some details. Before we can merge this it would > > > be very good if we could get tests that verify tmpfs being mounted > > > inside a userns with quotas enabled because I don't think this is > > > covered yet by xfstests. Or you punt on it for now and restricted quotas > > > to tmpfs instances mounted on the host. > > > > > > > Thanks for the link, I've read it before, and this is by now a limitation I'd > > like to keep in this series. I can extend it to be namespace aware later on, but > > the current goal of this series is to be able tmpfs mounts on the host to limit > > the amount of memory consumed by users. Being namespace aware is something I > > This is fine with me. But please point the restriction out in the > documentation and in the commit message. This is especially important > because the check is hidden in the bowls of dquot_load_quota_sb(). Sounds reasonable, I'll work on the comments I received and re-send this series next week if nothing urgent comes up. > > Ideally we'd probably check for fc->user_ns == &init_user_ns directly > when parsing the quota mount options instead of waiting until > fill_super.
From: Carlos Maiolino <cmaiolino@redhat.com> Hi folks. this work has been done originally by Lukas, but he left the company, so I'm taking over his work from where he left it of. This series is virtually done, and he had updated it with comments from the last version, but, I'm initially posting it as a RFC because it's been a while since he posted the last version. Most of what I did here was rebase his last work on top of current Linus's tree. Honza, there is one patch from you in this series, which I believe you had it suggested to Lukas on a previous version. The original cover-letter follows... people have been asking for quota support in tmpfs many times in the past mostly to avoid one malicious user, or misbehaving user/program to consume all of the system memory. This has been partially solved with the size mount option, but some problems still prevail. One of the problems is the fact that /dev/shm is still generally unprotected with this and another is administration overhead of managing multiple tmpfs mounts and lack of more fine grained control. Quota support can solve all these problems in a somewhat standard way people are already familiar with from regular file systems. It can give us more fine grained control over how much memory user/groups can consume. Additionally it can also control number of inodes and with special quota mount options introduced with a second patch we can set global limits allowing us to replace the size mount option with quota entirely. Currently the standard userspace quota tools (quota, xfs_quota) are only using quotactl ioctl which is expecting a block device. I patched quota [1] and xfs_quota [2] to use quotactl_fd in case we want to run the tools on mount point directory to work nicely with tmpfs. The implementation was tested on patched version of xfstests [3]. Jan Kara (1): quota: Check presence of quota operation structures instead of ->quota_read and ->quota_write callbacks Lukas Czerner (5): shmem: make shmem_inode_acct_block() return error shmem: make shmem_get_inode() return ERR_PTR instead of NULL shmem: prepare shmem quota infrastructure shmem: quota support Add default quota limit mount options Documentation/filesystems/tmpfs.rst | 28 ++ fs/Kconfig | 12 + fs/quota/dquot.c | 2 +- include/linux/shmem_fs.h | 25 ++ include/uapi/linux/quota.h | 1 + mm/Makefile | 2 +- mm/shmem.c | 452 +++++++++++++++++++++------- mm/shmem_quota.c | 327 ++++++++++++++++++++ 8 files changed, 740 insertions(+), 109 deletions(-) create mode 100644 mm/shmem_quota.c