[libvirt] [PATCH] util: Fix stack smashing in virNetDevGetFamilyId

[Erik Skultety]

On Wed, Sep 20, 2017 at 12:48:01PM +0200, Jiri Denemark wrote:
> After commit 8708ca01c0d libvirtd consistently aborts with
> "stack smashing detected" when nodedev driver is initialized.
>
> Apparently this is caused by nlmsg_parse trying to write more than
> CTRL_ATTR_MAX bytes in tb because it is told tb can accommodate
> CTRL_CMD_MAX bytes.
>
> Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
> ---
>  src/util/virnetdev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
> index 41a659732b..be8f4b7938 100644
> --- a/src/util/virnetdev.c
> +++ b/src/util/virnetdev.c
> @@ -3158,7 +3158,7 @@ virNetDevGetFamilyId(const char *family_name)
>      struct nl_msg *nl_msg = NULL;
>      struct nlmsghdr *resp = NULL;
>      struct genlmsghdr* gmsgh = NULL;
> -    struct nlattr *tb[CTRL_ATTR_MAX + 1] = {NULL, };
> +    struct nlattr *tb[CTRL_CMD_MAX + 1] = {NULL, };

Shouldn't the correct fix be to instruct nlmsg_parse to accommodate up to
CTRL_ATTR_MAX bytes instead? Judging from the nature of the enum in getlink.h
it appears to me that that's the correct enum we want to use, instead of the
CTR_CMD_ enum ??

I'm inclined to say ACK, but let me know what you think of the enum first.

Erik