| Message ID | 20240216232744.work.514-kees@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 1e63e5a813fa6203d7430af51d6bffb728525015 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: sched: Annotate struct tc_pedit with __counted_by | expand |
On 2/16/24 17:27, Kees Cook wrote: > Prepare for the coming implementation by GCC and Clang of the __counted_by > attribute. Flexible array members annotated with __counted_by can have > their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS > (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family > functions). > > As found with Coccinelle[1], add __counted_by for struct tc_pedit. > Additionally, since the element count member must be set before accessing > the annotated flexible array member, move its initialization earlier. > > Link: https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci [1] > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > Cc: Jakub Kicinski <kuba@kernel.org> > Cc: Jamal Hadi Salim <jhs@mojatatu.com> > Cc: Cong Wang <xiyou.wangcong@gmail.com> > Cc: Jiri Pirko <jiri@resnulli.us> > Cc: "David S. Miller" <davem@davemloft.net> > Cc: Eric Dumazet <edumazet@google.com> > Cc: Paolo Abeni <pabeni@redhat.com> > Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org> > Cc: netdev@vger.kernel.org > Cc: linux-hardening@vger.kernel.org `opt->nkeys` updated before `memcpy()`, looks good to me: Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org> Thanks! -- Gustavo > --- > include/uapi/linux/tc_act/tc_pedit.h | 2 +- > net/sched/act_pedit.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/uapi/linux/tc_act/tc_pedit.h b/include/uapi/linux/tc_act/tc_pedit.h > index f3e61b04fa01..f5cab7fc96ab 100644 > --- a/include/uapi/linux/tc_act/tc_pedit.h > +++ b/include/uapi/linux/tc_act/tc_pedit.h > @@ -62,7 +62,7 @@ struct tc_pedit_sel { > tc_gen; > unsigned char nkeys; > unsigned char flags; > - struct tc_pedit_key keys[0]; > + struct tc_pedit_key keys[] __counted_by(nkeys); > }; > > #define tc_pedit tc_pedit_sel > diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c > index 2ef22969f274..21e863d2898c 100644 > --- a/net/sched/act_pedit.c > +++ b/net/sched/act_pedit.c > @@ -515,11 +515,11 @@ static int tcf_pedit_dump(struct sk_buff *skb, struct tc_action *a, > spin_unlock_bh(&p->tcf_lock); > return -ENOBUFS; > } > + opt->nkeys = parms->tcfp_nkeys; > > memcpy(opt->keys, parms->tcfp_keys, > flex_array_size(opt, keys, parms->tcfp_nkeys)); > opt->index = p->tcf_index; > - opt->nkeys = parms->tcfp_nkeys; > opt->flags = parms->tcfp_flags; > opt->action = p->tcf_action; > opt->refcnt = refcount_read(&p->tcf_refcnt) - ref;
On Fri, Feb 16, 2024 at 7:04 PM Gustavo A. R. Silva <gustavo@embeddedor.com> wrote: > > > > On 2/16/24 17:27, Kees Cook wrote: > > Prepare for the coming implementation by GCC and Clang of the __counted_by > > attribute. Flexible array members annotated with __counted_by can have > > their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS > > (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family > > functions). > > > > As found with Coccinelle[1], add __counted_by for struct tc_pedit. > > Additionally, since the element count member must be set before accessing > > the annotated flexible array member, move its initialization earlier. > > > > Link: https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci [1] > > Signed-off-by: Kees Cook <keescook@chromium.org> > > --- > > Cc: Jakub Kicinski <kuba@kernel.org> > > Cc: Jamal Hadi Salim <jhs@mojatatu.com> > > Cc: Cong Wang <xiyou.wangcong@gmail.com> > > Cc: Jiri Pirko <jiri@resnulli.us> > > Cc: "David S. Miller" <davem@davemloft.net> > > Cc: Eric Dumazet <edumazet@google.com> > > Cc: Paolo Abeni <pabeni@redhat.com> > > Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org> > > Cc: netdev@vger.kernel.org > > Cc: linux-hardening@vger.kernel.org > > `opt->nkeys` updated before `memcpy()`, looks good to me: > > Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org> Looks good to me. Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> cheers, jamal > Thanks! > -- > Gustavo > > > --- > > include/uapi/linux/tc_act/tc_pedit.h | 2 +- > > net/sched/act_pedit.c | 2 +- > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/include/uapi/linux/tc_act/tc_pedit.h b/include/uapi/linux/tc_act/tc_pedit.h > > index f3e61b04fa01..f5cab7fc96ab 100644 > > --- a/include/uapi/linux/tc_act/tc_pedit.h > > +++ b/include/uapi/linux/tc_act/tc_pedit.h > > @@ -62,7 +62,7 @@ struct tc_pedit_sel { > > tc_gen; > > unsigned char nkeys; > > unsigned char flags; > > - struct tc_pedit_key keys[0]; > > + struct tc_pedit_key keys[] __counted_by(nkeys); > > }; > > > > #define tc_pedit tc_pedit_sel > > diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c > > index 2ef22969f274..21e863d2898c 100644 > > --- a/net/sched/act_pedit.c > > +++ b/net/sched/act_pedit.c > > @@ -515,11 +515,11 @@ static int tcf_pedit_dump(struct sk_buff *skb, struct tc_action *a, > > spin_unlock_bh(&p->tcf_lock); > > return -ENOBUFS; > > } > > + opt->nkeys = parms->tcfp_nkeys; > > > > memcpy(opt->keys, parms->tcfp_keys, > > flex_array_size(opt, keys, parms->tcfp_nkeys)); > > opt->index = p->tcf_index; > > - opt->nkeys = parms->tcfp_nkeys; > > opt->flags = parms->tcfp_flags; > > opt->action = p->tcf_action; > > opt->refcnt = refcount_read(&p->tcf_refcnt) - ref;
Hello: This patch was applied to netdev/net-next.git (main) by David S. Miller <davem@davemloft.net>: On Fri, 16 Feb 2024 15:27:44 -0800 you wrote: > Prepare for the coming implementation by GCC and Clang of the __counted_by > attribute. Flexible array members annotated with __counted_by can have > their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS > (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family > functions). > > As found with Coccinelle[1], add __counted_by for struct tc_pedit. > Additionally, since the element count member must be set before accessing > the annotated flexible array member, move its initialization earlier. > > [...] Here is the summary with links: - net: sched: Annotate struct tc_pedit with __counted_by https://git.kernel.org/netdev/net-next/c/1e63e5a813fa You are awesome, thank you!
diff --git a/include/uapi/linux/tc_act/tc_pedit.h b/include/uapi/linux/tc_act/tc_pedit.h index f3e61b04fa01..f5cab7fc96ab 100644 --- a/include/uapi/linux/tc_act/tc_pedit.h +++ b/include/uapi/linux/tc_act/tc_pedit.h @@ -62,7 +62,7 @@ struct tc_pedit_sel { tc_gen; unsigned char nkeys; unsigned char flags; - struct tc_pedit_key keys[0]; + struct tc_pedit_key keys[] __counted_by(nkeys); }; #define tc_pedit tc_pedit_sel diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c index 2ef22969f274..21e863d2898c 100644 --- a/net/sched/act_pedit.c +++ b/net/sched/act_pedit.c @@ -515,11 +515,11 @@ static int tcf_pedit_dump(struct sk_buff *skb, struct tc_action *a, spin_unlock_bh(&p->tcf_lock); return -ENOBUFS; } + opt->nkeys = parms->tcfp_nkeys; memcpy(opt->keys, parms->tcfp_keys, flex_array_size(opt, keys, parms->tcfp_nkeys)); opt->index = p->tcf_index; - opt->nkeys = parms->tcfp_nkeys; opt->flags = parms->tcfp_flags; opt->action = p->tcf_action; opt->refcnt = refcount_read(&p->tcf_refcnt) - ref;
Prepare for the coming implementation by GCC and Clang of the __counted_by attribute. Flexible array members annotated with __counted_by can have their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions). As found with Coccinelle[1], add __counted_by for struct tc_pedit. Additionally, since the element count member must be set before accessing the annotated flexible array member, move its initialization earlier. Link: https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci [1] Signed-off-by: Kees Cook <keescook@chromium.org> --- Cc: Jakub Kicinski <kuba@kernel.org> Cc: Jamal Hadi Salim <jhs@mojatatu.com> Cc: Cong Wang <xiyou.wangcong@gmail.com> Cc: Jiri Pirko <jiri@resnulli.us> Cc: "David S. Miller" <davem@davemloft.net> Cc: Eric Dumazet <edumazet@google.com> Cc: Paolo Abeni <pabeni@redhat.com> Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org> Cc: netdev@vger.kernel.org Cc: linux-hardening@vger.kernel.org --- include/uapi/linux/tc_act/tc_pedit.h | 2 +- net/sched/act_pedit.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)