From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/11627 Path: news.gmane.org!.POSTED!not-for-mail From: Alexander Monakov Newsgroups: gmane.linux.lib.musl.general Subject: [RFC PATCH] fix OOB reads in Xbyte_memmem Date: Fri, 30 Jun 2017 00:35:33 +0300 Message-ID: <20170629213533.18744-1-amonakov@ispras.ru> References: <87r2y2vrsg.fsf@gmail.com> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: blaine.gmane.org X-Trace: blaine.gmane.org 1498772153 17579 195.159.176.226 (29 Jun 2017 21:35:53 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Thu, 29 Jun 2017 21:35:53 +0000 (UTC) To: musl@lists.openwall.com Original-X-From: musl-return-11640-gllmg-musl=m.gmane.org@lists.openwall.com Thu Jun 29 23:35:49 2017 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.84_2) (envelope-from ) id 1dQh6H-0004CX-QE for gllmg-musl@m.gmane.org; Thu, 29 Jun 2017 23:35:45 +0200 Original-Received: (qmail 18101 invoked by uid 550); 29 Jun 2017 21:35:48 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 17999 invoked from network); 29 Jun 2017 21:35:46 -0000 X-Mailer: git-send-email 2.11.0 In-Reply-To: <87r2y2vrsg.fsf@gmail.com> Xref: news.gmane.org gmane.linux.lib.musl.general:11627 Archived-At: Reported by Leah Neukirchen. --- Presumably the loops were written that way to get good performance even in absence of loop unrolling and/or without relying on advanced compiler optimizations. In that case the manual loop pipelining should be kept as is, and the solution is to reduce iteration count by one, pushing out one test into the loop epilogue. Alexander src/string/memmem.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/string/memmem.c b/src/string/memmem.c index 4be6a310..54a66e46 100644 --- a/src/string/memmem.c +++ b/src/string/memmem.c @@ -5,27 +5,27 @@ static char *twobyte_memmem(const unsigned char *h, size_t k, const unsigned char *n) { uint16_t nw = n[0]<<8 | n[1], hw = h[0]<<8 | h[1]; - for (h++, k--; k; k--, hw = hw<<8 | *++h) - if (hw == nw) return (char *)h-1; - return 0; + for (h+=2, k-=2; k; k--, hw = hw<<8 | *h++) + if (hw == nw) return (char *)h-2; + return hw == nw ? (char *)h-2 : 0; } static char *threebyte_memmem(const unsigned char *h, size_t k, const unsigned char *n) { uint32_t nw = n[0]<<24 | n[1]<<16 | n[2]<<8; uint32_t hw = h[0]<<24 | h[1]<<16 | h[2]<<8; - for (h+=2, k-=2; k; k--, hw = (hw|*++h)<<8) - if (hw == nw) return (char *)h-2; - return 0; + for (h+=3, k-=3; k; k--, hw = (hw|*h++)<<8) + if (hw == nw) return (char *)h-3; + return hw == nw ? (char *)h-3 : 0; } static char *fourbyte_memmem(const unsigned char *h, size_t k, const unsigned char *n) { uint32_t nw = n[0]<<24 | n[1]<<16 | n[2]<<8 | n[3]; uint32_t hw = h[0]<<24 | h[1]<<16 | h[2]<<8 | h[3]; - for (h+=3, k-=3; k; k--, hw = hw<<8 | *++h) - if (hw == nw) return (char *)h-3; - return 0; + for (h+=4, k-=4; k; k--, hw = hw<<8 | *h++) + if (hw == nw) return (char *)h-4; + return hw == nw ? (char *)h-4 : 0; } #define MAX(a,b) ((a)>(b)?(a):(b)) -- 2.11.0