This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] add attribute nonstring


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]