linux-kernel - Re: [RESEND PATCH] scsi: make struct scsi_varlen_cdb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 11 Oct 2012 15:10:40 +0100
From:	James Hogan <james.hogan@...tec.com>
To:	James Bottomley <James.Bottomley@...senPartnership.com>
CC:	<linux-scsi@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [RESEND PATCH] scsi: make struct scsi_varlen_cdb_hdr packed

On 11/10/12 13:58, James Bottomley wrote:
> On Thu, 2012-10-11 at 12:32 +0100, James Hogan wrote:
>> On 11/10/12 11:24, James Bottomley wrote:
>>> On Thu, 2012-10-11 at 10:15 +0100, James Hogan wrote:
>>>> The struct scsi_varlen_cdb_hdr is expected to be exactly 10 bytes when
>>>> used in struct osd_cdb_head, but it isn't marked as packed. Some
>>>> architectures will round the struct size up which triggers BUILD_BUG_ON
>>>> compile errors in osd_initiator.c when the outer structs are unexpected
>>>> sizes. This is fixed by marking struct scsi_varlen_cdb_hdr as __packed.
>>>
>>> What actual problem have you encountered? The structure is {u8[8], u16}
>>> which is naturally packed on every architecture I know about.  I've even
>>> built osd_initiator without problem on parisc, which has some of the
>>> most rigid alignment rules I've seen.
>>
>> Hi James,
>>
>> The alignment is fine (the offset of the u16 is 8 bytes), but
>> unfortunately with the metag port of gcc, sizeof(struct
>> scsi_varlen_cdb_hdr) is rounded up to a 4 byte boundary (even though the
>> largest data member alignment is only 2 bytes), which is 12 bytes
>> instead of 10.
> 
> That sounds to be a bug in your compiler ... it shouldn't be rounding up
> structure sizes if the structure can fit in 10 bytes.  This isn't
> happening in any other architecture that I know of (otherwise we'd have
> had a reported build break).

This was my initial thought, and I share your feeling that this isn't
ideal compiler behaviour, however it's pretty much set in stone as part
of our ABI now. Having talked to one of our compiler folk about it
here's what he had to say:
> In GCC 4.2 the following backends have STRUCTURE_SIZE_BOUNDARY set to >=32 which will trigger this behaviour:
> 
> Alpha+unicos
> Arm prior to AAPCS
> Mt
> 
> This is within standards to my knowledge.

Cheers
James

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/