Re: [PATCH v2] quota: initialize dquot cache before mounting rootfs

[Jan Kara]

On Wed 21-10-26 18:12:17, YouYou Yu wrote:
> On some systems, a tmpfs backed rootfs is extensively used throughout
> system's lifecycle. tmpfs quotas are needed to limit per user rootfs
> usage. However, booting with CONFIG_TMPFS_QUOTA=y, an initramfs, no root=
> and "initramfs_options=usrquota" panics at boot:
> 
>   BUG: kernel NULL pointer dereference, address: 0000000000000000
>   RIP: 0010:dqget+0x1ac/0x490
>   Call Trace:
>    __dquot_initialize
>    dquot_initialize
>    shmem_get_inode
>    shmem_fill_super
>    ...
>    mnt_init
>    vfs_caches_init
>    start_kernel
> 
> The rootfs is mounted from mnt_init(), but the dquot hash table and
> cache are only set up later by dquot_init(), an fs_initcall. Creating
> the root inode of a quota enabled rootfs thus dereferences the NULL
> dquot_hash in dqget().
> 
> Rename dquot_init() to dquot_cache_init() and call it from
> vfs_caches_init() before mnt_init(). The slab, percpu and early
> workqueue code it relies on is available there, and the workqueue
> rescuer and shrinker debugfs entries are set up later by their own
> subsystems. Keep the sysctl registration in an fs_initcall, as done
> for the dcache, inode and mount sysctls. This only moves existing
> allocations earlier in boot.
> 
> Fixes: 278033a225e1 ("fs: Add 'initramfs_options' to set initramfs mount options")
> Cc: stable@xxxxxxxxxxxxxxx
> Assisted-by: LLM
> Co-developed-by: Dylan Johnson <dgj@xxxxxxxxxx>
> Signed-off-by: Dylan Johnson <dgj@xxxxxxxxxx>
> Signed-off-by: YouYou Yu <youyouyu@xxxxxxxxxx>
> Reviewed-by: Gunnar Kudrjavets <gunnarku@xxxxxxxxxx>

Thanks! I've added the patch to my tree.

								Honza

> ---
> 
> Notes:
>     v2:
>     - Mention the use case in the changelog (Jan)
>     
>     v1:
>     https://lore.kernel.org/all/20261015183608.22507-1-youyouyu@xxxxxxxxxx/
>     
>     Tested on v7.3-rc7, x86_64 QEMU/KVM, booting from an initramfs without root=:
>     - without this patch, initramfs_options=usrquota panics in dqget() as above
>     - with it, initramfs_options=usrquota,grpquota,usrquota_block_hardlimit=10M
>       boots, rootfs is mounted with quotas, and a non-root user is limited to
>       10M on rootfs
>     - /sys/kernel/slab/dquot, the quota_events_unbound rescuer, the fs/quota
>       sysctls and (with CONFIG_SHRINKER_DEBUG) the dquota-cache shrinker
>       debugfs entry are all created as expected
>     
>     An LLM (Claude Opus 5.5) helped drafting this changelog.
>     I reviewed the change and ran the tests above.
> 
>  fs/dcache.c              |  2 ++
>  fs/quota/dquot.c         | 10 ++++++----
>  include/linux/quotaops.h |  5 +++++
>  3 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/dcache.c b/fs/dcache.c
> index f44c7b39c3b..b208cbdda0f 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -32,6 +32,7 @@
>  #include <linux/bit_spinlock.h>
>  #include <linux/rculist_bl.h>
>  #include <linux/list_lru.h>
> +#include <linux/quotaops.h>
>  #include "internal.h"
>  #include "mount.h"
>  
> @@ -3525,6 +3526,7 @@ void __init vfs_caches_init(void)
>  	inode_init();
>  	files_init();
>  	files_maxfiles_init();
> +	dquot_cache_init();
>  	mnt_init();
>  	bdev_cache_init();
>  	chrdev_init();
> diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
> index 1c78c695d0d..cd142c9cc7e 100644
> --- a/fs/quota/dquot.c
> +++ b/fs/quota/dquot.c
> @@ -2942,7 +2942,7 @@ static const struct ctl_table fs_dqstats_table[] = {
>  	},
>  };
>  
> -static int __init dquot_init(void)
> +void __init dquot_cache_init(void)
>  {
>  	int i, ret;
>  	unsigned long nr_hash;
> @@ -2950,8 +2950,6 @@ static int __init dquot_init(void)
>  
>  	printk(KERN_NOTICE "VFS: Disk quotas %s\n", __DQUOT_VERSION__);
>  
> -	register_sysctl_init("fs/quota", fs_dqstats_table);
> -
>  	dquot_cachep = kmem_cache_create("dquot",
>  			sizeof(struct dquot), sizeof(unsigned long) * 4,
>  			(SLAB_HWCACHE_ALIGN|SLAB_RECLAIM_ACCOUNT|
> @@ -2992,7 +2990,11 @@ static int __init dquot_init(void)
>  					   WQ_UNBOUND | WQ_MEM_RECLAIM, WQ_MAX_ACTIVE);
>  	if (!quota_unbound_wq)
>  		panic("Cannot create quota_unbound_wq\n");
> +}
>  
> +static int __init init_fs_dquot_sysctls(void)
> +{
> +	register_sysctl_init("fs/quota", fs_dqstats_table);
>  	return 0;
>  }
> -fs_initcall(dquot_init);
> +fs_initcall(init_fs_dquot_sysctls);
> diff --git a/include/linux/quotaops.h b/include/linux/quotaops.h
> index f9c0f9d7c9d..bbe22c22439 100644
> --- a/include/linux/quotaops.h
> +++ b/include/linux/quotaops.h
> @@ -40,6 +40,7 @@ void __quota_error(struct super_block *sb, const char *func,
>  /*
>   * declaration of quota_function calls in kernel.
>   */
> +void dquot_cache_init(void);
>  int dquot_initialize(struct inode *inode);
>  bool dquot_initialize_needed(struct inode *inode);
>  void dquot_drop(struct inode *inode);
> @@ -206,6 +207,10 @@ static inline int sb_has_quota_active(struct super_block *sb, int type)
>  	return 0;
>  }
>  
> +static inline void dquot_cache_init(void)
> +{
> +}
> +
>  static inline int dquot_initialize(struct inode *inode)
>  {
>  	return 0;
> 
> base-commit: 7c6f8a2841d4eeeceacf42689cd2717fe76d9f7c
> -- 
> 2.47.3
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR