| Message ID | 20250304141858.3392957-2-juny24602@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | sched: address a potential NULL pointer dereference in the GRED scheduler. | expand |
On Tue, Mar 04, 2025 at 10:18:59PM +0800, kwqcheii wrote: > If kzalloc in gred_init returns a NULL pointer, the code follows the error handling path, > invoking gred_destroy. This, in turn, calls gred_offload, where memset could receive > a NULL pointer as input, potentially leading to a kernel crash. > > Signed-off-by: kwqcheii <juny24602@gmail.com> Please use your real name for Signed-off-by. > --- > net/sched/sch_gred.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/net/sched/sch_gred.c b/net/sched/sch_gred.c > index ab6234b4fcd5..fa643e5709bd 100644 > --- a/net/sched/sch_gred.c > +++ b/net/sched/sch_gred.c > @@ -317,10 +317,12 @@ static void gred_offload(struct Qdisc *sch, enum tc_gred_command command) > if (!tc_can_offload(dev) || !dev->netdev_ops->ndo_setup_tc) > return; > > - memset(opt, 0, sizeof(*opt)); > - opt->command = command; > - opt->handle = sch->handle; > - opt->parent = sch->parent; > + if (opt) { > + memset(opt, 0, sizeof(*opt)); > + opt->command = command; > + opt->handle = sch->handle; > + opt->parent = sch->parent; > + } I think the whole gred_offload() should be skipped when table->opt == NULL, espeically the last call of ->ndo_setup_tc(). Something like: diff --git a/net/sched/sch_gred.c b/net/sched/sch_gred.c index ab6234b4fcd5..532fde548b88 100644 --- a/net/sched/sch_gred.c +++ b/net/sched/sch_gred.c @@ -913,7 +913,8 @@ static void gred_destroy(struct Qdisc *sch) for (i = 0; i < table->DPs; i++) gred_destroy_vq(table->tab[i]); - gred_offload(sch, TC_GRED_DESTROY); + if (table->opt) + gred_offload(sch, TC_GRED_DESTROY); kfree(table->opt); } What do you think? Thanks.
diff --git a/net/sched/sch_gred.c b/net/sched/sch_gred.c index ab6234b4fcd5..fa643e5709bd 100644 --- a/net/sched/sch_gred.c +++ b/net/sched/sch_gred.c @@ -317,10 +317,12 @@ static void gred_offload(struct Qdisc *sch, enum tc_gred_command command) if (!tc_can_offload(dev) || !dev->netdev_ops->ndo_setup_tc) return; - memset(opt, 0, sizeof(*opt)); - opt->command = command; - opt->handle = sch->handle; - opt->parent = sch->parent; + if (opt) { + memset(opt, 0, sizeof(*opt)); + opt->command = command; + opt->handle = sch->handle; + opt->parent = sch->parent; + } if (command == TC_GRED_REPLACE) { unsigned int i;
If kzalloc in gred_init returns a NULL pointer, the code follows the error handling path, invoking gred_destroy. This, in turn, calls gred_offload, where memset could receive a NULL pointer as input, potentially leading to a kernel crash. Signed-off-by: kwqcheii <juny24602@gmail.com> --- net/sched/sch_gred.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)