On Fri, Oct 25, 2019 at 10:16:14AM +0200, Klemens Nanni wrote:
> The function argument is not checked at all and strlcpy(3) will segfault
> if it NULL.
>
> Slightly adjust the comment to reflect this and defer the error case's
> default value to the end: If a node is successfully opened, ifname gets
> rewritten.
>
> There's only one usage of this function in config.c which passes a
> non-NULL buffer and always expects ifname to be non-NULL and non-empty.
>
> config.c:config_setwm()
> 225 char ifname[IF_NAMESIZE], *s;
> ...
> 425 if (s != NULL) {
> 426 snprintf(path, PATH_MAX, "/dev/%s", s);
> 427 tapfds[i] = open(path, O_RDWR | O_NONBLOCK);
> 428 } else {
> 429 tapfds[i] = opentap(ifname);
> 430 s = ifname;
> 431 }
> 432 if (tapfds[i] == -1) {
> 433 log_warn("%s: can't open tap %s", __func__, s);
> 434 goto fail;
> 435 }
>
> OK?
>
You can also remove the "if (ifname != NULL)" check in opentap().
Otherwise OK.
Reyk
> Index: vmm.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/vmd/vmm.c,v
> retrieving revision 1.93
> diff -u -p -r1.93 vmm.c
> --- vmm.c 28 Jun 2019 13:32:51 -0000 1.93
> +++ vmm.c 24 Oct 2019 23:34:18 -0000
> @@ -590,7 +590,7 @@ terminate_vm(struct vm_terminate_params
> * Opens the next available tap device, up to MAX_TAP.
> *
> * Parameters
> - * ifname: an optional buffer of at least IF_NAMESIZE bytes.
> + * ifname: a buffer of at least IF_NAMESIZE bytes.
> *
> * Returns a file descriptor to the tap node opened, or -1 if no tap
> * devices were available.
> @@ -601,7 +601,6 @@ opentap(char *ifname)
> int i, fd;
> char path[PATH_MAX];
>
> - strlcpy(ifname, "tap", IF_NAMESIZE);
> for (i = 0; i < MAX_TAP; i++) {
> snprintf(path, PATH_MAX, "/dev/tap%d", i);
> fd = open(path, O_RDWR | O_NONBLOCK);
> @@ -611,6 +610,7 @@ opentap(char *ifname)
> return (fd);
> }
> }
> + strlcpy(ifname, "tap", IF_NAMESIZE);
>
> return (-1);
> }
>
