On Sun, Jul 31, 2016 at 09:15:47AM +0300, Leon Romanovsky wrote:
> On Fri, Jul 29, 2016 at 09:31:16PM +0000, Adit Ranadive wrote:
> > On Fri, 29 Jul 2016 12:37:29 +0000, Leon Romanovsky <leon@xxxxxxxxxx> wrote:
> > > On Thu, Jul 28, 2016 at 10:51:39PM +0000, Adit Ranadive wrote:
> > > > On Mon, 18 Jul 2016 15:46:53 +0300 Yuval Shaia <yuval.shaia@xxxxxxxxxx> wrote:
> > > > > On Tue, Jul 12, 2016 at 12:36:32PM -0700, Adit Ranadive wrote:
> > > > > > This patch enables posting Verb requests and receiving responses to/from
> > > > > > the backend PVRDMA emulation layer.
> > > > > >
> > > >
> > > > ...
> > > >
> > > > > > +int
> > > > > > +pvrdma_cmd_post(struct pvrdma_dev *dev, union pvrdma_cmd_req *req,
> > > > > > + bool expect_resp, union pvrdma_cmd_resp *resp)
> > > > > > +{
> > > > > > + int err;
> > > > > > +
> > > > > > + dev_dbg(&dev->pdev->dev, "post request to device\n");
> > > > > > +
> > > > > > + /* Serializiation */
> > > > > > + down(&dev->cmd_sema);
> > > > > > +
> > > > > > + spin_lock(&dev->cmd_lock);
> > > > > > + memcpy(dev->cmd_slot, req, sizeof(*req));
> > > > >
> > > > > Just to protect against memory corruption suggesting to do:
> > > > > memcpy(dev->cmd_slot, req, min(PAGE_SIZE, sizeof(*req)));
> > > > >
> > > >
> > > > Done.
> > >
> > > Hi Adit,
> > > I didn't check how "dev->cmd_slot" was declared and if it is equal to
> > > req in size. Additionally I didn't check if memcpy has any size
> > > limitations, but wanted to warn you that implementation of this
> > > suggestion as is will leave your system in much worse situation of
> > > partially copied data.
> > >
> > > if you really want to check sizes, please do it with BUILD_ON compilation
> > > macro. I have serious doubts if you need it.
> > >
> > > Thanks
> >
> > Hi Leon,
> >
> > Thanks for taking a look!
> > I think this is okay to do was that we allocate a page for cmd_slot anyways
> > and the size of req will always be smaller than PAGE_SIZE. If the request is
> > malformed, our virtual hardware should throw an error for it.
> > I dont think we would run into partially copied data.
> >
> > Having said that, I dont see a necessary advantage of using min here so am
> > willing to drop it if you feel strongly about it.
> >
> > Can you clarify what you mean by memcpy size limitations?
>
> I didn't have time to look after reasons for Yuval's comment about
> min(..), so I assumed that one of the possibilities will be limitation
> of memcpy which I'm not aware.
I understand that it is not such realistic that one of the structures in
union pvrdma_cmd_req will be more then PAGE_SIZE but any other alternative
to protect from such case?
>
> >
> > Thanks,
> > Adit
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
