Re: [PATCH] crypto: talitos: Prevent panic in probe error path

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



----- Original Message -----
> From: "Herbert Xu" <herbert@xxxxxxxxxxxxxxxxxxx>
> Sent: Tuesday, August 4, 2015 7:08:13 PM
> 
> 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.
> 

Herbert,
Thanks for the feedback. I'll address your concerns in the next
version.

-Aaron
--
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



[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]

  Powered by Linux