Re: [RESEND PATCH v2] platform/chrome: cros_ec_debugfs: detach log reader wq from devm

[Tzung-Bi Shih <tzungbi@google.com>]

On Wed, Feb 09, 2022 at 01:30:14PM -0800, Prashant Malani wrote:
> On Tue, Feb 8, 2022 at 9:11 PM Tzung-Bi Shih <tzungbi@google.com> wrote:
> > +/* waitqueue for log readers */
> > +static DECLARE_WAIT_QUEUE_HEAD(cros_ec_debugfs_log_wq);
> 
> Hmm, I'm always wary of introducing statics to address races like this.
> 
> Doesn't re-ordering cros_ec_debugfs_remove() avoid the race? :
> diff --git a/drivers/platform/chrome/cros_ec_debugfs.c
> b/drivers/platform/chrome/cros_ec_debugfs.c
> index 272c89837d74..80bd898c3271 100644
> --- a/drivers/platform/chrome/cros_ec_debugfs.c
> +++ b/drivers/platform/chrome/cros_ec_debugfs.c
> @@ -485,8 +485,8 @@ static int cros_ec_debugfs_remove(struct
> platform_device *pd)
>  {
>         struct cros_ec_dev *ec = dev_get_drvdata(pd->dev.parent);
> 
> -       debugfs_remove_recursive(ec->debug_info->dir);
>         cros_ec_cleanup_console_log(ec->debug_info);
> +       debugfs_remove_recursive(ec->debug_info->dir);
> 
>         return 0;
>  }

I have tested the patch and it doesn't help.  The issue is: cros_ec_debugfs
provides a wait queue to fs at [1]; fs remembers the address at [2].  The wait
queue is in devm memory.  After the memory has freed (i.e. the device has
removed), fs still gets chance to access the memory (e.g. [3] in the case).

[1]: https://elixir.bootlin.com/linux/v5.17-rc3/source/drivers/platform/chrome/cros_ec_debugfs.c#L176
[2]: https://elixir.bootlin.com/linux/v5.17-rc3/source/fs/eventpoll.c#L1256
[3]: https://elixir.bootlin.com/linux/v5.17-rc3/source/fs/eventpoll.c#L550