linux-kernel - Re: [PATCH v2] xen/scsifront: don't request a slot on the ring until request is ready

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 for Android: free password hash cracker in your pocket

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

Date:   Mon, 5 Dec 2016 16:35:13 +0100
From:   Juergen Gross <jgross@...e.com>
To:     Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        linux-kernel@...r.kernel.org, xen-devel@...ts.xenproject.org
Cc:     lambert.quentin@...il.com, linux-scsi@...r.kernel.org,
        dan.carpenter@...cle.com, jejb@...ux.vnet.ibm.com,
        martin.petersen@...cle.com
Subject: Re: [PATCH v2] xen/scsifront: don't request a slot on the ring until
 request is ready

On 05/12/16 16:32, Boris Ostrovsky wrote:
> On 12/02/2016 01:15 AM, Juergen Gross wrote:
>>  
>> -static struct vscsiif_request *scsifront_pre_req(struct vscsifrnt_info *info)
>> +static int scsifront_do_request(struct vscsifrnt_info *info,
>> +				struct vscsifrnt_shadow *shadow)
>>  {
>>  	struct vscsiif_front_ring *ring = &(info->ring);
>>  	struct vscsiif_request *ring_req;
>> +	struct scsi_cmnd *sc = shadow->sc;
>>  	uint32_t id;
>> +	int i, notify;
>> +
>> +	if (RING_FULL(&info->ring))
>> +		return -EBUSY;
>>  
>>  	id = scsifront_get_rqid(info);	/* use id in response */
>>  	if (id >= VSCSIIF_MAX_REQS)
>> -		return NULL;
>> +		return -EBUSY;
>> +
>> +	info->shadow[id] = shadow;
>> +	shadow->rqid = id;
>>  
>>  	ring_req = RING_GET_REQUEST(&(info->ring), ring->req_prod_pvt);
>> -
>>  	ring->req_prod_pvt++;
>>  
>> -	ring_req->rqid = (uint16_t)id;
>> +	ring_req->rqid        = id;
>> +	ring_req->act         = shadow->act;
>> +	ring_req->ref_rqid    = shadow->ref_rqid;
>> +	ring_req->nr_segments = shadow->nr_segments;
> 
> Shouldn't req_prod_pvt be incremented after you've copied everything? (I
> realize that there is not error until everything has been copied but still.)

That doesn't really matter as it is private.

>> @@ -473,44 +496,14 @@ static int map_data_for_request(struct vscsifrnt_info *info,
>>  	}
>>  
>>  	if (seg_grants)
>> -		ring_req->nr_segments = VSCSIIF_SG_GRANT | seg_grants;
>> +		shadow->nr_segments = VSCSIIF_SG_GRANT | seg_grants;
> 
> Are we guaranteed that seg_grants is not going to have VSCSIIF_SG_GRANT
> bit set?

Absolutely, yes. Can't be larger than VSCSIIF_SG_TABLESIZE which is 26.


Juergen