svn commit: r310086 - head/sys/dev/xen/blkfront

[Dimitry Andric]

On 15 Dec 2016, at 08:01, Colin Percival <cperciva at 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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 194 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://lists.freebsd.org/pipermail/svn-src-all/attachments/20161215/915edba7/attachment.sig>