| Message ID | 20220518134922.8833-1-eajames@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [linux,dev-5.15] fsi: occ: Prevent use after free | expand |
On Wed, 18 May 2022 at 13:49, Eddie James <eajames@linux.ibm.com> wrote: > > Use get_device and put_device in the open and close functions to > make sure the device doesn't get freed while a file descriptor is > open. > Also, lock around the freeing of the device buffer and check the > buffer before using it in the submit function. > > Signed-off-by: Eddie James <eajames@linux.ibm.com> > Reviewed-by: Guenter Roeck <linux@roeck-us.net> Applied, thanks. > --- > drivers/fsi/fsi-occ.c | 18 +++++++++++++++--- > 1 file changed, 15 insertions(+), 3 deletions(-) > > diff --git a/drivers/fsi/fsi-occ.c b/drivers/fsi/fsi-occ.c > index 3d04e8baecbb..8f7f602b909d 100644 > --- a/drivers/fsi/fsi-occ.c > +++ b/drivers/fsi/fsi-occ.c > @@ -94,6 +94,7 @@ static int occ_open(struct inode *inode, struct file *file) > client->occ = occ; > mutex_init(&client->lock); > file->private_data = client; > + get_device(occ->dev); > > /* We allocate a 1-page buffer, make sure it all fits */ > BUILD_BUG_ON((OCC_CMD_DATA_BYTES + 3) > PAGE_SIZE); > @@ -197,6 +198,7 @@ static int occ_release(struct inode *inode, struct file *file) > { > struct occ_client *client = file->private_data; > > + put_device(client->occ->dev); > free_page((unsigned long)client->buffer); > kfree(client); > > @@ -493,12 +495,19 @@ int fsi_occ_submit(struct device *dev, const void *request, size_t req_len, > for (i = 1; i < req_len - 2; ++i) > checksum += byte_request[i]; > > - mutex_lock(&occ->occ_lock); > + rc = mutex_lock_interruptible(&occ->occ_lock); > + if (rc) > + return rc; > > occ->client_buffer = response; > occ->client_buffer_size = user_resp_len; > occ->client_response_size = 0; > > + if (!occ->buffer) { > + rc = -ENOENT; > + goto done; > + } > + > /* > * Get a sequence number and update the counter. Avoid a sequence > * number of 0 which would pass the response check below even if the > @@ -674,10 +683,13 @@ static int occ_remove(struct platform_device *pdev) > { > struct occ *occ = platform_get_drvdata(pdev); > > - kvfree(occ->buffer); > - > misc_deregister(&occ->mdev); > > + mutex_lock(&occ->occ_lock); > + kvfree(occ->buffer); > + occ->buffer = NULL; > + mutex_unlock(&occ->occ_lock); > + > device_for_each_child(&pdev->dev, NULL, occ_unregister_child); > > ida_simple_remove(&occ_ida, occ->idx); > -- > 2.27.0 >
diff --git a/drivers/fsi/fsi-occ.c b/drivers/fsi/fsi-occ.c index 3d04e8baecbb..8f7f602b909d 100644 --- a/drivers/fsi/fsi-occ.c +++ b/drivers/fsi/fsi-occ.c @@ -94,6 +94,7 @@ static int occ_open(struct inode *inode, struct file *file) client->occ = occ; mutex_init(&client->lock); file->private_data = client; + get_device(occ->dev); /* We allocate a 1-page buffer, make sure it all fits */ BUILD_BUG_ON((OCC_CMD_DATA_BYTES + 3) > PAGE_SIZE); @@ -197,6 +198,7 @@ static int occ_release(struct inode *inode, struct file *file) { struct occ_client *client = file->private_data; + put_device(client->occ->dev); free_page((unsigned long)client->buffer); kfree(client); @@ -493,12 +495,19 @@ int fsi_occ_submit(struct device *dev, const void *request, size_t req_len, for (i = 1; i < req_len - 2; ++i) checksum += byte_request[i]; - mutex_lock(&occ->occ_lock); + rc = mutex_lock_interruptible(&occ->occ_lock); + if (rc) + return rc; occ->client_buffer = response; occ->client_buffer_size = user_resp_len; occ->client_response_size = 0; + if (!occ->buffer) { + rc = -ENOENT; + goto done; + } + /* * Get a sequence number and update the counter. Avoid a sequence * number of 0 which would pass the response check below even if the @@ -674,10 +683,13 @@ static int occ_remove(struct platform_device *pdev) { struct occ *occ = platform_get_drvdata(pdev); - kvfree(occ->buffer); - misc_deregister(&occ->mdev); + mutex_lock(&occ->occ_lock); + kvfree(occ->buffer); + occ->buffer = NULL; + mutex_unlock(&occ->occ_lock); + device_for_each_child(&pdev->dev, NULL, occ_unregister_child); ida_simple_remove(&occ_ida, occ->idx);