Re: crash on CONFIG_CFAG12864B=y in 2.6.20-rc3-mm1

Re: crash on CONFIG_CFAG12864B=y in 2.6.20-rc3-mm1

[Miguel Ojeda]

Ok, here it is the patch, just a draft.

I have checked the Makefile to ensure that ks0108 was being called before than cfag12864b at boot, still, I'm not sure and I don't know if this will prevent the crash if ks0108 fails to init.

Daniel, please test it.

drivers-add-lcd-support-update10.patch.draft
Signed-off-by: Miguel Ojeda Sandonis <maxextreme@gmail.com>
---
diff --git a/drivers/auxdisplay/cfag12864b.c b/drivers/auxdisplay/cfag12864b.c
index 889583d..cb44cb4 100644
--- a/drivers/auxdisplay/cfag12864b.c
+++ b/drivers/auxdisplay/cfag12864b.c
@@ -312,6 +312,17 @@ EXPORT_SYMBOL_GPL(cfag12864b_disable);
 EXPORT_SYMBOL_GPL(cfag12864b_isenabled);
 
 /*
+ * Is the module inited?
+ */
+
+static unsigned char cfag12864b_inited;
+unsigned char cfag12864b_isinited(void)
+{
+	return cfag12864b_inited;
+}
+EXPORT_SYMBOL_GPL(cfag12864b_isinited);
+
+/*
  * Module Init & Exit
  */
 
@@ -319,6 +330,13 @@ static int __init cfag12864b_init(void)
 {
 	int ret = -EINVAL;
 
+	/* ks0108_init() must be called first */
+	if (!ks0108_isinited()) {
+		printk(KERN_ERR CFAG12864B_NAME ": ERROR: "
+			"ks0108 is not initialized\n");
+		goto none;
+	}
+
 	if (PAGE_SIZE < CFAG12864B_SIZE) {
 		printk(KERN_ERR CFAG12864B_NAME ": ERROR: "
 			"page size (%i) < cfag12864b size (%i)\n",
@@ -354,6 +372,7 @@ static int __init cfag12864b_init(void)
 	cfag12864b_clear();
 	cfag12864b_on();
 
+	cfag12864b_inited = 1;
 	return 0;
 
 cachealloced:
diff --git a/drivers/auxdisplay/cfag12864bfb.c b/drivers/auxdisplay/cfag12864bfb.c
index 94765e7..66fafbb 100644
--- a/drivers/auxdisplay/cfag12864bfb.c
+++ b/drivers/auxdisplay/cfag12864bfb.c
@@ -137,7 +137,14 @@ static struct platform_device *cfag12864
 
 static int __init cfag12864bfb_init(void)
 {
-	int ret;
+	int ret = -EINVAL;
+
+	/* cfag12864b_init() must be called first */
+	if (!cfag12864b_isinited()) {
+		printk(KERN_ERR CFAG12864BFB_NAME ": ERROR: "
+			"cfag12864b is not initialized\n");
+		goto none;
+	}
 
 	if (cfag12864b_enable()) {
 		printk(KERN_ERR CFAG12864BFB_NAME ": ERROR: "
@@ -162,6 +169,7 @@ static int __init cfag12864bfb_init(void
 		}
 	}
 
+none:
 	return ret;
 }
 
diff --git a/drivers/auxdisplay/ks0108.c b/drivers/auxdisplay/ks0108.c
index a637575..e6c3646 100644
--- a/drivers/auxdisplay/ks0108.c
+++ b/drivers/auxdisplay/ks0108.c
@@ -111,6 +111,17 @@ EXPORT_SYMBOL_GPL(ks0108_address);
 EXPORT_SYMBOL_GPL(ks0108_page);
 
 /*
+ * Is the module inited?
+ */
+
+static unsigned char ks0108_inited;
+unsigned char ks0108_isinited(void)
+{
+	return ks0108_inited;
+}
+EXPORT_SYMBOL_GPL(ks0108_isinited);
+
+/*
  * Module Init & Exit
  */
 
@@ -142,6 +153,7 @@ static int __init ks0108_init(void)
 		goto registered;
 	}
 
+	ks0108_inited = 1;
 	return 0;
 
 registered:
diff --git a/include/linux/cfag12864b.h b/include/linux/cfag12864b.h
index 0bc45e6..1605dd8 100644
--- a/include/linux/cfag12864b.h
+++ b/include/linux/cfag12864b.h
@@ -73,5 +73,10 @@ extern void cfag12864b_disable(void);
  */
 extern unsigned char cfag12864b_isenabled(void);
 
+/*
+ * Is the module inited?
+ */
+extern unsigned char cfag12864b_isinited(void);
+
 #endif /* _CFAG12864B_H_ */
 
diff --git a/include/linux/ks0108.h b/include/linux/ks0108.h
index 8047d4b..a2c54ac 100644
--- a/include/linux/ks0108.h
+++ b/include/linux/ks0108.h
@@ -43,4 +43,7 @@ extern void ks0108_address(unsigned char
 /* Set the controller's current page (0..7) */
 extern void ks0108_page(unsigned char page);
 
+/* Is the module inited? */
+extern unsigned char ks0108_isinited(void);
+
 #endif /* _KS0108_H_ */

crash on CONFIG_CFAG12864B=y in 2.6.20-rc3-mm1

[Daniel Walker]

(forgot to CC LKML)

The options,

CONFIG_CFAG12864B=y
CONFIG_CFAG12864B_RATE=20

causes a crash at boot in 2.6.20-rc3-mm1. I don't have the hardware
associated with the options. It looks like it just doesn't have guards
to detect if the hardware doesn't exists.

Here is the crash,

ks0108: ERROR: parport didn't find 888 port
BUG: unable to handle kernel NULL pointer dereference at virtual address 0000004 printing eip:
c02dbff9
*pde = 00000000
Oops: 0000 [#1]
PREEMPT SMP
last sysfs file:
Modules linked in:
CPU:    3
EIP:    0060:[<c02dbff9>]    Not tainted VLI
EFLAGS: 00010246   (2.6.20-rc3-mm1 #11)
EIP is at ks0108_writecontrol+0x79/0xc0
eax: 00001008   ebx: 0000000a   ecx: 673e2eb8   edx: 00000001
esi: 0000000a   edi: 00000000   ebp: f7c3ff6c   esp: f7c3ff50
ds: 007b   es: 007b   fs: 00d8  gs: 0000  ss: 0068
Process swapper (pid: 1, ti=f7c3e000 task=f7c26a90 task.ti=f7c3e000)
Stack: 00000001 f7552c40 f7c3ff60 c0120e3f 00000000 c049f450 00000000 f7c3ff74
       c02dc159 f7c3ff80 c02dc177 00000000 f7c3ff98 c048feda 00000378 c02d74db
       00000000 00000000 f7c3ffe0 c0478610 c03d9d35 00000004 f7c26a90 c0473fc4
Call Trace:
 [<c01053da>] show_trace_log_lvl+0x1a/0x30
 [<c0105499>] show_stack_log_lvl+0xa9/0xd0
 [<c01056c7>] show_registers+0x207/0x370
 [<c0105949>] die+0x119/0x250
 [<c011d267>] do_page_fault+0x277/0x610
 [<c038e9d4>] error_code+0x7c/0x84
 [<c02dc159>] cfag12864b_e+0x19/0x20
 [<c02dc177>] cfag12864b_page+0x17/0x30
 [<c048feda>] cfag12864b_init+0x8a/0x130
 [<c0478610>] init+0x110/0x250
 [<c0104fd3>] kernel_thread_helper+0x7/0x14
 =======================
Code: 8b 98 ec 00 00 00 0f b6 03 24 df 88 45 f3 80 75 f3 20 0f b6 43 01 20 45 f
EIP: [<c02dbff9>] ks0108_writecontrol+0x79/0xc0 SS:ESP 0068:f7c3ff50
 <0>Kernel panic - not syncing: Attempted to kill init!

Re: crash on CONFIG_CFAG12864B=y in 2.6.20-rc3-mm1

[Miguel Ojeda]

On 1/7/07, Daniel Walker <dwalker@mvista.com> wrote:
> (forgot to CC LKML)
>
> The options,
>
> CONFIG_CFAG12864B=y
> CONFIG_CFAG12864B_RATE=20
>
> causes a crash at boot in 2.6.20-rc3-mm1. I don't have the hardware
> associated with the options. It looks like it just doesn't have guards
> to detect if the hardware doesn't exists.
>
> Here is the crash,
>
> ks0108: ERROR: parport didn't find 888 port
> BUG: unable to handle kernel NULL pointer dereference at virtual address
> 0000004 printing eip:
> c02dbff9
> *pde = 00000000
> Oops: 0000 [#1]
> PREEMPT SMP
> last sysfs file:
> Modules linked in:
> CPU:    3
> EIP:    0060:[<c02dbff9>]    Not tainted VLI
> EFLAGS: 00010246   (2.6.20-rc3-mm1 #11)
> EIP is at ks0108_writecontrol+0x79/0xc0
> eax: 00001008   ebx: 0000000a   ecx: 673e2eb8   edx: 00000001
> esi: 0000000a   edi: 00000000   ebp: f7c3ff6c   esp: f7c3ff50
> ds: 007b   es: 007b   fs: 00d8  gs: 0000  ss: 0068
> Process swapper (pid: 1, ti=f7c3e000 task=f7c26a90 task.ti=f7c3e000)
> Stack: 00000001 f7552c40 f7c3ff60 c0120e3f 00000000 c049f450 00000000
> f7c3ff74
>        c02dc159 f7c3ff80 c02dc177 00000000 f7c3ff98 c048feda 00000378
> c02d74db
>        00000000 00000000 f7c3ffe0 c0478610 c03d9d35 00000004 f7c26a90
> c0473fc4
> Call Trace:
>  [<c01053da>] show_trace_log_lvl+0x1a/0x30
>  [<c0105499>] show_stack_log_lvl+0xa9/0xd0
>  [<c01056c7>] show_registers+0x207/0x370
>  [<c0105949>] die+0x119/0x250
>  [<c011d267>] do_page_fault+0x277/0x610
>  [<c038e9d4>] error_code+0x7c/0x84
>  [<c02dc159>] cfag12864b_e+0x19/0x20
>  [<c02dc177>] cfag12864b_page+0x17/0x30
>  [<c048feda>] cfag12864b_init+0x8a/0x130
>  [<c0478610>] init+0x110/0x250
>  [<c0104fd3>] kernel_thread_helper+0x7/0x14
>  =======================
> Code: 8b 98 ec 00 00 00 0f b6 03 24 df 88 45 f3 80 75 f3 20 0f b6 43 01 20
> 45 f
> EIP: [<c02dbff9>] ks0108_writecontrol+0x79/0xc0 SS:ESP 0068:f7c3ff50
>  <0>Kernel panic - not syncing: Attempted to kill init!
>
>
>
>

As Daniel Walker pointed out, the driver doesn't probe for the
hardware because it just uses the parallel port for output (there
isn't any kind of input).

The driver shouldn't continue execution and using
ks0108_writecontrol() (which writes to the parallel port) after the
"ks0108: ERROR: parport didn't find 888 port" message.

Will check.

Thanks for the warning,
Miguel

(forgot to reply to all).

-- 
Miguel Ojeda
http://maxextreme.googlepages.com/index.htm

Re: crash on CONFIG_CFAG12864B=y in 2.6.20-rc3-mm1

[Miguel Ojeda]

The problem is easy:

ks0108_init() prints the error message and exit with -EINVAL, so the
module isn't loaded properly.

However, cfag12864b_init() is called, although ks0108 failed. It
returns 0 and after a while cfag12864b calls ks0108_writecontrol()
which dereferences the uninitialized pointer ks0108_parport:

	parport_write_control(ks0108_parport, byte ^ (bit(0) | bit(1) | bit(3)));

Why is cfag12864b_init() called if ks0108 module didn't load properly?
Is that normal? If so, how a module should alarm other modules about
it failed loading?

An easy solution woule be to export a function at ks0108.c like
ks0108_inited() that would return if the module was properly inited or
not. Is there any better solution?

Regards,
Miguel

On 2/1/07, Miguel Ojeda <maxextreme@gmail.com> wrote:
> On 1/7/07, Daniel Walker <dwalker@mvista.com> wrote:
> > (forgot to CC LKML)
> >
> > The options,
> >
> > CONFIG_CFAG12864B=y
> > CONFIG_CFAG12864B_RATE=20
> >
> > causes a crash at boot in 2.6.20-rc3-mm1. I don't have the hardware
> > associated with the options. It looks like it just doesn't have guards
> > to detect if the hardware doesn't exists.
> >
> > Here is the crash,
> >
> > ks0108: ERROR: parport didn't find 888 port
> > BUG: unable to handle kernel NULL pointer dereference at virtual address
> > 0000004 printing eip:
> > c02dbff9
> > *pde = 00000000
> > Oops: 0000 [#1]
> > PREEMPT SMP
> > last sysfs file:
> > Modules linked in:
> > CPU:    3
> > EIP:    0060:[<c02dbff9>]    Not tainted VLI
> > EFLAGS: 00010246   (2.6.20-rc3-mm1 #11)
> > EIP is at ks0108_writecontrol+0x79/0xc0
> > eax: 00001008   ebx: 0000000a   ecx: 673e2eb8   edx: 00000001
> > esi: 0000000a   edi: 00000000   ebp: f7c3ff6c   esp: f7c3ff50
> > ds: 007b   es: 007b   fs: 00d8  gs: 0000  ss: 0068
> > Process swapper (pid: 1, ti=f7c3e000 task=f7c26a90 task.ti=f7c3e000)
> > Stack: 00000001 f7552c40 f7c3ff60 c0120e3f 00000000 c049f450 00000000
> > f7c3ff74
> >        c02dc159 f7c3ff80 c02dc177 00000000 f7c3ff98 c048feda 00000378
> > c02d74db
> >        00000000 00000000 f7c3ffe0 c0478610 c03d9d35 00000004 f7c26a90
> > c0473fc4
> > Call Trace:
> >  [<c01053da>] show_trace_log_lvl+0x1a/0x30
> >  [<c0105499>] show_stack_log_lvl+0xa9/0xd0
> >  [<c01056c7>] show_registers+0x207/0x370
> >  [<c0105949>] die+0x119/0x250
> >  [<c011d267>] do_page_fault+0x277/0x610
> >  [<c038e9d4>] error_code+0x7c/0x84
> >  [<c02dc159>] cfag12864b_e+0x19/0x20
> >  [<c02dc177>] cfag12864b_page+0x17/0x30
> >  [<c048feda>] cfag12864b_init+0x8a/0x130
> >  [<c0478610>] init+0x110/0x250
> >  [<c0104fd3>] kernel_thread_helper+0x7/0x14
> >  =======================
> > Code: 8b 98 ec 00 00 00 0f b6 03 24 df 88 45 f3 80 75 f3 20 0f b6 43 01 20
> > 45 f
> > EIP: [<c02dbff9>] ks0108_writecontrol+0x79/0xc0 SS:ESP 0068:f7c3ff50
> >  <0>Kernel panic - not syncing: Attempted to kill init!
> >
> >
> >
> >
>
> As Daniel Walker pointed out, the driver doesn't probe for the
> hardware because it just uses the parallel port for output (there
> isn't any kind of input).
>
> The driver shouldn't continue execution and using
> ks0108_writecontrol() (which writes to the parallel port) after the
> "ks0108: ERROR: parport didn't find 888 port" message.
>
> Will check.
>
> Thanks for the warning,
> Miguel
>
> (forgot to reply to all).
>
> --
> Miguel Ojeda
> http://maxextreme.googlepages.com/index.htm
>


-- 
Miguel Ojeda
http://maxextreme.googlepages.com/index.htm

Re: crash on CONFIG_CFAG12864B=y in 2.6.20-rc3-mm1

[Daniel Walker]

On Thu, 2007-02-01 at 16:16 +0100, Miguel Ojeda wrote:
> The problem is easy:
> 
> ks0108_init() prints the error message and exit with -EINVAL, so the
> module isn't loaded properly.
> 
> However, cfag12864b_init() is called, although ks0108 failed. It
> returns 0 and after a while cfag12864b calls ks0108_writecontrol()
> which dereferences the uninitialized pointer ks0108_parport:
> 
> 	parport_write_control(ks0108_parport, byte ^ (bit(0) | bit(1) | bit(3)));
> 
> Why is cfag12864b_init() called if ks0108 module didn't load properly?
> Is that normal? If so, how a module should alarm other modules about
> it failed loading?

I don't know if this matters, but I had this driver built-in , and not a
module ..

Daniel

Re: crash on CONFIG_CFAG12864B=y in 2.6.20-rc3-mm1

[Miguel Ojeda]

On 2/1/07, Daniel Walker <dwalker@mvista.com> wrote:
> On Thu, 2007-02-01 at 16:16 +0100, Miguel Ojeda wrote:
> > The problem is easy:
> >
> > ks0108_init() prints the error message and exit with -EINVAL, so the
> > module isn't loaded properly.
> >
> > However, cfag12864b_init() is called, although ks0108 failed. It
> > returns 0 and after a while cfag12864b calls ks0108_writecontrol()
> > which dereferences the uninitialized pointer ks0108_parport:
> >
> > 	parport_write_control(ks0108_parport, byte ^ (bit(0) | bit(1) | bit(3)));
> >
> > Why is cfag12864b_init() called if ks0108 module didn't load properly?
> > Is that normal? If so, how a module should alarm other modules about
> > it failed loading?
>
> I don't know if this matters, but I had this driver built-in , and not a
> module ..
>
> Daniel
>
>

Well, I use the word "module" for both cases: When I modprobe
cfag12864b, if ks0108 fails, it doesn't get linked. So I thought the
same happen for built-in drivers (in other words, I didn't think
cfag12864b would be linked if ks0108 failed).

So I'm waiting until someone tell me what is the right way to avoid
drivers like cfag12864b been inited if their dependencies failed.

Anyway, thanks for discovering the bug (I wouldn't have discovered it
as my motherboard has parallel port ;).

Miguel

-- 
Miguel Ojeda
http://maxextreme.googlepages.com/index.htm

Re: crash on CONFIG_CFAG12864B=y in 2.6.20-rc3-mm1

[Daniel Walker]

On Thu, 2007-02-01 at 18:13 +0100, Miguel Ojeda wrote:

> >
> >
> 
> Well, I use the word "module" for both cases: When I modprobe
> cfag12864b, if ks0108 fails, it doesn't get linked. So I thought the
> same happen for built-in drivers (in other words, I didn't think
> cfag12864b would be linked if ks0108 failed).
> 
> So I'm waiting until someone tell me what is the right way to avoid
> drivers like cfag12864b been inited if their dependencies failed.
> 
> Anyway, thanks for discovering the bug (I wouldn't have discovered it
> as my motherboard has parallel port ;).

I have a parallel port, but I don't have the cfag12864b or ks0108 .. I
think the simplest way to fix it would be to add a flag to ks0108 that
signals successful initialization , then force cfag12864b to check for
that ..

I found this by booting a make allyesconfig kernel ..

Daniel

Re: crash on CONFIG_CFAG12864B=y in 2.6.20-rc3-mm1

[Miguel Ojeda]

On 2/1/07, Daniel Walker <dwalker@mvista.com> wrote:
> On Thu, 2007-02-01 at 18:13 +0100, Miguel Ojeda wrote:
>
> > >
> > >
> >
> > Well, I use the word "module" for both cases: When I modprobe
> > cfag12864b, if ks0108 fails, it doesn't get linked. So I thought the
> > same happen for built-in drivers (in other words, I didn't think
> > cfag12864b would be linked if ks0108 failed).
> >
> > So I'm waiting until someone tell me what is the right way to avoid
> > drivers like cfag12864b been inited if their dependencies failed.
> >
> > Anyway, thanks for discovering the bug (I wouldn't have discovered it
> > as my motherboard has parallel port ;).
>
> I have a parallel port, but I don't have the cfag12864b or ks0108 .. I

In fact, you don't need the cfag12864b display, as the driver just
needs the parallel port for output (you can put some LEDs attached to
the parallel port to check the driver's operations and it will be
fine).

BTW, why ks0108 didn't find the 888 parallel port? Haven't you got such number?

> think the simplest way to fix it would be to add a flag to ks0108 that
> signals successful initialization , then force cfag12864b to check for
> that ..

Sure, that won't be a problem; however, maybe there is other better way (?).

>
> I found this by booting a make allyesconfig kernel ..
>
> Daniel
>
>

Miguel

-- 
Miguel Ojeda
http://maxextreme.googlepages.com/index.htm

Re: crash on CONFIG_CFAG12864B=y in 2.6.20-rc3-mm1

[Daniel Walker]

On Thu, 2007-02-01 at 18:39 +0100, Miguel Ojeda wrote:
> On 2/1/07, Daniel Walker <dwalker@mvista.com> wrote:
> > On Thu, 2007-02-01 at 18:13 +0100, Miguel Ojeda wrote:
> >
> > > >
> > > >
> > >
> > > Well, I use the word "module" for both cases: When I modprobe
> > > cfag12864b, if ks0108 fails, it doesn't get linked. So I thought the
> > > same happen for built-in drivers (in other words, I didn't think
> > > cfag12864b would be linked if ks0108 failed).
> > >
> > > So I'm waiting until someone tell me what is the right way to avoid
> > > drivers like cfag12864b been inited if their dependencies failed.
> > >
> > > Anyway, thanks for discovering the bug (I wouldn't have discovered it
> > > as my motherboard has parallel port ;).
> >
> > I have a parallel port, but I don't have the cfag12864b or ks0108 .. I
> 
> In fact, you don't need the cfag12864b display, as the driver just
> needs the parallel port for output (you can put some LEDs attached to
> the parallel port to check the driver's operations and it will be
> fine).
> 
> BTW, why ks0108 didn't find the 888 parallel port? Haven't you got such number?

You know I think your right, it may not have a parallel port.. It's so
common I tend to take it for granted .

> > think the simplest way to fix it would be to add a flag to ks0108 that
> > signals successful initialization , then force cfag12864b to check for
> > that ..
> 
> Sure, that won't be a problem; however, maybe there is other better way (?).

When you build the driver as a module the module loader will only load
what is needed, and in order .. When the same driver is built-in the
module_init() becomes a device_initcall() , and all device_initcall's
get executed .. So you have to expect that cfag12864b drivers initcalls
will run eventually and will need to be careful about it's
initialization.

Daniel

Re: crash on CONFIG_CFAG12864B=y in 2.6.20-rc3-mm1

[Miguel Ojeda]

On 2/1/07, Daniel Walker <dwalker@mvista.com> wrote:
> On Thu, 2007-02-01 at 18:39 +0100, Miguel Ojeda wrote:
> > On 2/1/07, Daniel Walker <dwalker@mvista.com> wrote:
> > > On Thu, 2007-02-01 at 18:13 +0100, Miguel Ojeda wrote:
> > >
> > > > >
> > > > >
> > > >
> > > > Well, I use the word "module" for both cases: When I modprobe
> > > > cfag12864b, if ks0108 fails, it doesn't get linked. So I thought the
> > > > same happen for built-in drivers (in other words, I didn't think
> > > > cfag12864b would be linked if ks0108 failed).
> > > >
> > > > So I'm waiting until someone tell me what is the right way to avoid
> > > > drivers like cfag12864b been inited if their dependencies failed.
> > > >
> > > > Anyway, thanks for discovering the bug (I wouldn't have discovered it
> > > > as my motherboard has parallel port ;).
> > >
> > > I have a parallel port, but I don't have the cfag12864b or ks0108 .. I
> >
> > In fact, you don't need the cfag12864b display, as the driver just
> > needs the parallel port for output (you can put some LEDs attached to
> > the parallel port to check the driver's operations and it will be
> > fine).
> >
> > BTW, why ks0108 didn't find the 888 parallel port? Haven't you got such
> number?
>
> You know I think your right, it may not have a parallel port.. It's so
> common I tend to take it for granted .
>

Sadly, parallel ports are dying :(

> > > think the simplest way to fix it would be to add a flag to ks0108 that
> > > signals successful initialization , then force cfag12864b to check for
> > > that ..
> >
> > Sure, that won't be a problem; however, maybe there is other better way
> (?).
>
> When you build the driver as a module the module loader will only load
> what is needed, and in order .. When the same driver is built-in the
> module_init() becomes a device_initcall() , and all device_initcall's
> get executed .. So you have to expect that cfag12864b drivers initcalls
> will run eventually and will need to be careful about it's
> initialization.

That is the real problem, indeed.

>
> Daniel
>
>

All right, if there is no other better way to avoid that, I will send
the patch soon.

Miguel

-- 
Miguel Ojeda
http://maxextreme.googlepages.com/index.htm