linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RESEND PATCH] ipmi: Clear drvdata when interface is removed by hotmod
@ 2014-09-09  0:19 Takao Indoh
  2014-09-09 18:17 ` Corey Minyard
  0 siblings, 1 reply; 3+ messages in thread
From: Takao Indoh @ 2014-09-09  0:19 UTC (permalink / raw)
  To: Corey Minyard
  Cc: openipmi-developer, Corey Minyard, linux-kernel, gouji.masayuki

Add another email of maintainer just in case.


This patch fixes a bug on hotmod removing.

After ipmi interface is removed using hotmod, kernel panic occurs when
rmmod impi_si. For example, try this:

 # echo "remove,"`cat /proc/ipmi/0/params` > \
 /sys/module/ipmi_si/parameters/hotmod
 # rmmod ipmi_si

Then, rmmod fails with the following messages.

------------[ cut here ]------------
WARNING: CPU: 12 PID: 10819 at /mnt/repos/linux/lib/list_debug.c:53
__list_del_entry+0x63/0xd0()
(snip)
CPU: 12 PID: 10819 Comm: rmmod Not tainted 3.17.0-rc1 #19
Hardware name: FUJITSU-SV PRIMERGY BX920 S2/D3030, BIOS 080015
Rev.3D81.3030 02/10/2012
 0000000000000009 ffff88022d547d40 ffffffff81575778 ffff88022d547d88
 ffff88022d547d78 ffffffff8104ec5d ffff88023908cdb0 ffffffffa06fa4e0
 ffff8800bac20860 0000000000000000 0000000002046090 ffff88022d547dd8
Call Trace:
 [<ffffffff81575778>] dump_stack+0x45/0x56
 [<ffffffff8104ec5d>] warn_slowpath_common+0x7d/0xa0
 [<ffffffff8104eccc>] warn_slowpath_fmt+0x4c/0x50
 [<ffffffff811f60bf>] ? __kernfs_remove+0xdf/0x220
 [<ffffffff81291213>] __list_del_entry+0x63/0xd0
 [<ffffffff8129128d>] list_del+0xd/0x30
 [<ffffffffa06f285a>] cleanup_one_si+0x2a/0x230 [ipmi_si]
 [<ffffffffa06f2f05>] ipmi_pnp_remove+0x15/0x20 [ipmi_si]
 [<ffffffff8131c7d4>] pnp_device_remove+0x24/0x40
 [<ffffffff8137175f>] __device_release_driver+0x7f/0xf0
 [<ffffffff81372100>] driver_detach+0xb0/0xc0
 [<ffffffff81371415>] bus_remove_driver+0x55/0xd0
 [<ffffffff8137283c>] driver_unregister+0x2c/0x50
 [<ffffffff8131ca02>] pnp_unregister_driver+0x12/0x20
 [<ffffffffa06f347c>] cleanup_ipmi_si+0xbc/0xf0 [ipmi_si]
 [<ffffffff810c33f2>] SyS_delete_module+0x132/0x1c0
 [<ffffffff81002ab9>] ? do_notify_resume+0x59/0x80
 [<ffffffff8157c45a>] ? int_signal+0x12/0x17
 [<ffffffff8157c1d2>] system_call_fastpath+0x16/0x1b
---[ end trace 70b4377268f85c23 ]---

list_del in cleanup_one_si() fails because the smi_info is already
removed when hotmod removing.

When ipmi interface is removed by hotmod, smi_info is removed by
cleanup_one_si(), but it is still set in drvdata. Therefore when rmmod
ipmi_si, ipmi_pnp_remove tries to remove it again and fails.

By this patch, a pointer to smi_info in drvdata is cleared when hotmod
removing so that it will be not accessed when rmmod.

Signed-off-by: Takao Indoh <indou.takao@jp.fujitsu.com>
---
 drivers/char/ipmi/ipmi_si_intf.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
index 5d66568..20a3739 100644
--- a/drivers/char/ipmi/ipmi_si_intf.c
+++ b/drivers/char/ipmi/ipmi_si_intf.c
@@ -1904,8 +1904,14 @@ static int hotmod_handler(const char *val, struct kernel_param *kp)
 					continue;
 				if (e->si_type != si_type)
 					continue;
-				if (e->io.addr_data == addr)
-					cleanup_one_si(e);
+				if (e->io.addr_data != addr)
+					continue;
+
+				/* Clear driver data */
+				if (e->dev)
+					dev_set_drvdata(e->dev, NULL);
+
+				cleanup_one_si(e);
 			}
 			mutex_unlock(&smi_infos_lock);
 		}
@@ -2316,7 +2322,8 @@ static void ipmi_pnp_remove(struct pnp_dev *dev)
 {
 	struct smi_info *info = pnp_get_drvdata(dev);
 
-	cleanup_one_si(info);
+	if (info)
+		cleanup_one_si(info);
 }
 
 static const struct pnp_device_id pnp_dev_table[] = {
@@ -2621,7 +2628,8 @@ static int ipmi_pci_probe(struct pci_dev *pdev,
 static void ipmi_pci_remove(struct pci_dev *pdev)
 {
 	struct smi_info *info = pci_get_drvdata(pdev);
-	cleanup_one_si(info);
+	if (info)
+		cleanup_one_si(info);
 	pci_disable_device(pdev);
 }
 
@@ -2729,7 +2737,10 @@ static int ipmi_probe(struct platform_device *dev)
 static int ipmi_remove(struct platform_device *dev)
 {
 #ifdef CONFIG_OF
-	cleanup_one_si(dev_get_drvdata(&dev->dev));
+	struct smi_info *info = dev_get_drvdata(&dev->dev);
+
+	if (info)
+		cleanup_one_si(info);
 #endif
 	return 0;
 }
@@ -2796,7 +2807,11 @@ static int ipmi_parisc_probe(struct parisc_device *dev)
 
 static int ipmi_parisc_remove(struct parisc_device *dev)
 {
-	cleanup_one_si(dev_get_drvdata(&dev->dev));
+	struct smi_info *info = dev_get_drvdata(&dev->dev);
+
+	if (info)
+		cleanup_one_si(info);
+
 	return 0;
 }
 
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [RESEND PATCH] ipmi: Clear drvdata when interface is removed by hotmod
  2014-09-09  0:19 [RESEND PATCH] ipmi: Clear drvdata when interface is removed by hotmod Takao Indoh
@ 2014-09-09 18:17 ` Corey Minyard
  2014-09-10  0:42   ` Takao Indoh
  0 siblings, 1 reply; 3+ messages in thread
From: Corey Minyard @ 2014-09-09 18:17 UTC (permalink / raw)
  To: Takao Indoh; +Cc: openipmi-developer, linux-kernel, gouji.masayuki

Ok, I can see the problem.  Instead of the change you have, can you add
something like:

if (!info)
        return;
if (info->dev)
        dev_set_drvdata(info->dev, NULL);

to the top of cleanup_one_si() instead?  I think that would be a cleaner
and more general.

-corey


On 09/08/2014 07:19 PM, Takao Indoh wrote:
> Add another email of maintainer just in case.
>
>
> This patch fixes a bug on hotmod removing.
>
> After ipmi interface is removed using hotmod, kernel panic occurs when
> rmmod impi_si. For example, try this:
>
>  # echo "remove,"`cat /proc/ipmi/0/params` > \
>  /sys/module/ipmi_si/parameters/hotmod
>  # rmmod ipmi_si
>
> Then, rmmod fails with the following messages.
>
> ------------[ cut here ]------------
> WARNING: CPU: 12 PID: 10819 at /mnt/repos/linux/lib/list_debug.c:53
> __list_del_entry+0x63/0xd0()
> (snip)
> CPU: 12 PID: 10819 Comm: rmmod Not tainted 3.17.0-rc1 #19
> Hardware name: FUJITSU-SV PRIMERGY BX920 S2/D3030, BIOS 080015
> Rev.3D81.3030 02/10/2012
>  0000000000000009 ffff88022d547d40 ffffffff81575778 ffff88022d547d88
>  ffff88022d547d78 ffffffff8104ec5d ffff88023908cdb0 ffffffffa06fa4e0
>  ffff8800bac20860 0000000000000000 0000000002046090 ffff88022d547dd8
> Call Trace:
>  [<ffffffff81575778>] dump_stack+0x45/0x56
>  [<ffffffff8104ec5d>] warn_slowpath_common+0x7d/0xa0
>  [<ffffffff8104eccc>] warn_slowpath_fmt+0x4c/0x50
>  [<ffffffff811f60bf>] ? __kernfs_remove+0xdf/0x220
>  [<ffffffff81291213>] __list_del_entry+0x63/0xd0
>  [<ffffffff8129128d>] list_del+0xd/0x30
>  [<ffffffffa06f285a>] cleanup_one_si+0x2a/0x230 [ipmi_si]
>  [<ffffffffa06f2f05>] ipmi_pnp_remove+0x15/0x20 [ipmi_si]
>  [<ffffffff8131c7d4>] pnp_device_remove+0x24/0x40
>  [<ffffffff8137175f>] __device_release_driver+0x7f/0xf0
>  [<ffffffff81372100>] driver_detach+0xb0/0xc0
>  [<ffffffff81371415>] bus_remove_driver+0x55/0xd0
>  [<ffffffff8137283c>] driver_unregister+0x2c/0x50
>  [<ffffffff8131ca02>] pnp_unregister_driver+0x12/0x20
>  [<ffffffffa06f347c>] cleanup_ipmi_si+0xbc/0xf0 [ipmi_si]
>  [<ffffffff810c33f2>] SyS_delete_module+0x132/0x1c0
>  [<ffffffff81002ab9>] ? do_notify_resume+0x59/0x80
>  [<ffffffff8157c45a>] ? int_signal+0x12/0x17
>  [<ffffffff8157c1d2>] system_call_fastpath+0x16/0x1b
> ---[ end trace 70b4377268f85c23 ]---
>
> list_del in cleanup_one_si() fails because the smi_info is already
> removed when hotmod removing.
>
> When ipmi interface is removed by hotmod, smi_info is removed by
> cleanup_one_si(), but it is still set in drvdata. Therefore when rmmod
> ipmi_si, ipmi_pnp_remove tries to remove it again and fails.
>
> By this patch, a pointer to smi_info in drvdata is cleared when hotmod
> removing so that it will be not accessed when rmmod.
>
> Signed-off-by: Takao Indoh <indou.takao@jp.fujitsu.com>
> ---
>  drivers/char/ipmi/ipmi_si_intf.c | 27 +++++++++++++++++++++------
>  1 file changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
> index 5d66568..20a3739 100644
> --- a/drivers/char/ipmi/ipmi_si_intf.c
> +++ b/drivers/char/ipmi/ipmi_si_intf.c
> @@ -1904,8 +1904,14 @@ static int hotmod_handler(const char *val, struct kernel_param *kp)
>  					continue;
>  				if (e->si_type != si_type)
>  					continue;
> -				if (e->io.addr_data == addr)
> -					cleanup_one_si(e);
> +				if (e->io.addr_data != addr)
> +					continue;
> +
> +				/* Clear driver data */
> +				if (e->dev)
> +					dev_set_drvdata(e->dev, NULL);
> +
> +				cleanup_one_si(e);
>  			}
>  			mutex_unlock(&smi_infos_lock);
>  		}
> @@ -2316,7 +2322,8 @@ static void ipmi_pnp_remove(struct pnp_dev *dev)
>  {
>  	struct smi_info *info = pnp_get_drvdata(dev);
>  
> -	cleanup_one_si(info);
> +	if (info)
> +		cleanup_one_si(info);
>  }
>  
>  static const struct pnp_device_id pnp_dev_table[] = {
> @@ -2621,7 +2628,8 @@ static int ipmi_pci_probe(struct pci_dev *pdev,
>  static void ipmi_pci_remove(struct pci_dev *pdev)
>  {
>  	struct smi_info *info = pci_get_drvdata(pdev);
> -	cleanup_one_si(info);
> +	if (info)
> +		cleanup_one_si(info);
>  	pci_disable_device(pdev);
>  }
>  
> @@ -2729,7 +2737,10 @@ static int ipmi_probe(struct platform_device *dev)
>  static int ipmi_remove(struct platform_device *dev)
>  {
>  #ifdef CONFIG_OF
> -	cleanup_one_si(dev_get_drvdata(&dev->dev));
> +	struct smi_info *info = dev_get_drvdata(&dev->dev);
> +
> +	if (info)
> +		cleanup_one_si(info);
>  #endif
>  	return 0;
>  }
> @@ -2796,7 +2807,11 @@ static int ipmi_parisc_probe(struct parisc_device *dev)
>  
>  static int ipmi_parisc_remove(struct parisc_device *dev)
>  {
> -	cleanup_one_si(dev_get_drvdata(&dev->dev));
> +	struct smi_info *info = dev_get_drvdata(&dev->dev);
> +
> +	if (info)
> +		cleanup_one_si(info);
> +
>  	return 0;
>  }
>  


^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [RESEND PATCH] ipmi: Clear drvdata when interface is removed by hotmod
  2014-09-09 18:17 ` Corey Minyard
