[PATCH] MPBL0010 driver sysfs permissions wide open

[PATCH] MPBL0010 driver sysfs permissions wide open

[Mark Bellon]

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

The MPBL0010 Telco clock driver (drivers/char/tlclk.c) uses 0222 (anyone 
can write) permissions on its writable sysfs entries. IMHO this is a bit 
too wide open for proper security. The patch (against 2.6.16.1) alters 
the permissions to 0220 (owner and group can write).

Signed-off-by: Mark Bellon <mbellon@mvista.com>

mark



[-- Attachment #2: lkml-patch --]
[-- Type: text/plain, Size: 5144 bytes --]

diff -Naur linux-2.6.16.1-orig/drivers/char/tlclk.c linux-2.6.16.1/drivers/char/tlclk.c
--- linux-2.6.16.1-orig/drivers/char/tlclk.c	2006-03-27 23:49:02.000000000 -0700
+++ linux-2.6.16.1/drivers/char/tlclk.c	2006-04-04 12:17:14.000000000 -0700
@@ -327,7 +327,7 @@
 	return strnlen(buf, count);
 }
 
-static DEVICE_ATTR(received_ref_clk3a, S_IWUGO, NULL,
+static DEVICE_ATTR(received_ref_clk3a, (S_IWUSR|S_IWGRP), NULL,
 		store_received_ref_clk3a);
 
 
@@ -349,7 +349,7 @@
 	return strnlen(buf, count);
 }
 
-static DEVICE_ATTR(received_ref_clk3b, S_IWUGO, NULL,
+static DEVICE_ATTR(received_ref_clk3b, (S_IWUSR|S_IWGRP), NULL,
 		store_received_ref_clk3b);
 
 
@@ -371,7 +371,7 @@
 	return strnlen(buf, count);
 }
 
