Bug 2013 - memccpy() gives inconsistent results on mmapped files
Summary: memccpy() gives inconsistent results on mmapped files
Status: RESOLVED FIXED
Alias: None
Product: glibc
Classification: Unclassified
Component: libc (show other bugs)
Version: 2.3.5
: P2 normal
Target Milestone: 2.3.7
Assignee: Roland McGrath
URL:
Keywords:
Depends on:
Blocks: libc237
  Show dependency treegraph
 
Reported: 2005-12-09 16:52 UTC by Tony Ernst
Modified: 2018-04-19 14:16 UTC (History)
2 users (show)

See Also:
Host: ia64-*-linux
Target: ia64-*-linux
Build: ia64-*-linux
Last reconfirmed: 2006-03-02 04:50:17
fweimer: security-


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tony Ernst 2005-12-09 16:52:12 UTC
Glenn Fowler of ATT reported that memccpy() gives inconsistent results
on mmapped files.  This was reported to SGI, but the bug is ia64
generic, I get the same errors on a vanilla ia64 box.

Glenn's test case follows, it has been tweaked to add a PREFAULT
option.  Prefaulting the input file makes memccpy work.  The problem 
occurs on at least glibc 2.3.3, glibc 2.3.4 and glibc 2.3.5.

# for i in 1 257 513 1025;do cc -DNUM=$i testmemccpy.c; ./a.out; done
FAILED NUM 1 offset 0 size 64 no match
FAILED NUM 257 offset 16384 size 8
OLD xxxxxxxx
NEW xxxxxxxN
FAILED NUM 513 offset 32768 size 8
OLD xxxxxxxx
NEW xxxxxxxN
FAILED NUM 1025 offset 65536 size 8
OLD xxxxxxxx
NEW xxxxxxxN

# for i in 1 257 513 1025;do cc -DNUM=$i -DPREFAULT testmemccpy.c; ./a.out; done
No errors.

/*
 * the umpteenth linux ia64 memccpy indictment
 *
 * Glenn Fowler ATT
 *
 * creates/clobbers the file ./testmemccpy.dat
 *
 * fails with
 *
 *      -DNUM=1 
 *      -DNUM=257
 *      -DNUM=513
 *      -DNUM=1025
 *
 * works with -DOH and any -DNUM=n, n>0, n<memory-limit
 *
 * please eradicate the cute memccpy tricks
 * surely the processor is fast enough to minimize their impact
 *
 * Also works with -DPREFAULT - prefaulting the mmapped file gives the
 * correct results in memccpy.  
 */

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <fcntl.h>
#include <sys/mman.h>

#ifndef NUM
#define NUM 257
#endif

#if OH

#undef  memccpy
#define memccpy memccpy_that_works

void* memccpy(void* as1, const void* as2, int c, size_t n)
{
        char*           s1 = (char*)as1;
        const char*     s2 = (char*)as2;
        const char*     ep = s2 + n;

        while (s2 < ep)
                if ((*s1++ = *s2++) == c)
                        return s1;
        return 0;
}

#endif

static const char x[] =
"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxN";

int main()
{
        char*           b;
        char*           s;
        char*           e;
        char*           t;
        long            m;
        int             f;
        char            u[1024];

        if ((f = open("testmemccpy.dat", O_CREAT|O_RDWR|O_TRUNC, 0666)) < 0)
        {
                fprintf(stderr, "creat error\n");
                return 1;
        }
        for (m = 0; m < NUM; m++)
                if (write(f, x, sizeof(x)-1) != sizeof(x)-1)
                {
                        fprintf(stderr, "write error\n");
                        return 1;
                }
        if (lseek(f, (off_t)0, SEEK_SET))
        {
                fprintf(stderr, "seek error\n");
                return 1;
        }
        m *= (sizeof(x)-1);
        if (!(b = s = mmap((void*)0, m, PROT_READ|PROT_WRITE, MAP_PRIVATE, f,
(off_t)0)))
        {
                fprintf(stderr, "mmap error\n");
                return 1;
        }
#ifdef	PREFAULT
	for (s = b; s < b+m; s+=4096)
		f = *s;
	s = b;
#endif
        for (e = s + m; s < e && (t = memccpy(u, s, 'N', (e-s) > sizeof(u) ?
sizeof(u) : (e-s))); s += (t-u))
                if ((t-u) != (sizeof(x)-1) || memcmp(u, s, t-u))
                {
                        fprintf(stderr, "FAILED NUM %d offset %lu size %lu\n",
NUM, (unsigned long)(s-b), (unsigned long)(t-u));
                        fprintf(stderr, "OLD %-.*s\n", t-u, s);
                        fprintf(stderr, "NEW %-.*s\n", t-u, u);
                        return 1;
                }
        if (s < e)
        {
                fprintf(stderr, "FAILED NUM %d offset %lu size %lu no match\n",
NUM, (unsigned long)(s-b), (unsigned long)(e-s));
                return 1;
        }
        return 0;
}
Comment 1 H.J. Lu 2005-12-29 16:51:23 UTC
Doesn't SGI have a fix

http://www.gelato.unsw.edu.au/archives/linux-ia64/0412/11984.html
Comment 2 Tony Ernst 2006-01-02 21:31:27 UTC
No, I'm afraid not.  We thought this looked like something Jes Sorensen had fixed previously, but we 
verified that his fix was in, and we can still reproduce it.  So this appears to be a different bug.
Comment 3 H.J. Lu 2006-01-05 22:55:20 UTC
A patch is posted at

http://sources.redhat.com/ml/libc-alpha/2006-01/msg00048.html
Comment 4 Tony Ernst 2006-02-23 19:51:09 UTC
The patch from Comment #3 does fix the problem for us.
Comment 5 Sourceware Commits 2006-03-02 04:49:30 UTC
Subject: Bug 2013

CVSROOT:	/cvs/glibc
Module name:	libc
Changes by:	roland@sources.redhat.com	2006-03-02 04:49:27

Modified files:
	sysdeps/ia64   : memccpy.S 

Log message:
	2006-01-05  H.J. Lu  <hongjiu.lu@intel.com>
	
	[BZ #2013]
	* sysdeps/ia64/memccpy.S: Properly handle recovery for
	predicated speculative load.

Patches:
http://sources.redhat.com/cgi-bin/cvsweb.cgi/libc/sysdeps/ia64/memccpy.S.diff?cvsroot=glibc&r1=1.7&r2=1.8

Comment 6 Roland McGrath 2006-03-02 04:50:17 UTC
I put this on the trunk.  It seem right for 2.3 too.
Comment 7 Ulrich Drepper 2007-02-18 04:46:54 UTC
The 2.3 is outdated.  Closing.