[KJ] [PATCH 2.6.19-rc2] drivers/usb/serial/mos7840.c: check

[KJ] [PATCH 2.6.19-rc2] drivers/usb/serial/mos7840.c: check

[Amit Choudhary]

Description: Check the return value of kmalloc() in function mos7840_get_reg(), in file drivers/usb/serial/mos7840.c.

Signed-off-by: Amit Choudhary <amit2030@gmail.com>

diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c
index 021be39..91d474b 100644
--- a/drivers/usb/serial/mos7840.c
+++ b/drivers/usb/serial/mos7840.c
@@ -475,6 +475,14 @@ static int mos7840_get_reg(struct moschi
 	int ret = 0;
 	buffer = (__u8 *) mcs->ctrl_buf;
 
+	/* The memory for ctrl_buf is allocated in
+	 * mos7840_startup(), but it is not checked if
+	 * kmalloc failed. So, mcs->ctrl_buf might be NULL.
+	 * So, it should be checked here.
+	 */
+	if (!buffer)
+		return -ENOMEM;
+
 //      dr=(struct usb_ctrlrequest *)(buffer);
 	dr = (void *)(buffer + 2);
 	dr->bRequestType = MCS_RD_RTYPE;
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH 2.6.19-rc2] drivers/usb/serial/mos7840.c: check

[Greg KH]

On Sun, Oct 15, 2006 at 10:55:13PM -0700, Amit Choudhary wrote:
> Description: Check the return value of kmalloc() in function mos7840_get_reg(), in file drivers/usb/serial/mos7840.c.
> 
> Signed-off-by: Amit Choudhary <amit2030@gmail.com>
> 
> diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c
> index 021be39..91d474b 100644
> --- a/drivers/usb/serial/mos7840.c
> +++ b/drivers/usb/serial/mos7840.c
> @@ -475,6 +475,14 @@ static int mos7840_get_reg(struct moschi
>  	int ret = 0;
>  	buffer = (__u8 *) mcs->ctrl_buf;
>  
> +	/* The memory for ctrl_buf is allocated in
> +	 * mos7840_startup(), but it is not checked if
> +	 * kmalloc failed. So, mcs->ctrl_buf might be NULL.
> +	 * So, it should be checked here.
> +	 */
> +	if (!buffer)
> +		return -ENOMEM;

Why not just check it in the startup function instead?

thanks,

greg k-h
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH 2.6.19-rc2] drivers/usb/serial/mos7840.c: check

[Amit Choudhary]

> > @@ -475,6 +475,14 @@ static int mos7840_get_reg(struct moschi
> >  	int ret = 0;
> >  	buffer = (__u8 *) mcs->ctrl_buf;
> >  
> > +	/* The memory for ctrl_buf is allocated in
> > +	 * mos7840_startup(), but it is not checked if
> > +	 * kmalloc failed. So, mcs->ctrl_buf might be NULL.
> > +	 * So, it should be checked here.
> > +	 */
> > +	if (!buffer)
> > +		return -ENOMEM;
> 
> Why not just check it in the startup function instead?

After looking at the code, I inferred that it can still proceed if ctrl_buf is NULL because
ctrl_buf is referenced only in mos7840_get_reg(). But please let me know if this is not the right
way.



__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH 2.6.19-rc2] drivers/usb/serial/mos7840.c: check

[Greg KH]

On Mon, Oct 16, 2006 at 11:33:50PM -0700, Amit Choudhary wrote:
> 
> 
> 
> > > @@ -475,6 +475,14 @@ static int mos7840_get_reg(struct moschi
> > >  	int ret = 0;
> > >  	buffer = (__u8 *) mcs->ctrl_buf;
> > >  
> > > +	/* The memory for ctrl_buf is allocated in
> > > +	 * mos7840_startup(), but it is not checked if
> > > +	 * kmalloc failed. So, mcs->ctrl_buf might be NULL.
> > > +	 * So, it should be checked here.
> > > +	 */
> > > +	if (!buffer)
> > > +		return -ENOMEM;
> > 
> > Why not just check it in the startup function instead?
> 
> After looking at the code, I inferred that it can still proceed if ctrl_buf is NULL because
> ctrl_buf is referenced only in mos7840_get_reg(). But please let me know if this is not the right
> way.

Nah, just error out in the beginning when the buffer is created.  It
makes is simpler to handle the low memory condition.

thanks,

greg k-h
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors