Bug 1174267 - qemu linux-user syscall pwrite64 does not handle the case when the length of buffer is = 0
Summary: qemu linux-user syscall pwrite64 does not handle the case when the length of ...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: qemu
Version: 30
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Fedora Virtualization Maintainers
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2014-12-15 14:30 UTC by Ilya Palachev
Modified: 2019-03-28 17:40 UTC (History)
18 users (show)

Fixed In Version: qemu-3.1.0-6.fc30
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2019-03-28 17:40:57 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)
a.c from testcase described in bug report, compiled with GCC and linked with ld.gold for armv7l (8.45 KB, application/x-executable)
2014-12-15 14:50 UTC, Ilya Palachev
no flags Details
a.c from testcase described in bug report, compiled with GCC and linked with ld.gold for aarch64 (10.75 KB, application/x-executable)
2014-12-15 15:10 UTC, Ilya Palachev
no flags Details


Links
System ID Private Priority Status Summary Last Updated
Launchpad 1810433 0 None None None 2019-01-04 14:39:55 UTC

Description Ilya Palachev 2014-12-15 14:30:19 UTC
Description of problem:

eu-strip fails on binaries linked by ld.gold, without any special options specified.

Version-Release number of selected component (if applicable):
0.160

How reproducible:
always

Steps to Reproduce:

1. Create simple testcase:
# echo '
int main(void)
{
return 12;
}' > a.c

2. Compile and link it with gold:

# gcc a.c -o a -fuse-ld=gold

3. Check that binary really works:

# ./a
# echo $?
12

4. Run eu-strip on it:

# eu-strip -g -f a.debug a
eu-strip: while writing 'a': cannot write data to file

Actual results:

The binary is not stripped with eu-strip.

Expected results:

The binary is stripped with eu-strip.

Additional info:

# gcc -v
Using built-in specs.
COLLECT_GCC=/usr/bin/gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/armv7l-tizen-linux-gnueabi/4.9/lto-wrapper
Target: armv7l-tizen-linux-gnueabi
Configured with: ../configure --prefix=/usr --infodir=/usr/share/info --mandir=/usr/share/man --libdir=/usr/lib --libexecdir=/usr/lib --enable-languages=c,c++ --enable-checking=release --with-gxx-include-dir=/usr/include/c++/4.9 --enable-ssp --disable-libssp --disable-bootstrap --disable-libvtv --disable-plugin --with-bugurl=http://bugs.tizen.org/ --with-pkgversion=Tizen --disable-libquadmath --disable-libgcj --with-slibdir=/lib --with-system-zlib --enable-__cxa_atexit --enable-libstdcxx-allocator=new --disable-libstdcxx-pch --enable-version-specific-runtime-libs --enable-linker-build-id --enable-linux-futex --program-suffix=-4.9 --program-prefix=armv7l-tizen-linux-gnueabi- --target=armv7l-tizen-linux-gnueabi --disable-nls --with-sysroot=/usr/armv7l-tizen-linux-gnueabi --with-build-sysroot=/ --with-build-time-tools=/usr/arm-tizen-linux-gnueabi/bin --with-arch=armv7-a --with-tune=cortex-a8 --with-float=softfp --with-fpu=vfpv3 --with-mode=thumb --disable-sjlj-exceptions --build=i586-tizen-linux --host=i586-tizen-linux
Thread model: posix
gcc version 4.9.2 (Tizen)

# ld.gold --version

GNU gold (GNU Binutils; devel:arm_toolchain:Mobile:Tizen_Common / arm-wayland 2.24.90.20141014-24) 1.11
Copyright (C) 2014 Free Software Foundation, Inc.
This program is free software; you may redistribute it under the terms of
the GNU General Public License version 3 or (at your option) a later version.
This program has absolutely no warranty.

#  eu-strip --version
strip (elfutils) 0.160
Copyright (C) 2012 Red Hat, 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.
Written by Ulrich Drepper.

The error was obtained during ld.gold enabling for Tizen:Common project.
https://build.tizen.org/project/show?project=devel%3Aarm_toolchain%3AMobile%3ATizen_Common
Fail detected on systemd bootctl binary, then reduced to the testcase shown above.

[  666s] + /usr/lib/rpm/find-debuginfo.sh /home/abuild/rpmbuild/BUILD/systemd-216
[  666s] extracting debug info from /home/abuild/rpmbuild/BUILDROOT/systemd-216-9.2.aarch64/usr/bin/bootctl
[  666s] eu-strip: while writing '/home/abuild/rpmbuild/BUILDROOT/systemd-216-9.2.aarch64/usr/bin/bootctl': cannot write data to file
[  666s] error: Bad exit status from /var/tmp/rpm-tmp.1NRd9H (%install)
[  666s] 

