Re: [PATCH] netfilter: x_tables: replace strlcat() with snprintf()

[Ian Bridges <icb@fastmail.org>]

On Sat, Jun 27, 2026 at 10:16:43PM +0100, David Laight wrote:
> On Fri, 26 Jun 2026 17:25:35 -0500
> Ian Bridges <icb@fastmail.org> wrote:
> 
> > In preparation for removing the deprecated strlcat() API[1], replace the
> > strscpy()/strlcat() pairs in xt_proto_init() and xt_proto_fini() with
> > snprintf(), which builds each /proc file name in a single call.
> > 
> > Each name is "<prefix><suffix>", where <prefix> is the address-family
> > string xt_prefix[af] and <suffix> is one of the FORMAT_TABLES,
> > FORMAT_MATCHES or FORMAT_TARGETS literals. snprintf() with a "%s%s"
> > format produces the same NUL-terminated, length-bounded string as the
> > strscpy()/strlcat() chain it replaces, so the proc entry names are
> > unchanged.
> > 
> > Link: https://github.com/KSPP/linux/issues/370 [1]
> > Signed-off-by: Ian Bridges <icb@fastmail.org>
> > ---
> >  net/netfilter/x_tables.c | 24 ++++++++----------------
> >  1 file changed, 8 insertions(+), 16 deletions(-)
> > 
> > diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
> > index 4e6708c23922..56f4546be336 100644
> > --- a/net/netfilter/x_tables.c
> > +++ b/net/netfilter/x_tables.c
> > @@ -2033,8 +2033,7 @@ int xt_proto_init(struct net *net, u_int8_t af)
> >  	root_uid = make_kuid(net->user_ns, 0);
> >  	root_gid = make_kgid(net->user_ns, 0);
> >  
> > -	strscpy(buf, xt_prefix[af], sizeof(buf));
> > -	strlcat(buf, FORMAT_TABLES, sizeof(buf));
> > +	snprintf(buf, sizeof(buf), "%s%s", xt_prefix[af], FORMAT_TABLES);
> 
> If you are going to use snprintf either paste the strings together:
> 	snprintf(buf, sizeof(buf), "%s" FORMAT_TABLES, xt_prefix[af]);
> or prepend the "%s" onto the #define of FORMAT_TABLES itself:
> 	snprintf(buf, sizeof(buf), FORMAT_TABLES, xt_prefix[af]);
>

I learned something new today, thanks. I'll use the first form in v2.

> FORMAT_TABLES should also be FORMAT_NAMES.

The macro is already named FORMAT_TABLES today, so that rename would
be a cleanup of pre-existing code rather than part of the strlcat
conversion. I'm happy to fold it into v2 if a maintainer is fine
including the tidy-up in this patch.

Thanks for the review,
Ian

> 
> -- David
> 
> >  	proc = proc_create_net_data(buf, 0440, net->proc_net, &xt_table_seq_ops,
> >  			sizeof(struct seq_net_private),
> >  			(void *)(unsigned long)af);
> > @@ -2043,8 +2042,7 @@ int xt_proto_init(struct net *net, u_int8_t af)
> >  	if (uid_valid(root_uid) && gid_valid(root_gid))
> >  		proc_set_user(proc, root_uid, root_gid);
> >  
> > -	strscpy(buf, xt_prefix[af], sizeof(buf));
> > -	strlcat(buf, FORMAT_MATCHES, sizeof(buf));
> > +	snprintf(buf, sizeof(buf), "%s%s", xt_prefix[af], FORMAT_MATCHES);
> >  	proc = proc_create_seq_private(buf, 0440, net->proc_net,
> >  			&xt_match_seq_ops, sizeof(struct nf_mttg_trav),
> >  			(void *)(unsigned long)af);
> > @@ -2053,8 +2051,7 @@ int xt_proto_init(struct net *net, u_int8_t af)
> >  	if (uid_valid(root_uid) && gid_valid(root_gid))
> >  		proc_set_user(proc, root_uid, root_gid);
> >  
> > -	strscpy(buf, xt_prefix[af], sizeof(buf));
> > -	strlcat(buf, FORMAT_TARGETS, sizeof(buf));
> > +	snprintf(buf, sizeof(buf), "%s%s", xt_prefix[af], FORMAT_TARGETS);
> >  	proc = proc_create_seq_private(buf, 0440, net->proc_net,
> >  			 &xt_target_seq_ops, sizeof(struct nf_mttg_trav),
> >  			 (void *)(unsigned long)af);
> > @@ -2068,13 +2065,11 @@ int xt_proto_init(struct net *net, u_int8_t af)
> >  
> >  #ifdef CONFIG_PROC_FS
> >  out_remove_matches:
> > -	strscpy(buf, xt_prefix[af], sizeof(buf));
> > -	strlcat(buf, FORMAT_MATCHES, sizeof(buf));
> > +	snprintf(buf, sizeof(buf), "%s%s", xt_prefix[af], FORMAT_MATCHES);
> >  	remove_proc_entry(buf, net->proc_net);
> >  
> >  out_remove_tables:
> > -	strscpy(buf, xt_prefix[af], sizeof(buf));
> > -	strlcat(buf, FORMAT_TABLES, sizeof(buf));
> > +	snprintf(buf, sizeof(buf), "%s%s", xt_prefix[af], FORMAT_TABLES);
> >  	remove_proc_entry(buf, net->proc_net);
> >  out:
> >  	return -1;
> > @@ -2087,16 +2082,13 @@ void xt_proto_fini(struct net *net, u_int8_t af)
> >  #ifdef CONFIG_PROC_FS
> >  	char buf[XT_FUNCTION_MAXNAMELEN];
> >  
> > -	strscpy(buf, xt_prefix[af], sizeof(buf));
> > -	strlcat(buf, FORMAT_TABLES, sizeof(buf));
> > +	snprintf(buf, sizeof(buf), "%s%s", xt_prefix[af], FORMAT_TABLES);
> >  	remove_proc_entry(buf, net->proc_net);
> >  
> > -	strscpy(buf, xt_prefix[af], sizeof(buf));
> > -	strlcat(buf, FORMAT_TARGETS, sizeof(buf));
> > +	snprintf(buf, sizeof(buf), "%s%s", xt_prefix[af], FORMAT_TARGETS);
> >  	remove_proc_entry(buf, net->proc_net);
> >  
> > -	strscpy(buf, xt_prefix[af], sizeof(buf));
> > -	strlcat(buf, FORMAT_MATCHES, sizeof(buf));
> > +	snprintf(buf, sizeof(buf), "%s%s", xt_prefix[af], FORMAT_MATCHES);
> >  	remove_proc_entry(buf, net->proc_net);
> >  #endif /*CONFIG_PROC_FS*/
> >  }
>