Re: [PATCH v2 02/15] IB/pvrdma: Add device command support

[Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 1390 bytes --]

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-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> 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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]