Re: [PATCH nf-next 1/1] netfilter: Use strlcpy to copy dev name instead of strncpy

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Tue, Dec 27, 2016 at 07:16:53PM +0800, fgao@ikuai8.com wrote:
> From: Gao Feng <fgao@ikuai8.com>
> 
> The current codes use strncpy to copy the dev name in
> nf_tables_newchain, there is no real issue now. Because the sizes of
> src and dst are IFNAMSIZ. But normally it should use count-1 with
> strncpy, because strncpy doesn't make sure the dst must be
> NULL-terminated when src exceeds the count bytes.
> So use strlcpy to replace the strncpy to make it clearer.
> 
> Signed-off-by: Gao Feng <fgao@ikuai8.com>
> ---
>  net/netfilter/nf_tables_api.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index a019a87..dc50801 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -1488,7 +1488,7 @@ static int nf_tables_newchain(struct net *net, struct sock *nlsk,
>  		}
>  
>  		if (hook.dev != NULL)
> -			strncpy(basechain->dev_name, hook.dev->name, IFNAMSIZ);
> +			strlcpy(basechain->dev_name, hook.dev->name, IFNAMSIZ);

I think this is not a problem, look:

        [NFTA_HOOK_DEV]         = { .type = NLA_STRING,
                                    .len = IFNAMSIZ - 1 },

The netlink attribute policy enforces that NFTA_HOOK_DEV is always one
bytes smaller than the maximum length. So we guarantee that there's
room to nul-terminated this string.

>  		if (nla[NFTA_CHAIN_COUNTERS]) {
>  			stats = nft_stats_alloc(nla[NFTA_CHAIN_COUNTERS]);
> -- 
> 1.9.1
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html