On Sun, Feb 26, 2023 at 01:28:17PM +0100, Julian Anastasov wrote: > > Hello, > > On Sun, 26 Feb 2023, Sven Auhagen wrote: > > > On Sun, Feb 26, 2023 at 10:53:58AM +0200, Julian Anastasov wrote: > > > > > > > diff --git a/net/netfilter/nf_flow_table_procfs.c b/net/netfilter/nf_flow_table_procfs.c > > > > index 159b033a43e6..cff9ad58c7c9 100644 > > > > --- a/net/netfilter/nf_flow_table_procfs.c > > > > +++ b/net/netfilter/nf_flow_table_procfs.c > > > > @@ -64,6 +64,16 @@ static const struct seq_operations nf_flow_table_cpu_seq_ops = { > > > > .show = nf_flow_table_cpu_seq_show, > > > > }; > > > > > > > > +static int nf_flow_table_counter_show(struct seq_file *seq, void *v) > > > > +{ > > > > + struct net *net = seq_file_net(seq); > > > > + > > > > + seq_printf(seq, "%lld\n", > > > > + atomic64_read(&net->ft.count_flowoffload) > > > > + ); > > > > + return 0; > > > > +} > > > > + > > > > int nf_flow_table_init_proc(struct net *net) > > > > { > > > > struct proc_dir_entry *pde; > > > > @@ -71,6 +81,9 @@ int nf_flow_table_init_proc(struct net *net) > > > > pde = proc_create_net("nf_flowtable", 0444, net->proc_net_stat, > > > > &nf_flow_table_cpu_seq_ops, > > > > sizeof(struct seq_net_private)); > > > > > > Result should be checked, pde is not needed: > > > > > > > pde is needed to free the per cpu stat structure in nf_flow_table_fini_net > > Nothing is needed to add in nf_flow_table_fini_net(), > only in nf_flow_table_fini_proc() which is called by > nf_flow_table_pernet_exit(). > > > in the error case. This was implemented before this patch though. > > I understand that some kind of error check would be good. > > Since there is no consequence when this does not work I did not implement one > > and let the old error checking go along. > > The checking tools will warn for such leaks, so > every module should cleanup everything on init error. > > > Do you have any ideas how to solve the problem more? > > Nothing more is needed except the missing > remove_proc_entry("nf_flowtable_counter", net->proc_net_stat); > in nf_flow_table_fini_proc(). The var 'pde' is not needed. > On error in nf_flow_table_init_proc() it should free everything > allocated there and nf_flow_table_fini_proc() should free > everything that is allocated in nf_flow_table_init_proc(). Got it and you are correct about nf_flow_table_fini_proc but it is also called from nf_flow_table_pernet_init in case of a nf_flow_table_init_proc error which is checking the return of the pde variable. That what I was referring to. I will send a v2 with your proposed changes. > > > Best > > Sven > > > > > if (!proc_create_net()) > > > goto err; > > > > > > > + proc_create_net_single("nf_flowtable_counter", 0444, > > > > + net->proc_net, nf_flow_table_counter_show, NULL); > > > > > > if (!proc_create_net_single()) > > > goto err_net; > > > return 0; > > > > > > err_net: > > > remove_proc_entry("nf_flowtable", net->proc_net_stat); > > > > > > err: > > > return -ENOMEM; > > > > > > > + > > > > return pde ? 0 : -ENOMEM; > > > > } > > Regards > > -- > Julian Anastasov <ja@xxxxxx> >