-static DEVICE_ATTR(enable_clk3b_output, S_IWUGO, NULL,
+static DEVICE_ATTR(enable_clk3b_output, (S_IWUSR|S_IWGRP), NULL,
 		store_enable_clk3b_output);
 
 static ssize_t store_enable_clk3a_output(struct device *d,
@@ -392,7 +392,7 @@
 	return strnlen(buf, count);
 }
 
-static DEVICE_ATTR(enable_clk3a_output, S_IWUGO, NULL,
+static DEVICE_ATTR(enable_clk3a_output, (S_IWUSR|S_IWGRP), NULL,
 		store_enable_clk3a_output);
 
 static ssize_t store_enable_clkb1_output(struct device *d,
@@ -413,7 +413,7 @@
 	return strnlen(buf, count);
 }
 
-static DEVICE_ATTR(enable_clkb1_output, S_IWUGO, NULL,
+static DEVICE_ATTR(enable_clkb1_output, (S_IWUSR|S_IWGRP), NULL,
 		store_enable_clkb1_output);
 
 
@@ -435,7 +435,7 @@
 	return strnlen(buf, count);
 }
 
-static DEVICE_ATTR(enable_clka1_output, S_IWUGO, NULL,
+static DEVICE_ATTR(enable_clka1_output, (S_IWUSR|S_IWGRP), NULL,
 		store_enable_clka1_output);
 
 static ssize_t store_enable_clkb0_output(struct device *d,
@@ -456,7 +456,7 @@
 	return strnlen(buf, count);
 }
 
-static DEVICE_ATTR(enable_clkb0_output, S_IWUGO, NULL,
+static DEVICE_ATTR(enable_clkb0_output, (S_IWUSR|S_IWGRP), NULL,
 		store_enable_clkb0_output);
 
 static ssize_t store_enable_clka0_output(struct device *d,
@@ -477,7 +477,7 @@
 	return strnlen(buf, count);
 }
 
-static DEVICE_ATTR(enable_clka0_output, S_IWUGO, NULL,
+static DEVICE_ATTR(enable_clka0_output, (S_IWUSR|S_IWGRP), NULL,
 		store_enable_clka0_output);
 
 static ssize_t store_select_amcb2_transmit_clock(struct device *d,
@@ -519,7 +519,7 @@
 	return strnlen(buf, count);
 }
 
-static DEVICE_ATTR(select_amcb2_transmit_clock, S_IWUGO, NULL,
+static DEVICE_ATTR(select_amcb2_transmit_clock, (S_IWUSR|S_IWGRP), NULL,
 	store_select_amcb2_transmit_clock);
 
 static ssize_t store_select_amcb1_transmit_clock(struct device *d,
@@ -560,7 +560,7 @@
 	return strnlen(buf, count);
 }
 
-static DEVICE_ATTR(select_amcb1_transmit_clock, S_IWUGO, NULL,
+static DEVICE_ATTR(select_amcb1_transmit_clock, (S_IWUSR|S_IWGRP), NULL,
 		store_select_amcb1_transmit_clock);
 
 static ssize_t store_select_redundant_clock(struct device *d,
@@ -581,7 +581,7 @@
 	return strnlen(buf, count);
 }
 
-static DEVICE_ATTR(select_redundant_clock, S_IWUGO, NULL,
+static DEVICE_ATTR(select_redundant_clock, (S_IWUSR|S_IWGRP), NULL,
 		store_select_redundant_clock);
 
 static ssize_t store_select_ref_frequency(struct device *d,
@@ -602,7 +602,7 @@
 	return strnlen(buf, count);
 }
 
-static DEVICE_ATTR(select_ref_frequency, S_IWUGO, NULL,
+static DEVICE_ATTR(select_ref_frequency, (S_IWUSR|S_IWGRP), NULL,
 		store_select_ref_frequency);
 
 static ssize_t store_filter_select(struct device *d,
@@ -623,7 +623,7 @@
 	return strnlen(buf, count);
 }
 
-static DEVICE_ATTR(filter_select, S_IWUGO, NULL, store_filter_select);
+static DEVICE_ATTR(filter_select, (S_IWUSR|S_IWGRP), NULL, store_filter_select);
 
 static ssize_t store_hardware_switching_mode(struct device *d,
 		 struct device_attribute *attr, const char *buf, size_t count)
@@ -643,7 +643,7 @@
 	return strnlen(buf, count);
 }
 
-static DEVICE_ATTR(hardware_switching_mode, S_IWUGO, NULL,
+static DEVICE_ATTR(hardware_switching_mode, (S_IWUSR|S_IWGRP), NULL,
 		store_hardware_switching_mode);
 
 static ssize_t store_hardware_switching(struct device *d,
@@ -664,7 +664,7 @@
 	return strnlen(buf, count);
 }
 
-static DEVICE_ATTR(hardware_switching, S_IWUGO, NULL,
+static DEVICE_ATTR(hardware_switching, (S_IWUSR|S_IWGRP), NULL,
 		store_hardware_switching);
 
 static ssize_t store_refalign (struct device *d,
@@ -684,7 +684,7 @@
 	return strnlen(buf, count);
 }
 
-static DEVICE_ATTR(refalign, S_IWUGO, NULL, store_refalign);
+static DEVICE_ATTR(refalign, (S_IWUSR|S_IWGRP), NULL, store_refalign);
 
 static ssize_t store_mode_select (struct device *d,
 		 struct device_attribute *attr, const char *buf, size_t count)
@@ -704,7 +704,7 @@
 	return strnlen(buf, count);
 }
 
-static DEVICE_ATTR(mode_select, S_IWUGO, NULL, store_mode_select);
+static DEVICE_ATTR(mode_select, (S_IWUSR|S_IWGRP), NULL, store_mode_select);
 
 static ssize_t store_reset (struct device *d,
 		 struct device_attribute *attr, const char *buf, size_t count)
@@ -724,7 +724,7 @@
 	return strnlen(buf, count);
 }
 
-static DEVICE_ATTR(reset, S_IWUGO, NULL, store_reset);
+static DEVICE_ATTR(reset, (S_IWUSR|S_IWGRP), NULL, store_reset);
 
 static struct attribute *tlclk_sysfs_entries[] = {
 	&dev_attr_current_ref.attr,

RE: [PATCH] MPBL0010 driver sysfs permissions wide open

[Gross, Mark]

ACK.

This looks good to me.  

The use case for this driver is to configure the fail over behavior of
the clock hardware.  That should be done by the more privileged users.

Thanks,

--mgross

>-----Original Message-----
>From: Mark Bellon [mailto:mbellon@mvista.com]
>Sent: Tuesday, April 04, 2006 12:32 PM
>To: linux-kernel@vger.kernel.org; Gross, Mark;
>sebastien.bouchard@ca.kontron.com
>Subject: [PATCH] MPBL0010 driver sysfs permissions wide open
>
>The MPBL0010 Telco clock driver (drivers/char/tlclk.c) uses 0222
(anyone
>can write) permissions on its writable sysfs entries. IMHO this is a
bit
>too wide open for proper security. The patch (against 2.6.16.1) alters
>the permissions to 0220 (owner and group can write).
>
>Signed-off-by: Mark Bellon <mbellon@mvista.com>
>
>mark
>