[PATCH] add attribute nonstring

Florian Weimer fweimer@redhat.com
Tue Nov 14 16:50:00 GMT 2017


On 11/13/2017 06:35 PM, Steve Ellcey wrote:
> On Sun, 2017-11-12 at 16:49 -0700, Martin Sebor wrote:
>>
>> PS I still don't see it discussed on the Linux man page but
>> I did find such a requirement on an AIX 6.1 ioctl man page:
>> https://www.ibm.com/support/knowledgecenter/en/ssw_ibm_i_61/apis/ioct
>> l.htm
>>
>> The descriptions of the if_indextoname and if_nametoindex
>> functions specified by RFC 3493 also talk about the name being
>> a nul-terminated string so it looks to me like you are correct
>> and the warning has found a Glibc bug.  Yay! :)
> 
> I think this is a bug and that if_nametoindex should check for a name
> that is too long.  Based on RFC 3493 it would appear that we don't need
> to set errno in this case though I am not sure if that is a correct
> interpretation.  I tested this patch:
> 
> 
> 2017-11-13  Steve Ellcey  <sellcey@cavium.com>
> 
> 	* sysdeps/unix/sysv/linux/if_index.c (__if_nametoindex):
> 	Check if ifname is too long.
> 
> 
> diff --git a/sysdeps/unix/sysv/linux/if_index.c
> b/sysdeps/unix/sysv/linux/if_index.c
> index 56f3f13..1e081c0 100644
> --- a/sysdeps/unix/sysv/linux/if_index.c
> +++ b/sysdeps/unix/sysv/linux/if_index.c
> @@ -43,6 +43,9 @@ __if_nametoindex (const char *ifname)
>     if (fd < 0)
>       return 0;
>   
> +  if (strlen (ifname) >= IFNAMSIZ)
> +    return 0;
> +
>     strncpy (ifr.ifr_name, ifname, sizeof (ifr.ifr_name));
>     if (__ioctl (fd, SIOCGIFINDEX, &ifr) < 0)
>       {

The Linux kernel has this in include/linux/netdevice.h:

| struct net_device {
|         char                    name[IFNAMSIZ];

If I traced this correctly, the ioctl compares the passed-in name using 
strncmp:

| struct net_device *dev_get_by_name_rcu(struct net *net, const char *name)
| {
|         struct net_device *dev;
|         struct hlist_head *head = dev_name_hash(net, name);
|
|         hlist_for_each_entry_rcu(dev, head, name_hlist)
|                 if (!strncmp(dev->name, name, IFNAMSIZ))
|                         return dev;
|
|         return NULL;
| }

So this means that we should add the nostring attribute and not the 
length check.

I'll ask one of our networking people to double-check this.

Thanks,
Florian



More information about the Libc-alpha mailing list