@ 2014-09-10  0:42   ` Takao Indoh
  0 siblings, 0 replies; 3+ messages in thread
From: Takao Indoh @ 2014-09-10  0:42 UTC (permalink / raw)
  To: cminyard; +Cc: openipmi-developer, linux-kernel, gouji.masayuki

(2014/09/10 3:17), Corey Minyard wrote:
> Ok, I can see the problem.  Instead of the change you have, can you add
> something like:
> 
> if (!info)
>          return;
> if (info->dev)
>          dev_set_drvdata(info->dev, NULL);
> 
> to the top of cleanup_one_si() instead?  I think that would be a cleaner
> and more general.

Ok, agreed.

  > if (!info)
  >          return;

This is already done in the top of cleanup_one_si(), so I'll just 
add this:

  > if (info->dev)
  >          dev_set_drvdata(info->dev, NULL);

Thanks,
Takao Indoh


> 
> -corey
> 
> 
> On 09/08/2014 07:19 PM, Takao Indoh wrote:
>> Add another email of maintainer just in case.
>>
>>
>> This patch fixes a bug on hotmod removing.
>>
>> After ipmi interface is removed using hotmod, kernel panic occurs when
>> rmmod impi_si. For example, try this:
>>
>>   # echo "remove,"`cat /proc/ipmi/0/params` > \
>>   /sys/module/ipmi_si/parameters/hotmod
>>   # rmmod ipmi_si
>>
>> Then, rmmod fails with the following messages.
>>
>> ------------[ cut here ]------------
>> WARNING: CPU: 12 PID: 10819 at /mnt/repos/linux/lib/list_debug.c:53
>> __list_del_entry+0x63/0xd0()
>> (snip)
>> CPU: 12 PID: 10819 Comm: rmmod Not tainted 3.17.0-rc1 #19
>> Hardware name: FUJITSU-SV PRIMERGY BX920 S2/D3030, BIOS 080015
>> Rev.3D81.3030 02/10/2012
>>   0000000000000009 ffff88022d547d40 ffffffff81575778 ffff88022d547d88
>>   ffff88022d547d78 ffffffff8104ec5d ffff88023908cdb0 ffffffffa06fa4e0
>>   ffff8800bac20860 0000000000000000 0000000002046090 ffff88022d547dd8
>> Call Trace:
>>   [<ffffffff81575778>] dump_stack+0x45/0x56
>>   [<ffffffff8104ec5d>] warn_slowpath_common+0x7d/0xa0
>>   [<ffffffff8104eccc>] warn_slowpath_fmt+0x4c/0x50
>>   [<ffffffff811f60bf>] ? __kernfs_remove+0xdf/0x220
>>   [<ffffffff81291213>] __list_del_entry+0x63/0xd0
>>   [<ffffffff8129128d>] list_del+0xd/0x30
>>   [<ffffffffa06f285a>] cleanup_one_si+0x2a/0x230 [ipmi_si]
>>   [<ffffffffa06f2f05>] ipmi_pnp_remove+0x15/0x20 [ipmi_si]
>>   [<ffffffff8131c7d4>] pnp_device_remove+0x24/0x40
>>   [<ffffffff8137175f>] __device_release_driver+0x7f/0xf0
>>   [<ffffffff81372100>] driver_detach+0xb0/0xc0
>>   [<ffffffff81371415>] bus_remove_driver+0x55/0xd0
>>   [<ffffffff8137283c>] driver_unregister+0x2c/0x50
>>   [<ffffffff8131ca02>] pnp_unregister_driver+0x12/0x20
>>   [<ffffffffa06f347c>] cleanup_ipmi_si+0xbc/0xf0 [ipmi_si]
>>   [<ffffffff810c33f2>] SyS_delete_module+0x132/0x1c0
>>   [<ffffffff81002ab9>] ? do_notify_resume+0x59/0x80
>>   [<ffffffff8157c45a>] ? int_signal+0x12/0x17
>>   [<ffffffff8157c1d2>] system_call_fastpath+0x16/0x1b
>> ---[ end trace 70b4377268f85c23 ]---
>>
>> list_del in cleanup_one_si() fails because the smi_info is already
>> removed when hotmod removing.
>>
>> When ipmi interface is removed by hotmod, smi_info is removed by
>> cleanup_one_si(), but it is still set in drvdata. Therefore when rmmod
>> ipmi_si, ipmi_pnp_remove tries to remove it again and fails.
>>
>> By this patch, a pointer to smi_info in drvdata is cleared when hotmod
>> removing so that it will be not accessed when rmmod.
>>
>> Signed-off-by: Takao Indoh <indou.takao@jp.fujitsu.com>
>> ---
>>   drivers/char/ipmi/ipmi_si_intf.c | 27 +++++++++++++++++++++------
>>   1 file changed, 21 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
>> index 5d66568..20a3739 100644
>> --- a/drivers/char/ipmi/ipmi_si_intf.c
>> +++ b/drivers/char/ipmi/ipmi_si_intf.c
>> @@ -1904,8 +1904,14 @@ static int hotmod_handler(const char *val, struct kernel_param *kp)
>>   					continue;
>>   				if (e->si_type != si_type)
>>   					continue;
>> -				if (e->io.addr_data == addr)
>> -					cleanup_one_si(e);
>> +				if (e->io.addr_data != addr)
>> +					continue;
>> +
>> +				/* Clear driver data */
>> +				if (e->dev)
>> +					dev_set_drvdata(e->dev, NULL);
>> +
>> +				cleanup_one_si(e);
>>   			}
>>   			mutex_unlock(&smi_infos_lock);
>>   		}
>> @@ -2316,7 +2322,8 @@ static void ipmi_pnp_remove(struct pnp_dev *dev)
>>   {
>>   	struct smi_info *info = pnp_get_drvdata(dev);
>>   
>> -	cleanup_one_si(info);
>> +	if (info)
>> +		cleanup_one_si(info);
>>   }
>>   
>>   static const struct pnp_device_id pnp_dev_table[] = {
>> @@ -2621,7 +2628,8 @@ static int ipmi_pci_probe(struct pci_dev *pdev,
>>   static void ipmi_pci_remove(struct pci_dev *pdev)
>>   {
>>   	struct smi_info *info = pci_get_drvdata(pdev);
>> -	cleanup_one_si(info);
>> +	if (info)
>> +		cleanup_one_si(info);
>>   	pci_disable_device(pdev);
>>   }
>>   
>> @@ -2729,7 +2737,10 @@ static int ipmi_probe(struct platform_device *dev)
>>   static int ipmi_remove(struct platform_device *dev)
>>   {
>>   #ifdef CONFIG_OF
>> -	cleanup_one_si(dev_get_drvdata(&dev->dev));
>> +	struct smi_info *info = dev_get_drvdata(&dev->dev);
>> +
>> +	if (info)
>> +		cleanup_one_si(info);
>>   #endif
>>   	return 0;
>>   }
>> @@ -2796,7 +2807,11 @@ static int ipmi_parisc_probe(struct parisc_device *dev)
>>   
>>   static int ipmi_parisc_remove(struct parisc_device *dev)
>>   {
>> -	cleanup_one_si(dev_get_drvdata(&dev->dev));
>> +	struct smi_info *info = dev_get_drvdata(&dev->dev);
>> +
>> +	if (info)
>> +		cleanup_one_si(info);
>> +
>>   	return 0;
>>   }
>>   
> 
> 
> 



^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-09-10  0:42 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-09  0:19 [RESEND PATCH] ipmi: Clear drvdata when interface is removed by hotmod Takao Indoh
2014-09-09 18:17 ` Corey Minyard
2014-09-10  0:42   ` Takao Indoh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).