Re: [PATCH] mtd: nandsim: remove debugfs entries in error path

[Brian Norris <computersforpeace@gmail.com>]

Hi,

On Wed, Aug 23, 2017 at 09:03:04AM +0200, Uwe Kleine-König wrote:
> The debugfs entries must be removed before an error is returned in the
> probe function. Otherwise another try to load the module fails and when
> the debugfs files are accessed without the module loaded, the kernel
> still tries to call a function in that module.
> 
> Fixes: 5346c27c5fed ("mtd: nandsim: Introduce debugfs infrastructure")
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/mtd/nand/nandsim.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mtd/nand/nandsim.c b/drivers/mtd/nand/nandsim.c
> index 03a0d057bf2f..e4211c3cc49b 100644
> --- a/drivers/mtd/nand/nandsim.c
> +++ b/drivers/mtd/nand/nandsim.c
> @@ -2373,6 +2373,7 @@ static int __init ns_init_module(void)
>          return 0;
>  
>  err_exit:
> +	nandsim_debugfs_remove(nand);

The 'err_exit' label is used by error paths that occur before the
debugfs entries are registered. But I guess that's OK, since
debugfs_remove_recursive() handles NULL arguments fine?

We also have the same mismatch with init_nandsim()/free_nandsim(), which
is also similarly safe because of the nature of kfree(), vfree(), and
the fact that most of those are guarded with NULL checks anyway... So
it's just confusing and potentially dangerous for future developers --
but not wrong.

Given this code will promptly go away with the for-4.14 material, I
suppose this isn't that bad.

I'll look to queue this up and send to Linus within a day.

Thanks,
Brian

>  	free_nandsim(nand);
>  	nand_release(nsmtd);
>  	for (i = 0;i < ARRAY_SIZE(nand->partitions); ++i)
> -- 
> 2.11.0
>