wcslen reads beyond the end of the buffer

Bug #1089722 reported by Phillip Susi
22
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Valgrind
Unknown
Unknown
valgrind (Ubuntu)
Triaged
Medium
Unassigned

Bug Description

valgrind reports many errors of wcslen reading beyond the end of the buffer. It appears that sysdeps/x86_64/wcslen.S tries do do 64 bit reads which can result in reading beyond the end of the buffer if there are less than 64 bits left.

Revision history for this message
Dave Gilbert (ubuntu-treblig) wrote :

Have you got any examples?
IMHO this can just be a false positive since it's safe to read beyond a buffer end iff:
  * You know you're not crossing a 64bit boundary (since you know you're not crossing a page boundary and hence can't seg)
  * Don't cause any data in that 64bit read to influence the result if it's after the terminating character.

If you've got an example that shows where it breaks either of those two conditions then I agree it's eglibc bug, else I say it's a false positive from valgrind.

Changed in eglibc (Ubuntu):
status: New → Incomplete
Revision history for this message
Phillip Susi (psusi) wrote :

I suspected it may be a false positive in valgrind from such an optimization in eglibc, but looking at the uncommented glibc source, I can not tell if it actually is safe and won't cross a page boundary. If it can't cross a page boundary, then yes, valgrind should be configured to ignore it, but if it can, then it certainly is a bug.

Revision history for this message
Dave Gilbert (ubuntu-treblig) wrote :

Hmm my x86 is a bit rusty; have you got an actual test case that triggers the warning?

It seems to be basically:
  a) Do a bunch of individual character tests (32 bit at a time)
  b) Do a bunch of tests of 64 bit words (pcmpeqd's) - i.e. 2 wchar's at a time
  c) Get into an aligned loop doing 64 bytes at a time (using multiple pcmpeqd's)

a) looks safe

Before (b) there is the code:
        lea 32(%rdi), %rax
        lea 16(%rdi), %rcx
        and $-16, %rax

So I think that's aligning rax to a 16 byte boundary, so I'd hope what's after it is safe

and Before (c) there is :

        and $-0x40, %rax

        .p2align 4
L(aligned_64_loop):

so again before going into that loop it's aligned to a 64byte boundary

So that feels ok with out having worked every path - although I'd kind of expected valgrind to follow it.

If you've got the example and it shows exactly which instruction it's moaning about it might be able to spot something.

Dave

Revision history for this message
Phillip Susi (psusi) wrote :

It sounds like it's not going to cross a page boundary then, so it safe, in which case, it's a false positive and valgrind should be configured to ignore it.

affects: eglibc (Ubuntu) → valgrind (Ubuntu)
Changed in valgrind (Ubuntu):
importance: Undecided → Medium
status: Incomplete → Triaged
Revision history for this message
Andreas W Wylach (aw-s) wrote :

I can confirm this bug. I already posted a question on stackoverflow a few weeks ago (stackoverflow.com/questions/14474691/wifstream-with-imbue-locale-produces-valgrind-errors) before I did deeper investigation. In my current project I work with wstring/wchar_t and also locale / imbue and valgrind reports errors as stated above.

Using Ubuntu 12.04 LTS

- libc6:
  Installed: 2.15-0ubuntu10.3
  Candidate: 2.15-0ubuntu10.3
  Version table:
 *** 2.15-0ubuntu10.3 0
        500 http://mo.archive.ubuntu.com/ubuntu/ precise-updates/main amd64 Packages
        100 /var/lib/dpkg/status
     2.15-0ubuntu10.2 0
        500 http://security.ubuntu.com/ubuntu/ precise-security/main amd64 Packages
     2.15-0ubuntu10 0
        500 http://mo.archive.ubuntu.com/ubuntu/ precise/main amd64 Packages

$ /lib/x86_64-linux-gnu/libc.so.6
GNU C Library (Ubuntu EGLIBC 2.15-0ubuntu10.3) stable release version 2.15, by Roland McGrath et al.
Copyright (C) 2012 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.
There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A
PARTICULAR PURPOSE.
Compiled by GNU CC version 4.6.3.
Compiled on a Linux 3.2.30 system on 2012-10-05.
Available extensions:
 crypt add-on version 2.1 by Michael Glad and others
 GNU Libidn by Simon Josefsson
 Native POSIX Threads Library by Ulrich Drepper et al
 BIND-8.2.3-T5B
libc ABIs: UNIQUE IFUNC
For bug reporting instructions, please see:
<http://www.debian.org/Bugs/>.

Revision history for this message
scrawl (scrawl-deactivatedaccount) wrote :

According to https://bugs.kde.org/show_bug.cgi?id=307828, should be fixed in valgrind 3.9

Revision history for this message
Pjfloyd (pjfloyd) wrote :

This should be closed.

commit ec3a16088b1992e3ab3ca3f2cb93adeacc247064
Author: Julian Seward <email address hidden>
Date: Fri Aug 24 14:44:27 2012 +0000

    Implement a wrapper for wcslen on Linux, assuming that
    sizeof(wchar_t) == 4, which I believe to be true on both Linux
    and MacOSX. Fixes #298281.

    git-svn-id: svn://svn.valgrind.org/valgrind/trunk@12893

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.