Re: [PATCH] ceph: use a flexible array for monitor addresses

[Alejandro Colomar <alx+linux-hardening@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3529 bytes --]

Hi,

> Date: 2026-10-04 17:19:55-0700
> From: Rosen Penev <rosenp@gmail.com>
>
> ceph_alloc_options() allocates the monitor address array separately,
> always sized for CEPH_MAX_MON entries. Make it a flexible array member
> at the end of struct ceph_options and allocate both together with
> kzalloc_flex(). The array was already an 8 KiB slab object, so this
> only drops the separate allocation and its error path.
> 
> num_mon is the number of addresses in use, not the capacity, so the
> array cannot be annotated with __counted_by().
> 
> ceph_compare_options() memcmp()s the simple fields up to the first
> member that needs a custom comparison. With mon_addr gone from there,
> use num_mon as that boundary; the compared fields are unchanged.
> Document that num_mon has to stay first past that point.
> 
> Assisted-by: LLM
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  include/linux/ceph/libceph.h |  7 ++++---
>  net/ceph/ceph_common.c       | 11 ++---------
>  2 files changed, 6 insertions(+), 12 deletions(-)
> 
> diff --git a/include/linux/ceph/libceph.h b/include/linux/ceph/libceph.h
> index f92fdd853f1f..e585d6d11303 100644
> --- a/include/linux/ceph/libceph.h
> +++ b/include/linux/ceph/libceph.h
> @@ -57,15 +57,16 @@ struct ceph_options {
>  	/*
>  	 * any type that can't be simply compared or doesn't need
>  	 * to be compared should go beyond this point,
> -	 * ceph_compare_options() should be updated accordingly
> +	 * ceph_compare_options() should be updated accordingly.
> +	 * num_mon must stay the first member past this point, as
> +	 * ceph_compare_options() uses it as the end of the memcmp().
>  	 */
>  
> -	struct ceph_entity_addr *mon_addr; /* should be the first
> -					      pointer type of args */
>  	int num_mon;
>  	char *name;
>  	struct ceph_crypto_key *key;
>  	struct rb_root crush_locs;
> +	struct ceph_entity_addr mon_addr[];
>  };
>  
>  /*
> diff --git a/net/ceph/ceph_common.c b/net/ceph/ceph_common.c
> index a797c7360e3c..95dc18257056 100644
> --- a/net/ceph/ceph_common.c
> +++ b/net/ceph/ceph_common.c
> @@ -133,7 +133,7 @@ int ceph_compare_options(struct ceph_options *new_opt,
>  {
>  	struct ceph_options *opt1 = new_opt;
>  	struct ceph_options *opt2 = client->options;
> -	int ofs = offsetof(struct ceph_options, mon_addr);
> +	int ofs = offsetof(struct ceph_options, num_mon);
>  	int i;
>  	int ret;
>  
> @@ -309,17 +309,11 @@ struct ceph_options *ceph_alloc_options(void)
>  {
>  	struct ceph_options *opt;
>  
> -	opt = kzalloc_obj(*opt);
> +	opt = kzalloc_flex(*opt, mon_addr, CEPH_MAX_MON);

Oh, there's indeed a kzalloc_flex().  Why is it not used in the other
patches?  (I'm not all that happy with the current API of this _flex()
variant, but it's better than nothing.)


Have a lovely day!
Alex

>  	if (!opt)
>  		return NULL;
>  
>  	opt->crush_locs = RB_ROOT;
> -	opt->mon_addr = kzalloc_objs(*opt->mon_addr, CEPH_MAX_MON);
> -	if (!opt->mon_addr) {
> -		kfree(opt);
> -		return NULL;
> -	}
> -
>  	opt->flags = CEPH_OPT_DEFAULT;
>  	opt->osd_keepalive_timeout = CEPH_OSD_KEEPALIVE_DEFAULT;
>  	opt->mount_timeout = CEPH_MOUNT_TIMEOUT_DEFAULT;
> @@ -344,7 +338,6 @@ void ceph_destroy_options(struct ceph_options *opt)
>  		ceph_crypto_key_destroy(opt->key);
>  		kfree(opt->key);
>  	}
> -	kfree(opt->mon_addr);
>  	kfree(opt);
>  }
>  EXPORT_SYMBOL(ceph_destroy_options);
> -- 
> 2.56.0
> 
> 

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]