[PATCH] ipmi poweroff: fix chassis control

The IPMI power control function proc_write_chassctrl was badly written, it directly used userspace pointers, it assumed that strings were NULL terminated, and it used the evil sscanf function. This converts over to using the sysctl interface for this data and changes the semantics to be a little more logical. Signed-off-by: Corey Minyard <minyard@acm.org> Cc: <viro@parcelfarce.linux.theplanet.co.uk> Signed-off-by: Andrew Morton <akpm@osdl.org> Signed-off-by: Linus Torvalds <torvalds@osdl.org>

[PATCH] ipmi poweroff: fix chassis control
The IPMI power control function proc_write_chassctrl was badly written, it directly used userspace pointers, it assumed that strings were NULL terminated, and it used the evil sscanf function. This converts over to using the sysctl interface for this data and changes the semantics to be a little more logical. Signed-off-by: Corey Minyard <minyard@acm.org> Cc: <viro@parcelfarce.linux.theplanet.co.uk> Signed-off-by: Andrew Morton <akpm@osdl.org> Signed-off-by: Linus Torvalds <torvalds@osdl.org>
Corey Minyard · Linus Torvalds
1 parent 877197ef89
Showing 3 changed files with 71 additions and 80 deletions Side-by-side Diff
Documentation/IPMI.txt
drivers/char/ipmi/ipmi_poweroff.c
include/linux/sysctl.h
@@ -605,12 +605,13 @@
 it will send the proper IPMI commands to do this.  This is supported on
 several platforms.
  
-There is a module parameter named "poweroff_control" that may either be zero
-(do a power down) or 2 (do a power cycle, power the system off, then power
-it on in a few seconds).  Setting ipmi_poweroff.poweroff_control=x will do
-the same thing on the kernel command line.  The parameter is also available
-via the proc filesystem in /proc/ipmi/poweroff_control.  Note that if the
-system does not support power cycling, it will always to the power off.
+There is a module parameter named "poweroff_powercycle" that may
+either be zero (do a power down) or non-zero (do a power cycle, power
+the system off, then power it on in a few seconds).  Setting
+ipmi_poweroff.poweroff_control=x will do the same thing on the kernel
+command line.  The parameter is also available via the proc filesystem
+in /proc/sys/dev/ipmi/poweroff_powercycle.  Note that if the system
+does not support power cycling, it will always do the power off.
  
 Note that if you have ACPI enabled, the system will prefer using ACPI to
 power off.
@@ -52,11 +52,11 @@
 #define IPMI_CHASSIS_POWER_CYCLE	0x02	/* power cycle */
  
 /* the IPMI data command */
-static int poweroff_control = IPMI_CHASSIS_POWER_DOWN;
+static int poweroff_powercycle;
  
 /* parameter definition to allow user to flag power cycle */
-module_param(poweroff_control, int, IPMI_CHASSIS_POWER_DOWN);
-MODULE_PARM_DESC(poweroff_control, " Set to 2 to enable power cycle instead of power down. Power cycle is contingent on hardware support, otherwise it defaults back to power down.");
+module_param(poweroff_powercycle, int, 0);
+MODULE_PARM_DESC(poweroff_powercycles, " Set to non-zero to enable power cycle instead of power down. Power cycle is contingent on hardware support, otherwise it defaults back to power down.");
  
 /* Stuff from the get device id command. */
 static unsigned int mfg_id;
  
  
  
  
@@ -385,37 +385,34 @@
  
  powercyclefailed:
 	printk(KERN_INFO PFX "Powering %s via IPMI chassis control command\n",
-		((poweroff_control != IPMI_CHASSIS_POWER_CYCLE) ? "down" : "cycle"));
+		(poweroff_powercycle ? "cycle" : "down"));
  
 	/*
 	 * Power down
 	 */
 	send_msg.netfn = IPMI_NETFN_CHASSIS_REQUEST;
 	send_msg.cmd = IPMI_CHASSIS_CONTROL_CMD;
-	data[0] = poweroff_control;
+	if (poweroff_powercycle)
+		data[0] = IPMI_CHASSIS_POWER_CYCLE;
+	else
+		data[0] = IPMI_CHASSIS_POWER_DOWN;
 	send_msg.data = data;
 	send_msg.data_len = sizeof(data);
 	rv = ipmi_request_in_rc_mode(user,
 				     (struct ipmi_addr *) &smi_addr,
 				     &send_msg);
 	if (rv) {
-		switch (poweroff_control) {
-			case IPMI_CHASSIS_POWER_CYCLE:
-				/* power cycle failed, default to power down */
-				printk(KERN_ERR PFX "Unable to send chassis power " \
-					"cycle message, IPMI error 0x%x\n", rv);
-				poweroff_control = IPMI_CHASSIS_POWER_DOWN;
-				goto powercyclefailed;
-
-			case IPMI_CHASSIS_POWER_DOWN:
-			default:
-				printk(KERN_ERR PFX "Unable to send chassis power " \
-					"down message, IPMI error 0x%x\n", rv);
-				break;
+		if (poweroff_powercycle) {
+			/* power cycle failed, default to power down */
+			printk(KERN_ERR PFX "Unable to send chassis power " \
+			       "cycle message, IPMI error 0x%x\n", rv);
+			poweroff_powercycle = 0;
+			goto powercyclefailed;
 		}
-	}
  
-	return;
+		printk(KERN_ERR PFX "Unable to send chassis power " \
+		       "down message, IPMI error 0x%x\n", rv);
+	}
 }
  
  
  
  
  
  
@@ -561,39 +558,35 @@
  
  
 #ifdef CONFIG_PROC_FS
-/* displays properties to proc */
-static int proc_read_chassctrl(char *page, char **start, off_t off, int count,
-			       int *eof, void *data)
-{
-	return sprintf(page, "%d\t[ 0=powerdown 2=powercycle ]\n",
-			poweroff_control);
-}
+#include <linux/sysctl.h>
  
-/* process property writes from proc */
-static int proc_write_chassctrl(struct file *file, const char *buffer,
-			        unsigned long count, void *data)
-{
-	int          rv = count;
-	unsigned int newval = 0;
+static ctl_table ipmi_table[] = {
+	{ .ctl_name	= DEV_IPMI_POWEROFF_POWERCYCLE,
+	  .procname	= "poweroff_powercycle",
+	  .data		= &poweroff_powercycle,
+	  .maxlen	= sizeof(poweroff_powercycle),
+	  .mode		= 0644,
+	  .proc_handler	= &proc_dointvec },
+	{ }
+};
  
-	sscanf(buffer, "%d", &newval);
-	switch (newval) {
-		case IPMI_CHASSIS_POWER_CYCLE:
-			printk(KERN_INFO PFX "power cycle is now enabled\n");
-			poweroff_control = newval;
-			break;
+static ctl_table ipmi_dir_table[] = {
+	{ .ctl_name	= DEV_IPMI,
+	  .procname	= "ipmi",
+	  .mode		= 0555,
+	  .child	= ipmi_table },
+	{ }
+};
  
-		case IPMI_CHASSIS_POWER_DOWN:
-			poweroff_control = IPMI_CHASSIS_POWER_DOWN;
-			break;
+static ctl_table ipmi_root_table[] = {
+	{ .ctl_name	= CTL_DEV,
+	  .procname	= "dev",
+	  .mode		= 0555,
+	  .child	= ipmi_dir_table },
+	{ }
+};
  
-		default:
-			rv = -EINVAL;
-			break;
-	}
-
-	return rv;
-}
+static struct ctl_table_header *ipmi_table_header;
 #endif /* CONFIG_PROC_FS */
  
 /*
  
  
  
  
  
  
  
@@ -601,41 +594,32 @@
  */
 static int ipmi_poweroff_init (void)
 {
-	int                   rv;
-	struct proc_dir_entry *file;
+	int rv;
  
 	printk ("Copyright (C) 2004 MontaVista Software -"
 		" IPMI Powerdown via sys_reboot.\n");
  
-	switch (poweroff_control) {
-		case IPMI_CHASSIS_POWER_CYCLE:
-			printk(KERN_INFO PFX "Power cycle is enabled.\n");
-			break;
+	if (poweroff_powercycle)
+		printk(KERN_INFO PFX "Power cycle is enabled.\n");
  
-		case IPMI_CHASSIS_POWER_DOWN:
-		default:
-			poweroff_control = IPMI_CHASSIS_POWER_DOWN;
-			break;
+#ifdef CONFIG_PROC_FS
+	ipmi_table_header = register_sysctl_table(ipmi_root_table, 1);
+	if (!ipmi_table_header) {
+		printk(KERN_ERR PFX "Unable to register powercycle sysctl\n");
+		rv = -ENOMEM;
+		goto out_err;
 	}
+#endif
  
+#ifdef CONFIG_PROC_FS
 	rv = ipmi_smi_watcher_register(&smi_watcher);
+#endif
 	if (rv) {
+		unregister_sysctl_table(ipmi_table_header);
 		printk(KERN_ERR PFX "Unable to register SMI watcher: %d\n", rv);
 		goto out_err;
 	}
  
-#ifdef CONFIG_PROC_FS
-	file = create_proc_entry("poweroff_control", 0, proc_ipmi_root);
-	if (!file) {
-		printk(KERN_ERR PFX "Unable to create proc power control\n");
-	} else {
-		file->nlink = 1;
-		file->read_proc = proc_read_chassctrl;
-		file->write_proc = proc_write_chassctrl;
-		file->owner = THIS_MODULE;
-	}
-#endif
-
  out_err:
 	return rv;
 }
@@ -646,7 +630,7 @@
 	int rv;
  
 #ifdef CONFIG_PROC_FS
-	remove_proc_entry("poweroff_control", proc_ipmi_root);
+	unregister_sysctl_table(ipmi_table_header);
 #endif
  
 	ipmi_smi_watcher_unregister(&smi_watcher);
@@ -711,6 +711,7 @@
 	DEV_RAID=4,
 	DEV_MAC_HID=5,
 	DEV_SCSI=6,
+	DEV_IPMI=7,
 };
  
 /* /proc/sys/dev/cdrom */
