linux-kernel - Re: [PROBLEM]: WARNING: lock held when returning to user space! (5.4.1 #16 Tainted: G )

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20191211174536.GG4516@linux.intel.com>
Date:   Wed, 11 Dec 2019 19:45:36 +0200
From:   Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
To:     Tadeusz Struk <tadeusz.struk@...el.com>
Cc:     Will Deacon <will@...nel.org>,
        Jeffrin Jose <jeffrin@...agiritech.edu.in>,
        peterz@...radead.org, mingo@...hat.com,
        linux-kernel@...r.kernel.org, linux-integrity@...r.kernel.org,
        peterhuewe@....de, jgg@...pe.ca
Subject: Re: [PROBLEM]: WARNING: lock held when returning to user space!
 (5.4.1 #16 Tainted: G )

On Mon, Dec 09, 2019 at 01:34:29PM -0800, Tadeusz Struk wrote:
> I think that's expected for a non-blocking operation.

What do you mean by "expected"?

It is a locking bug. When you implemented the feature you failed
to free locks before going back to the user space and I failed to
notice this when I reviewed the code.

> To get rid of the warning it should be changed to something like this:
> 
> diff --git a/drivers/char/tpm/tpm-dev-common.c
> b/drivers/char/tpm/tpm-dev-common.c
> index 2ec47a69a2a6..47f1c0c5c8de 100644
> --- a/drivers/char/tpm/tpm-dev-common.c
> +++ b/drivers/char/tpm/tpm-dev-common.c
> @@ -61,6 +61,12 @@ static void tpm_dev_async_work(struct work_struct *work)
> 
>  	mutex_lock(&priv->buffer_mutex);
>  	priv->command_enqueued = false;
> +	ret = tpm_try_get_ops(priv->chip);
> +	if (ret) {
> +		priv->response_length = ret;
> +		goto out;
> +	}
> +
>  	ret = tpm_dev_transmit(priv->chip, priv->space, priv->data_buffer,
>  			       sizeof(priv->data_buffer));
>  	tpm_put_ops(priv->chip);
> @@ -68,6 +74,7 @@ static void tpm_dev_async_work(struct work_struct *work)
>  		priv->response_length = ret;
>  		mod_timer(&priv->user_read_timer, jiffies + (120 * HZ));
>  	}
> +out:
>  	mutex_unlock(&priv->buffer_mutex);
>  	wake_up_interruptible(&priv->async_wait);
>  }
> @@ -205,6 +212,7 @@ ssize_t tpm_common_write(struct file *file, const
> char __user *buf,
>  		priv->command_enqueued = true;
>  		queue_work(tpm_dev_wq, &priv->async_work);
>  		mutex_unlock(&priv->buffer_mutex);
> +		tpm_put_ops(priv->chip);
>  		return size;
>  	}
> 
> 
> 
> -- 
> Tadeusz

The fix looks appropriate but needs to be formalized as a patch.

/Jarkko