Re: [PATCH 5.15] crypto: sun4i-ss - Remove insecure and unused rng_alg

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The driver fails to remove its debugfs directory on device removal or module unload, leading to a dangling debugfs entry that can cause a use-after-free and kernel panic.
--

--- Patch [1]: [PATCH 5.15] crypto: sun4i-ss - Remove insecure and unused rng_alg ---
commit 170a30200701519fc546c370a0ab6113fdc3a608
Author: Eric Biggers <[email protected]>

crypto: sun4i-ss - Remove insecure and unused rng_alg

This commit removes the sun4i_ss_rng implementation because it is insecure,
has multiple vulnerabilities including a buffer overflow, and is unused.
There is no point in fixing these issues individually when the code is
unneeded.

Closes: https://lore.kernel.org/r/[email protected]/

> diff --git a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-core.c b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-core.c
> --- a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-core.c
> +++ b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-core.c

[ ... ]

> @@ -535,9 +502,6 @@ static int sun4i_ss_remove(struct platform_device *pdev)
>  		case CRYPTO_ALG_TYPE_AHASH:
>  			crypto_unregister_ahash(&ss_algs[i].alg.hash);
>  			break;
> -		case CRYPTO_ALG_TYPE_RNG:
> -			crypto_unregister_rng(&ss_algs[i].alg.rng);
> -			break;
>  		}
>  	}
>  

[Severity: High]
This is a pre-existing issue, but does this function leak the debugfs
directory when the module is removed?

In sun4i_ss_probe(), the driver sets up the debugfs entries:

    ss->dbgfs_dir = debugfs_create_dir("sun4i-ss", NULL);
    ss->dbgfs_stats = debugfs_create_file("stats", 0444, ss->dbgfs_dir, ss,
                                          &sun4i_ss_debugfs_fops);

Since sun4i_ss_remove() lacks a corresponding debugfs_remove_recursive()
call, could unloading the module leave the debugfs entries registered in the
VFS, leading to a use-after-free and kernel panic if someone opens
/sys/kernel/debug/sun4i-ss/stats after the module memory is freed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1