On 2/15/18 11:34 AM, Serhey Popovych wrote:
> Be consistent in handling of IFLA_IFNAME attribute in all places: if
> there is no attribute report bug to stderr and use ll_idx_n2a() as
> last measure to get name in "if%u" format instead of "<nil>".
>
> Use check_ifname() to validate network device name: this catches both
> unexpected return from kernel and ll_idx_n2a().
>
> Signed-off-by: Serhey Popovych <serhe.popov...@gmail.com>
> ---
> bridge/link.c | 8 ++++----
> include/utils.h | 1 +
> ip/ipaddress.c | 20 ++++++++------------
> lib/utils.c | 19 +++++++++++++++++++
> 4 files changed, 32 insertions(+), 16 deletions(-)
>
> diff --git a/bridge/link.c b/bridge/link.c
> index 870ebe0..a11cbb1 100644
> --- a/bridge/link.c
> +++ b/bridge/link.c
> @@ -99,9 +99,10 @@ int print_linkinfo(const struct sockaddr_nl *who,
> struct nlmsghdr *n, void *arg)
> {
> FILE *fp = arg;
> - int len = n->nlmsg_len;
> struct ifinfomsg *ifi = NLMSG_DATA(n);
> struct rtattr *tb[IFLA_MAX+1];
> + int len = n->nlmsg_len;
> + const char *name;
>
> len -= NLMSG_LENGTH(sizeof(*ifi));
> if (len < 0) {
> @@ -117,10 +118,9 @@ int print_linkinfo(const struct sockaddr_nl *who,
>
> parse_rtattr_flags(tb, IFLA_MAX, IFLA_RTA(ifi), len, NLA_F_NESTED);
>
> - if (tb[IFLA_IFNAME] == NULL) {
> - fprintf(stderr, "BUG: nil ifname\n");
> + name = get_ifname_rta(ifi->ifi_index, tb[IFLA_IFNAME]);
> + if (!name)
> return -1;
> - }
>
> if (n->nlmsg_type == RTM_DELLINK)
> fprintf(fp, "Deleted ");
> diff --git a/include/utils.h b/include/utils.h
> index 867e540..84ca873 100644
> --- a/include/utils.h
> +++ b/include/utils.h
> @@ -183,6 +183,7 @@ void duparg(const char *, const char *)
> __attribute__((noreturn));
> void duparg2(const char *, const char *) __attribute__((noreturn));
> int check_ifname(const char *);
> int get_ifname(char *, const char *);
> +const char *get_ifname_rta(int ifindex, const struct rtattr *rta);
> int matches(const char *arg, const char *pattern);
> int inet_addr_match(const inet_prefix *a, const inet_prefix *b, int bits);
> int inet_addr_match_rta(const inet_prefix *m, const struct rtattr *rta);
> diff --git a/ip/ipaddress.c b/ip/ipaddress.c
> index 749178d..08d2576 100644
> --- a/ip/ipaddress.c
> +++ b/ip/ipaddress.c
> @@ -776,12 +776,10 @@ int print_linkinfo_brief(const struct sockaddr_nl *who,
> return -1;
>
> parse_rtattr(tb, IFLA_MAX, IFLA_RTA(ifi), len);
> - if (tb[IFLA_IFNAME] == NULL) {
> - fprintf(stderr, "BUG: device with ifindex %d has nil ifname\n",
> ifi->ifi_index);
> - name = ll_idx_n2a(ifi->ifi_index);
> - } else {
> - name = rta_getattr_str(tb[IFLA_IFNAME]);
> - }
> +
> + name = get_ifname_rta(ifi->ifi_index, tb[IFLA_IFNAME]);
> + if (!name)
> + return -1;
>
> if (filter.label &&
> (!filter.family || filter.family == AF_PACKET) &&
> @@ -903,12 +901,10 @@ int print_linkinfo(const struct sockaddr_nl *who,
> return -1;
>
> parse_rtattr(tb, IFLA_MAX, IFLA_RTA(ifi), len);
> - if (tb[IFLA_IFNAME] == NULL) {
> - fprintf(stderr, "BUG: device with ifindex %d has nil ifname\n",
> ifi->ifi_index);
> - name = ll_idx_n2a(ifi->ifi_index);
> - } else {
> - name = rta_getattr_str(tb[IFLA_IFNAME]);
> - }
> +
> + name = get_ifname_rta(ifi->ifi_index, tb[IFLA_IFNAME]);
> + if (!name)
> + return -1;
>
> if (filter.label &&
> (!filter.family || filter.family == AF_PACKET) &&
> diff --git a/lib/utils.c b/lib/utils.c
> index d86c2ee..572d42a 100644
> --- a/lib/utils.c
> +++ b/lib/utils.c
> @@ -871,6 +871,25 @@ int get_ifname(char *buf, const char *name)
> return ret;
> }
>
> +const char *get_ifname_rta(int ifindex, const struct rtattr *rta)
> +{
> + const char *name;
> +
> + if (rta) {
> + name = rta_getattr_str(rta);
> + } else {
> + fprintf(stderr,
> + "BUG: device with ifindex %d has nil ifname\n",
> + ifindex);
> + name = ll_idx_n2a(ifindex);
> + }
> +
> + if (check_ifname(name))
> + return NULL;
> +
> + return name;
> +}
> +
> int matches(const char *cmd, const char *pattern)
> {
> int len = strlen(cmd);
>
Getting build failures with this one:
$ make clean; make -j 8
...
devlink
CC devlink.o
CC mnlg.o
LINK devlink
../lib/libutil.a(ll_map.o): In function `ll_remember_index':
ll_map.c:(.text+0xf6): undefined reference to `parse_rtattr'
ll_map.c:(.text+0x1c5): undefined reference to `parse_rtattr'
../lib/libutil.a(ll_map.o): In function `ll_init_map':
ll_map.c:(.text+0x47c): undefined reference to `rtnl_wilddump_request'
ll_map.c:(.text+0x493): undefined reference to `rtnl_dump_filter_nc'
