public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] Fix ide-disk.c oops caused by hwif == NULL
       [not found] <200508100459.j7A4xTn7016128@hera.kernel.org>
@ 2005-08-10 11:11 ` Geert Uytterhoeven
  2005-08-10 13:07   ` Christoph Lameter
  0 siblings, 1 reply; 6+ messages in thread
From: Geert Uytterhoeven @ 2005-08-10 11:11 UTC (permalink / raw)
  To: Christoph Lameter, Linus Torvalds; +Cc: Linux Kernel Development

On Tue, 9 Aug 2005, Linux Kernel Mailing List wrote:
> tree 518f62158f0923573decb8f072ac7282fb7575cb
> parent aeb3f76350e78aba90653b563de6677b442d21d6
> author Christoph Lameter <christoph@lameter.com> Wed, 10 Aug 2005 09:59:21 -0700
> committer Linus Torvalds <torvalds@g5.osdl.org> Wed, 10 Aug 2005 10:21:31 -0700
> 
> [PATCH] Fix ide-disk.c oops caused by hwif == NULL

How can this patch fix that? It still dereferences hwif without checking for a
NULL pointer.

> 1. Move hwif_to_node to ide.h
> 
> 2. Use hwif_to_node in ide-disk.c
> 
> Signed-off-by: Christoph Lameter <clameter@sgi.com>
> Signed-off-by: Linus Torvalds <torvalds@osdl.org>
> 
>  drivers/ide/ide-disk.c  |    2 +-
>  drivers/ide/ide-probe.c |    9 ---------
>  include/linux/ide.h     |    6 ++++++
>  3 files changed, 7 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/ide/ide-disk.c b/drivers/ide/ide-disk.c
> --- a/drivers/ide/ide-disk.c
> +++ b/drivers/ide/ide-disk.c
> @@ -1220,7 +1220,7 @@ static int ide_disk_probe(struct device 
>  		goto failed;
>  
>  	g = alloc_disk_node(1 << PARTN_BITS,
> -			pcibus_to_node(drive->hwif->pci_dev->bus));
> +			hwif_to_node(drive->hwif));
>  	if (!g)
>  		goto out_free_idkp;
>  
> diff --git a/drivers/ide/ide-probe.c b/drivers/ide/ide-probe.c
> --- a/drivers/ide/ide-probe.c
> +++ b/drivers/ide/ide-probe.c
> @@ -960,15 +960,6 @@ static void save_match(ide_hwif_t *hwif,
>  }
>  #endif /* MAX_HWIFS > 1 */
>  
> -static inline int hwif_to_node(ide_hwif_t *hwif)
> -{
> -	if (hwif->pci_dev)
> -		return pcibus_to_node(hwif->pci_dev->bus);
> -	else
> -		/* Add ways to determine the node of other busses here */
> -		return -1;
> -}
> -
>  /*
>   * init request queue
>   */
> diff --git a/include/linux/ide.h b/include/linux/ide.h
> --- a/include/linux/ide.h
> +++ b/include/linux/ide.h
> @@ -1501,4 +1501,10 @@ extern struct bus_type ide_bus_type;
>  #define ide_id_has_flush_cache_ext(id)	\
>  	(((id)->cfs_enable_2 & 0x2400) == 0x2400)
>  
> +static inline int hwif_to_node(ide_hwif_t *hwif)
> +{
> +	struct pci_dev *dev = hwif->pci_dev;
                              ^^^^^^^^^^^^^
> +	return dev ? pcibus_to_node(dev->bus) : -1;
> +}
> +
>  #endif /* _IDE_H */

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix ide-disk.c oops caused by hwif == NULL
  2005-08-10 11:11 ` [PATCH] Fix ide-disk.c oops caused by hwif == NULL Geert Uytterhoeven
@ 2005-08-10 13:07   ` Christoph Lameter
  2005-08-10 13:13     ` Bartlomiej Zolnierkiewicz
  2005-08-10 14:59     ` Alan Cox
  0 siblings, 2 replies; 6+ messages in thread
From: Christoph Lameter @ 2005-08-10 13:07 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Linus Torvalds, kiran, Linux Kernel Development

On Wed, 10 Aug 2005, Geert Uytterhoeven wrote:

> On Tue, 9 Aug 2005, Linux Kernel Mailing List wrote:
> > tree 518f62158f0923573decb8f072ac7282fb7575cb
> > parent aeb3f76350e78aba90653b563de6677b442d21d6
> > author Christoph Lameter <christoph@lameter.com> Wed, 10 Aug 2005 09:59:21 -0700
> > committer Linus Torvalds <torvalds@g5.osdl.org> Wed, 10 Aug 2005 10:21:31 -0700
> > 
> > [PATCH] Fix ide-disk.c oops caused by hwif == NULL
> 
> How can this patch fix that? It still dereferences hwif without checking for a
> NULL pointer.