eu-strip is used during final stage of RPM creation.

Comment 1 Ilya Palachev 2014-12-15 14:37:21 UTC
Please, when you try to reproduce the error, make sure that ld.gold (not ld.bfd) is used. If compiler fails to find ld.gold, it will run ld.bfd and eu-strip will not fail on the produced binary.

So, the error happens only with gold-linked binaries, not with bfd-linked ones.

Comment 2 Mark Wielaard 2014-12-15 14:44:13 UTC
Could you please attach the 'a' binary from step 2.
It should be testable on non-aarch64 if one has the aarch64 ELF file available.
Thanks.

Comment 3 Ilya Palachev 2014-12-15 14:50:59 UTC
Created attachment 969057 [details]
a.c from testcase described in bug report, compiled with GCC and linked with ld.gold for armv7l

Attached a.c from testcase described in bug report, compiled with GCC and linked with ld.gold for armv7l.

Comment 4 Ilya Palachev 2014-12-15 15:10:11 UTC
Created attachment 969076 [details]
a.c from testcase described in bug report, compiled with GCC and linked with ld.gold for aarch64

Attached a.c from testcase described in bug report, compiled with GCC and linked with ld.gold for aarch64

Comment 5 Ilya Palachev 2014-12-15 16:44:01 UTC
(In reply to Mark Wielaard from comment #2)
> Could you please attach the 'a' binary from step 2.

I've attached 2 binaries: compiled for arvm7l and for aarch64:
- a.arvm7l
- a.aarch64

Comment 6 Ilya Palachev 2014-12-16 10:10:57 UTC
Sorry. I've built elfutils outside Tizen buildroot and checked eu-strip on attached files. It works. So the problem is actually in our environment.

Comment 7 Mark Wielaard 2014-12-16 10:23:04 UTC
Thanks for letting us know. I hadn't had a chance to test myself yet.

If you figure out what went wrong with the build inside the Tizen buildroot please do make a note here in case it is something that could happen to others. Maybe we can change something in the build setup so that the issue is more easily detected.

Note that "cannot write data to file" is ELF_E_WRITE_ERROR which is only set when pwrite, fstat, ftruncate or fchmod fail.

Comment 8 Ilya Palachev 2014-12-16 11:48:06 UTC
(In reply to Mark Wielaard from comment #7)
> Note that "cannot write data to file" is ELF_E_WRITE_ERROR which is only set
> when pwrite, fstat, ftruncate or fchmod fail.

I've tried to debug the testcase with GDB. Here is the block of code where the error is set to ELF_E_WRITE_ERROR:

ssize_t n = pwrite_retry (elf->fildes, buf,
		      dl->data.d.d_size,
		      last_offset);
if (unlikely ((size_t) n != dl->data.d.d_size))
{
    if (buf != dl->data.d.d_buf && buf != tmpbuf)
        free (buf);

    __libelf_seterrno (ELF_E_WRITE_ERROR);
    return 1;
}

The error happens in pwrite_retry, which calls pwrite64:

static inline ssize_t __attribute__ ((unused))
pwrite_retry (int fd, const void *buf, size_t len, off_t off)
{
  ssize_t recvd = 0;

  do
    {
      ssize_t ret = TEMP_FAILURE_RETRY (pwrite (fd, buf + recvd, len - recvd,
                                                off + recvd));
      if (ret <= 0)
        return ret < 0 ? ret : recvd;

      recvd += ret;
    }
  while ((size_t) recvd < len);

  return recvd;
}

Comment 9 Mark Wielaard 2014-12-16 13:21:56 UTC
Does pwrite_retry returns a negative value? If so what does errno say?

Comment 10 Ilya Palachev 2014-12-16 15:28:19 UTC
(In reply to Mark Wielaard from comment #9)
> Does pwrite_retry returns a negative value? If so what does errno say?

After erroneous call to pwrite, the errno is set to 14 (EFAULT), that means "bad address", according to "man errno".

During this call pwrite_retry was called with the following arguments:

(off=2288, len=0, buf=0x0, fd=5)

Here is the full backtrace:

#0  0x0000004000958cf4 in pwrite64 () from /home/ilya/obs/buildroots/systemd.aarch64/usr/lib64/libc-2.20.so
#1  0x00000040008424ac in pwrite_retry (off=2288, len=0, buf=0x0, fd=5) at ../lib/system.h:79
#2  __elf64_updatefile (elf=elf@entry=0x4276b0, change_bo=change_bo@entry=0, shnum=shnum@entry=33) at elf32_updatefile.c:728
#3  0x000000400083eda4 in write_file (shnum=33, change_bo=0, size=7096, elf=0x4276b0) at elf_update.c:92
#4  elf_update (elf=0x4276b0, cmd=<optimized out>) at elf_update.c:192
#5  0x0000000000405478 in handle_elf ()
#6  0x0000000000406b60 in process_file ()
#7  0x0000000000402e58 in main ()

It means that pwrite was called with the same arguments (at least first time), and it was called only once.

Moreover, we have found that on armv7l target machine eu-strip works correctly. In Tizen buildroot eu-strip is run under qemu-arm. So it seems to be a qemu bug.

IMO qemu erroneously processes call with buf = 0. How do you think?

Comment 11 Ilya Palachev 2014-12-17 12:18:36 UTC
The error was in the fact that qemu implementation of syscall pwrite64 does not handle the case when the length of buffer is = 0.

I've submitted a patch to qemu-devel that fixes this:
http://lists.gnu.org/archive/html/qemu-devel/2014-12/msg02702.html

See also official POSIX.1-2001. specification of pread64 and pwrite64 at
http://pubs.opengroup.org/onlinepubs/009695399/functions/read.html
http://pubs.opengroup.org/onlinepubs/009695399/functions/write.html

Comment 12 Mark Wielaard 2014-12-17 13:22:44 UTC
Wow, good you dug deeper and produced that patch for qemu. Nice find. Thanks for sharing the result.

All pread/[p]write calls in elfutils should normally go through the lib/system.h pread/[p]write_retry wrappers. Those all do the following:

  ssize_t recvd = 0;

  do
    {
      ssize_t ret = TEMP_FAILURE_RETRY (write (fd, buf + recvd, len - recvd));
      if (ret <= 0)
        return ret < 0 ? ret : recvd;

      recvd += ret;
    }
  while ((size_t) recvd < len);

  return recvd;

Flipping the "do-while" loop to a "while" loop should be a workaround for this issue because it would skip the system call when len == 0.

Comment 13 Matwey V. Kornilov 2019-01-03 17:11:58 UTC
Hi,

I am still able to reproduce the issue with elfutils master (e8b9832af19e5975fb2a9dbe729eaba0373c781f) and qemu 2.11.2

Comment 14 Mark Wielaard 2019-01-04 11:13:30 UTC
(In reply to Matwey V. Kornilov from comment #13) 
> I am still able to reproduce the issue with elfutils master
> (e8b9832af19e5975fb2a9dbe729eaba0373c781f) and qemu 2.11.2

It looks like Ilya's patch from comment #11 was never applied to qemu.

Comment 15 Mark Wielaard 2019-01-18 10:48:19 UTC
There is now a similar fix to Ilya Palachev's patch upstream:

commit 2bd3f8998e1e7dcd9afc29fab252fb9936f9e956
Author: Peter Maydell <peter.maydell>
Date:   Tue Jan 8 18:49:00 2019 +0000

    linux-user: make pwrite64/pread64(fd, NULL, 0, offset) return 0
    
    Linux returns success if pwrite64() or pread64() are called with a
    zero length NULL buffer, but QEMU was returning -TARGET_EFAULT.
    
    This is the same bug that we fixed in commit 58cfa6c2e6eb51b23cc9
    for the write syscall, and long before that in 38d840e6790c29f59
    for the read syscall.
    
    Fixes: https://bugs.launchpad.net/qemu/+bug/1810433
    
    Signed-off-by: Peter Maydell <peter.maydell>
    Reviewed-by: Laurent Vivier <laurent>
    Reviewed-by: Philippe Mathieu-Daudé <philmd>
    Message-Id: <20190108184900.9654-1-peter.maydell>
    Signed-off-by: Laurent Vivier <laurent>

Comment 16 Ben Cotton 2019-02-19 17:12:10 UTC
This bug appears to have been reported against 'rawhide' during the Fedora 30 development cycle.
Changing version to '30.

Comment 17 Fedora Update System 2019-03-21 19:12:59 UTC
qemu-3.1.0-5.fc30 has been pushed to the Fedora 30 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2019-0664c7724d

Comment 18 Fedora Update System 2019-03-25 23:24:18 UTC
qemu-3.1.0-6.fc30 has been submitted as an update to Fedora 30. https://bodhi.fedoraproject.org/updates/FEDORA-2019-0664c7724d

Comment 19 Fedora Update System 2019-03-27 00:44:48 UTC
qemu-3.1.0-6.fc30 has been pushed to the Fedora 30 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2019-0664c7724d

Comment 20 Fedora Update System 2019-03-28 17:40:57 UTC
qemu-3.1.0-6.fc30 has been pushed to the Fedora 30 stable repository. If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.