Re: [Xen-devel] [PATCH] xen-blkback: correctly respond to unknown, non-native requests

[David Vrabel <david.vrabel@xxxxxxxxxx>]

On 04/03/13 09:57, Jan Beulich wrote:
>>>> On 01.03.13 at 18:33, David Vrabel <david.vrabel@xxxxxxxxxx> wrote:
>> If the frontend is using a non-native protocol (e.g., a 64-bit
>> frontend with a 32-bit backend) and it sent an unrecognized request,
>> the request was not translated and the response would have the
>> incorrect ID.  This may cause the frontend driver to behave
>> incorrectly or crash.
>>
>> Since the ID field in the request is always in the same place,
>> regardless of the request type we can get the correct ID and make a
>> valid response (which will report BLKIF_RSP_EOPNOTSUPP).
>>
>> This bug affected 64-bit SLES 11 guests when using a 32-bit backend.
>> This guest does a BLKIF_OP_RESERVED_1 (BLKIF_OP_PACKET in the SLES
>> source) and would crash in blkif_int() as the ID in the response would
>> be invalid.
> 
> I recognize the need for the change, and I'm also fine with the
> patch, but ...
> 
>> --- a/include/xen/interface/io/blkif.h
>> +++ b/include/xen/interface/io/blkif.h
>> @@ -138,11 +138,21 @@ struct blkif_request_discard {
>>      uint8_t        _pad3;
>>  } __attribute__((__packed__));
>>  
>> +struct blkif_request_unknown {
>> +    uint8_t      _pad1;
>> +    blkif_vdev_t _pad2;        /* only for read/write requests         */
>> +#ifdef CONFIG_X86_64
>> +    uint32_t     _pad3;        /* offsetof(blkif_req..,u.unknown.id)==8*/
>> +#endif
>> +    uint64_t     id;           /* private guest value, echoed in resp  */
>> +} __attribute__((__packed__));
>> +
>>  struct blkif_request {
>>      uint8_t        operation;    /* BLKIF_OP_???                         */
>>      union {
>>              struct blkif_request_rw rw;
>>              struct blkif_request_discard discard;
>> +            struct blkif_request_unknown unknown;
>>      } u;
>>  } __attribute__((__packed__));
>>  
> 
> ... I wonder whether "unknown" here really is a suitable term.
> I'd favor "generic" or "other", as "unknown" suggests we know
> _nothing_ about it, which would include the position of the ID
> field.

'other' it is then.

> Also, we'd need a matching patch for the master copy of the
> public header...

The header in Xen only has the native structures doesn't have any the
union so there's nothing to fix.

David

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel