linux-2.6.today: rtc_cmos init oops/panic in rtc_sysfs_remove_device()

linux-2.6.today: rtc_cmos init oops/panic in rtc_sysfs_remove_device()

[Mike Galbraith]

[-- Attachment #1: Type: text/plain, Size: 2901 bytes --]

Kernel I captured this from is dirty, but virgin source does exactly the
same.  gzipped config attached.


[   32.211999] rtc_cmos 00:03: rtc core: registered rtc_cmos as rtc0
[   32.227226] BUG: unable to handle kernel NULL pointer dereference at virtual address 00000018
[   32.245198]  printing eip:
[   32.257172] c03c7cc8
[   32.268434] *pde = 00000000
[   32.280222] Oops: 0000 [#1]
[   32.291997] PREEMPT SMP 
[   32.303352] Modules linked in:
[   32.315009] CPU:    0
[   32.315010] EIP:    0060:[<c03c7cc8>]    Not tainted VLI
[   32.315011] EFLAGS: 00010202   (2.6.20-smp-g575d5e72-dirty #25)
[   32.353865] EIP is at rtc_sysfs_remove_device+0x18/0x4b
[   32.367596] eax: 00000000   ebx: dfef0454   ecx: c03c7cb0   edx: c064212c
[   32.383044] esi: dfef0454   edi: c19c5f00   ebp: c18ede3c   esp: c18ede38
[   32.398590] ds: 007b   es: 007b   fs: 00d8  gs: 0000  ss: 0068
[   32.413219] Process swapper (pid: 1, ti=c18ed000 task=c18eca50 task.ti=c18ed000)
[   32.420821] Stack: c064212c c18ede5c c0350b5e c19c5e74 00000000 c19c5f08 dfef0454 dfef050c 
[   32.438377]        c1997244 c18ede68 c0350c21 dfef0454 c18ede78 c03c752f fffffff0 c199741c 
[   32.456025]        c18edea8 c03c931d 00000000 00000020 00000004 c04f8c60 00000008 00000000 
[   32.473692] Call Trace:
[   32.493941]  [<c0105146>] show_trace_log_lvl+0x1a/0x30
[   32.508144]  [<c0105201>] show_stack_log_lvl+0xa5/0xca
[   32.522210]  [<c0105422>] show_registers+0x1fc/0x341
[   32.536006]  [<c0105683>] die+0x11c/0x22b
[   32.548692]  [<c011cb3b>] do_page_fault+0x160/0x575
[   32.562185]  [<c04cea7c>] error_code+0x7c/0x84
[   32.575029]  [<c0350b5e>] class_device_del+0x72/0x12a
[   32.588332]  [<c0350c21>] class_device_unregister+0xb/0x15
[   32.601816]  [<c03c752f>] rtc_device_unregister+0x2c/0x30
[   32.615146]  [<c03c931d>] cmos_pnp_probe+0x147/0x1ee
[   32.627927]  [<c03200fc>] pnp_device_probe+0x4c/0xa0
[   32.640614]  [<c03501f3>] really_probe+0x5c/0x163
[   32.652999]  [<c0350397>] driver_probe_device+0x9d/0xa9
[   32.665800]  [<c03504bc>] __driver_attach+0x84/0x86
[   32.678364]  [<c034f7e3>] bus_for_each_dev+0x44/0x62
[   32.690966]  [<c03500bf>] driver_attach+0x19/0x1b
[   32.703177]  [<c034fb0a>] bus_add_driver+0x6a/0x185
[   32.715467]  [<c0350659>] driver_register+0x54/0x84
[   32.727547]  [<c031fef1>] pnp_register_driver+0x17/0x19
[   32.739810]  [<c067f648>] cmos_init+0xd/0xf
[   32.751078]  [<c066358c>] init+0x114/0x23c
[   32.762285]  [<c0104dbb>] kernel_thread_helper+0x7/0x1c
[   32.774782]  =======================
[   32.785651] Code: 8b 45 f0 e8 0a 5a 10 00 89 f8 83 c4 04 5b 5e 5f 5d c3 90 55 89 e5 53 89 c3 8b 40 6c f6 80 2c 01 00 00 01 74 0d 8b 83 b4 00 00 00 <8b> 40 18 85 c0 75 10 8d 43 08 ba 48 21 64 c0 e8 75 e4 de ff 5b 
[   32.830191] EIP: [<c03c7cc8>] rtc_sysfs_remove_device+0x18/0x4b SS:ESP 0068:c18ede38
[   32.846381] Kernel panic - not syncing: Attempted to kill init!


[-- Attachment #2: config.gz --]
[-- Type: application/x-gzip, Size: 14012 bytes --]

[patch] Re: linux-2.6.today: rtc_cmos init oops/panic in rtc_sysfs_remove_device()

[Mike Galbraith]

On Tue, 2007-02-20 at 11:26 +0100, Mike Galbraith wrote:
> Kernel I captured this from is dirty, but virgin source does exactly the
> same.  gzipped config attached.
> 
> 
> [   32.211999] rtc_cmos 00:03: rtc core: registered rtc_cmos as rtc0
> [   32.227226] BUG: unable to handle kernel NULL pointer dereference at virtual address 00000018
> [   32.245198]  printing eip:
> [   32.257172] c03c7cc8
> [   32.268434] *pde = 00000000
> [   32.280222] Oops: 0000 [#1]
> [   32.291997] PREEMPT SMP 
> [   32.303352] Modules linked in:
> [   32.315009] CPU:    0
> [   32.315010] EIP:    0060:[<c03c7cc8>]    Not tainted VLI
> [   32.315011] EFLAGS: 00010202   (2.6.20-smp-g575d5e72-dirty #25)
> [   32.353865] EIP is at rtc_sysfs_remove_device+0x18/0x4b
> [   32.367596] eax: 00000000   ebx: dfef0454   ecx: c03c7cb0   edx: c064212c
> [   32.383044] esi: dfef0454   edi: c19c5f00   ebp: c18ede3c   esp: c18ede38
> [   32.398590] ds: 007b   es: 007b   fs: 00d8  gs: 0000  ss: 0068
> [   32.413219] Process swapper (pid: 1, ti=c18ed000 task=c18eca50 task.ti=c18ed000)
> [   32.420821] Stack: c064212c c18ede5c c0350b5e c19c5e74 00000000 c19c5f08 dfef0454 dfef050c 
> [   32.438377]        c1997244 c18ede68 c0350c21 dfef0454 c18ede78 c03c752f fffffff0 c199741c 
> [   32.456025]        c18edea8 c03c931d 00000000 00000020 00000004 c04f8c60 00000008 00000000 
> [   32.473692] Call Trace:
> [   32.493941]  [<c0105146>] show_trace_log_lvl+0x1a/0x30
> [   32.508144]  [<c0105201>] show_stack_log_lvl+0xa5/0xca
> [   32.522210]  [<c0105422>] show_registers+0x1fc/0x341
> [   32.536006]  [<c0105683>] die+0x11c/0x22b
> [   32.548692]  [<c011cb3b>] do_page_fault+0x160/0x575
> [   32.562185]  [<c04cea7c>] error_code+0x7c/0x84
> [   32.575029]  [<c0350b5e>] class_device_del+0x72/0x12a
> [   32.588332]  [<c0350c21>] class_device_unregister+0xb/0x15
> [   32.601816]  [<c03c752f>] rtc_device_unregister+0x2c/0x30
> [   32.615146]  [<c03c931d>] cmos_pnp_probe+0x147/0x1ee
> [   32.627927]  [<c03200fc>] pnp_device_probe+0x4c/0xa0
> [   32.640614]  [<c03501f3>] really_probe+0x5c/0x163
> [   32.652999]  [<c0350397>] driver_probe_device+0x9d/0xa9
> [   32.665800]  [<c03504bc>] __driver_attach+0x84/0x86
> [   32.678364]  [<c034f7e3>] bus_for_each_dev+0x44/0x62
> [   32.690966]  [<c03500bf>] driver_attach+0x19/0x1b
> [   32.703177]  [<c034fb0a>] bus_add_driver+0x6a/0x185
> [   32.715467]  [<c0350659>] driver_register+0x54/0x84
> [   32.727547]  [<c031fef1>] pnp_register_driver+0x17/0x19
> [   32.739810]  [<c067f648>] cmos_init+0xd/0xf
> [   32.751078]  [<c066358c>] init+0x114/0x23c
> [   32.762285]  [<c0104dbb>] kernel_thread_helper+0x7/0x1c
> [   32.774782]  =======================
> [   32.785651] Code: 8b 45 f0 e8 0a 5a 10 00 89 f8 83 c4 04 5b 5e 5f 5d c3 90 55 89 e5 53 89 c3 8b 40 6c f6 80 2c 01 00 00 01 74 0d 8b 83 b4 00 00 00 <8b> 40 18 85 c0 75 10 8d 43 08 ba 48 21 64 c0 e8 75 e4 de ff 5b 
> [   32.830191] EIP: [<c03c7cc8>] rtc_sysfs_remove_device+0x18/0x4b SS:ESP 0068:c18ede38
> [   32.846381] Kernel panic - not syncing: Attempted to kill init!

I took a look at it this morning.  If CONFIG_PNPACPI is set, and
request_resource() fails in cmos_do_probe(), rtc_does_wakealarm()
dereferences rtc->ops which we NULLed in rtc_device_unregister().

Fix NULL pointer dereference in cmos_rtc registration failure path.

Signed-off-by: Mike Galbraith <efault@gmx.de>

diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c
index 7a0d8ee..9201786 100644
--- a/drivers/rtc/class.c
+++ b/drivers/rtc/class.c
@@ -113,10 +113,10 @@ EXPORT_SYMBOL_GPL(rtc_device_register);
  */
 void rtc_device_unregister(struct rtc_device *rtc)
 {
+	class_device_unregister(&rtc->class_dev);
 	mutex_lock(&rtc->ops_lock);
 	rtc->ops = NULL;
 	mutex_unlock(&rtc->ops_lock);
-	class_device_unregister(&rtc->class_dev);
 }
 EXPORT_SYMBOL_GPL(rtc_device_unregister);
 

Re: [patch] Re: linux-2.6.today: rtc_cmos init oops/panic in rtc_sysfs_remove_device()

[Mike Galbraith]

On Sun, 2007-02-25 at 09:32 +0100, Mike Galbraith wrote:

> Fix NULL pointer dereference in cmos_rtc registration failure path.
> 
> Signed-off-by: Mike Galbraith <efault@gmx.de>
> 
> diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c
> index 7a0d8ee..9201786 100644
> --- a/drivers/rtc/class.c
> +++ b/drivers/rtc/class.c
> @@ -113,10 +113,10 @@ EXPORT_SYMBOL_GPL(rtc_device_register);
>   */
>  void rtc_device_unregister(struct rtc_device *rtc)
>  {
> +	class_device_unregister(&rtc->class_dev);
>  	mutex_lock(&rtc->ops_lock);
>  	rtc->ops = NULL;
>  	mutex_unlock(&rtc->ops_lock);
> -	class_device_unregister(&rtc->class_dev);
>  }
>  EXPORT_SYMBOL_GPL(rtc_device_unregister);

However, re-enabling CONFIG_DEBUG_SLAB which somehow got disabled
emitted the below.

[   36.765977] rtc_cmos 00:03: rtc intf: sysfs
[   36.779301] rtc_cmos 00:03: rtc intf: proc
[   36.792557] rtc_cmos 00:03: rtc intf: dev (239:0)
[   36.806139] rtc_cmos 00:03: rtc core: registered rtc_cmos as rtc0
[   36.821017] rtc_cmos 00:03: i/o registers already in use
[   36.834992] rtc_cmos 00:03: removing char 239:0
[   36.848268] pnp: Device 00:03 does not support disabling.
[   36.862282] rtc_cmos: probe of 00:03 failed with error -16
[   36.876394] md: linear personality registered for level -1
[   36.890508] md: raid0 personality registered for level 0
[   36.904563] md: raid1 personality registered for level 1
[   36.918454] md: raid10 personality registered for level 10
[   36.949187] raid6: int32x1    722 MB/s
[   36.978092] raid6: int32x2    800 MB/s
[   37.006101] raid6: int32x4    917 MB/s
[   37.034047] raid6: int32x8    671 MB/s
[   37.062030] raid6: mmxx1     2371 MB/s
[   37.090000] raid6: mmxx2     3066 MB/s
[   37.118001] raid6: sse1x1    1449 MB/s
[   37.145964] raid6: sse1x2    2699 MB/s
[   37.172947] raid6: sse2x1    2222 MB/s
[   37.199912] raid6: sse2x2    3375 MB/s
[   37.210371] raid6: using algorithm sse2x2 (3375 MB/s)
[   37.222143] md: raid6 personality registered for level 6
[   37.234127] md: raid5 personality registered for level 5
[   37.245889] md: raid4 personality registered for level 4
[   37.257313] raid5: automatically using best checksumming function: pIII_sse
[   37.274853]    pIII_sse  :  3812.000 MB/sec
[   37.284947] raid5: using function: pIII_sse (3812.000 MB/sec)
[   37.296648] md: multipath personality registered for level -4
[   37.308377] Slab corruption: start=dfede248, len=512
[   37.319200] Redzone: 0x5a2cf071/0x5a2cf071.
[   37.329211] Last user: [<c03c8ab2>](rtc_device_release+0x31/0x34)
[   37.341274] 0b0: 6b 6b 6b 6b 00 00 00 00 6b 6b 6b 6b 6b 6b 6b 6b
[   37.353894] Prev obj: start=dfede03c, len=512
[   37.364285] Redzone: 0x5a2cf071/0x5a2cf071.
[   37.374409] Last user: [<c0408643>](skb_release_data+0x57/0x79)
[   37.386402] 000: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
[   37.399116] 010: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
[   37.411788] Next obj: start=dfede454, len=512
[   37.422309] Redzone: 0x170fc2a5/0x170fc2a5.
[   37.432642] Last user: [<c0350168>](device_create+0x2b/0xa3)
[   37.444670] 000: 01 00 00 00 00 00 00 00 5c e4 ed df 5c e4 ed df
[   37.457532] 010: 73 f2 34 c0 ff f0 34 c0 00 00 00 00 00 00 00 00
[   37.470429] device-mapper: ioctl: 4.11.0-ioctl (2006-10-12) initialised: dm-devel@redhat.com

Darn.

	-Mike

[patch take 2] Re: linux-2.6.today: rtc_cmos init oops/panic in rtc_sysfs_remove_device()

[Mike Galbraith]

On Tue, 2007-02-27 at 10:25 +0100, Mike Galbraith wrote:
> On Sun, 2007-02-25 at 09:32 +0100, Mike Galbraith wrote:
> 
> > Fix NULL pointer dereference in cmos_rtc registration failure path.
> > 
> > Signed-off-by: Mike Galbraith <efault@gmx.de>
> > 
> > diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c
> > index 7a0d8ee..9201786 100644
> > --- a/drivers/rtc/class.c
> > +++ b/drivers/rtc/class.c
> > @@ -113,10 +113,10 @@ EXPORT_SYMBOL_GPL(rtc_device_register);
> >   */
> >  void rtc_device_unregister(struct rtc_device *rtc)
> >  {
> > +	class_device_unregister(&rtc->class_dev);
> >  	mutex_lock(&rtc->ops_lock);
> >  	rtc->ops = NULL;
> >  	mutex_unlock(&rtc->ops_lock);
> > -	class_device_unregister(&rtc->class_dev);
> >  }
> >  EXPORT_SYMBOL_GPL(rtc_device_unregister);
> 
> However, re-enabling CONFIG_DEBUG_SLAB which somehow got disabled
> emitted the below.
> 
> [   36.765977] rtc_cmos 00:03: rtc intf: sysfs
> [   36.779301] rtc_cmos 00:03: rtc intf: proc
> [   36.792557] rtc_cmos 00:03: rtc intf: dev (239:0)
> [   36.806139] rtc_cmos 00:03: rtc core: registered rtc_cmos as rtc0
> [   36.821017] rtc_cmos 00:03: i/o registers already in use
> [   36.834992] rtc_cmos 00:03: removing char 239:0
> [   36.848268] pnp: Device 00:03 does not support disabling.
> [   36.862282] rtc_cmos: probe of 00:03 failed with error -16
> [   36.876394] md: linear personality registered for level -1
> [   36.890508] md: raid0 personality registered for level 0
> [   36.904563] md: raid1 personality registered for level 1
> [   36.918454] md: raid10 personality registered for level 10
> [   36.949187] raid6: int32x1    722 MB/s
> [   36.978092] raid6: int32x2    800 MB/s
> [   37.006101] raid6: int32x4    917 MB/s
> [   37.034047] raid6: int32x8    671 MB/s
> [   37.062030] raid6: mmxx1     2371 MB/s
> [   37.090000] raid6: mmxx2     3066 MB/s
> [   37.118001] raid6: sse1x1    1449 MB/s
> [   37.145964] raid6: sse1x2    2699 MB/s
> [   37.172947] raid6: sse2x1    2222 MB/s
> [   37.199912] raid6: sse2x2    3375 MB/s
> [   37.210371] raid6: using algorithm sse2x2 (3375 MB/s)
> [   37.222143] md: raid6 personality registered for level 6
> [   37.234127] md: raid5 personality registered for level 5
> [   37.245889] md: raid4 personality registered for level 4
> [   37.257313] raid5: automatically using best checksumming function: pIII_sse
> [   37.274853]    pIII_sse  :  3812.000 MB/sec
> [   37.284947] raid5: using function: pIII_sse (3812.000 MB/sec)
> [   37.296648] md: multipath personality registered for level -4
> [   37.308377] Slab corruption: start=dfede248, len=512
> [   37.319200] Redzone: 0x5a2cf071/0x5a2cf071.
> [   37.329211] Last user: [<c03c8ab2>](rtc_device_release+0x31/0x34)
> [   37.341274] 0b0: 6b 6b 6b 6b 00 00 00 00 6b 6b 6b 6b 6b 6b 6b 6b
> [   37.353894] Prev obj: start=dfede03c, len=512
> [   37.364285] Redzone: 0x5a2cf071/0x5a2cf071.
> [   37.374409] Last user: [<c0408643>](skb_release_data+0x57/0x79)
> [   37.386402] 000: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> [   37.399116] 010: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
> [   37.411788] Next obj: start=dfede454, len=512
> [   37.422309] Redzone: 0x170fc2a5/0x170fc2a5.
> [   37.432642] Last user: [<c0350168>](device_create+0x2b/0xa3)
> [   37.444670] 000: 01 00 00 00 00 00 00 00 5c e4 ed df 5c e4 ed df
> [   37.457532] 010: 73 f2 34 c0 ff f0 34 c0 00 00 00 00 00 00 00 00
> [   37.470429] device-mapper: ioctl: 4.11.0-ioctl (2006-10-12) initialised: dm-devel@redhat.com

Dummy here created a use after free.

Fix NULL pointer dereference in cmos_rtc registration failure path.
Since we're freeing rtc in rtc_device_release(), there should be no need
to NULL rtc->ops.  Anybody who has a reference to the freed rtc after
device release, and uses it, will hopefully explode violently.

Signed-off-by: Mike Galbraith <efault@gmx.de>

diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c
index 7a0d8ee..d338fb8 100644
--- a/drivers/rtc/class.c
+++ b/drivers/rtc/class.c
@@ -113,9 +113,6 @@ EXPORT_SYMBOL_GPL(rtc_device_register);
  */
 void rtc_device_unregister(struct rtc_device *rtc)
 {
-	mutex_lock(&rtc->ops_lock);
-	rtc->ops = NULL;
-	mutex_unlock(&rtc->ops_lock);
 	class_device_unregister(&rtc->class_dev);
 }
 EXPORT_SYMBOL_GPL(rtc_device_unregister);

Re: [patch take 2] Re: linux-2.6.today: rtc_cmos init oops/panic in rtc_sysfs_remove_device()

[Andrew Morton]

On Thu, 01 Mar 2007 09:55:06 +0100
Mike Galbraith <efault@gmx.de> wrote:

> Dummy here created a use after free.
> 
> Fix NULL pointer dereference in cmos_rtc registration failure path.
> Since we're freeing rtc in rtc_device_release(), there should be no need
> to NULL rtc->ops.  Anybody who has a reference to the freed rtc after
> device release, and uses it, will hopefully explode violently.
> 
> Signed-off-by: Mike Galbraith <efault@gmx.de>
> 
> diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c
> index 7a0d8ee..d338fb8 100644
> --- a/drivers/rtc/class.c
> +++ b/drivers/rtc/class.c
> @@ -113,9 +113,6 @@ EXPORT_SYMBOL_GPL(rtc_device_register);
>   */
>  void rtc_device_unregister(struct rtc_device *rtc)
>  {
> -	mutex_lock(&rtc->ops_lock);
> -	rtc->ops = NULL;
> -	mutex_unlock(&rtc->ops_lock);
>  	class_device_unregister(&rtc->class_dev);
>  }
>  EXPORT_SYMBOL_GPL(rtc_device_unregister);

Linus today merged the below, which I cunningly forgot to cc you on.

Can you please review current mainline, see if we still need fixes?

Thanks.


From: David Brownell <david-b@pacbell.net>

Fix an oops on the rtc_device_unregister() path by waiting until the last
moment before nulling the rtc->ops vector.  Fix some potential oopses by
having the rtc_class_open()/rtc_class_close() interface increase the RTC's
reference count while an RTC handle is available outside the RTC framework.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/rtc/class.c     |   14 ++++++++++----
 drivers/rtc/interface.c |    3 ++-
 2 files changed, 12 insertions(+), 5 deletions(-)

diff -puN drivers/rtc/class.c~rtc_cmos-oops-fix drivers/rtc/class.c
--- a/drivers/rtc/class.c~rtc_cmos-oops-fix
+++ a/drivers/rtc/class.c
@@ -113,10 +113,16 @@ EXPORT_SYMBOL_GPL(rtc_device_register);
  */
 void rtc_device_unregister(struct rtc_device *rtc)
 {
-	mutex_lock(&rtc->ops_lock);
-	rtc->ops = NULL;
-	mutex_unlock(&rtc->ops_lock);
-	class_device_unregister(&rtc->class_dev);
+	if (class_device_get(&rtc->class_dev) != NULL) {
+		mutex_lock(&rtc->ops_lock);
+		/* remove innards of this RTC, then disable it, before
+		 * letting any rtc_class_open() users access it again
+		 */
+		class_device_unregister(&rtc->class_dev);
+		rtc->ops = NULL;
+		mutex_unlock(&rtc->ops_lock);
+		class_device_put(&rtc->class_dev);
+	}
 }
 EXPORT_SYMBOL_GPL(rtc_device_unregister);
 
diff -puN drivers/rtc/interface.c~rtc_cmos-oops-fix drivers/rtc/interface.c
--- a/drivers/rtc/interface.c~rtc_cmos-oops-fix
+++ a/drivers/rtc/interface.c
@@ -179,7 +179,7 @@ struct class_device *rtc_class_open(char
 	down(&rtc_class->sem);
 	list_for_each_entry(class_dev_tmp, &rtc_class->children, node) {
 		if (strncmp(class_dev_tmp->class_id, name, BUS_ID_SIZE) == 0) {
-			class_dev = class_dev_tmp;
+			class_dev = class_device_get(class_dev_tmp);
 			break;
 		}
 	}
@@ -197,6 +197,7 @@ EXPORT_SYMBOL_GPL(rtc_class_open);
 void rtc_class_close(struct class_device *class_dev)
 {
 	module_put(to_rtc_device(class_dev)->owner);
+	class_device_put(class_dev);
 }
 EXPORT_SYMBOL_GPL(rtc_class_close);
 
_