* [PATCH v2] net: ceph: make free_choose_arg_map() resilient to partial allocation
@ 2025-12-20 18:11 Tuo Li
2025-12-23 23:01 ` Viacheslav Dubeyko
0 siblings, 1 reply; 3+ messages in thread
From: Tuo Li @ 2025-12-20 18:11 UTC (permalink / raw)
To: idryomov, xiubli; +Cc: ceph-devel, linux-kernel, Tuo Li
free_choose_arg_map() may dereference a NULL pointer if its caller fails
after a partial allocation.
For example, in decode_choose_args(), if allocation of arg_map->args
fails, execution jumps to the fail label and free_choose_arg_map() is
called. Since arg_map->size is updated to a non-zero value before memory
allocation, free_choose_arg_map() will iterate over arg_map->args and
dereference a NULL pointer.
To prevent this potential NULL pointer dereference and make
free_choose_arg_map() more resilient, add checks for pointers before
iterating.
Signed-off-by: Tuo Li <islituo@gmail.com>
---
v2:
* Add pointer checks before iterating in free_choose_arg_map(), instead of
moving the arg_map->size assignment in decode_choose_args().
Thanks to Viacheslav Dubeyko for pointing out the issue with the previous
patch, and to Ilya Dryomov for the helpful advice.
---
net/ceph/osdmap.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/net/ceph/osdmap.c b/net/ceph/osdmap.c
index 34b3ab59602f..08157945af43 100644
--- a/net/ceph/osdmap.c
+++ b/net/ceph/osdmap.c
@@ -241,22 +241,26 @@ static struct crush_choose_arg_map *alloc_choose_arg_map(void)
static void free_choose_arg_map(struct crush_choose_arg_map *arg_map)
{
- if (arg_map) {
- int i, j;
+ int i, j;
+
+ if (!arg_map)
+ return;
- WARN_ON(!RB_EMPTY_NODE(&arg_map->node));
+ WARN_ON(!RB_EMPTY_NODE(&arg_map->node));
+ if (arg_map->args) {
for (i = 0; i < arg_map->size; i++) {
struct crush_choose_arg *arg = &arg_map->args[i];
-
- for (j = 0; j < arg->weight_set_size; j++)
- kfree(arg->weight_set[j].weights);
- kfree(arg->weight_set);
+ if (arg->weight_set) {
+ for (j = 0; j < arg->weight_set_size; j++)
+ kfree(arg->weight_set[j].weights);
+ kfree(arg->weight_set);
+ }
kfree(arg->ids);
}
kfree(arg_map->args);
- kfree(arg_map);
}
+ kfree(arg_map);
}
DEFINE_RB_FUNCS(choose_arg_map, struct crush_choose_arg_map, choose_args_index,
--
2.48.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH v2] net: ceph: make free_choose_arg_map() resilient to partial allocation
2025-12-20 18:11 [PATCH v2] net: ceph: make free_choose_arg_map() resilient to partial allocation Tuo Li
@ 2025-12-23 23:01 ` Viacheslav Dubeyko
2026-01-05 12:53 ` Ilya Dryomov
0 siblings, 1 reply; 3+ messages in thread
From: Viacheslav Dubeyko @ 2025-12-23 23:01 UTC (permalink / raw)
To: idryomov@gmail.com, Xiubo Li, islituo@gmail.com
Cc: ceph-devel@vger.kernel.org, linux-kernel@vger.kernel.org
On Sun, 2025-12-21 at 02:11 +0800, Tuo Li wrote:
> free_choose_arg_map() may dereference a NULL pointer if its caller fails
> after a partial allocation.
>
> For example, in decode_choose_args(), if allocation of arg_map->args
> fails, execution jumps to the fail label and free_choose_arg_map() is
> called. Since arg_map->size is updated to a non-zero value before memory
> allocation, free_choose_arg_map() will iterate over arg_map->args and
> dereference a NULL pointer.
>
> To prevent this potential NULL pointer dereference and make
> free_choose_arg_map() more resilient, add checks for pointers before
> iterating.
>
> Signed-off-by: Tuo Li <islituo@gmail.com>
> ---
> v2:
> * Add pointer checks before iterating in free_choose_arg_map(), instead of
> moving the arg_map->size assignment in decode_choose_args().
> Thanks to Viacheslav Dubeyko for pointing out the issue with the previous
> patch, and to Ilya Dryomov for the helpful advice.
> ---
> net/ceph/osdmap.c | 20 ++++++++++++--------
> 1 file changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/net/ceph/osdmap.c b/net/ceph/osdmap.c
> index 34b3ab59602f..08157945af43 100644
> --- a/net/ceph/osdmap.c
> +++ b/net/ceph/osdmap.c
> @@ -241,22 +241,26 @@ static struct crush_choose_arg_map *alloc_choose_arg_map(void)
>
> static void free_choose_arg_map(struct crush_choose_arg_map *arg_map)
> {
> - if (arg_map) {
> - int i, j;
> + int i, j;
> +
> + if (!arg_map)
> + return;
>
> - WARN_ON(!RB_EMPTY_NODE(&arg_map->node));
> + WARN_ON(!RB_EMPTY_NODE(&arg_map->node));
>
> + if (arg_map->args) {
> for (i = 0; i < arg_map->size; i++) {
> struct crush_choose_arg *arg = &arg_map->args[i];
> -
> - for (j = 0; j < arg->weight_set_size; j++)
> - kfree(arg->weight_set[j].weights);
> - kfree(arg->weight_set);
> + if (arg->weight_set) {
> + for (j = 0; j < arg->weight_set_size; j++)
> + kfree(arg->weight_set[j].weights);
> + kfree(arg->weight_set);
> + }
> kfree(arg->ids);
> }
> kfree(arg_map->args);
> - kfree(arg_map);
> }
> + kfree(arg_map);
> }
>
> DEFINE_RB_FUNCS(choose_arg_map, struct crush_choose_arg_map, choose_args_index,
Looks good. Thanks a lot for the fix.
Reviewed-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
Thanks,
Slava.
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v2] net: ceph: make free_choose_arg_map() resilient to partial allocation
2025-12-23 23:01 ` Viacheslav Dubeyko
@ 2026-01-05 12:53 ` Ilya Dryomov
0 siblings, 0 replies; 3+ messages in thread
From: Ilya Dryomov @ 2026-01-05 12:53 UTC (permalink / raw)
To: Viacheslav Dubeyko
Cc: Xiubo Li, islituo@gmail.com, ceph-devel@vger.kernel.org,
linux-kernel@vger.kernel.org
On Wed, Dec 24, 2025 at 12:01 AM Viacheslav Dubeyko
<Slava.Dubeyko@ibm.com> wrote:
>
> On Sun, 2025-12-21 at 02:11 +0800, Tuo Li wrote:
> > free_choose_arg_map() may dereference a NULL pointer if its caller fails
> > after a partial allocation.
> >
> > For example, in decode_choose_args(), if allocation of arg_map->args
> > fails, execution jumps to the fail label and free_choose_arg_map() is
> > called. Since arg_map->size is updated to a non-zero value before memory
> > allocation, free_choose_arg_map() will iterate over arg_map->args and
> > dereference a NULL pointer.
> >
> > To prevent this potential NULL pointer dereference and make
> > free_choose_arg_map() more resilient, add checks for pointers before
> > iterating.
> >
> > Signed-off-by: Tuo Li <islituo@gmail.com>
> > ---
> > v2:
> > * Add pointer checks before iterating in free_choose_arg_map(), instead of
> > moving the arg_map->size assignment in decode_choose_args().
> > Thanks to Viacheslav Dubeyko for pointing out the issue with the previous
> > patch, and to Ilya Dryomov for the helpful advice.
> > ---
> > net/ceph/osdmap.c | 20 ++++++++++++--------
> > 1 file changed, 12 insertions(+), 8 deletions(-)
> >
> > diff --git a/net/ceph/osdmap.c b/net/ceph/osdmap.c
> > index 34b3ab59602f..08157945af43 100644
> > --- a/net/ceph/osdmap.c
> > +++ b/net/ceph/osdmap.c
> > @@ -241,22 +241,26 @@ static struct crush_choose_arg_map *alloc_choose_arg_map(void)
> >
> > static void free_choose_arg_map(struct crush_choose_arg_map *arg_map)
> > {
> > - if (arg_map) {
> > - int i, j;
> > + int i, j;
> > +
> > + if (!arg_map)
> > + return;
> >
> > - WARN_ON(!RB_EMPTY_NODE(&arg_map->node));
> > + WARN_ON(!RB_EMPTY_NODE(&arg_map->node));
> >
> > + if (arg_map->args) {
> > for (i = 0; i < arg_map->size; i++) {
> > struct crush_choose_arg *arg = &arg_map->args[i];
> > -
> > - for (j = 0; j < arg->weight_set_size; j++)
> > - kfree(arg->weight_set[j].weights);
> > - kfree(arg->weight_set);
> > + if (arg->weight_set) {
> > + for (j = 0; j < arg->weight_set_size; j++)
> > + kfree(arg->weight_set[j].weights);
> > + kfree(arg->weight_set);
> > + }
> > kfree(arg->ids);
> > }
> > kfree(arg_map->args);
> > - kfree(arg_map);
> > }
> > + kfree(arg_map);
> > }
> >
> > DEFINE_RB_FUNCS(choose_arg_map, struct crush_choose_arg_map, choose_args_index,
>
> Looks good. Thanks a lot for the fix.
>
> Reviewed-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
>
> Thanks,
> Slava.
Applied.
Thanks,
Ilya
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-01-05 12:54 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-20 18:11 [PATCH v2] net: ceph: make free_choose_arg_map() resilient to partial allocation Tuo Li
2025-12-23 23:01 ` Viacheslav Dubeyko
2026-01-05 12:53 ` Ilya Dryomov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox