From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg1-x531.google.com (mail-pg1-x531.google.com [IPv6:2607:f8b0:4864:20::531]) by sourceware.org (Postfix) with ESMTPS id 689D23858D28 for ; Tue, 3 Oct 2023 16:59:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 689D23858D28 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-pg1-x531.google.com with SMTP id 41be03b00d2f7-584bfb14c59so794298a12.0 for ; Tue, 03 Oct 2023 09:59:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1696352349; x=1696957149; darn=sourceware.org; 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=ZvaRW2BR4ozPCGqKbQj/SJNdZWex3CqJ4w+orWbN+Eo=; b=f6ZlCW88ZrPRogzpGAW7GqqWrJ85seJXx5o1bbACFL3Roux6fg6OZbt3Lc2556K26K edBYyjGWAIMun6uuxOBmPK/8qGkFE6LenCbFQOei7Akd4BlA57RWJf/nRV57JCYXplQ7 vrkeJnZSPJzJqJMR+op4e7H4bZ5SMALdj7guXW9JJiEUiVIlatTQWGaOVGM+JP+kdHxc qoqrFNrC6+1suOI8Hd8NpxyGUKTZvPEip59ucGsgoPdn+K/Y9tPJ04lC3MX9PM/gzXdy CjWVYN+yqvo0VAPUXArv+qGm5FNQ2u4I0KHkGDe1iiaJKcdXm04j0Mr2X+tt4vFLsBhm iQKA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696352349; x=1696957149; 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=ZvaRW2BR4ozPCGqKbQj/SJNdZWex3CqJ4w+orWbN+Eo=; b=g48aij39bzGS5Dm+n6OpcrU1/egXqrIw/Tna7TfyUbOCcwyP5woBz6RWACSHAQsZU4 ziyYoF+2TVFuwC/AGoTELlmzaY+RI6/16rhpYU8rhYI7MltAO6Ab31HzFcUllJlKIYyY BDie7AMLHFjXxcCVuAhxUseGQXCogBCxB3ZHcVTKmcBnC3P8VXbf8FGbBBawwfpaR/hy YUwoapk8Ts+7EAp1ANYFuKTjyCwVXRMKwxjVkRZxVM0jqvVr+HKDFjO42WnIBOOKicdZ Ip8L1GogX0vUkqdNOb7bm5n5EdPAcnRxmgaGiQ5mpojFmRdeFeDq0mb+uKYpRks5rGTZ dCgw== X-Gm-Message-State: AOJu0YzzKUXZDvm+SnKqH9WE+wiJHQtew9PPDARyKN0+NCph05GZl3vr dmQL6DASKcUoUQslZRwNhSzD9g== X-Google-Smtp-Source: AGHT+IEzLGLF8xmIsULFSvxIKE8tKX7Z75fiADkIZu83vwRW+lUlFyo5p4AVoN7atMSRwrHg4lFDkw== X-Received: by 2002:a05:6a20:441c:b0:12f:c0c1:d70 with SMTP id ce28-20020a056a20441c00b0012fc0c10d70mr32679pzb.40.1696352349431; Tue, 03 Oct 2023 09:59:09 -0700 (PDT) Received: from ?IPV6:2804:1b3:a7c1:feaf:c9f1:61ab:649c:ad56? ([2804:1b3:a7c1:feaf:c9f1:61ab:649c:ad56]) by smtp.gmail.com with ESMTPSA id f11-20020a17090274cb00b001c0c86a5415sm1817596plt.154.2023.10.03.09.59.07 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 03 Oct 2023 09:59:08 -0700 (PDT) Message-ID: Date: Tue, 3 Oct 2023 13:59:06 -0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3] Fix off-by-one OOB write in iconv/tst-iconv-mt Content-Language: en-US To: Szabolcs Nagy , libc-alpha@sourceware.org References: <20231002141631.1882760-1-szabolcs.nagy@arm.com> From: Adhemerval Zanella Netto Organization: Linaro In-Reply-To: <20231002141631.1882760-1-szabolcs.nagy@arm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP 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 02/10/23 11:16, Szabolcs Nagy wrote: > The iconv buffer sizes must not include the \0 string terminator. > And the output termination with *outbufpos = '\0' was OOB. > > Consistently use non-null-terminated buffer sizes. LGTM, valgrind does show the issue: ==3486612== Thread 9: ==3486612== Invalid write of size 1 ==3486612== at 0x10A78F: worker (tst-iconv-mt.c:94) ==3486612== by 0x48E3FCB: start_thread (pthread_create.c:444) ==3486612== by 0x496DFFF: clone (clone.S:100) ==3486612== Address 0x52579ae is 0 bytes after a block of size 14 alloc'd ==3486612== at 0x4845A83: calloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==3486612== by 0x10B6E0: xcalloc (xcalloc.c:30) ==3486612== by 0x10A70A: worker (tst-iconv-mt.c:63) ==3486612== by 0x48E3FCB: start_thread (pthread_create.c:444) ==3486612== by 0x496DFFF: clone (clone.S:100) Reviewed-by: Adhemerval Zanella > > --- > v2: dropped \0 and replaced strncmp with TEST_COMPARE_BLOB. > v3: unchanged. (rebase) > --- > iconv/tst-iconv-mt.c | 15 +++++---------- > 1 file changed, 5 insertions(+), 10 deletions(-) > > diff --git a/iconv/tst-iconv-mt.c b/iconv/tst-iconv-mt.c > index e634eec1b7..8d7867b323 100644 > --- a/iconv/tst-iconv-mt.c > +++ b/iconv/tst-iconv-mt.c > @@ -57,12 +57,13 @@ worker (void * arg) > iconv_t cd; > > char ascii[] = CONV_INPUT; > + size_t bytes = sizeof (CONV_INPUT) - 1; > char *inbufpos = ascii; > - size_t inbytesleft = sizeof (CONV_INPUT); > + size_t inbytesleft = bytes; > > - char *utf8 = xcalloc (sizeof (CONV_INPUT), 1); > + char *utf8 = xcalloc (bytes, 1); > char *outbufpos = utf8; > - size_t outbytesleft = sizeof (CONV_INPUT); > + size_t outbytesleft = bytes; > > if (tidx < TCOUNT/2) > /* The first half of the worker thread pool synchronize together here, > @@ -91,8 +92,6 @@ worker (void * arg) > &outbytesleft) > != (size_t) -1); > > - *outbufpos = '\0'; > - > xpthread_barrier_wait (&sync); > > TEST_VERIFY_EXIT (iconv_close (cd) == 0); > @@ -104,11 +103,7 @@ worker (void * arg) > if (tidx < TCOUNT/2) > xpthread_barrier_wait (&sync); > > - if (strncmp (utf8, CONV_INPUT, sizeof CONV_INPUT)) > - { > - printf ("FAIL: thread %lx: invalid conversion output from iconv\n", tidx); > - pthread_exit ((void *) (long int) 1); > - } > + TEST_COMPARE_BLOB (utf8, bytes, CONV_INPUT, bytes); > > pthread_exit (NULL); > }