[PATCH v2] make function rtnl_link_set_name() more reasonable

[Hongwei Bi]

Hi Thomas,

The full check is overkill surely. I think there are two method we can
do to solve it.

first: we don't tune the code. when the length of name is equal or
greater then 16, we just notify the user that  the length has been
truncated to 15 bytes by means of modifying the code comment.

second:  tune the code as follows:

int rtnl_link_set_name(struct rtnl_link *link, const char *name)
{
int err = 0;
if (strlen(name) >= IFNAMSIZ)
   err = -1;

strncpy(link->l_name, name, sizeof(link->l_name) - 1);
link->ce_mask |= LINK_ATTR_IFNAME;

return err;
}

If you have any other suggests, please let me know. Thanks.

Hongwei


2014/1/22 Thomas Haller <thaller at redhat.com>:
> 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;
>>  }
>>
>>  /**
>