From patchwork Sat Jun 17 16:40:25 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sergey Bugaev X-Patchwork-Id: 1796194 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=sourceware.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=sourceware.org; envelope-from=libc-alpha-bounces+incoming=patchwork.ozlabs.org@sourceware.org; receiver=) Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=og+yUUHJ; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4Qk1xK6L2zz20ZG for ; Sun, 18 Jun 2023 02:40:50 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id A87A53858291 for ; Sat, 17 Jun 2023 16:40:48 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org A87A53858291 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1687020048; bh=jU5pmyMNO9FiobckIdssjqSaVLF9dlCA2xwOEmgYWME=; h=To:Cc:Subject:Date:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=og+yUUHJnLH02jWz3/fUVs8tGQCth9lHAKUvQWoXn9rCI+wSwV9DFcbAPeNZP/51y 7QfuA6W0cLofp6VdBQIxU4deeMw7jyBHbwu0y3ROIrAm4re/prqbkGwGouJnJAkQUM 1o9hy8V6OFJq3I4gWJ+U89gRYVoXq3BMtZvQoNoM= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-lj1-x233.google.com (mail-lj1-x233.google.com [IPv6:2a00:1450:4864:20::233]) by sourceware.org (Postfix) with ESMTPS id 08ACA3858D35 for ; Sat, 17 Jun 2023 16:40:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 08ACA3858D35 Received: by mail-lj1-x233.google.com with SMTP id 38308e7fff4ca-2b46cad2fd9so4392191fa.1 for ; Sat, 17 Jun 2023 09:40:30 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1687020029; x=1689612029; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=jU5pmyMNO9FiobckIdssjqSaVLF9dlCA2xwOEmgYWME=; b=lDuOUk+8C1Tv6JeNBwLT3bBQJtrMVhUndrAi1SypTph2xamw9TJ3UuOGF22tZw94Pj 6obcWbwrtDsLXDBNlyZ+pt7UQ2EdPtPVMFxbCDGPdui9NiPz/mMMUhJjeikDCFY5HYAz gPLWyz0z11W4tehtzbqT1Xhy4f9C2tqMbD4yeCWMqmWNB6jrh+hybLHCheuChY3KoKXX FPQ7uvBMlsStmJtjeBB+vKHHZODtBFiilrHSPbIjVtfSx7nK6FMaHHXvc4OqKF7760dI Gv6Yt8gCyvurwkO5+PQgiMx4UVDL+gYRuAGbjteVX6z8Le88+83l0cjuTCwQM22/fN25 6DRQ== X-Gm-Message-State: AC+VfDzYAp1k46odQEHgt3A/9l709GK34cPEi/0fpGnRsjvD5G3SiXr5 /D0b6ER6SuTv6UmCMZHZU8Oo4T7747U= X-Google-Smtp-Source: ACHHUZ45BSZTgyv6Z2U8MM1wMA2BEOVmuke2YBn5raQ7H/uoWFyC6ccTVYukbAlFIRbEU+kSMBa+8A== X-Received: by 2002:a2e:95c2:0:b0:2b3:43e6:4335 with SMTP id y2-20020a2e95c2000000b002b343e64335mr3917901ljh.11.1687020028571; Sat, 17 Jun 2023 09:40:28 -0700 (PDT) Received: from surface-pro-6.. ([2a00:1370:818c:173b:cc34:4e0d:9ea6:c16c]) by smtp.gmail.com with ESMTPSA id d3-20020a2e8903000000b002b4430dda25sm1559251lji.92.2023.06.17.09.40.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 17 Jun 2023 09:40:27 -0700 (PDT) To: libc-alpha@sourceware.org Cc: Adhemerval Zanella , Florian Weimer Subject: [PATCH v3 1/2] elf: Port ldconfig away from stack-allocated paths Date: Sat, 17 Jun 2023 19:40:25 +0300 Message-ID: <20230617164026.124004-1-bugaevc@gmail.com> X-Mailer: git-send-email 2.41.0 MIME-Version: 1.0 X-Spam-Status: No, score=-11.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, 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 X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Sergey Bugaev via Libc-alpha From: Sergey Bugaev Reply-To: Sergey Bugaev Errors-To: libc-alpha-bounces+incoming=patchwork.ozlabs.org@sourceware.org Sender: "Libc-alpha" ldconfig was allocating PATH_MAX bytes on the stack for the library file name. The issues with PATH_MAX usage are well documented [0][1]; even if a program does not rely on paths being limited to PATH_MAX bytes, allocating 4096 bytes on the stack for paths that are typically rather short (strlen ("/lib64/libc.so.6") is 16) is wasteful and dangerous. [0]: https://insanecoding.blogspot.com/2007/11/pathmax-simply-isnt.html [1]: https://eklitzke.org/path-max-is-tricky Instead, make use of asprintf to dynamically allocate memory of just the right size on the heap. Reviewed-by: Adhemerval Zanella Reviewed-by: Florian Weimer Signed-off-by: Sergey Bugaev --- The only change since v2: Simplified logic a bit by useing xstrdup + an unconditional free, as suggested by Florian. This is slightly less efficient, but then so is using heap memory instead of alloca, so, fine. I kept Adhemerval's Reviewed-by (again), but I'm not sure if that is the appropriate thing to do. In general, what am I supposed to do when getting a R-b from someone? (other than understanding that the person approves the patch) * Am I supposed to re-send the series with the R-b added and no other changes? (Pushing a new version is what I would do when using a branch-based workflow like that of GitLab merge requests.) * Am I supposed to add the R-b the next time I send the series, but only if I made no changes to that particular patch? * Am I supposed to add the R-b the next time I send the series, but only if I made minor or no changes to that particular patch, and the reviewer can be reasonably assumed to still be happy with it? (This variant makes most sense to me.) * Am I supposed to just change nothing? -- in this case, would the eventual committer (the person who pushes the patch) collect and append the R-b's that the patch has received? Sergey elf/ldconfig.c | 59 +++++++++++++++++++------------------------------- 1 file changed, 22 insertions(+), 37 deletions(-) diff --git a/elf/ldconfig.c b/elf/ldconfig.c index 349b7356..d26eef1f 100644 --- a/elf/ldconfig.c +++ b/elf/ldconfig.c @@ -677,28 +677,18 @@ search_dir (const struct dir_entry *entry) char *dir_name; char *real_file_name; - size_t real_file_name_len; - size_t file_name_len = PATH_MAX; - char *file_name = alloca (file_name_len); + char *file_name; if (opt_chroot != NULL) - { - dir_name = chroot_canon (opt_chroot, entry->path); - real_file_name_len = PATH_MAX; - real_file_name = alloca (real_file_name_len); - } + dir_name = chroot_canon (opt_chroot, entry->path); else - { - dir_name = entry->path; - real_file_name_len = 0; - real_file_name = file_name; - } + dir_name = entry->path; DIR *dir; if (dir_name == NULL || (dir = opendir (dir_name)) == NULL) { if (opt_verbose) error (0, errno, _("Can't open directory %s"), entry->path); - if (opt_chroot != NULL && dir_name != NULL) + if (opt_chroot != NULL) free (dir_name); return; } @@ -733,25 +723,16 @@ search_dir (const struct dir_entry *entry) + 1, ".#prelink#.", sizeof (".#prelink#.") - 1) == 0) continue; } - len += strlen (entry->path) + 2; - if (len > file_name_len) - { - file_name_len = len; - file_name = alloca (file_name_len); - if (!opt_chroot) - real_file_name = file_name; - } - sprintf (file_name, "%s/%s", entry->path, direntry->d_name); + if (asprintf (&file_name, "%s/%s", entry->path, direntry->d_name) < 0) + error (EXIT_FAILURE, errno, _("Could not form library path")); if (opt_chroot != NULL) { - len = strlen (dir_name) + strlen (direntry->d_name) + 2; - if (len > real_file_name_len) - { - real_file_name_len = len; - real_file_name = alloca (real_file_name_len); - } - sprintf (real_file_name, "%s/%s", dir_name, direntry->d_name); + if (asprintf (&real_file_name, "%s/%s", + dir_name, direntry->d_name) < 0) + error (EXIT_FAILURE, errno, _("Could not form library path")); } + else + real_file_name = xstrdup (file_name); struct stat lstat_buf; /* We optimize and try to do the lstat call only if needed. */ @@ -761,7 +742,7 @@ search_dir (const struct dir_entry *entry) if (__glibc_unlikely (lstat (real_file_name, &lstat_buf))) { error (0, errno, _("Cannot lstat %s"), file_name); - continue; + goto next; } struct stat stat_buf; @@ -778,7 +759,7 @@ search_dir (const struct dir_entry *entry) { if (strstr (file_name, ".so") == NULL) error (0, 0, _("Input file %s not found.\n"), file_name); - continue; + goto next; } } if (__glibc_unlikely (stat (target_name, &stat_buf))) @@ -793,7 +774,7 @@ search_dir (const struct dir_entry *entry) if (opt_chroot != NULL) free (target_name); - continue; + goto next; } if (opt_chroot != NULL) @@ -806,7 +787,7 @@ search_dir (const struct dir_entry *entry) lstat_buf.st_ctime = stat_buf.st_ctime; } else if (!S_ISREG (lstat_buf.st_mode)) - continue; + goto next; char *real_name; if (opt_chroot != NULL && is_link) @@ -816,7 +797,7 @@ search_dir (const struct dir_entry *entry) { if (strstr (file_name, ".so") == NULL) error (0, 0, _("Input file %s not found.\n"), file_name); - continue; + goto next; } } else @@ -828,7 +809,7 @@ search_dir (const struct dir_entry *entry) && __builtin_expect (lstat (real_file_name, &lstat_buf), 0)) { error (0, errno, _("Cannot lstat %s"), file_name); - continue; + goto next; } /* First search whether the auxiliary cache contains this @@ -842,7 +823,7 @@ search_dir (const struct dir_entry *entry) { if (real_name != real_file_name) free (real_name); - continue; + goto next; } else if (opt_build_cache) add_to_aux_cache (&lstat_buf, flag, isa_level, soname); @@ -948,6 +929,10 @@ search_dir (const struct dir_entry *entry) dlib_ptr->next = dlibs; dlibs = dlib_ptr; } + + next: + free (file_name); + free (real_file_name); } closedir (dir);