Date: Thu, 15 Dec 2016 20:07:04 +0100 From: Dimitry Andric <dim@FreeBSD.org> To: Colin Percival <cperciva@tarsnap.com> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r310086 - head/sys/dev/xen/blkfront Message-ID: <69D559F0-45E9-42DB-821A-864CEEB9C741@FreeBSD.org> In-Reply-To: <0100015901493cd9-99c1edd7-8f8f-4a53-b86a-ae400a37f1c5-000000@email.amazonses.com> References: <201612141928.uBEJSJor074348@repo.freebsd.org> <0100015901493cd9-99c1edd7-8f8f-4a53-b86a-ae400a37f1c5-000000@email.amazonses.com>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
On 15 Dec 2016, at 08:01, Colin Percival <cperciva@tarsnap.com> wrote:
>
> On 12/14/16 11:28, Dimitry Andric wrote:
>> Log:
>> In xbd_connect(), use correct scanf conversion specifiers for the
>> feature_barrier and feature_flush variables. Otherwise, adjacent
>> variables on the stack, such as sector_size, may be overwritten, with
>> disastrous results.
>
> Thanks! Did you happen to notice what stack variable (if any?) was being
> overwritten under clang 3.8.0? Just wondering if there might be some
> undiscovered issue lurking in FreeBSD releases which will cause other less
> obvious problems.
Here is a little overview of the locations on the stack (e.g. offsets
from %rbp) with different compiler versions:
clang 3.8.x and earlier:
[ -56: -48) sectors
[ -64: -56) sector_size
[ -72: -64) phys_sector_size
[ -76: -72) binfo
[ -80: -76) feature_barrier
[ -84: -80) feature_flush
Here, writing 8 bytes of data to feature_barrier will most likely
overwrite binfo with zeroes, but since that is usually zero already,
not much will happen.
Similarly, writing 8 bytes of data to feature_flush will most likely
overwrite feature_barrier with zeroes, effectively always turning off
that feature.
clang 3.9.0 and later:
[ -80: -72) phys_sector_size
[ -88: -80) sector_size
[ -92: -88) feature_flush
[ -96: -92) feature_barrier
[-104: -96) indirectpages
[-112:-104) sectors
[-132:-128) binfo
As is now known, here the effect was that sector_size is effectively
zeroed when feature_flush is written. Not good. :)
gcc 4.2.1:
[ -44: -40) binfo
[ -48: -44) feature_barrier
[ -52: -48) feature_flush
[ -64: -56) sectors
[ -72: -64) sector_size
[ -80: -72) phys_sector_size
For our base gcc, the results are similar to clang 3.8.x and earlier:
writing 8 bytes of data to feature_barrier will most likely overwrite
binfo with zeroes, to not much effect. Same story for feature_flush.
gcc 6.2.0:
[ -64: -56) phys_sector_size
[ -72: -64) sector_size
[ -80: -72) sectors
[ -84: -80) feature_flush
[ -88: -84) feature_barrier
[ -92: -88) binfo
With a more recent version of gcc, writing 8 bytes of data to
feature_flush will most likely zero least significant half of sectors.
Unless the virtual disk has 2^32 or more sectors, it will turn up as
zero length.
-Dimitry
[-- Attachment #2 --]
-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.30
iEYEARECAAYFAlhS6eUACgkQsF6jCi4glqP+7ACgh/RxNSTD+zFFuwBm97bj8xiB
IeUAoIDnfUyuK+dXuPezdOgCOXUb7kvl
=obF1
-----END PGP SIGNATURE-----
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?69D559F0-45E9-42DB-821A-864CEEB9C741>