From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oa1-x30.google.com (mail-oa1-x30.google.com [IPv6:2001:4860:4864:20::30]) by sourceware.org (Postfix) with ESMTPS id 5781B3858D32 for ; Mon, 29 May 2023 13:15:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 5781B3858D32 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org Received: by mail-oa1-x30.google.com with SMTP id 586e51a60fabf-19f30256921so1734878fac.1 for ; Mon, 29 May 2023 06:15:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1685366137; x=1687958137; h=content-transfer-encoding:in-reply-to:organization:from:references :to:content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=0CqnhNQkcTyrh34+KASQw4lZZ4XXfon/hY4wELYdmXo=; b=rWD75fc2wyvOM39UTYBC9Nhu/Xbb44m5qDYnvfatUcAU/lmZcWSKvU/ihOC56JgwLx rcDjWp35uQ0jYE/zapG1T28EtCIRRs88ktz2AJt01yI3lAY05jlHICC2hw7al7bQq3ZY OngOXQ2xyS0twwgVhCL5VJMCpBI2hzpDjqZHNv08L9e27oOOl3ps/EXqP8QXnzZA8H+h V9YD6gCuMDsB53u/+68SvFdaYDq/L2SDIBW1m04AC/X6fWmP9uEsTfkK/6aDuxG9GWKU 6JHb9Ciag9fMrOgWSLV3zcy2bRPkhZpqqa4dy2vC3vK62u5RbDuOtrO6gms2A+z2XMTW AbXA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1685366137; x=1687958137; h=content-transfer-encoding:in-reply-to:organization:from:references :to:content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=0CqnhNQkcTyrh34+KASQw4lZZ4XXfon/hY4wELYdmXo=; b=L10HVqNo4Cshd+pwo2ENMlNQCZ+1r686wlFLXdLalovfD2uKmvDLcuoi6FEVTmuAcn p8MwTAhinbDtGDuIfAWSkRxoWkDg3IV3H1z9vUraUtQVhJxX6P8RFR8WxOY1kHMsoUvs S9fZ7Iu6C9eG5wmGJuHl6SYpo3ZGTrBudSJ5h71rU/a3bQ0Qp9dFzBcHmpAU/XRvZWMb ZUODiowM2w4YFm4jT5akBEiwQSSWa7ibLDV7fIQdFoMX8gEW76vcEyIFWklgpl8lcMPe T25volXbQeJ1E64gyiMNNOB/R7ZaacR1Ex8AwhjezFfZgiCtr+UU04XzK/xR42TU3CEZ pG3Q== X-Gm-Message-State: AC+VfDzGdaau+BXBEMw/gZ98u2XkhMksgY8pi1bq7skcCxa8DbDNNCaU QfuwSmnmTf/Gcq6CWIucjf1YaQ== X-Google-Smtp-Source: ACHHUZ5jr/LpG1g2jHKy8ncS1suJtngxvd+ViTk3P8ArWGDizpjW/SpE+Y45MYagFEsZlAj62DbdBg== X-Received: by 2002:a05:6808:8ca:b0:398:523a:f1ee with SMTP id k10-20020a05680808ca00b00398523af1eemr5294913oij.21.1685366136841; Mon, 29 May 2023 06:15:36 -0700 (PDT) Received: from ?IPV6:2804:1b3:a7c1:4dd5:7120:a40d:fc97:d6db? ([2804:1b3:a7c1:4dd5:7120:a40d:fc97:d6db]) by smtp.gmail.com with ESMTPSA id z6-20020a544586000000b00397c07e8061sm4389963oib.6.2023.05.29.06.15.35 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 29 May 2023 06:15:35 -0700 (PDT) Message-ID: Date: Mon, 29 May 2023 10:15:33 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.11.1 Subject: Re: [PATCH v2] ifaddrs: Get rid of alloca Content-Language: en-US To: Joe Simmons-Talbott , libc-alpha@sourceware.org References: <20230526165419.1543475-1-josimmon@redhat.com> From: Adhemerval Zanella Netto Organization: Linaro In-Reply-To: <20230526165419.1543475-1-josimmon@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 26/05/23 13:54, Joe Simmons-Talbott via Libc-alpha wrote: > Use scratch_buffer and malloc rather than alloca to avoid potential stack > overflows. > --- > Changes to v1: > * in __netlink_request use an 8kb buffer size and malloc rather than a > scratch_buffer. > > Suggested-by: Adhemerval Zanella Netto > > sysdeps/unix/sysv/linux/ifaddrs.c | 51 +++++++++++++++---------------- > 1 file changed, 25 insertions(+), 26 deletions(-) > > diff --git a/sysdeps/unix/sysv/linux/ifaddrs.c b/sysdeps/unix/sysv/linux/ifaddrs.c > index 184ee224cb..e42da62e9b 100644 > --- a/sysdeps/unix/sysv/linux/ifaddrs.c > +++ b/sysdeps/unix/sysv/linux/ifaddrs.c > @@ -16,13 +16,13 @@ > License along with the GNU C Library; if not, see > . */ > > -#include > #include > #include > #include > #include > #include > #include > +#include > #include > #include > #include > @@ -122,6 +122,13 @@ __netlink_sendreq (struct netlink_handle *h, int type) > } > > > +static void > +ifree (char **ptr) > +{ > + free (*ptr); > +} > + > + > int > __netlink_request (struct netlink_handle *h, int type) > { > @@ -131,26 +138,14 @@ __netlink_request (struct netlink_handle *h, int type) > ssize_t read_len; > bool done = false; > > -#ifdef PAGE_SIZE > - /* Help the compiler optimize out the malloc call if PAGE_SIZE > - is constant and smaller or equal to PTHREAD_STACK_MIN/4. */ > - const size_t buf_size = PAGE_SIZE; > -#else > - const size_t buf_size = __getpagesize (); > -#endif > - bool use_malloc = false; > - char *buf; > - > - if (__libc_use_alloca (buf_size)) > - buf = alloca (buf_size); > - else > - { > - buf = malloc (buf_size); > - if (buf != NULL) > - use_malloc = true; > - else > - goto out_fail; > - } > + /* Netlink requires that user buffer needs to be either 8kb or page size > + (whichever is bigger), however this has been changed over time and now > + 8Kb is sufficient (check NLMSG_DEFAULT_SIZE on Linux > + linux/include/linux/netlink.h). */ > + const size_t buf_size = 8192; > + char *buf __attribute__ ((__cleanup__ (ifree))) = malloc (buf_size); > + if (buf == NULL) > + return -1; > > struct iovec iov = { buf, buf_size }; > > @@ -229,13 +224,9 @@ __netlink_request (struct netlink_handle *h, int type) > h->end_ptr = nlm_next; > } > > - if (use_malloc) > - free (buf); > return 0; > > out_fail: > - if (use_malloc) > - free (buf); > return -1; > } > > @@ -324,6 +315,7 @@ getifaddrs_internal (struct ifaddrs **ifap) > char *ifa_data_ptr; /* Pointer to the unused part of memory for > ifa_data. */ > int result = 0; > + struct scratch_buffer buf; Move the scratch_buffer_init here, otherwise previous failures (such as the calloc) will trigger invalid memory access by the scratch_buffer_free. The rest looks ok. > > *ifap = NULL; > > @@ -425,7 +417,13 @@ getifaddrs_internal (struct ifaddrs **ifap) > } > > /* Table for mapping kernel index to entry in our list. */ > - map_newlink_data = alloca (newlink * sizeof (int)); > + scratch_buffer_init (&buf)> + if (!scratch_buffer_set_array_size (&buf, 1, newlink * sizeof (int))) > + { > + result = -1; > + goto exit_free; > + } > + map_newlink_data = buf.data; > memset (map_newlink_data, '\xff', newlink * sizeof (int)); > > ifa_data_ptr = (char *) &ifas[newlink + newaddr]; > @@ -820,6 +818,7 @@ getifaddrs_internal (struct ifaddrs **ifap) > exit_free: > __netlink_free_handle (&nh); > __netlink_close (&nh); > + scratch_buffer_free (&buf); > > return result; > }