All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: Henry Wertz <hwertz10@gmail.com>, qemu-arm <qemu-arm@nongnu.org>,
	QEMU Developers <qemu-devel@nongnu.org>
Subject: Re: [Qemu-arm] [Qemu-devel] getdents patch for 64-bit app on 32-bit host
Date: Thu, 19 Apr 2018 12:11:08 +0100	[thread overview]
Message-ID: <20180419111108.GM10259@redhat.com> (raw)
In-Reply-To: <CAFEAcA9d=oEKfnW0Jr2vrSD8mu0xPbUYk6PAk7dxUqXjJUTooQ@mail.gmail.com>

On Thu, Apr 19, 2018 at 12:00:00PM +0100, Peter Maydell wrote:
> On 17 April 2018 at 22:53, Henry Wertz <hwertz10@gmail.com> wrote:
> > Peter Maydell has raised a concern about possible buffer overflows in this
> > code (which was meant to handle 32-bit app on 64-bit system, not 64-bit on
> > 32-bit).  I must admit I haven't gone through the dirent-copying code with a
> > fine-toothed comb... it appeared to work for my use case.  That said, the
> > code seems to be careful about using offsetof() rather than making any
> > assumptions.  In addition, the dirent-copying code appears to have an assert
> > that would crash qemu if it was going to write past the end of the dirent
> > buffer -- always nice to have plenty of sanity checks!
> 
> If you build the attached test program for x86-64 (which is a
> minor tweak on the test program in the Linux getdents manpage):
>  gcc -g -Wall -o /tmp/getdents getdents.c -static
> 
> and then on a 32-bit Arm host take a qemu-x86_64 with your patch
> applied, and a test directory like this:
> 
> $ ls /tmp/testdir/
> abcd  abcde
> 
> and run it, QEMU will abort on the assert that we don't run off
> the end of the buffer:
> 
> $ ./build/all-a32/x86_64-linux-user/qemu-x86_64 ~/getdents /tmp/testdir
> linux_dirent struct size 24 bytes
> buffer space 32 bytes
> qemu-x86_64: /home/peter.maydell/qemu/linux-user/syscall.c:10197:
> do_syscall: Assertion `count1 + treclen <= count' failed.
> 
> This is because the guest linux_dirent is bigger than the host
> linux_dirent, and therefore just because the host syscall
> successfully fit the record into the buffer doesn't mean we
> can fit the guest record into the buffer.
> 
> I don't see any way to fix this, because the records are variable size.

If we can't even get something common like dirents working with 64-bit
guest on 32-bit host, should we refuse to even build 64-bit linux-user
emulation on a 32-bit host ?  There must be many other similar cases
where the 64-bit guest syscall is going have insufficient space in the
host 32-bit syscall structs.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

WARNING: multiple messages have this Message-ID (diff)
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: Henry Wertz <hwertz10@gmail.com>, qemu-arm <qemu-arm@nongnu.org>,
	QEMU Developers <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] [Qemu-arm] getdents patch for 64-bit app on 32-bit host
Date: Thu, 19 Apr 2018 12:11:08 +0100	[thread overview]
Message-ID: <20180419111108.GM10259@redhat.com> (raw)
In-Reply-To: <CAFEAcA9d=oEKfnW0Jr2vrSD8mu0xPbUYk6PAk7dxUqXjJUTooQ@mail.gmail.com>

On Thu, Apr 19, 2018 at 12:00:00PM +0100, Peter Maydell wrote:
> On 17 April 2018 at 22:53, Henry Wertz <hwertz10@gmail.com> wrote:
> > Peter Maydell has raised a concern about possible buffer overflows in this
> > code (which was meant to handle 32-bit app on 64-bit system, not 64-bit on
> > 32-bit).  I must admit I haven't gone through the dirent-copying code with a
> > fine-toothed comb... it appeared to work for my use case.  That said, the
> > code seems to be careful about using offsetof() rather than making any
> > assumptions.  In addition, the dirent-copying code appears to have an assert
> > that would crash qemu if it was going to write past the end of the dirent
> > buffer -- always nice to have plenty of sanity checks!
> 
> If you build the attached test program for x86-64 (which is a
> minor tweak on the test program in the Linux getdents manpage):
>  gcc -g -Wall -o /tmp/getdents getdents.c -static
> 
> and then on a 32-bit Arm host take a qemu-x86_64 with your patch
> applied, and a test directory like this:
> 
> $ ls /tmp/testdir/
> abcd  abcde
> 
> and run it, QEMU will abort on the assert that we don't run off
> the end of the buffer:
> 
> $ ./build/all-a32/x86_64-linux-user/qemu-x86_64 ~/getdents /tmp/testdir
> linux_dirent struct size 24 bytes
> buffer space 32 bytes
> qemu-x86_64: /home/peter.maydell/qemu/linux-user/syscall.c:10197:
> do_syscall: Assertion `count1 + treclen <= count' failed.
> 
> This is because the guest linux_dirent is bigger than the host
> linux_dirent, and therefore just because the host syscall
> successfully fit the record into the buffer doesn't mean we
> can fit the guest record into the buffer.
> 
> I don't see any way to fix this, because the records are variable size.

If we can't even get something common like dirents working with 64-bit
guest on 32-bit host, should we refuse to even build 64-bit linux-user
emulation on a 32-bit host ?  There must be many other similar cases
where the 64-bit guest syscall is going have insufficient space in the
host 32-bit syscall structs.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

  reply	other threads:[~2018-04-19 11:11 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-17 21:53 [Qemu-arm] getdents patch for 64-bit app on 32-bit host Henry Wertz
2018-04-17 21:53 ` [Qemu-devel] " Henry Wertz
2018-04-18  5:51 ` [Qemu-arm] " Thomas Huth
2018-04-18  5:51   ` Thomas Huth
2018-04-19 11:00 ` [Qemu-arm] " Peter Maydell
2018-04-19 11:00   ` [Qemu-devel] " Peter Maydell
2018-04-19 11:11   ` Daniel P. Berrangé [this message]
2018-04-19 11:11     ` Daniel P. Berrangé
2018-04-19 11:58     ` [Qemu-arm] [Qemu-devel] " Peter Maydell
2018-04-19 11:58       ` [Qemu-devel] [Qemu-arm] " Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180419111108.GM10259@redhat.com \
    --to=berrange@redhat.com \
    --cc=hwertz10@gmail.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.