From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) by sourceware.org (Postfix) with ESMTPS id 354DA3858D35 for ; Mon, 6 Jul 2020 14:30:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 354DA3858D35 Received: from pps.filterd (m0187473.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id 066E3Q0M174187 for ; Mon, 6 Jul 2020 10:30:27 -0400 Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com with ESMTP id 322ndvbwy5-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Mon, 06 Jul 2020 10:30:26 -0400 Received: from m0187473.ppops.net (m0187473.ppops.net [127.0.0.1]) by pps.reinject (8.16.0.36/8.16.0.36) with SMTP id 066E3bRS174551 for ; Mon, 6 Jul 2020 10:30:25 -0400 Received: from ppma05fra.de.ibm.com (6c.4a.5195.ip4.static.sl-reverse.com [149.81.74.108]) by mx0a-001b2d01.pphosted.com with ESMTP id 322ndvbwv8-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 06 Jul 2020 10:30:25 -0400 Received: from pps.filterd (ppma05fra.de.ibm.com [127.0.0.1]) by ppma05fra.de.ibm.com (8.16.0.42/8.16.0.42) with SMTP id 066EB0Wm002318; Mon, 6 Jul 2020 14:30:19 GMT Received: from b06cxnps3074.portsmouth.uk.ibm.com (d06relay09.portsmouth.uk.ibm.com [9.149.109.194]) by ppma05fra.de.ibm.com with ESMTP id 322hd8175c-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 06 Jul 2020 14:30:19 +0000 Received: from d06av25.portsmouth.uk.ibm.com (d06av25.portsmouth.uk.ibm.com [9.149.105.61]) by b06cxnps3074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 066EUGsk20447434 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 6 Jul 2020 14:30:16 GMT Received: from d06av25.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 9FF1E11C058; Mon, 6 Jul 2020 14:30:16 +0000 (GMT) Received: from d06av25.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 6FAA711C064; Mon, 6 Jul 2020 14:30:16 +0000 (GMT) Received: from oc4452167425.ibm.com (unknown [9.145.17.26]) by d06av25.portsmouth.uk.ibm.com (Postfix) with ESMTP; Mon, 6 Jul 2020 14:30:16 +0000 (GMT) Subject: Re: [PATCH] Fix stringop-overflow errors from gcc 10 in iconv. To: libc-alpha@sourceware.org, "Carlos O'Donell" References: <20200616122420.5175-1-stli@linux.ibm.com> From: Stefan Liebler Message-ID: Date: Mon, 6 Jul 2020 16:30:16 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.9.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.235, 18.0.687 definitions=2020-07-06_11:2020-07-06, 2020-07-06 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 mlxscore=0 suspectscore=0 clxscore=1015 cotscore=-2147483648 mlxlogscore=999 impostorscore=0 phishscore=0 lowpriorityscore=0 malwarescore=0 adultscore=0 priorityscore=1501 bulkscore=0 spamscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2004280000 definitions=main-2007060107 X-Spam-Status: No, score=-13.6 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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-List-Received-Date: Mon, 06 Jul 2020 14:30:29 -0000 On 6/26/20 8:00 PM, Matheus Castanho wrote: > Hi Stefan, > > On 6/16/20 9:24 AM, Stefan Liebler via Libc-alpha wrote: >> On s390x, I've recognize various -Werror=stringop-overflow messages >> in iconv/loop.c and iconv/skeleton.c if build with gcc10 -O3. >> >> With this commit gcc knows the size and do not raise those errors anymore. >> --- >> iconv/loop.c | 14 ++++++++------ >> iconv/skeleton.c | 8 +++++--- >> 2 files changed, 13 insertions(+), 9 deletions(-) >> >> diff --git a/iconv/loop.c b/iconv/loop.c >> index 9f7570d591..b032fcd9ac 100644 >> --- a/iconv/loop.c >> +++ b/iconv/loop.c >> @@ -420,8 +420,10 @@ SINGLE(LOOPFCT) (struct __gconv_step *step, >> # else >> /* We don't have enough input for another complete input >> character. */ >> - while (inptr < inend) >> - state->__value.__wchb[inlen++] = *inptr++; >> + size_t inlen_after = inlen + (inend - inptr); >> + assert (inlen_after <= sizeof (state->__value.__wchb)); >> + for (; inlen < inlen_after; inlen++) >> + state->__value.__wchb[inlen] = *inptr++; >> # endif >> >> return __GCONV_INCOMPLETE_INPUT; >> @@ -483,11 +485,11 @@ SINGLE(LOOPFCT) (struct __gconv_step *step, >> /* We don't have enough input for another complete input >> character. */ >> assert (inend - inptr > (state->__count & ~7)); >> - assert (inend - inptr <= sizeof (state->__value)); >> + assert (inend - inptr <= sizeof (state->__value.__wchb)); >> state->__count = (state->__count & ~7) | (inend - inptr); >> - inlen = 0; >> - while (inptr < inend) >> - state->__value.__wchb[inlen++] = *inptr++; >> + for (inlen = 0; inlen < inend - inptr; inlen++) >> + state->__value.__wchb[inlen] = inptr[inlen]; >> + inptr = inend; >> # endif >> } >> >> diff --git a/iconv/skeleton.c b/iconv/skeleton.c >> index 1dc642e2fc..1a38b51a5a 100644 >> --- a/iconv/skeleton.c >> +++ b/iconv/skeleton.c >> @@ -795,11 +795,13 @@ FUNCTION_NAME (struct __gconv_step *step, struct __gconv_step_data *data, >> # else >> /* Make sure the remaining bytes fit into the state objects >> buffer. */ >> - assert (inend - *inptrp < 4); >> + size_t cnt_after = inend - *inptrp; >> + assert (cnt_after <= sizeof (data->__statep->__value.__wchb)); >> >> size_t cnt; >> - for (cnt = 0; *inptrp < inend; ++cnt) >> - data->__statep->__value.__wchb[cnt] = *(*inptrp)++; >> + for (cnt = 0; cnt < cnt_after; ++cnt) >> + data->__statep->__value.__wchb[cnt] = (*inptrp)[cnt]; >> + *inptrp = inend; >> data->__statep->__count &= ~7; >> data->__statep->__count |= cnt; >> # endif >> > > > Thanks for working on this! I also noticed this same issue on power. > This patch indeed fixes it there as well. > > As for the patch, I'm not that familiar with iconv code, but by checking > the modified snippet, the loops seem equivalent and the pointers are > properly modified as before. So it's mostly harmless, basically > rewriting those lines in a different way to please GCC. > > LGTM. > > -- > Matheus Castanho > @Carlos: Is this patch okay to commit before the release? Bye. Stefan