@@ -774,6 +775,11 @@
 /* /proc/sys/dev/scsi */
 enum {
 	DEV_SCSI_LOGGING_LEVEL=1,
+};
+
+/* /proc/sys/dev/ipmi */
+enum {
+	DEV_IPMI_POWEROFF_POWERCYCLE=1,
 };
  
 /* /proc/sys/abi */
...	...	@@ -605,12 +605,13 @@
605	605	it will send the proper IPMI commands to do this. This is supported on
606	606	several platforms.
607	607
608		-There is a module parameter named "poweroff_control" that may either be zero
609		-(do a power down) or 2 (do a power cycle, power the system off, then power
610		-it on in a few seconds). Setting ipmi_poweroff.poweroff_control=x will do
611		-the same thing on the kernel command line. The parameter is also available
612		-via the proc filesystem in /proc/ipmi/poweroff_control. Note that if the
613		-system does not support power cycling, it will always to the power off.
	608	+There is a module parameter named "poweroff_powercycle" that may
	609	+either be zero (do a power down) or non-zero (do a power cycle, power
	610	+the system off, then power it on in a few seconds). Setting
	611	+ipmi_poweroff.poweroff_control=x will do the same thing on the kernel
	612	+command line. The parameter is also available via the proc filesystem
	613	+in /proc/sys/dev/ipmi/poweroff_powercycle. Note that if the system
	614	+does not support power cycling, it will always do the power off.
614	615
615	616	Note that if you have ACPI enabled, the system will prefer using ACPI to
616	617	power off.
...	...	@@ -52,11 +52,11 @@
52	52	#define IPMI_CHASSIS_POWER_CYCLE 0x02 /* power cycle */
53	53
54	54	/* the IPMI data command */
55		-static int poweroff_control = IPMI_CHASSIS_POWER_DOWN;
	55	+static int poweroff_powercycle;
56	56
57	57	/* parameter definition to allow user to flag power cycle */
58		-module_param(poweroff_control, int, IPMI_CHASSIS_POWER_DOWN);
59		-MODULE_PARM_DESC(poweroff_control, " Set to 2 to enable power cycle instead of power down. Power cycle is contingent on hardware support, otherwise it defaults back to power down.");
	58	+module_param(poweroff_powercycle, int, 0);
	59	+MODULE_PARM_DESC(poweroff_powercycles, " Set to non-zero to enable power cycle instead of power down. Power cycle is contingent on hardware support, otherwise it defaults back to power down.");