Correct. So we need to indeed go back to a version that does check for 
NULL that I initially proposed.

Index: linux-2.6/include/linux/ide.h
===================================================================
--- linux-2.6.orig/include/linux/ide.h	2005-08-09 19:47:14.000000000 -0700
+++ linux-2.6/include/linux/ide.h	2005-08-10 06:05:44.000000000 -0700
@@ -1503,7 +1503,7 @@
 
 static inline int hwif_to_node(ide_hwif_t *hwif)
 {
-	if (hwif->pci_dev)
+	if (hwif && hwif->pci_dev)
 		return pcibus_to_node(hwif->pci_dev->bus);
 	else
 		/* Add ways to determine the node of other busses here */

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix ide-disk.c oops caused by hwif == NULL
  2005-08-10 13:07   ` Christoph Lameter
@ 2005-08-10 13:13     ` Bartlomiej Zolnierkiewicz
  2005-08-10 13:28       ` Christoph Lameter
  2005-08-10 14:59     ` Alan Cox
  1 sibling, 1 reply; 6+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2005-08-10 13:13 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: Geert Uytterhoeven, Linus Torvalds, kiran,
	Linux Kernel Development

NAK

hwif can't be NULL or something is *really* wrong

On 8/10/05, Christoph Lameter <clameter@engr.sgi.com> wrote:
> On Wed, 10 Aug 2005, Geert Uytterhoeven wrote:
> 
> > On Tue, 9 Aug 2005, Linux Kernel Mailing List wrote:
> > > tree 518f62158f0923573decb8f072ac7282fb7575cb
> > > parent aeb3f76350e78aba90653b563de6677b442d21d6
> > > author Christoph Lameter <christoph@lameter.com> Wed, 10 Aug 2005 09:59:21 -0700
> > > committer Linus Torvalds <torvalds@g5.osdl.org> Wed, 10 Aug 2005 10:21:31 -0700
> > >
> > > [PATCH] Fix ide-disk.c oops caused by hwif == NULL
> >
> > How can this patch fix that? It still dereferences hwif without checking for a
> > NULL pointer.
> 
> Correct. So we need to indeed go back to a version that does check for
> NULL that I initially proposed.
> 
> Index: linux-2.6/include/linux/ide.h
> ===================================================================
> --- linux-2.6.orig/include/linux/ide.h  2005-08-09 19:47:14.000000000 -0700
> +++ linux-2.6/include/linux/ide.h       2005-08-10 06:05:44.000000000 -0700
> @@ -1503,7 +1503,7 @@
> 
>  static inline int hwif_to_node(ide_hwif_t *hwif)
>  {
> -       if (hwif->pci_dev)
> +       if (hwif && hwif->pci_dev)
>                 return pcibus_to_node(hwif->pci_dev->bus);
>         else
>                 /* Add ways to determine the node of other busses here */

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix ide-disk.c oops caused by hwif == NULL
  2005-08-10 13:13     ` Bartlomiej Zolnierkiewicz
@ 2005-08-10 13:28       ` Christoph Lameter
  0 siblings, 0 replies; 6+ messages in thread
From: Christoph Lameter @ 2005-08-10 13:28 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: Geert Uytterhoeven, Linus Torvalds, kiran,
	Linux Kernel Development

On Wed, 10 Aug 2005, Bartlomiej Zolnierkiewicz wrote:

> hwif can't be NULL or something is *really* wrong

Ahh.. Yes.... Not enough time to think about this email properly since I 
need to get to the LWCE in SF. Wrong description. The oops was caused by 
pci_dev being NULL..

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix ide-disk.c oops caused by hwif == NULL
  2005-08-10 13:07   ` Christoph Lameter
  2005-08-10 13:13     ` Bartlomiej Zolnierkiewicz
@ 2005-08-10 14:59     ` Alan Cox
  2005-08-11  2:18       ` Christoph Lameter
  1 sibling, 1 reply; 6+ messages in thread
From: Alan Cox @ 2005-08-10 14:59 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: Geert Uytterhoeven, Linus Torvalds, kiran,
	Linux Kernel Development

On Mer, 2005-08-10 at 06:07 -0700, Christoph Lameter wrote:
> Correct. So we need to indeed go back to a version that does check for 
> NULL that I initially proposed.

No, you need to fix the caller. "hwif_to_node(NULL)" is a nonsense
operation rather like strlen(NULL). The caller need to be fixed.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix ide-disk.c oops caused by hwif == NULL
  2005-08-10 14:59     ` Alan Cox
@ 2005-08-11  2:18       ` Christoph Lameter
  0 siblings, 0 replies; 6+ messages in thread
From: Christoph Lameter @ 2005-08-11  2:18 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Geert Uytterhoeven, Alan Cox, kiran, BartlomiejZolnierkiewicz,
	Linux Kernel Development

Just to make sure to avoid all confusion. The title of this thread should 
be

"[PATCH] Fix ide-disk.c oops caused by hwif->pci_dev == NULL"
 
And this is the patch that was acknowledged by Andrew and that fixes the 
issue AFAIK. This patch needs to be included in 2.6.13. Lets make sure 
that we are all on the same page on this patch now:

---

From: Christoph Lameter <christoph@lameter.com>

There is one additional place where pcibus_to_node is used with the hwif
that we did not cover.

- Move hwif_to_node to ide.h

- Use hwif_to_node in ide-disk.c

Signed-off-by: Christoph Lameter <clameter@sgi.com>
Signed-off-by: Andrew Morton <akpm@osdl.org>
---

 drivers/ide/ide-disk.c  |    2 +-
 drivers/ide/ide-probe.c |    9 ---------
 include/linux/ide.h     |    9 +++++++++
 3 files changed, 10 insertions(+), 10 deletions(-)

diff -puN drivers/ide/ide-disk.c~fix-ide-diskc-oops-caused-by-hwif-==-null drivers/ide/ide-disk.c
--- devel/drivers/ide/ide-disk.c~fix-ide-diskc-oops-caused-by-hwif-==-null	2005-08-09 20:16:45.000000000 -0700
+++ devel-akpm/drivers/ide/ide-disk.c	2005-08-09 20:16:45.000000000 -0700
@@ -1220,7 +1220,7 @@ static int ide_disk_probe(struct device 
 		goto failed;
 
 	g = alloc_disk_node(1 << PARTN_BITS,
-			pcibus_to_node(drive->hwif->pci_dev->bus));
+			hwif_to_node(drive->hwif));
 	if (!g)
 		goto out_free_idkp;
 
diff -puN drivers/ide/ide-probe.c~fix-ide-diskc-oops-caused-by-hwif-==-null drivers/ide/ide-probe.c
--- devel/drivers/ide/ide-probe.c~fix-ide-diskc-oops-caused-by-hwif-==-null	2005-08-09 20:16:45.000000000 -0700
+++ devel-akpm/drivers/ide/ide-probe.c	2005-08-09 20:16:45.000000000 -0700
@@ -960,15 +960,6 @@ static void save_match(ide_hwif_t *hwif,
 }
 #endif /* MAX_HWIFS > 1 */
 
-static inline int hwif_to_node(ide_hwif_t *hwif)
-{
-	if (hwif->pci_dev)
-		return pcibus_to_node(hwif->pci_dev->bus);
-	else
-		/* Add ways to determine the node of other busses here */
-		return -1;
-}
-
 /*
  * init request queue
  */
diff -puN include/linux/ide.h~fix-ide-diskc-oops-caused-by-hwif-==-null include/linux/ide.h
--- devel/include/linux/ide.h~fix-ide-diskc-oops-caused-by-hwif-==-null	2005-08-09 20:16:45.000000000 -0700
+++ devel-akpm/include/linux/ide.h	2005-08-09 20:16:45.000000000 -0700
@@ -1501,4 +1501,13 @@ extern struct bus_type ide_bus_type;
 #define ide_id_has_flush_cache_ext(id)	\
 	(((id)->cfs_enable_2 & 0x2400) == 0x2400)
 
+static inline int hwif_to_node(ide_hwif_t *hwif)
+{
+	if (hwif->pci_dev)
+		return pcibus_to_node(hwif->pci_dev->bus);
+	else
+		/* Add ways to determine the node of other busses here */
+		return -1;
+}
+
 #endif /* _IDE_H */
_

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2005-08-11  2:18 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <200508100459.j7A4xTn7016128@hera.kernel.org>
2005-08-10 11:11 ` [PATCH] Fix ide-disk.c oops caused by hwif == NULL Geert Uytterhoeven
2005-08-10 13:07   ` Christoph Lameter
2005-08-10 13:13     ` Bartlomiej Zolnierkiewicz
2005-08-10 13:28       ` Christoph Lameter
2005-08-10 14:59     ` Alan Cox
2005-08-11  2:18       ` Christoph Lameter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox