[PATCH v2] make function rtnl_link_set_name() more reasonable

[Thomas Haller]

Hi Hongwei,

sorry, re-thinking this again... :)

The users ~should~ provide valid arguments.
If they don't, it's basically a bug on their side and libnl is generally
not forgiving about that (just note, that most functions don't do any
input validation or range checking).

If you want to validate the ifname, shouldn't you also check that the
whole name is valid? Something like
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/net/core/dev.c?id=d8ec26d7f8287f5788a494f56e8814210f0e64be#n929

Otherwise the return value, just tells you, "yeah, it's maybe
valid" -- which isn't much use. And the full check I think is overkill.

What do you guys think?


Thomas



On Wed, 2014-01-15 at 22:47 +0800, Hongwei Bi wrote:
> We set the maximum value of IFNAMSIZ is 16 bytes.So when the length of
> the new link's name is equal or greater then 16,we should notify the user
> that the length of the name has been truncated to 15 bytes.
> 
> Signed-off-by: Hongwei Bi <hwbi2008 at gmail.com>
> ---
>  include/netlink/route/link.h |  2 +-
>  lib/route/link.c             | 19 ++++++++++++++++++-
>  2 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/include/netlink/route/link.h b/include/netlink/route/link.h
> index 3345787..566e224 100644
> --- a/include/netlink/route/link.h
> +++ b/include/netlink/route/link.h
> @@ -151,7 +151,7 @@ extern int	rtnl_link_str2carrier(const char *);
>  extern void	rtnl_link_set_qdisc(struct rtnl_link *, const char *);
>  extern char *	rtnl_link_get_qdisc(struct rtnl_link *);
>  
> -extern void	rtnl_link_set_name(struct rtnl_link *, const char *);
> +extern int	rtnl_link_set_name(struct rtnl_link *, const char *);
>  extern char *	rtnl_link_get_name(struct rtnl_link *);
>  
>  extern void	rtnl_link_set_group(struct rtnl_link *, uint32_t);
> diff --git a/lib/route/link.c b/lib/route/link.c
> index 9979b5b..9b2b561 100644
> --- a/lib/route/link.c
> +++ b/lib/route/link.c
> @@ -1651,11 +1651,28 @@ void rtnl_link_put(struct rtnl_link *link)
>   * @route_doc{link_attr_name, Link Name}
>   * @see rtnl_link_get_name()
>   * @see rtnl_link_set_ifindex()
> + *
> + *@return -1 when the length of the name is equal or greater than IFNAMSIZ.
> + *      In this case,the length of the name has been truncated to 15 bytes. 
> + *@return 0 normally
>   */
> -void rtnl_link_set_name(struct rtnl_link *link, const char *name)
> +int rtnl_link_set_name(struct rtnl_link *link, const char *name)
>  {
> +	int err = 0;
> +	if (!name)
> +		goto errout;
> +	else if (strlen(name) == 0)
> +		goto errout;
> +	else if (IFNAMSIZ > strlen(name))
> +		err = 0;
> +	else
> +		err = -1;
> +
>  	strncpy(link->l_name, name, sizeof(link->l_name) - 1);
>  	link->ce_mask |= LINK_ATTR_IFNAME;
> +
> +	errout:
> +		return err;
>  }
>  
>  /**

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/libnl/attachments/20140121/39eb9032/attachment.sig>