60	60
61	61	/* Stuff from the get device id command. */
62	62	static unsigned int mfg_id;
63	63
64	64
65	65
66	66
...	...	@@ -385,37 +385,34 @@
385	385
386	386	powercyclefailed:
387	387	printk(KERN_INFO PFX "Powering %s via IPMI chassis control command\n",
388		- ((poweroff_control != IPMI_CHASSIS_POWER_CYCLE) ? "down" : "cycle"));
	388	+ (poweroff_powercycle ? "cycle" : "down"));
389	389
390	390	/*
391	391	* Power down
392	392	*/
393	393	send_msg.netfn = IPMI_NETFN_CHASSIS_REQUEST;
394	394	send_msg.cmd = IPMI_CHASSIS_CONTROL_CMD;
395		- data[0] = poweroff_control;
	395	+ if (poweroff_powercycle)
	396	+ data[0] = IPMI_CHASSIS_POWER_CYCLE;
	397	+ else
	398	+ data[0] = IPMI_CHASSIS_POWER_DOWN;
396	399	send_msg.data = data;
397	400	send_msg.data_len = sizeof(data);
398	401	rv = ipmi_request_in_rc_mode(user,
399	402	(struct ipmi_addr *) &smi_addr,
400	403	&send_msg);
401	404	if (rv) {
402		- switch (poweroff_control) {
403		- case IPMI_CHASSIS_POWER_CYCLE:
404		- /* power cycle failed, default to power down */
405		- printk(KERN_ERR PFX "Unable to send chassis power " \
406		- "cycle message, IPMI error 0x%x\n", rv);
407		- poweroff_control = IPMI_CHASSIS_POWER_DOWN;
408		- goto powercyclefailed;
409		-
410		- case IPMI_CHASSIS_POWER_DOWN:
411		- default:
412		- printk(KERN_ERR PFX "Unable to send chassis power " \
413		- "down message, IPMI error 0x%x\n", rv);
414		- break;
	405	+ if (poweroff_powercycle) {
	406	+ /* power cycle failed, default to power down */
	407	+ printk(KERN_ERR PFX "Unable to send chassis power " \
	408	+ "cycle message, IPMI error 0x%x\n", rv);
	409	+ poweroff_powercycle = 0;
	410	+ goto powercyclefailed;
415	411	}
416		- }
417	412
418		- return;
	413	+ printk(KERN_ERR PFX "Unable to send chassis power " \
	414	+ "down message, IPMI error 0x%x\n", rv);
	415	+ }
419	416	}
420	417
421	418
422	419
423	420
424	421
425	422
...	...	@@ -561,39 +558,35 @@
561	558
562	559
563	560	#ifdef CONFIG_PROC_FS
564		-/* displays properties to proc */
565		-static int proc_read_chassctrl(char page, char *start, off_t off, int count,
566		- int eof, void data)
567		-{
568		- return sprintf(page, "%d\t[ 0=powerdown 2=powercycle ]\n",
569		- poweroff_control);
570		-}
	561	+#include <linux/sysctl.h>
571	562
572		-/* process property writes from proc */
573		-static int proc_write_chassctrl(struct file file, const char buffer,
574		- unsigned long count, void *data)
575		-{
576		- int rv = count;
577		- unsigned int newval = 0;
	563	+static ctl_table ipmi_table[] = {
	564	+ { .ctl_name = DEV_IPMI_POWEROFF_POWERCYCLE,
	565	+ .procname = "poweroff_powercycle",
	566	+ .data = &poweroff_powercycle,
	567	+ .maxlen = sizeof(poweroff_powercycle),
	568	+ .mode = 0644,
	569	+ .proc_handler = &proc_dointvec },
	570	+ { }
	571	+};
578	572
579		- sscanf(buffer, "%d", &newval);
580		- switch (newval) {
581		- case IPMI_CHASSIS_POWER_CYCLE:
582		- printk(KERN_INFO PFX "power cycle is now enabled\n");
583		- poweroff_control = newval;
584		- break;
	573	+static ctl_table ipmi_dir_table[] = {
	574	+ { .ctl_name = DEV_IPMI,
	575	+ .procname = "ipmi",
	576	+ .mode = 0555,
	577	+ .child = ipmi_table },
	578	+ { }
	579	+};
585	580
586		- case IPMI_CHASSIS_POWER_DOWN:
587		- poweroff_control = IPMI_CHASSIS_POWER_DOWN;
588		- break;
	581	+static ctl_table ipmi_root_table[] = {
	582	+ { .ctl_name = CTL_DEV,
	583	+ .procname = "dev",
	584	+ .mode = 0555,
	585	+ .child = ipmi_dir_table },
	586	+ { }
	587	+};
589	588
590		- default:
591		- rv = -EINVAL;
592		- break;
593		- }
594		-
595		- return rv;
596		-}
	589	+static struct ctl_table_header *ipmi_table_header;
