Re: [PATCH] ifconfig: keep track of allowed ips pointer correctly

[Klemens Nanni]

On Tue, Oct 27, 2020 at 06:16:08PM +0100, Jason A. Donenfeld wrote:
> Somebody on IRC mentioned that using ifconfig to set wgallowedips wasn't
> working on macppc. I don't have a macppc to test this on, but it seems
> like the code is assuming that the two values printed out by this test
> program must always be the same:
> 
>   struct s {
>       int i;
>   };
> 
>   struct p {
>       long l;
>       char c;
>       struct s a[];
>   };
> 
>   int main(int argc, char *argv[])
>   {
>       printf("%zu %zu\n", sizeof(struct p), (size_t)&((struct p *)0)->a[0]);
>       return 0;
>   }
> 
> But actually, on my amd64 system, that little test prints out "16 12".
> This patch fixes up ifconfig.c to do the right thing, so that it
> corresponds with how the kernel handles iteration.
> 
> I don't have a macppc in order to test this, but it works on amd64.
Using the wg(4) EXAMPLES section as test, this fixes wg(4) on macppc.
amd64 continues to work for me as well.

FWIW, both platforms produced the same ifconfig.o regardless of using a
void pointer or char pointer cast as guenther suggested.  I don't know
if `char *' might be beneficial on other platform/compiler combinations.

OK?
Otherwise I'll commit this on friday unless I hear objections or further
feedback.

Jason's diff (with `char *' as per guenther) reattached below.


Index: ifconfig.c
===================================================================
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.429
diff -u -p -r1.429 ifconfig.c
--- ifconfig.c  7 Oct 2020 14:38:54 -0000       1.429
+++ ifconfig.c  4 Nov 2020 19:35:20 -0000
@@ -5696,11 +5696,10 @@ ensurewginterface(void)
                err(1, "calloc");
 }
 
-void *
+void
 growwgdata(size_t by)
 {
        ptrdiff_t peer_offset, aip_offset;
-       void *ret;
 
        if (wg_interface == NULL)
                wgdata.wgd_size = sizeof(*wg_interface);
@@ -5721,16 +5720,18 @@ growwgdata(size_t by)
        if (wg_aip != NULL)
                wg_aip = (void *)wg_interface + aip_offset;
 
-       ret = (void *)wg_interface + wgdata.wgd_size - by;
-       bzero(ret, by);
-
-       return ret;
+       bzero((char *)wg_interface + wgdata.wgd_size - by, by);
 }
 
 void
 setwgpeer(const char *peerkey_b64, int param)
 {
-       wg_peer = growwgdata(sizeof(*wg_peer));
+       growwgdata(sizeof(*wg_peer));
+       if (wg_aip)
+               wg_peer = (struct wg_peer_io *)wg_aip;
+       else
+               wg_peer = &wg_interface->i_peers[0];
+       wg_aip = &wg_peer->p_aips[0];
        wg_peer->p_flags |= WG_PEER_HAS_PUBLIC;
        WG_LOAD_KEY(wg_peer->p_public, peerkey_b64, "wgpeer");
        wg_interface->i_peers_count++;
@@ -5743,7 +5744,7 @@ setwgpeeraip(const char *aip, int param)
        if (wg_peer == NULL)
                errx(1, "wgaip: wgpeer not set");
 
-       wg_aip = growwgdata(sizeof(*wg_aip));
+       growwgdata(sizeof(*wg_aip));
 
        if ((res = inet_net_pton(AF_INET, aip, &wg_aip->a_ipv4,
            sizeof(wg_aip->a_ipv4))) != -1) {
@@ -5759,6 +5760,8 @@ setwgpeeraip(const char *aip, int param)
 
        wg_peer->p_flags |= WG_PEER_REPLACE_AIPS;
        wg_peer->p_aips_count++;
+
+       wg_aip++;
 }
 
 void