[v2] staging: mmal-vchiq: add a check for the return of vmalloc()

[PATCH v2] staging: mmal-vchiq: add a check for the return of vmalloc()

Posted by xkernel.wang@foxmail.com 4 years, 3 months ago

From: Xiaoke Wang <xkernel.wang@foxmail.com>

vmalloc() is a memory allocation API which can return NULL when some
internal memory errors happen. So it is better to check the return
value of it to catch the error in time.

Signed-off-by: Xiaoke Wang <xkernel.wang@foxmail.com>
---
ChangeLog:
v1->v2  jump to the proper location and remove redundant instruction.
 drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
index 76d3f03..ff4b484 100644
--- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
+++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
@@ -1909,6 +1909,10 @@ int vchiq_mmal_init(struct vchiq_mmal_instance **out_instance)
 	mutex_init(&instance->vchiq_mutex);
 
 	instance->bulk_scratch = vmalloc(PAGE_SIZE);
+	if (!instance->bulk_scratch) {
+		err = -ENOMEM;
+		goto err_free;
+	}
 	instance->vchiq_instance = vchiq_instance;
 
 	mutex_init(&instance->context_map_lock);
--

Re: [PATCH v2] staging: mmal-vchiq: add a check for the return of vmalloc()

Posted by Greg KH 4 years, 3 months ago

On Tue, Mar 15, 2022 at 10:42:07PM +0800, xkernel.wang@foxmail.com wrote:
> From: Xiaoke Wang <xkernel.wang@foxmail.com>
> 
> vmalloc() is a memory allocation API which can return NULL when some
> internal memory errors happen. So it is better to check the return
> value of it to catch the error in time.
> 
> Signed-off-by: Xiaoke Wang <xkernel.wang@foxmail.com>
> ---
> ChangeLog:
> v1->v2  jump to the proper location and remove redundant instruction.
>  drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
> index 76d3f03..ff4b484 100644
> --- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
> +++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
> @@ -1909,6 +1909,10 @@ int vchiq_mmal_init(struct vchiq_mmal_instance **out_instance)
>  	mutex_init(&instance->vchiq_mutex);
>  
>  	instance->bulk_scratch = vmalloc(PAGE_SIZE);
> +	if (!instance->bulk_scratch) {
> +		err = -ENOMEM;
> +		goto err_free;
> +	}
>  	instance->vchiq_instance = vchiq_instance;
>  
>  	mutex_init(&instance->context_map_lock);
> -- 

Your change just crashed the kernel :(

Please be more careful.

greg k-h

Re: [PATCH v2] staging: mmal-vchiq: add a check for the return of vmalloc()

Posted by Dan Carpenter 4 years, 3 months ago

On Tue, Mar 15, 2022 at 04:27:38PM +0100, Greg KH wrote:
> On Tue, Mar 15, 2022 at 10:42:07PM +0800, xkernel.wang@foxmail.com wrote:
> > From: Xiaoke Wang <xkernel.wang@foxmail.com>
> > 
> > vmalloc() is a memory allocation API which can return NULL when some
> > internal memory errors happen. So it is better to check the return
> > value of it to catch the error in time.
> > 
> > Signed-off-by: Xiaoke Wang <xkernel.wang@foxmail.com>
> > ---
> > ChangeLog:
> > v1->v2  jump to the proper location and remove redundant instruction.
> >  drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
> > index 76d3f03..ff4b484 100644
> > --- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
> > +++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
> > @@ -1909,6 +1909,10 @@ int vchiq_mmal_init(struct vchiq_mmal_instance **out_instance)
> >  	mutex_init(&instance->vchiq_mutex);
> >  
> >  	instance->bulk_scratch = vmalloc(PAGE_SIZE);
> > +	if (!instance->bulk_scratch) {

Is ->bulk_scratch even used anywhere?

regards,
dan carpenter

Re: [PATCH v2] staging: mmal-vchiq: add a check for the return of vmalloc()

Posted by Xiaoke Wang 4 years, 3 months ago

T24gVHVlLCAxNSBNYXIgMjAyMiAxNjoyNzozOCArMDEwMCwgR3JlZyBLSCB3cm90ZToKJmd0
OyBZb3VyIGNoYW5nZSBqdXN0IGNyYXNoZWQgdGhlIGtlcm5lbCA6KAomZ3Q7CiZndDsgUGxl
YXNlIGJlIG1vcmUgY2FyZWZ1bAoKSSBhbSBzb3JyeS4gSSBldmVyIGJlZW4gdG9sZCB0aGF0
IHZmcmVlKE5VTEwpIG9yIGtmcmVlKE5VTEwpIGlzIHNhZmUsCnNvIEkganVzdCBqdW1wIHRv
IHRoZSB0YWcgYGVycl9mcmVlYCBpbiB0aGlzIHZlcnNpb24uCkkgd2lsbCByZXNlbmQgb25l
IGJ5IGFkZGluZyBhIG5ldyBwcm9wZXIgdGFnLgpBbmQgaW4gZmFjdCwgSSBkaWQgbm90IGZp
bmQgd2hlcmUgLSZndDtidWxrX3NjcmF0Y2ggaXMgdXNlZC4KClJlZ2FyZHMsClhpYW9rZSBX
YW5n

Re: [PATCH v2] staging: mmal-vchiq: add a check for the return of vmalloc()

Posted by gregkh 4 years, 3 months ago

On Wed, Mar 16, 2022 at 12:23:05AM +0800, Xiaoke Wang wrote:
> On Tue, 15 Mar 2022 16:27:38 +0100, Greg KH wrote:
> &gt; Your change just crashed the kernel :(
> &gt;
> &gt; Please be more careful
> 
> I am sorry. I ever been told that vfree(NULL) or kfree(NULL) is safe,

Sorry, you are right, I was thinking that there would be an error value
there.  My mistake.

greg k-h