| Message ID | 1472977778-23996-1-git-send-email-prasannatsmkumar@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Herbert Xu |
| Headers | show |
On Sun, Sep 04, 2016 at 01:59:38PM +0530, PrasannaKumar Muralidharan wrote: > > @@ -573,6 +557,17 @@ EXPORT_SYMBOL_GPL(devm_hwrng_unregister); > > static int __init hwrng_modinit(void) > { > + /* kmalloc makes this safe for virt_to_page() in virtio_rng.c */ > + rng_buffer = kmalloc(rng_buffer_size(), GFP_KERNEL); > + if (!rng_buffer) > + return -ENOMEM; > + > + rng_fillbuf = kmalloc(rng_buffer_size(), GFP_KERNEL); > + if (!rng_fillbuf) { > + kfree(rng_buffer); > + return -ENOMEM; > + } > + > return register_miscdev(); You're leaking memory if register_miscdev fails. Cheers,
> On Sun, Sep 04, 2016 at 01:59:38PM +0530, PrasannaKumar Muralidharan wrote: >> >> @@ -573,6 +557,17 @@ EXPORT_SYMBOL_GPL(devm_hwrng_unregister); >> >> static int __init hwrng_modinit(void) >> { >> + /* kmalloc makes this safe for virt_to_page() in virtio_rng.c */ >> + rng_buffer = kmalloc(rng_buffer_size(), GFP_KERNEL); >> + if (!rng_buffer) >> + return -ENOMEM; >> + >> + rng_fillbuf = kmalloc(rng_buffer_size(), GFP_KERNEL); >> + if (!rng_fillbuf) { >> + kfree(rng_buffer); >> + return -ENOMEM; >> + } >> + >> return register_miscdev(); > > You're leaking memory if register_miscdev fails. Ah, yes! I can send a revised patch. Do you think allocating rng_buffer, rng_fillbuf in module init useful? If not there is no point in sending a new patch with fix. Thanks, PrasannaKumar -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Sep 07, 2016 at 06:44:32PM +0530, PrasannaKumar Muralidharan wrote: > > Ah, yes! I can send a revised patch. > Do you think allocating rng_buffer, rng_fillbuf in module init useful? > If not there is no point in sending a new patch with fix. Yes I think it makes sense. Those who want to save a few bytes by keeping the RNGs as modules can always build core HWRNG as a module too. Cheers,
diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c index 9203f2d..ec6846b 100644 --- a/drivers/char/hw_random/core.c +++ b/drivers/char/hw_random/core.c @@ -449,22 +449,6 @@ int hwrng_register(struct hwrng *rng) goto out; mutex_lock(&rng_mutex); - - /* kmalloc makes this safe for virt_to_page() in virtio_rng.c */ - err = -ENOMEM; - if (!rng_buffer) { - rng_buffer = kmalloc(rng_buffer_size(), GFP_KERNEL); - if (!rng_buffer) - goto out_unlock; - } - if (!rng_fillbuf) { - rng_fillbuf = kmalloc(rng_buffer_size(), GFP_KERNEL); - if (!rng_fillbuf) { - kfree(rng_buffer); - goto out_unlock; - } - } - /* Must not register two RNGs with the same name. */ err = -EEXIST; list_for_each_entry(tmp, &rng_list, list) { @@ -573,6 +557,17 @@ EXPORT_SYMBOL_GPL(devm_hwrng_unregister); static int __init hwrng_modinit(void) { + /* kmalloc makes this safe for virt_to_page() in virtio_rng.c */ + rng_buffer = kmalloc(rng_buffer_size(), GFP_KERNEL); + if (!rng_buffer) + return -ENOMEM; + + rng_fillbuf = kmalloc(rng_buffer_size(), GFP_KERNEL); + if (!rng_fillbuf) { + kfree(rng_buffer); + return -ENOMEM; + } + return register_miscdev(); }
In core rng_buffer and rng_fillbuf is allocated in hwrng_register only once and it is freed during module exit. This patch moves allocating rng_buffer and rng_fillbuf from hwrng_register to rng core's init. This avoids checking whether rng_buffer and rng_fillbuf was allocated from every hwrng_register call. Also moving them to module init makes it explicit that it is freed in module exit. Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com> --- drivers/char/hw_random/core.c | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-)