Re: [Xen-devel] [PATCH 6/6] xen-blkfront: prepare request locally, only then put it on the shared ring

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, May 01, 2018 at 09:22:31AM +0100, Roger Pau Monné wrote:
> On Mon, Apr 30, 2018 at 11:01:50PM +0200, Marek Marczykowski-Górecki wrote:
> >                                         struct request *req,
> > -                                       struct blkif_request **ring_req)
> > +                                       struct blkif_request *ring_req)
> >  {
> >     unsigned long id;
> >  
> > -   *ring_req = RING_GET_REQUEST(&rinfo->ring, rinfo->ring.req_prod_pvt);
> > -   rinfo->ring.req_prod_pvt++;
> > -
> >     id = get_id_from_freelist(rinfo);
> >     rinfo->shadow[id].request = req;
> >     rinfo->shadow[id].status = REQ_WAITING;
> >     rinfo->shadow[id].associated_id = NO_ASSOCIATED_ID;
> >  
> > -   (*ring_req)->u.rw.id = id;
> > +   ring_req->u.rw.id = id;
> >  
> >     return id;
> >  }
> > @@ -545,23 +542,28 @@ static unsigned long blkif_ring_get_request(struct 
> > blkfront_ring_info *rinfo,
> >  static int blkif_queue_discard_req(struct request *req, struct 
> > blkfront_ring_info *rinfo)
> >  {
> >     struct blkfront_info *info = rinfo->dev_info;
> > -   struct blkif_request *ring_req;
> > +   struct blkif_request ring_req = { 0 };
> >     unsigned long id;
> >  
> >     /* Fill out a communications ring structure. */
> >     id = blkif_ring_get_request(rinfo, req, &ring_req);
> 
> Maybe I'm missing something obvious here, but you are adding a struct
> allocated on the stack to the shadow ring copy, isn't this dangerous?

The above comment is wrong, you are storing a pointer to 'req' in the
shadow ring copy, which is fine and is not the ring request.

Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel