On Tue, Aug 04, 2015 at 09:43:50AM -0500, Aaron Sierra wrote:

You are correct about the leak and I regret introducing that (I am
also leaking priv->rng), but I disagree with your dismissal of the
rest of the changes as unrelated to the changelog.

I understand the problem, but your change description is way
too vague and does not correspond to the change, especially the
bit where you're replacing the static variable with kzalloc.  You
should have included the following text in your description.

The probe error path for this driver, for all intents and purposes,
is the talitos_remove() function due to the common "goto err_out".

Without my patch applied, talitos_remove() will panic under the
two conditions that I outlined in the changelog:

1. If the RNG device hasn't been registered via
   talitos_register_rng() prior to entry into talitos_remove(),
   then the attempt to unregister the RNG "device" will cause a panic.

2. If the priv->chan array has not been allocated prior to entry
   into talitos_remove(), then the per-channel FIFO cleanup will panic
   because of the dereference of that NULL "array".

Both of the above scenarios occur if talitos_probe_irq() fails.

The patch that I submitted resolves issue #1 by changing priv->rng
to a struct hwrng pointer, which allows talitos_unregister_rng() to
know whether an RNG device has been registered (or at least prepared
for registration). As an alternative, I considered introducing a
boolean to serve the same purpose.

I would prefer a boolean as that means less churn.

Thanks,
-- 
Email: Herbert Xu [off-list ref]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt