From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-Status: No, score=-3.8 required=3.0 tests=AWL,BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_PASS, SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by dcvr.yhbt.net (Postfix) with ESMTP id DA8061F9FE for ; Wed, 24 Feb 2021 21:06:02 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233178AbhBXVFv (ORCPT ); Wed, 24 Feb 2021 16:05:51 -0500 Received: from cloud.peff.net ([104.130.231.41]:43888 "EHLO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232147AbhBXVFv (ORCPT ); Wed, 24 Feb 2021 16:05:51 -0500 Received: (qmail 2047 invoked by uid 109); 24 Feb 2021 21:05:10 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Wed, 24 Feb 2021 21:05:10 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 19218 invoked by uid 111); 24 Feb 2021 21:05:09 -0000 Received: from coredump.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Wed, 24 Feb 2021 16:05:09 -0500 Authentication-Results: peff.net; auth=none Date: Wed, 24 Feb 2021 16:05:09 -0500 From: Jeff King To: Junio C Hamano Cc: Jeff Hostetler via GitGitGadget , git@vger.kernel.org, Jeff Hostetler Subject: Re: [PATCH] dir: fix malloc of root untracked_cache_dir Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org On Wed, Feb 24, 2021 at 12:08:42PM -0800, Junio C Hamano wrote: > > Use FLEX_ALLOC_STR() to allocate the `struct untracked_cache_dir` > > for the root directory. Get rid of unsafe code that might fail to > > initialize the `name` field (if FLEX_ARRAY is not 1). This will > > make it clear that we intend to have a structure with an empty > > string following it. > [...] > The problematic code was introduced in 2015, a year before these > FLEX_ALLOC_*() helpers were introduced. The result is of course > correct and much easier to read, as the necessary "ask for a region > of calloc'ed memory with an additional byte for terminating NUL > beyond strlen()" is hidden in the helper. When I added the FLEX_ALLOC_* helpers, I audited for existing callers to convert. But I did so by looking for places where we were doing manual size computations. The bug here was that it was not doing any computation at all (when it need to be doing "+1"). So that's my guess why it got overlooked (which is not super important, but may give a hint about how to look for similar bugs). -Peff