[PATCH] ibmphp : read the length of ebda and map entire ebda region

[PATCH] ibmphp : read the length of ebda and map entire ebda region

[Chandru]

ibmphp driver currently maps only 1KB of ebda memory area into kernel address 
space during driver initialization. This causes kernel oops when the driver is 
modprobe'd and it accesses memory area beyond 1KB within ebda segment. The first 
byte of ebda segment actually stores the length of the ebda region in 
Kilobytes. Hence make use of the length parameter and map the entire ebda 
region.


Signed-off-by: Chandru Siddalingappa <chandru@linux.vnet.ibm.com>
---

 drivers/pci/hotplug/ibmphp_ebda.c |   11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

--- linux-2.6.33-rc2/drivers/pci/hotplug/ibmphp_ebda.c.orig	2010-01-09 
15:23:54.000000000 +0530
+++ linux-2.6.33-rc2/drivers/pci/hotplug/ibmphp_ebda.c	2010-01-09 
17:01:06.000000000 +0530
@@ -245,7 +245,7 @@ static void __init print_ebda_hpc (void)
 
 int __init ibmphp_access_ebda (void)
 {
-	u8 format, num_ctlrs, rio_complete, hs_complete;
+	u8 format, num_ctlrs, rio_complete, hs_complete, ebda_sz;
 	u16 ebda_seg, num_entries, next_offset, offset, blk_id, sub_addr, re, rc_id, 
re_id, base;
 	int rc = 0;
 
@@ -260,7 +260,14 @@ int __init ibmphp_access_ebda (void)
 	iounmap (io_mem);
 	debug ("returned ebda segment: %x\n", ebda_seg);
 	
-	io_mem = ioremap(ebda_seg<<4, 1024);
+	io_mem = ioremap(ebda_seg<<4, 1);
+	ebda_sz = readb(io_mem);
+	iounmap(io_mem);
+	debug("ebda size: %d(KiB)\n", ebda_sz);
+	if (ebda_sz == 0)
+		return -ENOMEM;
+
+	io_mem = ioremap(ebda_seg<<4, (ebda_sz * 1024));
 	if (!io_mem )
 		return -ENOMEM;
 	next_offset = 0x180;

Re: [PATCH] ibmphp : read the length of ebda and map entire ebda region

[Greg KH]

On Sat, Jan 09, 2010 at 05:12:25PM +0530, Chandru wrote:
> ibmphp driver currently maps only 1KB of ebda memory area into kernel address 
> space during driver initialization. This causes kernel oops when the driver is 
> modprobe'd and it accesses memory area beyond 1KB within ebda segment. The first 
> byte of ebda segment actually stores the length of the ebda region in 
> Kilobytes. Hence make use of the length parameter and map the entire ebda 
> region.
> 
> 
> Signed-off-by: Chandru Siddalingappa <chandru@linux.vnet.ibm.com>
> ---
> 
>  drivers/pci/hotplug/ibmphp_ebda.c |   11 +++++++++--

Please use the scripts/get_maintainer.pl script to get the proper person
and mailing list to send this patch to (hint, it's not me.)

thanks,

greg k-h

Re: [PATCH] ibmphp : read the length of ebda and map entire ebda region

[Chandru]

On Saturday 09 January 2010 23:53:43 Greg KH wrote:
> >  drivers/pci/hotplug/ibmphp_ebda.c |   11 +++++++++--
> 
> Please use the scripts/get_maintainer.pl script to get the proper person
> and mailing list to send this patch to (hint, it's not me.)
> 
> thanks,
> 
> greg k-h

Thanks Greg, thanks for pointing. Your name showed up in the source file as 'send feedback to Greg KH' , so I cc'ed you on this. 

thanks,
Chandru

Re: [PATCH] ibmphp : read the length of ebda and map entire ebda region

[Andrew Morton]

On Sat, 9 Jan 2010 17:12:25 +0530
Chandru <chandru@in.ibm.com> wrote:

> ibmphp driver currently maps only 1KB of ebda memory area into kernel address 
> space during driver initialization. This causes kernel oops when the driver is 
> modprobe'd and it accesses memory area beyond 1KB within ebda segment. The first 
> byte of ebda segment actually stores the length of the ebda region in 
> Kilobytes. Hence make use of the length parameter and map the entire ebda 
> region.
> 
> 
> Signed-off-by: Chandru Siddalingappa <chandru@linux.vnet.ibm.com>
> ---
> 
>  drivers/pci/hotplug/ibmphp_ebda.c |   11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> --- linux-2.6.33-rc2/drivers/pci/hotplug/ibmphp_ebda.c.orig	2010-01-09 
> 15:23:54.000000000 +0530
> +++ linux-2.6.33-rc2/drivers/pci/hotplug/ibmphp_ebda.c	2010-01-09 
> 17:01:06.000000000 +0530
> @@ -245,7 +245,7 @@ static void __init print_ebda_hpc (void)
>  
>  int __init ibmphp_access_ebda (void)
>  {
> -	u8 format, num_ctlrs, rio_complete, hs_complete;
> +	u8 format, num_ctlrs, rio_complete, hs_complete, ebda_sz;
>  	u16 ebda_seg, num_entries, next_offset, offset, blk_id, sub_addr, re, rc_id, 
> re_id, base;

Your email client is performing wordwrapping on the patches.

>  	int rc = 0;
>  
> @@ -260,7 +260,14 @@ int __init ibmphp_access_ebda (void)
>  	iounmap (io_mem);
>  	debug ("returned ebda segment: %x\n", ebda_seg);
>  	
> -	io_mem = ioremap(ebda_seg<<4, 1024);
> +	io_mem = ioremap(ebda_seg<<4, 1);
> +	ebda_sz = readb(io_mem);
> +	iounmap(io_mem);

All the other ioremap() calls are checked for failure, so this one
should also be checked, no?

--- a/drivers/pci/hotplug/ibmphp_ebda.c~ibmphp-read-the-length-of-ebda-and-map-entire-ebda-region-fix
+++ a/drivers/pci/hotplug/ibmphp_ebda.c
@@ -261,6 +261,8 @@ int __init ibmphp_access_ebda (void)
 	debug ("returned ebda segment: %x\n", ebda_seg);
 	
 	io_mem = ioremap(ebda_seg<<4, 1);
+	if (!io_mem)
+		return -ENOMEM;
 	ebda_sz = readb(io_mem);
 	iounmap(io_mem);
 	debug("ebda size: %d(KiB)\n", ebda_sz);
_

> +	debug("ebda size: %d(KiB)\n", ebda_sz);
> +	if (ebda_sz == 0)
> +		return -ENOMEM;
> +
> +	io_mem = ioremap(ebda_seg<<4, (ebda_sz * 1024));

A kernel oops is somewhat serious.  Would I be correct in assuming that
this fix is needed in 2.6.32.x and perhaps earlier kernels?

Re: [PATCH] ibmphp : read the length of ebda and map entire ebda region

[Chandru]

On Wednesday 13 January 2010 06:56:40 Andrew Morton wrote:
> 
> Your email client is performing wordwrapping on the patches.

Sorry for this, I changed the word wrap settings of my client

> 
> All the other ioremap() calls are checked for failure, so this one
> should also be checked, no?

Yes, it needs to be checked. thanks for adding the additional check.

> 
> --- a/drivers/pci/hotplug/ibmphp_ebda.c~ibmphp-read-the-length-of-ebda-and-map-entire-ebda-region-fix
> +++ a/drivers/pci/hotplug/ibmphp_ebda.c
> @@ -261,6 +261,8 @@ int __init ibmphp_access_ebda (void)
>  	debug ("returned ebda segment: %x\n", ebda_seg);
>  	
>  	io_mem = ioremap(ebda_seg<<4, 1);
> +	if (!io_mem)
> +		return -ENOMEM;
>  	ebda_sz = readb(io_mem);
>  	iounmap(io_mem);
>  	debug("ebda size: %d(KiB)\n", ebda_sz);
> _
> 

> 
> A kernel oops is somewhat serious.  Would I be correct in assuming that
> this fix is needed in 2.6.32.x and perhaps earlier kernels?

Yes, I just checked with 2.6.30 kernel and the issue exists over there too. So it applies to all older kernels.

Thanks, 
Chandru