Nick,
Do not merge this patch. It will fix the problem, but cause a regression
if there was a failure during setup.
On 05/17/2017 02:59 AM, Mike Christie wrote:
> We currently do
>
> tcmu_free_device ->tcmu_netlink_event(TCMU_CMD_REMOVED_DEVICE) ->
> kfree(tcmu_dev) via rcu call.
>
> The problem is that the kernel does not wait for userspace to
> do close() on the uio device before freeing the tcmu_dev.
> We can then hit a race where the kernel frees the tcmu_dev before
> userspace does close() and so when close() -> release -> tcmu_release
> is done, we try to access a freed tcmu_dev.
>
> This patch made over the target-pending master branch moves the freeing of
> the tcmu_dev to tcmu_release.
>
> Signed-off-by: Mike Christie <mchristi@xxxxxxxxxx>
>
> ---
> drivers/target/target_core_user.c | 25 ++++++++++++++-----------
> 1 file changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c
> index 9045837..cda0bd7 100644
> --- a/drivers/target/target_core_user.c
> +++ b/drivers/target/target_core_user.c
> @@ -1145,14 +1145,26 @@ static int tcmu_open(struct uio_info *info, struct inode *inode)
> return 0;
> }
>
> +static void tcmu_dev_call_rcu(struct rcu_head *p)
> +{
> + struct se_device *dev = container_of(p, struct se_device, rcu_head);
> + struct tcmu_dev *udev = TCMU_DEV(dev);
> +
> + kfree(udev);
> +}
> +
> static int tcmu_release(struct uio_info *info, struct inode *inode)
> {
> struct tcmu_dev *udev = container_of(info, struct tcmu_dev, uio_info);
> + struct se_device *dev = &udev->se_dev;
>
> clear_bit(TCMU_DEV_BIT_OPEN, &udev->flags);
>
> pr_debug("close\n");
>
> + kfree(udev->uio_info.name);
> + kfree(udev->name);
> + call_rcu(&dev->rcu_head, tcmu_dev_call_rcu);
> return 0;
> }
>
> @@ -1302,14 +1314,6 @@ static int tcmu_check_and_free_pending_cmd(struct tcmu_cmd *cmd)
> return -EINVAL;
> }
>
> -static void tcmu_dev_call_rcu(struct rcu_head *p)
> -{
> - struct se_device *dev = container_of(p, struct se_device, rcu_head);
> - struct tcmu_dev *udev = TCMU_DEV(dev);
> -
> - kfree(udev);
> -}
> -
> static bool tcmu_dev_configured(struct tcmu_dev *udev)
> {
> return udev->uio_info.uio_dev ? true : false;
> @@ -1364,10 +1368,9 @@ static void tcmu_free_device(struct se_device *dev)
> udev->uio_info.uio_dev->minor);
>
> uio_unregister_device(&udev->uio_info);
> - kfree(udev->uio_info.name);
> - kfree(udev->name);
> + } else {
> + call_rcu(&dev->rcu_head, tcmu_dev_call_rcu);
> }
> - call_rcu(&dev->rcu_head, tcmu_dev_call_rcu);
> }
>
> enum {
>
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
