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 <herbert@xxxxxxxxxxxxxxxxxxx> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
