On Wed, Sep 27, 2023 at 09:47:14AM -0700, joao@xxxxxxxxxxxxxxxxxx wrote: > From: Joao Moreira <joao.moreira@xxxxxxxxx> > > Both flow_rule_alloc and offload_action_alloc functions received an > unsigned num_actions parameters which are then operated within a loop. > The index of this loop is declared as a signed int. If it was possible > to pass a large enough num_actions to these functions, it would lead to > an out of bounds write. > > After checking with maintainers, it was mentioned that front-end will > cap the num_actions value and that it is not possible to reach this > function with such a large number. Yet, for correctness, it is still > better to fix this. > > This issue was observed by the commit author while reviewing a write-up > regarding a CVE within the same subsystem [1]. > > 1 - https://nickgregory.me/post/2022/03/12/cve-2022-25636/ > > Signed-off-by: Joao Moreira <joao.moreira@xxxxxxxxx> > --- > net/core/flow_offload.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/net/core/flow_offload.c b/net/core/flow_offload.c > index bc5169482710..bc3f53a09d8f 100644 > --- a/net/core/flow_offload.c > +++ b/net/core/flow_offload.c > @@ -10,7 +10,7 @@ > struct flow_rule *flow_rule_alloc(unsigned int num_actions) > { > struct flow_rule *rule; > - int i; > + unsigned int i; With the 2^8 cap, I don't think this patch is required anymore. > > rule = kzalloc(struct_size(rule, action.entries, num_actions), > GFP_KERNEL); > @@ -31,7 +31,7 @@ EXPORT_SYMBOL(flow_rule_alloc); > struct flow_offload_action *offload_action_alloc(unsigned int num_actions) > { > struct flow_offload_action *fl_action; > - int i; > + unsigned int i; > > fl_action = kzalloc(struct_size(fl_action, action.entries, num_actions), > GFP_KERNEL); > -- > 2.42.0 >
