Re: [PATCH 3/4] sg: check for valid direction before starting the request

[Hannes Reinecke <hare@suse.de>]

On 02/03/2017 11:28 AM, Christoph Hellwig wrote:
> On Fri, Feb 03, 2017 at 09:54:50AM +0100, Hannes Reinecke wrote:
>> From: Johannes Thumshirn <jthumshirn@suse.de>
>>
>> Check for a valid direction before starting the request, otherwise we risk
>> running into an assertion in the scsi midlayer checking for vaild requests.
> 
> Good idea, but..
> 
>> +static bool sg_is_valid_direction(int dxfer_direction)
>> +{
>> +	switch (dxfer_direction) {
>> +	case SG_DXFER_NONE:
>> +	case SG_DXFER_TO_DEV:
>> +	case SG_DXFER_FROM_DEV:
>> +	case SG_DXFER_TO_FROM_DEV:
> 
> This isn't strictly valid as sg doesn't actually handle real bidi
> commands, but we work around it.
> 
> It might be a good idea to move the warning about it from sg_write
> to here and use printk_ratelimited instead of the hand-rolled
> per-thread warning there.
> 
Well, that warning is only applicable to sg_write(); sg_new_write() is
supposed to have the correct header, so that warning doesn't apply.
We could drop the per-thread-thingie, though.


>> +	case SG_DXFER_UNKNOWN:
>> +		return true;
> 
> And how valid is this one (Question to Doug I guess): for a 0-sized
> transfer we should be fine, but otherwise it should probably be
> rejected.
> 
Yes, true.
Will be fixing it up.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)