597	590	#endif /* CONFIG_PROC_FS */
598	591
599	592	/*
600	593
601	594
602	595
603	596
604	597
605	598
606	599
...	...	@@ -601,41 +594,32 @@
601	594	*/
602	595	static int ipmi_poweroff_init (void)
603	596	{
604		- int rv;
605		- struct proc_dir_entry *file;
	597	+ int rv;
606	598
607	599	printk ("Copyright (C) 2004 MontaVista Software -"
608	600	" IPMI Powerdown via sys_reboot.\n");
609	601
610		- switch (poweroff_control) {
611		- case IPMI_CHASSIS_POWER_CYCLE:
612		- printk(KERN_INFO PFX "Power cycle is enabled.\n");
613		- break;
	602	+ if (poweroff_powercycle)
	603	+ printk(KERN_INFO PFX "Power cycle is enabled.\n");
614	604
615		- case IPMI_CHASSIS_POWER_DOWN:
616		- default:
617		- poweroff_control = IPMI_CHASSIS_POWER_DOWN;
618		- break;
	605	+#ifdef CONFIG_PROC_FS
	606	+ ipmi_table_header = register_sysctl_table(ipmi_root_table, 1);
	607	+ if (!ipmi_table_header) {
	608	+ printk(KERN_ERR PFX "Unable to register powercycle sysctl\n");
	609	+ rv = -ENOMEM;
	610	+ goto out_err;
619	611	}
	612	+#endif
620	613
	614	+#ifdef CONFIG_PROC_FS
621	615	rv = ipmi_smi_watcher_register(&smi_watcher);
	616	+#endif
622	617	if (rv) {
	618	+ unregister_sysctl_table(ipmi_table_header);
623	619	printk(KERN_ERR PFX "Unable to register SMI watcher: %d\n", rv);
624	620	goto out_err;
625	621	}
626	622
627		-#ifdef CONFIG_PROC_FS
628		- file = create_proc_entry("poweroff_control", 0, proc_ipmi_root);
629		- if (!file) {
630		- printk(KERN_ERR PFX "Unable to create proc power control\n");
631		- } else {
632		- file->nlink = 1;
633		- file->read_proc = proc_read_chassctrl;
634		- file->write_proc = proc_write_chassctrl;
635		- file->owner = THIS_MODULE;
636		- }
637		-#endif
638		-
639	623	out_err:
640	624	return rv;
641	625	}
...	...	@@ -646,7 +630,7 @@
646	630	int rv;
647	631
648	632	#ifdef CONFIG_PROC_FS
649		- remove_proc_entry("poweroff_control", proc_ipmi_root);
	633	+ unregister_sysctl_table(ipmi_table_header);
650	634	#endif
651	635
652	636	ipmi_smi_watcher_unregister(&smi_watcher);
...	...	@@ -711,6 +711,7 @@
711	711	DEV_RAID=4,
712	712	DEV_MAC_HID=5,
713	713	DEV_SCSI=6,
	714	+ DEV_IPMI=7,
714	715	};
715	716
716	717	/* /proc/sys/dev/cdrom */
...	...	@@ -774,6 +775,11 @@
774	775	/* /proc/sys/dev/scsi */
775	776	enum {
776	777	DEV_SCSI_LOGGING_LEVEL=1,
	778	+};
	779	+
	780	+/* /proc/sys/dev/ipmi */
	781	+enum {
	782	+ DEV_IPMI_POWEROFF_POWERCYCLE=1,
777	783	};
778	784
779	785	/* /proc/sys/abi */