[libvirt] [PATCH v3 1/4] util: introduce helper to parse /proc/net/arp

[Peter Krempa]

On Mon, Jan 29, 2018 at 16:35:33 +0800, Chen Hanxiao wrote:
> From: Chen Hanxiao <chenhanxiao at gmail.com>
> 
> introduce helper to parse /proc/net/arp and
> store it in struct virArpTable.
> 
> Signed-off-by: Chen Hanxiao <chenhanxiao at gmail.com>
> ---
> v3:
>   s/virGetArpTable/virArpTableGet
>   alloc virArpTable in virArpTableGet
>   return ENOSUPP on none-Linux platform
>   move helpers to virarptable.[ch]
> 
>  po/POTFILES.in           |   1 +
>  src/Makefile.am          |   1 +
>  src/libvirt_private.syms |   5 +++
>  src/util/virarptable.c   | 114 +++++++++++++++++++++++++++++++++++++++++++++++
>  src/util/virarptable.h   |  48 ++++++++++++++++++++
>  5 files changed, 169 insertions(+)
>  create mode 100644 src/util/virarptable.c
>  create mode 100644 src/util/virarptable.h
> 
> diff --git a/po/POTFILES.in b/po/POTFILES.in
> index 8382ee633..91a4c5730 100644
> --- a/po/POTFILES.in
> +++ b/po/POTFILES.in
> @@ -193,6 +193,7 @@ src/uml/uml_conf.c
>  src/uml/uml_driver.c
>  src/util/iohelper.c
>  src/util/viralloc.c
> +src/util/virarptable.c
>  src/util/viraudit.c
>  src/util/virauth.c
>  src/util/virauthconfig.c
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 289b03747..923eea6fe 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -98,6 +98,7 @@ augeastest_DATA =
>  UTIL_SOURCES = \
>  		util/viralloc.c util/viralloc.h \
>  		util/virarch.h util/virarch.c \
> +		util/virarptable.h util/virarptable.c \
>  		util/viratomic.h util/viratomic.c \
>  		util/viraudit.c util/viraudit.h \
>  		util/virauth.c util/virauth.h \
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index f2a2c8650..08229797a 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1332,6 +1332,11 @@ virArchGetWordSize;
>  virArchToString;
>  
>  
> +# util/virarptable.h
> +virArpTableFree;
> +virArpTableGet;
> +
> +
>  # util/viraudit.h
>  virAuditClose;
>  virAuditEncode;
> diff --git a/src/util/virarptable.c b/src/util/virarptable.c
> new file mode 100644
> index 000000000..48a38352f
> --- /dev/null
> +++ b/src/util/virarptable.c
> @@ -0,0 +1,114 @@
> +/*
> + * virarptable.c Linux ARP table handling
> + *
> + * Copyright (C) 2018 Chen Hanxiao
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library.  If not, see
> + * <http://www.gnu.org/licenses/>.
> + *
> + * Authors:
> + *     Chen Hanxiao <chenhanxiao at gmail.com>
> + */
> +
> +#include <config.h>
> +
> +#include <stdlib.h>
> +#include <stdio.h>
> +
> +#include "viralloc.h"
> +#include "virarptable.h"
> +#include "virfile.h"
> +#include "virstring.h"
> +
> +#define VIR_FROM_THIS VIR_FROM_NONE
> +
> +#ifdef __linux__
> +
> +virArpTablePtr virArpTableGet(void)
> +{
> +    FILE *fp = NULL;
> +    char line[1024];
> +    int num = 0;
> +    virArpTablePtr table = NULL;
> +
> +    if (VIR_ALLOC(table) < 0)
> +        return NULL;
> +
> +    if (!(fp = fopen("/proc/net/arp", "r")))
> +        goto cleanup;
> +
> +    while (fgets(line, sizeof(line), fp)) {
> +        char ip[32], mac[32], dev_name[32], hwtype[32],

I don't think hardcoding the lenghts ...



> +             flags[32], mask[32], nouse[32];
> +
> +        if (STRPREFIX(line, "IP address"))
> +            continue;
> +
> +        if (VIR_REALLOC_N(table->t, num + 1) < 0)
> +            goto cleanup;
> +
> +        table->n = num + 1;
> +
> +        /* /proc/net/arp looks like:
> +         * 172.16.17.254  0x1 0x2  e4:68:a3:8d:ed:d3  *   enp3s0
> +         */
> +        sscanf(line, "%[0-9.]%[ ]%[^ ]%[ ]%[^ ]%[ ]%[^ ]%[ ]%[^ ]%[ ]%[^ \t\n]",


without limiting the size here is a great idea. That is a buffer
overflow right here.

Also parsing /proc/net/arp is not enough, it will not list IPv6
neighbors.

Additionally I'd stay away from parsing this file completely. Not even
the obsolete 'arp' utility is using this but rather uses AF_NETLINK
socket to discover it.

NACK on this approach.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180129/335a27ee/attachment-0001.sig>