vtpm_module_init calls vtpmx_init which calls misc_register. The file
operations callbacks are registered. So, vtpmx_fops_ioctl can execute in
parallel with rest of vtpm_module_init. vtpmx_fops_ioctl calls
vtpmx_ioc_new_dev, which calls vtpm_proxy_create_device, which calls
vtpm_proxy_work_start, which could read uninitialized workqueue.
To avoid this, create workqueue before vtpmx init.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Saubhik Mukherjee <[email protected]>
---
drivers/char/tpm/tpm_vtpm_proxy.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
index 91c772e38bb5..225dfa026a8f 100644
--- a/drivers/char/tpm/tpm_vtpm_proxy.c
+++ b/drivers/char/tpm/tpm_vtpm_proxy.c
@@ -697,23 +697,22 @@ static int __init vtpm_module_init(void)
{
int rc;
- rc = vtpmx_init();
- if (rc) {
- pr_err("couldn't create vtpmx device\n");
- return rc;
- }
-
workqueue = create_workqueue("tpm-vtpm");
if (!workqueue) {
pr_err("couldn't create workqueue\n");
- rc = -ENOMEM;
- goto err_vtpmx_cleanup;
+ return -ENOMEM;
+ }
+
+ rc = vtpmx_init();
+ if (rc) {
+ pr_err("couldn't create vtpmx device\n");
+ goto err_destroy_workqueue;
}
return 0;
-err_vtpmx_cleanup:
- vtpmx_cleanup();
+err_destroy_workqueue:
+ destroy_workqueue(workqueue);
return rc;
}
--
2.30.2
On Wed, Jun 23, 2021 at 06:52:26PM +0530, Saubhik Mukherjee wrote:
> vtpm_module_init calls vtpmx_init which calls misc_register. The file
> operations callbacks are registered. So, vtpmx_fops_ioctl can execute in
> parallel with rest of vtpm_module_init. vtpmx_fops_ioctl calls
> vtpmx_ioc_new_dev, which calls vtpm_proxy_create_device, which calls
> vtpm_proxy_work_start, which could read uninitialized workqueue.
>
> To avoid this, create workqueue before vtpmx init.
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Saubhik Mukherjee <[email protected]>
> ---
> drivers/char/tpm/tpm_vtpm_proxy.c | 19 +++++++++----------
> 1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
> index 91c772e38bb5..225dfa026a8f 100644
> --- a/drivers/char/tpm/tpm_vtpm_proxy.c
> +++ b/drivers/char/tpm/tpm_vtpm_proxy.c
> @@ -697,23 +697,22 @@ static int __init vtpm_module_init(void)
> {
> int rc;
>
> - rc = vtpmx_init();
> - if (rc) {
> - pr_err("couldn't create vtpmx device\n");
> - return rc;
> - }
> -
> workqueue = create_workqueue("tpm-vtpm");
> if (!workqueue) {
> pr_err("couldn't create workqueue\n");
> - rc = -ENOMEM;
> - goto err_vtpmx_cleanup;
> + return -ENOMEM;
> + }
> +
> + rc = vtpmx_init();
> + if (rc) {
> + pr_err("couldn't create vtpmx device\n");
> + goto err_destroy_workqueue;
> }
>
> return 0;
>
> -err_vtpmx_cleanup:
> - vtpmx_cleanup();
> +err_destroy_workqueue:
> + destroy_workqueue(workqueue);
>
> return rc;
> }
> --
> 2.30.2
>
>
Reviewed-by: Jarkko Sakkinen <[email protected]>
/Jarkko
On Tue, Jun 29, 2021 at 08:27:00PM +0300, Jarkko Sakkinen wrote:
> On Wed, Jun 23, 2021 at 06:52:26PM +0530, Saubhik Mukherjee wrote:
> > vtpm_module_init calls vtpmx_init which calls misc_register. The file
> > operations callbacks are registered. So, vtpmx_fops_ioctl can execute in
> > parallel with rest of vtpm_module_init. vtpmx_fops_ioctl calls
> > vtpmx_ioc_new_dev, which calls vtpm_proxy_create_device, which calls
> > vtpm_proxy_work_start, which could read uninitialized workqueue.
> >
> > To avoid this, create workqueue before vtpmx init.
> >
> > Found by Linux Driver Verification project (linuxtesting.org).
> >
> > Signed-off-by: Saubhik Mukherjee <[email protected]>
> > ---
> > drivers/char/tpm/tpm_vtpm_proxy.c | 19 +++++++++----------
> > 1 file changed, 9 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
> > index 91c772e38bb5..225dfa026a8f 100644
> > --- a/drivers/char/tpm/tpm_vtpm_proxy.c
> > +++ b/drivers/char/tpm/tpm_vtpm_proxy.c
> > @@ -697,23 +697,22 @@ static int __init vtpm_module_init(void)
> > {
> > int rc;
> >
> > - rc = vtpmx_init();
> > - if (rc) {
> > - pr_err("couldn't create vtpmx device\n");
> > - return rc;
> > - }
> > -
> > workqueue = create_workqueue("tpm-vtpm");
> > if (!workqueue) {
> > pr_err("couldn't create workqueue\n");
> > - rc = -ENOMEM;
> > - goto err_vtpmx_cleanup;
> > + return -ENOMEM;
> > + }
> > +
> > + rc = vtpmx_init();
> > + if (rc) {
> > + pr_err("couldn't create vtpmx device\n");
> > + goto err_destroy_workqueue;
> > }
> >
> > return 0;
> >
> > -err_vtpmx_cleanup:
> > - vtpmx_cleanup();
> > +err_destroy_workqueue:
> > + destroy_workqueue(workqueue);
> >
> > return rc;
> > }
> > --
> > 2.30.2
> >
> >
>
> Reviewed-by: Jarkko Sakkinen <[email protected]>
Taking reviewed-by back.
You're lacking fixes tag. Please re-send with one.
/Jarkko
vtpm_module_init calls vtpmx_init which calls misc_register. The file
operations callbacks are registered. So, vtpmx_fops_ioctl can execute in
parallel with rest of vtpm_module_init. vtpmx_fops_ioctl calls
vtpmx_ioc_new_dev, which calls vtpm_proxy_create_device, which calls
vtpm_proxy_work_start, which could read uninitialized workqueue.
To avoid this, create workqueue before vtpmx init.
Found by Linux Driver Verification project (linuxtesting.org).
Fixes: 6f99612e2500 ("tpm: Proxy driver for supporting multiple emulated TPMs")
Signed-off-by: Saubhik Mukherjee <[email protected]>
---
drivers/char/tpm/tpm_vtpm_proxy.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
index 91c772e38bb5..225dfa026a8f 100644
--- a/drivers/char/tpm/tpm_vtpm_proxy.c
+++ b/drivers/char/tpm/tpm_vtpm_proxy.c
@@ -697,23 +697,22 @@ static int __init vtpm_module_init(void)
{
int rc;
- rc = vtpmx_init();
- if (rc) {
- pr_err("couldn't create vtpmx device\n");
- return rc;
- }
-
workqueue = create_workqueue("tpm-vtpm");
if (!workqueue) {
pr_err("couldn't create workqueue\n");
- rc = -ENOMEM;
- goto err_vtpmx_cleanup;
+ return -ENOMEM;
+ }
+
+ rc = vtpmx_init();
+ if (rc) {
+ pr_err("couldn't create vtpmx device\n");
+ goto err_destroy_workqueue;
}
return 0;
-err_vtpmx_cleanup:
- vtpmx_cleanup();
+err_destroy_workqueue:
+ destroy_workqueue(workqueue);
return rc;
}
--
2.30.2
On 6/30/21 2:35 AM, Jarkko Sakkinen wrote:
> On Tue, Jun 29, 2021 at 08:27:00PM +0300, Jarkko Sakkinen wrote:
>> On Wed, Jun 23, 2021 at 06:52:26PM +0530, Saubhik Mukherjee wrote:
>>> vtpm_module_init calls vtpmx_init which calls misc_register. The file
>>> operations callbacks are registered. So, vtpmx_fops_ioctl can execute in
>>> parallel with rest of vtpm_module_init. vtpmx_fops_ioctl calls
>>> vtpmx_ioc_new_dev, which calls vtpm_proxy_create_device, which calls
>>> vtpm_proxy_work_start, which could read uninitialized workqueue.
>>>
>>> To avoid this, create workqueue before vtpmx init.
>>>
>>> Found by Linux Driver Verification project (linuxtesting.org).
>>>
>>> Signed-off-by: Saubhik Mukherjee <[email protected]>
>>> ---
>>> drivers/char/tpm/tpm_vtpm_proxy.c | 19 +++++++++----------
>>> 1 file changed, 9 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
>>> index 91c772e38bb5..225dfa026a8f 100644
>>> --- a/drivers/char/tpm/tpm_vtpm_proxy.c
>>> +++ b/drivers/char/tpm/tpm_vtpm_proxy.c
>>> @@ -697,23 +697,22 @@ static int __init vtpm_module_init(void)
>>> {
>>> int rc;
>>>
>>> - rc = vtpmx_init();
>>> - if (rc) {
>>> - pr_err("couldn't create vtpmx device\n");
>>> - return rc;
>>> - }
>>> -
>>> workqueue = create_workqueue("tpm-vtpm");
>>> if (!workqueue) {
>>> pr_err("couldn't create workqueue\n");
>>> - rc = -ENOMEM;
>>> - goto err_vtpmx_cleanup;
>>> + return -ENOMEM;
>>> + }
>>> +
>>> + rc = vtpmx_init();
>>> + if (rc) {
>>> + pr_err("couldn't create vtpmx device\n");
>>> + goto err_destroy_workqueue;
>>> }
>>>
>>> return 0;
>>>
>>> -err_vtpmx_cleanup:
>>> - vtpmx_cleanup();
>>> +err_destroy_workqueue:
>>> + destroy_workqueue(workqueue);
>>>
>>> return rc;
>>> }
>>> --
>>> 2.30.2
>>>
>>>
>>
>> Reviewed-by: Jarkko Sakkinen <[email protected]>
>
> Taking reviewed-by back.
>
> You're lacking fixes tag. Please re-send with one.
>
> /Jarkko
>
Thank you for noticing. I sent the patch containing the Fixes tag in
reply to your last message. (https://lkml.org/lkml/2021/6/30/104)
On Wed, Jun 30, 2021 at 12:44:51PM +0530, Saubhik Mukherjee wrote:
> vtpm_module_init calls vtpmx_init which calls misc_register. The file
> operations callbacks are registered. So, vtpmx_fops_ioctl can execute in
> parallel with rest of vtpm_module_init. vtpmx_fops_ioctl calls
> vtpmx_ioc_new_dev, which calls vtpm_proxy_create_device, which calls
> vtpm_proxy_work_start, which could read uninitialized workqueue.
>
> To avoid this, create workqueue before vtpmx init.
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Fixes: 6f99612e2500 ("tpm: Proxy driver for supporting multiple emulated TPMs")
> Signed-off-by: Saubhik Mukherjee <[email protected]>
> ---
Is this v2? What was changed?
> drivers/char/tpm/tpm_vtpm_proxy.c | 19 +++++++++----------
> 1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
> index 91c772e38bb5..225dfa026a8f 100644
> --- a/drivers/char/tpm/tpm_vtpm_proxy.c
> +++ b/drivers/char/tpm/tpm_vtpm_proxy.c
> @@ -697,23 +697,22 @@ static int __init vtpm_module_init(void)
> {
> int rc;
>
> - rc = vtpmx_init();
> - if (rc) {
> - pr_err("couldn't create vtpmx device\n");
> - return rc;
> - }
> -
> workqueue = create_workqueue("tpm-vtpm");
> if (!workqueue) {
> pr_err("couldn't create workqueue\n");
> - rc = -ENOMEM;
> - goto err_vtpmx_cleanup;
> + return -ENOMEM;
> + }
> +
> + rc = vtpmx_init();
> + if (rc) {
> + pr_err("couldn't create vtpmx device\n");
> + goto err_destroy_workqueue;
> }
>
> return 0;
>
> -err_vtpmx_cleanup:
> - vtpmx_cleanup();
> +err_destroy_workqueue:
> + destroy_workqueue(workqueue);
>
> return rc;
> }
> --
> 2.30.2
>
>
/Jarkko
On Wed, Jun 30, 2021 at 12:54:30PM +0530, Saubhik Mukherjee wrote:
> On 6/30/21 2:35 AM, Jarkko Sakkinen wrote:
> > On Tue, Jun 29, 2021 at 08:27:00PM +0300, Jarkko Sakkinen wrote:
> > > On Wed, Jun 23, 2021 at 06:52:26PM +0530, Saubhik Mukherjee wrote:
> > > > vtpm_module_init calls vtpmx_init which calls misc_register. The file
> > > > operations callbacks are registered. So, vtpmx_fops_ioctl can execute in
> > > > parallel with rest of vtpm_module_init. vtpmx_fops_ioctl calls
> > > > vtpmx_ioc_new_dev, which calls vtpm_proxy_create_device, which calls
> > > > vtpm_proxy_work_start, which could read uninitialized workqueue.
> > > >
> > > > To avoid this, create workqueue before vtpmx init.
> > > >
> > > > Found by Linux Driver Verification project (linuxtesting.org).
> > > >
> > > > Signed-off-by: Saubhik Mukherjee <[email protected]>
> > > > ---
> > > > drivers/char/tpm/tpm_vtpm_proxy.c | 19 +++++++++----------
> > > > 1 file changed, 9 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
> > > > index 91c772e38bb5..225dfa026a8f 100644
> > > > --- a/drivers/char/tpm/tpm_vtpm_proxy.c
> > > > +++ b/drivers/char/tpm/tpm_vtpm_proxy.c
> > > > @@ -697,23 +697,22 @@ static int __init vtpm_module_init(void)
> > > > {
> > > > int rc;
> > > > - rc = vtpmx_init();
> > > > - if (rc) {
> > > > - pr_err("couldn't create vtpmx device\n");
> > > > - return rc;
> > > > - }
> > > > -
> > > > workqueue = create_workqueue("tpm-vtpm");
> > > > if (!workqueue) {
> > > > pr_err("couldn't create workqueue\n");
> > > > - rc = -ENOMEM;
> > > > - goto err_vtpmx_cleanup;
> > > > + return -ENOMEM;
> > > > + }
> > > > +
> > > > + rc = vtpmx_init();
> > > > + if (rc) {
> > > > + pr_err("couldn't create vtpmx device\n");
> > > > + goto err_destroy_workqueue;
> > > > }
> > > > return 0;
> > > > -err_vtpmx_cleanup:
> > > > - vtpmx_cleanup();
> > > > +err_destroy_workqueue:
> > > > + destroy_workqueue(workqueue);
> > > > return rc;
> > > > }
> > > > --
> > > > 2.30.2
> > > >
> > > >
> > >
> > > Reviewed-by: Jarkko Sakkinen <[email protected]>
> >
> > Taking reviewed-by back.
> >
> > You're lacking fixes tag. Please re-send with one.
> >
> > /Jarkko
> >
>
> Thank you for noticing. I sent the patch containing the Fixes tag in reply
> to your last message. (https://lkml.org/lkml/2021/6/30/104)
Please do not do that. Instead, version your patches (git format-patch -vX)
and send them as separate threads.
It's all documented: https://www.kernel.org/doc/html/v5.13/process/submitting-patches.html
/Jarkko