[PATCH 01/20] mach-omap2: mux: world-writable debugfs files

[PATCH 01/20] mach-omap2: mux: world-writable debugfs files

[Vasiliy Kulikov]

Do not create mux debugfs files as world-writable.

Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
---
 Cannot compile the driver, so it is not tested at all.

 arch/arm/mach-omap2/mux.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/mux.c b/arch/arm/mach-omap2/mux.c
index 98148b6..6c84659 100644
--- a/arch/arm/mach-omap2/mux.c
+++ b/arch/arm/mach-omap2/mux.c
@@ -605,7 +605,7 @@ static void __init omap_mux_dbg_create_entry(
 	list_for_each_entry(e, &partition->muxmodes, node) {
 		struct omap_mux *m = &e->mux;
 
-		(void)debugfs_create_file(m->muxnames[0], S_IWUGO, mux_dbg_dir,
+		(void)debugfs_create_file(m->muxnames[0], S_IWUSR, mux_dbg_dir,
 					  m, &omap_mux_dbg_signal_fops);
 	}
 }
-- 
1.7.0.4

Re: [PATCH 01/20] mach-omap2: mux: world-writable debugfs files

[Tony Lindgren]

* Vasiliy Kulikov <segoon@openwall.com> [110204 04:21]:
> Do not create mux debugfs files as world-writable.
> 
> Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
> ---
>  Cannot compile the driver, so it is not tested at all.

Thanks, will queue this as a fix for the -rc cycle.

Tony
 
>  arch/arm/mach-omap2/mux.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/mux.c b/arch/arm/mach-omap2/mux.c
> index 98148b6..6c84659 100644
> --- a/arch/arm/mach-omap2/mux.c
> +++ b/arch/arm/mach-omap2/mux.c
> @@ -605,7 +605,7 @@ static void __init omap_mux_dbg_create_entry(
>  	list_for_each_entry(e, &partition->muxmodes, node) {
>  		struct omap_mux *m = &e->mux;
>  
> -		(void)debugfs_create_file(m->muxnames[0], S_IWUGO, mux_dbg_dir,
> +		(void)debugfs_create_file(m->muxnames[0], S_IWUSR, mux_dbg_dir,
>  					  m, &omap_mux_dbg_signal_fops);
>  	}
>  }
> -- 
> 1.7.0.4
> 

[PATCH 02/20] mach-omap2: pm: world-writable debugfs timer files

[Vasiliy Kulikov]

Don't allow all users to change timer settings.

Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
---
 Cannot compile the driver, so it is not tested at all.

 arch/arm/mach-omap2/pm-debug.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-omap2/pm-debug.c b/arch/arm/mach-omap2/pm-debug.c
index 125f565..a5a83b3 100644
--- a/arch/arm/mach-omap2/pm-debug.c
+++ b/arch/arm/mach-omap2/pm-debug.c
@@ -637,14 +637,14 @@ static int __init pm_dbg_init(void)
 
 		}
 
-	(void) debugfs_create_file("enable_off_mode", S_IRUGO | S_IWUGO, d,
+	(void) debugfs_create_file("enable_off_mode", S_IRUGO | S_IWUSR, d,
 				   &enable_off_mode, &pm_dbg_option_fops);
-	(void) debugfs_create_file("sleep_while_idle", S_IRUGO | S_IWUGO, d,
+	(void) debugfs_create_file("sleep_while_idle", S_IRUGO | S_IWUSR, d,
 				   &sleep_while_idle, &pm_dbg_option_fops);
-	(void) debugfs_create_file("wakeup_timer_seconds", S_IRUGO | S_IWUGO, d,
+	(void) debugfs_create_file("wakeup_timer_seconds", S_IRUGO | S_IWUSR, d,
 				   &wakeup_timer_seconds, &pm_dbg_option_fops);
 	(void) debugfs_create_file("wakeup_timer_milliseconds",
-			S_IRUGO | S_IWUGO, d, &wakeup_timer_milliseconds,
+			S_IRUGO | S_IWUSR, d, &wakeup_timer_milliseconds,
 			&pm_dbg_option_fops);
 	pm_dbg_init_done = 1;
 
-- 
1.7.0.4

Re: [PATCH 02/20] mach-omap2: pm: world-writable debugfs timer files

[Tony Lindgren]

* Vasiliy Kulikov <segoon@openwall.com> [110204 04:21]:
> Don't allow all users to change timer settings.
> 
> Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
> ---
>  Cannot compile the driver, so it is not tested at all.

Taking this one too.

Tony

Re: [PATCH 02/20] mach-omap2: pm: world-writable debugfs timer files

[Kevin Hilman]

Vasiliy Kulikov <segoon@openwall.com> writes:

> Don't allow all users to change timer settings.
>
> Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
> ---
>  Cannot compile the driver, so it is not tested at all.

Acked-by: Kevin Hilman <khilman@ti.com>


>  arch/arm/mach-omap2/pm-debug.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm-debug.c b/arch/arm/mach-omap2/pm-debug.c
> index 125f565..a5a83b3 100644
> --- a/arch/arm/mach-omap2/pm-debug.c
> +++ b/arch/arm/mach-omap2/pm-debug.c
> @@ -637,14 +637,14 @@ static int __init pm_dbg_init(void)
>  
>  		}
>  
> -	(void) debugfs_create_file("enable_off_mode", S_IRUGO | S_IWUGO, d,
> +	(void) debugfs_create_file("enable_off_mode", S_IRUGO | S_IWUSR, d,
>  				   &enable_off_mode, &pm_dbg_option_fops);
> -	(void) debugfs_create_file("sleep_while_idle", S_IRUGO | S_IWUGO, d,
> +	(void) debugfs_create_file("sleep_while_idle", S_IRUGO | S_IWUSR, d,
>  				   &sleep_while_idle, &pm_dbg_option_fops);
> -	(void) debugfs_create_file("wakeup_timer_seconds", S_IRUGO | S_IWUGO, d,
> +	(void) debugfs_create_file("wakeup_timer_seconds", S_IRUGO | S_IWUSR, d,
>  				   &wakeup_timer_seconds, &pm_dbg_option_fops);
>  	(void) debugfs_create_file("wakeup_timer_milliseconds",
> -			S_IRUGO | S_IWUGO, d, &wakeup_timer_milliseconds,
> +			S_IRUGO | S_IWUSR, d, &wakeup_timer_milliseconds,
>  			&pm_dbg_option_fops);
>  	pm_dbg_init_done = 1;

[PATCH 03/20] mach-omap2: smartreflex: world-writable debugfs voltage files

[Vasiliy Kulikov]

Don't allow everybody to change voltage settings.

Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
---
 Cannot compile the driver, so it is not tested at all.

 arch/arm/mach-omap2/smartreflex.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/smartreflex.c b/arch/arm/mach-omap2/smartreflex.c
index c37e823..95ac336 100644
--- a/arch/arm/mach-omap2/smartreflex.c
+++ b/arch/arm/mach-omap2/smartreflex.c
@@ -900,7 +900,7 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 		return PTR_ERR(dbg_dir);
 	}
 
-	(void) debugfs_create_file("autocomp", S_IRUGO | S_IWUGO, dbg_dir,
+	(void) debugfs_create_file("autocomp", S_IRUGO | S_IWUSR, dbg_dir,
 				(void *)sr_info, &pm_sr_fops);
 	(void) debugfs_create_x32("errweight", S_IRUGO, dbg_dir,
 			&sr_info->err_weight);
@@ -939,7 +939,7 @@ static int __init omap_sr_probe(struct platform_device *pdev)
 		strcpy(name, "volt_");
 		sprintf(volt_name, "%d", volt_data[i].volt_nominal);
 		strcat(name, volt_name);
-		(void) debugfs_create_x32(name, S_IRUGO | S_IWUGO, nvalue_dir,
+		(void) debugfs_create_x32(name, S_IRUGO | S_IWUSR, nvalue_dir,
 				&(sr_info->nvalue_table[i].nvalue));
 	}
 
-- 
1.7.0.4

Re: [PATCH 03/20] mach-omap2: smartreflex: world-writable debugfs voltage files

[Tony Lindgren]

* Vasiliy Kulikov <segoon@openwall.com> [110204 04:22]:
> Don't allow everybody to change voltage settings.
> 
> Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
> ---
>  Cannot compile the driver, so it is not tested at all.

And this one.

Tony

Re: [PATCH 03/20] mach-omap2: smartreflex: world-writable debugfs voltage files

[Kevin Hilman]

Vasiliy Kulikov <segoon@openwall.com> writes:

> Don't allow everybody to change voltage settings.
>
> Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
> ---
>  Cannot compile the driver, so it is not tested at all.

Acked-by: Kevin Hilman <khilman@ti.com>


>  arch/arm/mach-omap2/smartreflex.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/smartreflex.c b/arch/arm/mach-omap2/smartreflex.c
> index c37e823..95ac336 100644
> --- a/arch/arm/mach-omap2/smartreflex.c
> +++ b/arch/arm/mach-omap2/smartreflex.c
> @@ -900,7 +900,7 @@ static int __init omap_sr_probe(struct platform_device *pdev)
>  		return PTR_ERR(dbg_dir);
>  	}
>  
> -	(void) debugfs_create_file("autocomp", S_IRUGO | S_IWUGO, dbg_dir,
> +	(void) debugfs_create_file("autocomp", S_IRUGO | S_IWUSR, dbg_dir,
>  				(void *)sr_info, &pm_sr_fops);
>  	(void) debugfs_create_x32("errweight", S_IRUGO, dbg_dir,
>  			&sr_info->err_weight);
> @@ -939,7 +939,7 @@ static int __init omap_sr_probe(struct platform_device *pdev)
>  		strcpy(name, "volt_");
>  		sprintf(volt_name, "%d", volt_data[i].volt_nominal);
>  		strcat(name, volt_name);
> -		(void) debugfs_create_x32(name, S_IRUGO | S_IWUGO, nvalue_dir,
> +		(void) debugfs_create_x32(name, S_IRUGO | S_IWUSR, nvalue_dir,
>  				&(sr_info->nvalue_table[i].nvalue));
>  	}

Re: [PATCH 03/20] mach-omap2: smartreflex: world-writable debugfs voltage files

[Menon, Nishanth]

On Sat, Feb 5, 2011 at 04:24, Kevin Hilman <khilman@ti.com> wrote:
> Vasiliy Kulikov <segoon@openwall.com> writes:
>
>> Don't allow everybody to change voltage settings.
>>
>> Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
>> ---
>>  Cannot compile the driver, so it is not tested at all.
>
> Acked-by: Kevin Hilman <khilman@ti.com>
Also fixes the checkpatch warning:
scripts/checkpatch.pl -f arch/arm/mach-omap2/smartreflex.c
WARNING: Exporting world writable files is usually an error. Consider
more restrictive permissions.
#903: FILE: arm/mach-omap2/smartreflex.c:903:
+	(void) debugfs_create_file("autocomp", S_IRUGO | S_IWUGO, dbg_dir,

total: 0 errors, 1 warnings, 1028 lines checked

Acked-by: Nishanth Menon <nm@ti.com>

Regards,
Nishanth Menon

[PATCH 04/20] mach-ux500: mbox-db5500: world-writable sysfs fifo file

[Vasiliy Kulikov]

Don't allow everybody to use a modem.

Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
---
 Cannot compile the driver, so it is not tested at all.

 arch/arm/mach-ux500/mbox-db5500.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-ux500/mbox-db5500.c b/arch/arm/mach-ux500/mbox-db5500.c
index cbf1571..402c00d 100644
--- a/arch/arm/mach-ux500/mbox-db5500.c
+++ b/arch/arm/mach-ux500/mbox-db5500.c
@@ -168,7 +168,7 @@ static ssize_t mbox_read_fifo(struct device *dev,
 	return sprintf(buf, "0x%X\n", mbox_value);
 }
 
-static DEVICE_ATTR(fifo, S_IWUGO | S_IRUGO, mbox_read_fifo, mbox_write_fifo);
+static DEVICE_ATTR(fifo, S_IWUSR | S_IRUGO, mbox_read_fifo, mbox_write_fifo);
 
 static int mbox_show(struct seq_file *s, void *data)
 {
-- 
1.7.0.4

[PATCH 05/20] leds: lp5521: world-writable sysfs engine* files

[Vasiliy Kulikov]

Don't allow everybody to change LED settings.

Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
---
 Compile tested only.

 drivers/leds/leds-lp5521.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/leds/leds-lp5521.c b/drivers/leds/leds-lp5521.c
index 80a3ae3..c0cff64 100644
--- a/drivers/leds/leds-lp5521.c
+++ b/drivers/leds/leds-lp5521.c
@@ -534,7 +534,7 @@ static ssize_t lp5521_selftest(struct device *dev,
 }
 
 /* led class device attributes */
-static DEVICE_ATTR(led_current, S_IRUGO | S_IWUGO, show_current, store_current);
+static DEVICE_ATTR(led_current, S_IRUGO | S_IWUSR, show_current, store_current);
 static DEVICE_ATTR(max_current, S_IRUGO , show_max_current, NULL);
 
 static struct attribute *lp5521_led_attributes[] = {
@@ -548,15 +548,15 @@ static struct attribute_group lp5521_led_attribute_group = {
 };
 
 /* device attributes */
-static DEVICE_ATTR(engine1_mode, S_IRUGO | S_IWUGO,
+static DEVICE_ATTR(engine1_mode, S_IRUGO | S_IWUSR,
 		   show_engine1_mode, store_engine1_mode);
-static DEVICE_ATTR(engine2_mode, S_IRUGO | S_IWUGO,
+static DEVICE_ATTR(engine2_mode, S_IRUGO | S_IWUSR,
 		   show_engine2_mode, store_engine2_mode);
-static DEVICE_ATTR(engine3_mode, S_IRUGO | S_IWUGO,
+static DEVICE_ATTR(engine3_mode, S_IRUGO | S_IWUSR,
 		   show_engine3_mode, store_engine3_mode);
-static DEVICE_ATTR(engine1_load, S_IWUGO, NULL, store_engine1_load);
-static DEVICE_ATTR(engine2_load, S_IWUGO, NULL, store_engine2_load);
-static DEVICE_ATTR(engine3_load, S_IWUGO, NULL, store_engine3_load);
+static DEVICE_ATTR(engine1_load, S_IWUSR, NULL, store_engine1_load);
+static DEVICE_ATTR(engine2_load, S_IWUSR, NULL, store_engine2_load);
+static DEVICE_ATTR(engine3_load, S_IWUSR, NULL, store_engine3_load);
 static DEVICE_ATTR(selftest, S_IRUGO, lp5521_selftest, NULL);
 
 static struct attribute *lp5521_attributes[] = {
-- 
1.7.0.4

[PATCH 06/20] leds: lp5523: world-writable engine* sysfs files

[Vasiliy Kulikov]

Don't allow everybody to change LED settings.

Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
---
 Compile tested only.

 drivers/leds/leds-lp5523.c |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/leds/leds-lp5523.c b/drivers/leds/leds-lp5523.c
index d0c4068..e19fed2 100644
--- a/drivers/leds/leds-lp5523.c
+++ b/drivers/leds/leds-lp5523.c
@@ -713,7 +713,7 @@ static ssize_t store_current(struct device *dev,
 }
 
 /* led class device attributes */
-static DEVICE_ATTR(led_current, S_IRUGO | S_IWUGO, show_current, store_current);
+static DEVICE_ATTR(led_current, S_IRUGO | S_IWUSR, show_current, store_current);
 static DEVICE_ATTR(max_current, S_IRUGO , show_max_current, NULL);
 
 static struct attribute *lp5523_led_attributes[] = {
@@ -727,21 +727,21 @@ static struct attribute_group lp5523_led_attribute_group = {
 };
 
 /* device attributes */
-static DEVICE_ATTR(engine1_mode, S_IRUGO | S_IWUGO,
+static DEVICE_ATTR(engine1_mode, S_IRUGO | S_IWUSR,
 		   show_engine1_mode, store_engine1_mode);
-static DEVICE_ATTR(engine2_mode, S_IRUGO | S_IWUGO,
+static DEVICE_ATTR(engine2_mode, S_IRUGO | S_IWUSR,
 		   show_engine2_mode, store_engine2_mode);
-static DEVICE_ATTR(engine3_mode, S_IRUGO | S_IWUGO,
+static DEVICE_ATTR(engine3_mode, S_IRUGO | S_IWUSR,
 		   show_engine3_mode, store_engine3_mode);
-static DEVICE_ATTR(engine1_leds, S_IRUGO | S_IWUGO,
+static DEVICE_ATTR(engine1_leds, S_IRUGO | S_IWUSR,
 		   show_engine1_leds, store_engine1_leds);
-static DEVICE_ATTR(engine2_leds, S_IRUGO | S_IWUGO,
+static DEVICE_ATTR(engine2_leds, S_IRUGO | S_IWUSR,
 		   show_engine2_leds, store_engine2_leds);
-static DEVICE_ATTR(engine3_leds, S_IRUGO | S_IWUGO,
+static DEVICE_ATTR(engine3_leds, S_IRUGO | S_IWUSR,
 		   show_engine3_leds, store_engine3_leds);
-static DEVICE_ATTR(engine1_load, S_IWUGO, NULL, store_engine1_load);
-static DEVICE_ATTR(engine2_load, S_IWUGO, NULL, store_engine2_load);
-static DEVICE_ATTR(engine3_load, S_IWUGO, NULL, store_engine3_load);
+static DEVICE_ATTR(engine1_load, S_IWUSR, NULL, store_engine1_load);
+static DEVICE_ATTR(engine2_load, S_IWUSR, NULL, store_engine2_load);
+static DEVICE_ATTR(engine3_load, S_IWUSR, NULL, store_engine3_load);
 static DEVICE_ATTR(selftest, S_IRUGO, lp5523_selftest, NULL);
 
 static struct attribute *lp5523_attributes[] = {
-- 
1.7.0.4

[PATCH 07/20] video: sn9c102: world-wirtable sysfs files

[Vasiliy Kulikov]

Don't allow everybody to change video settings.

Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
---
 Compile tested only.

 drivers/media/video/sn9c102/sn9c102_core.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/video/sn9c102/sn9c102_core.c b/drivers/media/video/sn9c102/sn9c102_core.c
index 84984f6..ce56a1c 100644
--- a/drivers/media/video/sn9c102/sn9c102_core.c
+++ b/drivers/media/video/sn9c102/sn9c102_core.c
@@ -1430,9 +1430,9 @@ static DEVICE_ATTR(i2c_reg, S_IRUGO | S_IWUSR,
 		   sn9c102_show_i2c_reg, sn9c102_store_i2c_reg);
 static DEVICE_ATTR(i2c_val, S_IRUGO | S_IWUSR,
 		   sn9c102_show_i2c_val, sn9c102_store_i2c_val);
-static DEVICE_ATTR(green, S_IWUGO, NULL, sn9c102_store_green);
-static DEVICE_ATTR(blue, S_IWUGO, NULL, sn9c102_store_blue);
-static DEVICE_ATTR(red, S_IWUGO, NULL, sn9c102_store_red);
+static DEVICE_ATTR(green, S_IWUSR, NULL, sn9c102_store_green);
+static DEVICE_ATTR(blue, S_IWUSR, NULL, sn9c102_store_blue);
+static DEVICE_ATTR(red, S_IWUSR, NULL, sn9c102_store_red);
 static DEVICE_ATTR(frame_header, S_IRUGO, sn9c102_show_frame_header, NULL);
 
 
-- 
1.7.0.4

Re: [PATCH 07/20] video: sn9c102: world-wirtable sysfs files

[Mauro Carvalho Chehab]

Em 04-02-2011 10:23, Vasiliy Kulikov escreveu:
> Don't allow everybody to change video settings.
> 
> Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
> ---
>  Compile tested only.
> 
>  drivers/media/video/sn9c102/sn9c102_core.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/video/sn9c102/sn9c102_core.c b/drivers/media/video/sn9c102/sn9c102_core.c
> index 84984f6..ce56a1c 100644
> --- a/drivers/media/video/sn9c102/sn9c102_core.c
> +++ b/drivers/media/video/sn9c102/sn9c102_core.c
> @@ -1430,9 +1430,9 @@ static DEVICE_ATTR(i2c_reg, S_IRUGO | S_IWUSR,
>  		   sn9c102_show_i2c_reg, sn9c102_store_i2c_reg);
>  static DEVICE_ATTR(i2c_val, S_IRUGO | S_IWUSR,
>  		   sn9c102_show_i2c_val, sn9c102_store_i2c_val);
> -static DEVICE_ATTR(green, S_IWUGO, NULL, sn9c102_store_green);
> -static DEVICE_ATTR(blue, S_IWUGO, NULL, sn9c102_store_blue);
> -static DEVICE_ATTR(red, S_IWUGO, NULL, sn9c102_store_red);
> +static DEVICE_ATTR(green, S_IWUSR, NULL, sn9c102_store_green);
> +static DEVICE_ATTR(blue, S_IWUSR, NULL, sn9c102_store_blue);
> +static DEVICE_ATTR(red, S_IWUSR, NULL, sn9c102_store_red);
>  static DEVICE_ATTR(frame_header, S_IRUGO, sn9c102_show_frame_header, NULL);

Acked-by: Mauro Carvalho Chehab <mchehab@redhat.com>

Re: [PATCH 07/20] video: sn9c102: world-wirtable sysfs files

[Luca Risolia]

Thanks.

Acked-by: Luca Risolia <luca.risolia@studio.unibo.it>

Vasiliy Kulikov ha scritto:
> Don't allow everybody to change video settings.
> 
> Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
> ---
>  Compile tested only.
> 
>  drivers/media/video/sn9c102/sn9c102_core.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/video/sn9c102/sn9c102_core.c b/drivers/media/video/sn9c102/sn9c102_core.c
> index 84984f6..ce56a1c 100644
> --- a/drivers/media/video/sn9c102/sn9c102_core.c
> +++ b/drivers/media/video/sn9c102/sn9c102_core.c
> @@ -1430,9 +1430,9 @@ static DEVICE_ATTR(i2c_reg, S_IRUGO | S_IWUSR,
>  		   sn9c102_show_i2c_reg, sn9c102_store_i2c_reg);
>  static DEVICE_ATTR(i2c_val, S_IRUGO | S_IWUSR,
>  		   sn9c102_show_i2c_val, sn9c102_store_i2c_val);
> -static DEVICE_ATTR(green, S_IWUGO, NULL, sn9c102_store_green);
> -static DEVICE_ATTR(blue, S_IWUGO, NULL, sn9c102_store_blue);
> -static DEVICE_ATTR(red, S_IWUGO, NULL, sn9c102_store_red);
> +static DEVICE_ATTR(green, S_IWUSR, NULL, sn9c102_store_green);
> +static DEVICE_ATTR(blue, S_IWUSR, NULL, sn9c102_store_blue);
> +static DEVICE_ATTR(red, S_IWUSR, NULL, sn9c102_store_red);
>  static DEVICE_ATTR(frame_header, S_IRUGO, sn9c102_show_frame_header, NULL);
>  
>  

[PATCH 08/20] mfd: ab3100: world-writable debugfs *_priv files

[Vasiliy Kulikov]

Don't allow everybody to change device hardware registers.

Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
---
 Compile tested only.

 drivers/mfd/ab3100-core.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/ab3100-core.c b/drivers/mfd/ab3100-core.c
index 4193af5..1707d22 100644
--- a/drivers/mfd/ab3100-core.c
+++ b/drivers/mfd/ab3100-core.c
@@ -613,7 +613,7 @@ static void ab3100_setup_debugfs(struct ab3100 *ab3100)
 	ab3100_get_priv.ab3100 = ab3100;
 	ab3100_get_priv.mode = false;
 	ab3100_get_reg_file = debugfs_create_file("get_reg",
-				S_IWUGO, ab3100_dir, &ab3100_get_priv,
+				S_IWUSR, ab3100_dir, &ab3100_get_priv,
 				&ab3100_get_set_reg_fops);
 	if (!ab3100_get_reg_file) {
 		err = -ENOMEM;
@@ -623,7 +623,7 @@ static void ab3100_setup_debugfs(struct ab3100 *ab3100)
 	ab3100_set_priv.ab3100 = ab3100;
 	ab3100_set_priv.mode = true;
 	ab3100_set_reg_file = debugfs_create_file("set_reg",
-				S_IWUGO, ab3100_dir, &ab3100_set_priv,
+				S_IWUSR, ab3100_dir, &ab3100_set_priv,
 				&ab3100_get_set_reg_fops);
 	if (!ab3100_set_reg_file) {
 		err = -ENOMEM;
-- 
1.7.0.4

Re: [PATCH 08/20] mfd: ab3100: world-writable debugfs *_priv files

[Vasiliy Kulikov]

2011/2/4 Vasiliy Kulikov <segoon@openwall.com>:
> Don't allow everybody to change device hardware registers.

Ping.

> Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
> ---
>  Compile tested only.
>
>  drivers/mfd/ab3100-core.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mfd/ab3100-core.c b/drivers/mfd/ab3100-core.c
> index 4193af5..1707d22 100644
> --- a/drivers/mfd/ab3100-core.c
> +++ b/drivers/mfd/ab3100-core.c
> @@ -613,7 +613,7 @@ static void ab3100_setup_debugfs(struct ab3100 *ab3100)
>        ab3100_get_priv.ab3100 = ab3100;
>        ab3100_get_priv.mode = false;
>        ab3100_get_reg_file = debugfs_create_file("get_reg",
> -                               S_IWUGO, ab3100_dir, &ab3100_get_priv,
> +                               S_IWUSR, ab3100_dir, &ab3100_get_priv,
>                                &ab3100_get_set_reg_fops);
>        if (!ab3100_get_reg_file) {
>                err = -ENOMEM;
> @@ -623,7 +623,7 @@ static void ab3100_setup_debugfs(struct ab3100 *ab3100)
>        ab3100_set_priv.ab3100 = ab3100;
>        ab3100_set_priv.mode = true;
>        ab3100_set_reg_file = debugfs_create_file("set_reg",
> -                               S_IWUGO, ab3100_dir, &ab3100_set_priv,
> +                               S_IWUSR, ab3100_dir, &ab3100_set_priv,
>                                &ab3100_get_set_reg_fops);
>        if (!ab3100_set_reg_file) {
>                err = -ENOMEM;
> --
> 1.7.0.4
>
>

[PATCH 09/20] mfd: ab3500: world-writable debugfs register-* files

[Vasiliy Kulikov]

Don't allow everybody to interact with hardware registers.

Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
---
 Compile tested only.

 drivers/mfd/ab3550-core.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mfd/ab3550-core.c b/drivers/mfd/ab3550-core.c
index 5fbca34..681984d 100644
--- a/drivers/mfd/ab3550-core.c
+++ b/drivers/mfd/ab3550-core.c
@@ -1053,17 +1053,17 @@ static inline void ab3550_setup_debugfs(struct ab3550 *ab)
 		goto exit_destroy_dir;
 
 	ab3550_bank_file = debugfs_create_file("register-bank",
-		(S_IRUGO | S_IWUGO), ab3550_dir, ab, &ab3550_bank_fops);
+		(S_IRUGO | S_IWUSR), ab3550_dir, ab, &ab3550_bank_fops);
 	if (!ab3550_bank_file)
 		goto exit_destroy_reg;
 
 	ab3550_address_file = debugfs_create_file("register-address",
-		(S_IRUGO | S_IWUGO), ab3550_dir, ab, &ab3550_address_fops);
+		(S_IRUGO | S_IWUSR), ab3550_dir, ab, &ab3550_address_fops);
 	if (!ab3550_address_file)
 		goto exit_destroy_bank;
 
 	ab3550_val_file = debugfs_create_file("register-value",
-		(S_IRUGO | S_IWUGO), ab3550_dir, ab, &ab3550_val_fops);
+		(S_IRUGO | S_IWUSR), ab3550_dir, ab, &ab3550_val_fops);
 	if (!ab3550_val_file)
 		goto exit_destroy_address;
 
-- 
1.7.0.4

[PATCH 10/20] mfd: ab8500: world-writable debugfs register-* files

[Vasiliy Kulikov]

Don't allow everybody to interact with hardware registers.

Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
---
 Compile tested only.

 drivers/mfd/ab8500-debugfs.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mfd/ab8500-debugfs.c b/drivers/mfd/ab8500-debugfs.c
index 3c1541a..64748e4 100644
--- a/drivers/mfd/ab8500-debugfs.c
+++ b/drivers/mfd/ab8500-debugfs.c
@@ -585,18 +585,18 @@ static int __devinit ab8500_debug_probe(struct platform_device *plf)
 		goto exit_destroy_dir;
 
 	ab8500_bank_file = debugfs_create_file("register-bank",
-		(S_IRUGO | S_IWUGO), ab8500_dir, &plf->dev, &ab8500_bank_fops);
+		(S_IRUGO | S_IWUSR), ab8500_dir, &plf->dev, &ab8500_bank_fops);
 	if (!ab8500_bank_file)
 		goto exit_destroy_reg;
 
 	ab8500_address_file = debugfs_create_file("register-address",
-		(S_IRUGO | S_IWUGO), ab8500_dir, &plf->dev,
+		(S_IRUGO | S_IWUSR), ab8500_dir, &plf->dev,
 		&ab8500_address_fops);
 	if (!ab8500_address_file)
 		goto exit_destroy_bank;
 
 	ab8500_val_file = debugfs_create_file("register-value",
-		(S_IRUGO | S_IWUGO), ab8500_dir, &plf->dev, &ab8500_val_fops);
+		(S_IRUGO | S_IWUSR), ab8500_dir, &plf->dev, &ab8500_val_fops);
 	if (!ab8500_val_file)
 		goto exit_destroy_address;
 
-- 
1.7.0.4

[PATCH 11/20] misc: ep93xx_pwm: world-writable sysfs files

[Vasiliy Kulikov]

Don't allow everybody to change device settings.

Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
---
 Cannot compile the driver, so it is not tested at all.

 drivers/misc/ep93xx_pwm.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/misc/ep93xx_pwm.c b/drivers/misc/ep93xx_pwm.c
index 46b3439..16d7179 100644
--- a/drivers/misc/ep93xx_pwm.c
+++ b/drivers/misc/ep93xx_pwm.c
@@ -249,11 +249,11 @@ static ssize_t ep93xx_pwm_set_invert(struct device *dev,
 
 static DEVICE_ATTR(min_freq, S_IRUGO, ep93xx_pwm_get_min_freq, NULL);
 static DEVICE_ATTR(max_freq, S_IRUGO, ep93xx_pwm_get_max_freq, NULL);
-static DEVICE_ATTR(freq, S_IWUGO | S_IRUGO,
+static DEVICE_ATTR(freq, S_IWUSR | S_IRUGO,
 		   ep93xx_pwm_get_freq, ep93xx_pwm_set_freq);
-static DEVICE_ATTR(duty_percent, S_IWUGO | S_IRUGO,
+static DEVICE_ATTR(duty_percent, S_IWUSR | S_IRUGO,
 		   ep93xx_pwm_get_duty_percent, ep93xx_pwm_set_duty_percent);
-static DEVICE_ATTR(invert, S_IWUGO | S_IRUGO,
+static DEVICE_ATTR(invert, S_IWUSR | S_IRUGO,
 		   ep93xx_pwm_get_invert, ep93xx_pwm_set_invert);
 
 static struct attribute *ep93xx_pwm_attrs[] = {
-- 
1.7.0.4

[PATCH 12/20] net: can: at91_can: world-writable sysfs files

[Vasiliy Kulikov]

Don't allow everybody to write to mb0_id file.

Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
---
 Cannot compile the driver, so it is not tested at all.

 drivers/net/can/at91_can.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c
index 2532b96..57d2ffb 100644
--- a/drivers/net/can/at91_can.c
+++ b/drivers/net/can/at91_can.c
@@ -1109,7 +1109,7 @@ static ssize_t at91_sysfs_set_mb0_id(struct device *dev,
 	return ret;
 }
 
-static DEVICE_ATTR(mb0_id, S_IWUGO | S_IRUGO,
+static DEVICE_ATTR(mb0_id, S_IWUSR | S_IRUGO,
 	at91_sysfs_show_mb0_id, at91_sysfs_set_mb0_id);
 
 static struct attribute *at91_sysfs_attrs[] = {
-- 
1.7.0.4

Re: [PATCH 12/20] net: can: at91_can: world-writable sysfs files

[Kurt Van Dijck]

On Fri, Feb 04, 2011 at 03:23:50PM +0300, Vasiliy Kulikov wrote:
> Don't allow everybody to write to mb0_id file.
> 
very well!

Acked-by: Kurt Van Dijck <kurt.van.dijck@eia.be>

Re: [PATCH 12/20] net: can: at91_can: world-writable sysfs files

[David Miller]

From: Kurt Van Dijck <kurt.van.dijck@eia.be>
Date: Fri, 4 Feb 2011 13:42:33 +0100

> On Fri, Feb 04, 2011 at 03:23:50PM +0300, Vasiliy Kulikov wrote:
>> Don't allow everybody to write to mb0_id file.
>> 
> very well!
> 
> Acked-by: Kurt Van Dijck <kurt.van.dijck@eia.be>

Applied.

About bittiming calculation result

[Tomoya MORINAGA]

Hi,

I have a question for bittiming-value calculated by Can-core.

In case setting like below,
 - ip link set can0 type can bitrate 800000
 - clock=50MHz
 - Use pch_can

Can-core calculates like below
brp=21
seg1=1
seg2=1
sjw=1
prop_seg=0

Is "prop_seg=0" true ?
seg1/seg2/sjw/prop_seg must be more than 1 ?

Also I can see the following kernel error log.
bitrate error 0.7%

Thanks,
-----------------------------------------
Tomoya MORINAGA
OKI SEMICONDUCTOR CO., LTD.

Re: About bittiming calculation result

[Wolfgang Grandegger]

Hi Tomoya,

On 02/07/2011 12:38 PM, Tomoya MORINAGA wrote:
> Hi,
> 
> I have a question for bittiming-value calculated by Can-core.
> 
> In case setting like below,
>  - ip link set can0 type can bitrate 800000
>  - clock=50MHz
>  - Use pch_can
> 
> Can-core calculates like below
> brp=21
> seg1=1
> seg2=1
> sjw=1
> prop_seg=0
> 
> Is "prop_seg=0" true ?

Well, only prop_seg+phase_seg=tseg1 is relevant and the pch_can driver
sets the allowed minimum "tseg1_min1" currently to 1:

static struct can_bittiming_const pch_can_bittiming_const = {
        .name = KBUILD_MODNAME,
        .tseg1_min = 1,
        .tseg1_max = 16,
        .tseg2_min = 1,
        .tseg2_max = 8,
        .sjw_max = 4,
        .brp_min = 1,
        .brp_max = 1024, /* 6bit + extended 4bit */
        .brp_inc = 1,
};

> seg1/seg2/sjw/prop_seg must be more than 1 ?

Then "tseg1_min" should be set to *2*.

> Also I can see the following kernel error log.
> bitrate error 0.7%

A clock frequency of 50 MHz is sub-optimal for CAN and some
bit-rates cannot be reproduced properly. Here is the output of
the can-utils program "can-calc-bit-timing" (with an entry for
the pch-can added):

$ ./can-calc-bit-timing pch-can
Bit timing parameters for pch-can with 50.000000 MHz ref clock
nominal                                 real Bitrt   nom  real SampP
Bitrate TQ[ns] PrS PhS1 PhS2 SJW BRP Bitrate Error SampP SampP Error CNF1 CNF2 CNF3
1000000    100   3    3    3   1   5 1000000  0.0% 75.0% 70.0%  6.7% 0x05 0x92 0x02
 800000    420   0    1    1   1  21  793650  0.8% 80.0% 66.6% 16.8% 0x15 0xff 0x00
 500000    100   8    8    3   1   5  500000  0.0% 87.5% 85.0%  2.9% 0x05 0xbf 0x02
 250000    500   3    3    1   1  25  250000  0.0% 87.5% 87.5%  0.0% 0x19 0x92 0x00
 125000    500   6    7    2   1  25  125000  0.0% 87.5% 87.5%  0.0% 0x19 0xb5 0x01
 100000    500   8    8    3   1  25  100000  0.0% 87.5% 85.0%  2.9% 0x19 0xbf 0x02
  50000   2500   3    3    1   1 125   50000  0.0% 87.5% 87.5%  0.0% 0x7d 0x92 0x00
  20000   2500   8    8    3   1 125   20000  0.0% 87.5% 85.0%  2.9% 0x7d 0xbf 0x02
  10000  12500   3    3    1   1 625   10000  0.0% 87.5% 87.5%  0.0% 0x71 0x92 0x00

As you can see, especially 800000 gives rather bad results.

Wolfgang.

Re: About bittiming calculation result

[Wolfgang Grandegger]

On 02/07/2011 01:00 PM, Wolfgang Grandegger wrote:
> Hi Tomoya,
> 
> On 02/07/2011 12:38 PM, Tomoya MORINAGA wrote:
>> Hi,
>>
>> I have a question for bittiming-value calculated by Can-core.
>>
>> In case setting like below,
>>  - ip link set can0 type can bitrate 800000
>>  - clock=50MHz
>>  - Use pch_can
>>
>> Can-core calculates like below
>> brp=21
>> seg1=1
>> seg2=1
>> sjw=1
>> prop_seg=0
>>
>> Is "prop_seg=0" true ?
> 
> Well, only prop_seg+phase_seg=tseg1 is relevant and the pch_can driver
> sets the allowed minimum "tseg1_min1" currently to 1:
> 
> static struct can_bittiming_const pch_can_bittiming_const = {
>         .name = KBUILD_MODNAME,
>         .tseg1_min = 1,
>         .tseg1_max = 16,
>         .tseg2_min = 1,
>         .tseg2_max = 8,
>         .sjw_max = 4,
>         .brp_min = 1,
>         .brp_max = 1024, /* 6bit + extended 4bit */
>         .brp_inc = 1,
> };
> 
>> seg1/seg2/sjw/prop_seg must be more than 1 ?
> 
> Then "tseg1_min" should be set to *2*.
> 
>> Also I can see the following kernel error log.
>> bitrate error 0.7%
> 
> A clock frequency of 50 MHz is sub-optimal for CAN and some
> bit-rates cannot be reproduced properly. Here is the output of
> the can-utils program "can-calc-bit-timing" (with an entry for
> the pch-can added):
> 
> $ ./can-calc-bit-timing pch-can
> Bit timing parameters for pch-can with 50.000000 MHz ref clock
> nominal                                 real Bitrt   nom  real SampP
> Bitrate TQ[ns] PrS PhS1 PhS2 SJW BRP Bitrate Error SampP SampP Error CNF1 CNF2 CNF3
> 1000000    100   3    3    3   1   5 1000000  0.0% 75.0% 70.0%  6.7% 0x05 0x92 0x02
>  800000    420   0    1    1   1  21  793650  0.8% 80.0% 66.6% 16.8% 0x15 0xff 0x00
>  500000    100   8    8    3   1   5  500000  0.0% 87.5% 85.0%  2.9% 0x05 0xbf 0x02
>  250000    500   3    3    1   1  25  250000  0.0% 87.5% 87.5%  0.0% 0x19 0x92 0x00
>  125000    500   6    7    2   1  25  125000  0.0% 87.5% 87.5%  0.0% 0x19 0xb5 0x01
>  100000    500   8    8    3   1  25  100000  0.0% 87.5% 85.0%  2.9% 0x19 0xbf 0x02
>   50000   2500   3    3    1   1 125   50000  0.0% 87.5% 87.5%  0.0% 0x7d 0x92 0x00
>   20000   2500   8    8    3   1 125   20000  0.0% 87.5% 85.0%  2.9% 0x7d 0xbf 0x02
>   10000  12500   3    3    1   1 625   10000  0.0% 87.5% 87.5%  0.0% 0x71 0x92 0x00
> 
> As you can see, especially 800000 gives rather bad results.

BTW, it's always possible to specify optimized bit-timing parameters
directly, e.g. the following seem better:

   800000     60  12    4    4   4   3  793650  0.8% 80.0% 81.0%  1.2%

You could set these with:

  $ ip link set can0 type can \
    tq 60 prop-seg 12 phase-seg1 4 phase-seg2 4 sjw 4

Wolfgang.

RE: About bittiming calculation result

[Tomoya MORINAGA]

On Tuesday, February 08, 2011 12:53 AM,  Wolfgang Grandegger wrote:

> BTW, it's always possible to specify optimized bit-timing 
> parameters directly, e.g. the following seem better:
> 
>    800000     60  12    4    4   4   3  793650  0.8% 80.0% 81.0%  1.2%
> 
> You could set these with:
> 
>   $ ip link set can0 type can \
>     tq 60 prop-seg 12 phase-seg1 4 phase-seg2 4 sjw 4

I can confirm 800K comms works well using the above.

I wish Can-core could calculate like above.

>> seg1/seg2/sjw/prop_seg must be more than 1 ?
BTW, according to EG20T PCH data sheet, 
CAN bit-timing parameters(BRP, Prop_Seg, Phase_Seg1, Phase_Seg2, SJW) must not be set 0.

Thanks,
-----------------------------------------
Tomoya MORINAGA
OKI SEMICONDUCTOR CO., LTD.


> -----Original Message-----
> From: Wolfgang Grandegger [mailto:wg@grandegger.com] 
> Sent: Tuesday, February 08, 2011 12:53 AM
> To: Tomoya MORINAGA
> Cc: socketcan-core@lists.berlios.de; netdev@vger.kernel.org; 
> linux-kernel@vger.kernel.org
> Subject: Re: About bittiming calculation result
> 
> On 02/07/2011 01:00 PM, Wolfgang Grandegger wrote:
> > Hi Tomoya,
> > 
> > On 02/07/2011 12:38 PM, Tomoya MORINAGA wrote:
> >> Hi,
> >>
> >> I have a question for bittiming-value calculated by Can-core.
> >>
> >> In case setting like below,
> >>  - ip link set can0 type can bitrate 800000
> >>  - clock=50MHz
> >>  - Use pch_can
> >>
> >> Can-core calculates like below
> >> brp=21
> >> seg1=1
> >> seg2=1
> >> sjw=1
> >> prop_seg=0
> >>
> >> Is "prop_seg=0" true ?
> > 
> > Well, only prop_seg+phase_seg=tseg1 is relevant and the 
> pch_can driver 
> > sets the allowed minimum "tseg1_min1" currently to 1:
> > 
> > static struct can_bittiming_const pch_can_bittiming_const = {
> >         .name = KBUILD_MODNAME,
> >         .tseg1_min = 1,
> >         .tseg1_max = 16,
> >         .tseg2_min = 1,
> >         .tseg2_max = 8,
> >         .sjw_max = 4,
> >         .brp_min = 1,
> >         .brp_max = 1024, /* 6bit + extended 4bit */
> >         .brp_inc = 1,
> > };
> > 
> >> seg1/seg2/sjw/prop_seg must be more than 1 ?
> > 
> > Then "tseg1_min" should be set to *2*.
> > 
> >> Also I can see the following kernel error log.
> >> bitrate error 0.7%
> > 
> > A clock frequency of 50 MHz is sub-optimal for CAN and some 
> bit-rates 
> > cannot be reproduced properly. Here is the output of the can-utils 
> > program "can-calc-bit-timing" (with an entry for the pch-can added):
> > 
> > $ ./can-calc-bit-timing pch-can
> > Bit timing parameters for pch-can with 50.000000 MHz ref clock
> > nominal                                 real Bitrt   nom  real SampP
> > Bitrate TQ[ns] PrS PhS1 PhS2 SJW BRP Bitrate Error SampP 
> SampP Error CNF1 CNF2 CNF3
> > 1000000    100   3    3    3   1   5 1000000  0.0% 75.0% 
> 70.0%  6.7% 0x05 0x92 0x02
> >  800000    420   0    1    1   1  21  793650  0.8% 80.0% 
> 66.6% 16.8% 0x15 0xff 0x00
> >  500000    100   8    8    3   1   5  500000  0.0% 87.5% 
> 85.0%  2.9% 0x05 0xbf 0x02
> >  250000    500   3    3    1   1  25  250000  0.0% 87.5% 
> 87.5%  0.0% 0x19 0x92 0x00
> >  125000    500   6    7    2   1  25  125000  0.0% 87.5% 
> 87.5%  0.0% 0x19 0xb5 0x01
> >  100000    500   8    8    3   1  25  100000  0.0% 87.5% 
> 85.0%  2.9% 0x19 0xbf 0x02
> >   50000   2500   3    3    1   1 125   50000  0.0% 87.5% 
> 87.5%  0.0% 0x7d 0x92 0x00
> >   20000   2500   8    8    3   1 125   20000  0.0% 87.5% 
> 85.0%  2.9% 0x7d 0xbf 0x02
> >   10000  12500   3    3    1   1 625   10000  0.0% 87.5% 
> 87.5%  0.0% 0x71 0x92 0x00
> > 
> > As you can see, especially 800000 gives rather bad results.
> 
> BTW, it's always possible to specify optimized bit-timing 
> parameters directly, e.g. the following seem better:
> 
>    800000     60  12    4    4   4   3  793650  0.8% 80.0% 81.0%  1.2%
> 
> You could set these with:
> 
>   $ ip link set can0 type can \
>     tq 60 prop-seg 12 phase-seg1 4 phase-seg2 4 sjw 4
> 
> Wolfgang.
> 

Re: About bittiming calculation result

[Wolfgang Grandegger]

Hi Tomoya,

On 02/08/2011 02:27 AM, Tomoya MORINAGA wrote:
> On Tuesday, February 08, 2011 12:53 AM,  Wolfgang Grandegger wrote:
> 
>> BTW, it's always possible to specify optimized bit-timing 
>> parameters directly, e.g. the following seem better:
>>
>>    800000     60  12    4    4   4   3  793650  0.8% 80.0% 81.0%  1.2%
>>
>> You could set these with:
>>
>>   $ ip link set can0 type can \
>>     tq 60 prop-seg 12 phase-seg1 4 phase-seg2 4 sjw 4
> 
> I can confirm 800K comms works well using the above.

Cool, I got these magic values from a CAN hardware expert.

> I wish Can-core could calculate like above.

Me too! I also got some indication on how to improve our algorithm in
case the bit-rate does not match. Hope to find some time soon to work
on this issue.

>>> seg1/seg2/sjw/prop_seg must be more than 1 ?
> BTW, according to EG20T PCH data sheet, 
> CAN bit-timing parameters(BRP, Prop_Seg, Phase_Seg1, Phase_Seg2, SJW) must not be set 0.

OK, then please provide a patch setting tseg1_min to 2.

Wolfgang.

RE: About bittiming calculation result

[Tomoya MORINAGA]

On Monday, February 07, 2011 9:01 PM, Wolfgang Grandegger wrote
> 
> Well, only prop_seg+phase_seg=tseg1 is relevant and the 
> pch_can driver sets the allowed minimum "tseg1_min1" currently to 1:
> 
> static struct can_bittiming_const pch_can_bittiming_const = {
>         .name = KBUILD_MODNAME,
>         .tseg1_min = 1,
>         .tseg1_max = 16,
>         .tseg2_min = 1,
>         .tseg2_max = 8,
>         .sjw_max = 4,
>         .brp_min = 1,
>         .brp_max = 1024, /* 6bit + extended 4bit */
>         .brp_inc = 1,
> };
> 
> > seg1/seg2/sjw/prop_seg must be more than 1 ?
> 
> Then "tseg1_min" should be set to *2*.

Though some drivers accepted by upstream have parameter "tseg1_min" as 1,
Sould we release the patch like below ?
 -         .tseg1_min = 1,
+         .tseg1_min = 2,

Thanks,
-----------------------------------------
Tomoya MORINAGA
OKI SEMICONDUCTOR CO., LTD.

RE: About bittiming calculation result

[Bhupesh SHARMA]

Hi Tomoya,

> -----Original Message-----
> From: socketcan-core-bounces@lists.berlios.de [mailto:socketcan-core-
> bounces@lists.berlios.de] On Behalf Of Tomoya MORINAGA
> Sent: Tuesday, February 08, 2011 6:40 AM
> To: 'Wolfgang Grandegger'
> Cc: socketcan-core@lists.berlios.de; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: RE: About bittiming calculation result
> 
> On Monday, February 07, 2011 9:01 PM, Wolfgang Grandegger wrote
> >
> > Well, only prop_seg+phase_seg=tseg1 is relevant and the
> > pch_can driver sets the allowed minimum "tseg1_min1" currently to 1:
> >
> > static struct can_bittiming_const pch_can_bittiming_const = {
> >         .name = KBUILD_MODNAME,
> >         .tseg1_min = 1,
> >         .tseg1_max = 16,
> >         .tseg2_min = 1,
> >         .tseg2_max = 8,
> >         .sjw_max = 4,
> >         .brp_min = 1,
> >         .brp_max = 1024, /* 6bit + extended 4bit */
> >         .brp_inc = 1,
> > };
> >
> > > seg1/seg2/sjw/prop_seg must be more than 1 ?
> >
> > Then "tseg1_min" should be set to *2*.
> 
> Though some drivers accepted by upstream have parameter "tseg1_min" as
> 1,
> Sould we release the patch like below ?
>  -         .tseg1_min = 1,
> +         .tseg1_min = 2,
> 
AFAIK pch uses the Bosch C_CAN core internally.
As per Bosch C_CAN user manual tseg1= prop_seg + phase_seg1
So, ideally tseg1_min should be 2. My version of Bosch C_CAN driver
Uses the value 2.

Regards,
Bhupesh

RE: About bittiming calculation result

[Tomoya MORINAGA]

Hi  Bhupesh,

On Tuesday, February 08, 2011 12:30 PM, Bhupesh SHARMA wrote:
> AFAIK pch uses the Bosch C_CAN core internally.
> As per Bosch C_CAN user manual tseg1= prop_seg + phase_seg1 
> So, ideally tseg1_min should be 2. My version of Bosch C_CAN 
> driver Uses the value 2.

Thank you for your suggestions.
I will submit the patch.

With Best Regards,
-----------------------------------------
Tomoya MORINAGA
OKI SEMICONDUCTOR CO., LTD.

[PATCH 13/20] net: can: janz-ican3: world-writable sysfs termination file

[Vasiliy Kulikov]

Don't allow everybody to set terminator via sysfs.

Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
---
 Compile tested only.

 drivers/net/can/janz-ican3.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/can/janz-ican3.c b/drivers/net/can/janz-ican3.c
index b9a6d7a..366f5cc 100644
--- a/drivers/net/can/janz-ican3.c
+++ b/drivers/net/can/janz-ican3.c
@@ -1618,7 +1618,7 @@ static ssize_t ican3_sysfs_set_term(struct device *dev,
 	return count;
 }
 
-static DEVICE_ATTR(termination, S_IWUGO | S_IRUGO, ican3_sysfs_show_term,
+static DEVICE_ATTR(termination, S_IWUSR | S_IRUGO, ican3_sysfs_show_term,
 						   ican3_sysfs_set_term);
 
 static struct attribute *ican3_sysfs_attrs[] = {
-- 
1.7.0.4

Re: [PATCH 13/20] net: can: janz-ican3: world-writable sysfs termination file

[David Miller]

From: Vasiliy Kulikov <segoon@openwall.com>
Date: Fri,  4 Feb 2011 15:23:53 +0300

> Don't allow everybody to set terminator via sysfs.
> 
> Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>

Applied.

[PATCH 14/20] platform: x86: acer-wmi: world-writable sysfs threeg file

[Vasiliy Kulikov]

Don't allow everybody to write to hardware registers.

Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
---
 Compile tested only.

 drivers/platform/x86/acer-wmi.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
index f21eb53..1dfd966 100644
--- a/drivers/platform/x86/acer-wmi.c
+++ b/drivers/platform/x86/acer-wmi.c
@@ -1279,7 +1279,7 @@ static ssize_t set_bool_threeg(struct device *dev,
 			return -EINVAL;
 	return count;
 }
-static DEVICE_ATTR(threeg, S_IWUGO | S_IRUGO | S_IWUSR, show_bool_threeg,
+static DEVICE_ATTR(threeg, S_IRUGO | S_IWUSR, show_bool_threeg,
 	set_bool_threeg);
 
 static ssize_t show_interface(struct device *dev, struct device_attribute *attr,
-- 
1.7.0.4

[PATCH 15/20] platform: x86: asus_acpi: world-writable procfs files

[Vasiliy Kulikov]

Don't allow everybody to change ACPI settings.  The comment says that it
is done deliberatelly, however, the comment before disp_proc_write()
says that at least one of these setting is experimental.

Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
---
 Compile tested only.

 drivers/platform/x86/asus_acpi.c |    8 +-------
 1 files changed, 1 insertions(+), 7 deletions(-)

diff --git a/drivers/platform/x86/asus_acpi.c b/drivers/platform/x86/asus_acpi.c
index 4633fd8..fe49593 100644
--- a/drivers/platform/x86/asus_acpi.c
+++ b/drivers/platform/x86/asus_acpi.c
@@ -1081,14 +1081,8 @@ static int asus_hotk_add_fs(struct acpi_device *device)
 	struct proc_dir_entry *proc;
 	mode_t mode;
 
-	/*
-	 * If parameter uid or gid is not changed, keep the default setting for
-	 * our proc entries (-rw-rw-rw-) else, it means we care about security,
-	 * and then set to -rw-rw----
-	 */
-
 	if ((asus_uid == 0) && (asus_gid == 0)) {
-		mode = S_IFREG | S_IRUGO | S_IWUGO;
+		mode = S_IFREG | S_IRUGO | S_IWUSR | S_IWGRP;
 	} else {
 		mode = S_IFREG | S_IRUSR | S_IRGRP | S_IWUSR | S_IWGRP;
 		printk(KERN_WARNING "  asus_uid and asus_gid parameters are "
-- 
1.7.0.4

Re: [PATCH 15/20] platform: x86: asus_acpi: world-writable procfs files

[Corentin Chary]

Acked-By: Corentin Chary <corentincj@iksaif.net>

Anyway, the driver is deprecated and will be removed someday.

[PATCH 16/20] platform: x86: tc1100-wmi: world-writable sysfs wireless and jogdial files

[Vasiliy Kulikov]

Don't allow everybody to change WMI settings.

Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
---
 Compile tested only.

 drivers/platform/x86/tc1100-wmi.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/platform/x86/tc1100-wmi.c b/drivers/platform/x86/tc1100-wmi.c
index 1fe0f1f..865ef78 100644
--- a/drivers/platform/x86/tc1100-wmi.c
+++ b/drivers/platform/x86/tc1100-wmi.c
@@ -162,7 +162,7 @@ set_bool_##value(struct device *dev, struct device_attribute *attr, \
 			return -EINVAL; \
 	return count; \
 } \
-static DEVICE_ATTR(value, S_IWUGO | S_IRUGO | S_IWUSR, \
+static DEVICE_ATTR(value, S_IRUGO | S_IWUSR, \
 	show_bool_##value, set_bool_##value);
 
 show_set_bool(wireless, TC1100_INSTANCE_WIRELESS);
-- 
1.7.0.4

[PATCH 17/20] rtc: rtc-ds1511: world-writable sysfs nvram file

[Vasiliy Kulikov]

Don't allow everybogy to write to NVRAM.

Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
---
 Compile tested only.

 drivers/rtc/rtc-ds1511.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/rtc/rtc-ds1511.c b/drivers/rtc/rtc-ds1511.c
index 37268e9..afeb546 100644
--- a/drivers/rtc/rtc-ds1511.c
+++ b/drivers/rtc/rtc-ds1511.c
@@ -485,7 +485,7 @@ ds1511_nvram_write(struct file *filp, struct kobject *kobj,
 static struct bin_attribute ds1511_nvram_attr = {
 	.attr = {
 		.name = "nvram",
-		.mode = S_IRUGO | S_IWUGO,
+		.mode = S_IRUGO | S_IWUSR,
 	},
 	.size = DS1511_RAM_MAX,
 	.read = ds1511_nvram_read,
-- 
1.7.0.4

[PATCH 18/20] scsi: aic94xx: world-writable sysfs update_bios file

[Vasiliy Kulikov]

Don't allow everybody to load firmware.

Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
---
 Compile tested only.

 drivers/scsi/aic94xx/aic94xx_init.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/aic94xx/aic94xx_init.c b/drivers/scsi/aic94xx/aic94xx_init.c
index 3b7e83d..d5ff142 100644
--- a/drivers/scsi/aic94xx/aic94xx_init.c
+++ b/drivers/scsi/aic94xx/aic94xx_init.c
@@ -486,7 +486,7 @@ static ssize_t asd_show_update_bios(struct device *dev,
 			flash_error_table[i].reason);
 }
 
-static DEVICE_ATTR(update_bios, S_IRUGO|S_IWUGO,
+static DEVICE_ATTR(update_bios, S_IRUGO|S_IWUSR,
 	asd_show_update_bios, asd_store_update_bios);
 
 static int asd_create_dev_attrs(struct asd_ha_struct *asd_ha)
-- 
1.7.0.4

[PATCH 19/20] scsi: iscsi: world-writable sysfs priv_sess file

[Vasiliy Kulikov]

Don't allow everybody to change iSCSI settings.

Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
---
 Compile tested only.

 drivers/scsi/scsi_transport_iscsi.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
index f905ecb..01543d2 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -1847,7 +1847,7 @@ store_priv_session_##field(struct device *dev,				\
 #define iscsi_priv_session_rw_attr(field, format)			\
 	iscsi_priv_session_attr_show(field, format)			\
 	iscsi_priv_session_attr_store(field)				\
-static ISCSI_CLASS_ATTR(priv_sess, field, S_IRUGO | S_IWUGO,		\
+static ISCSI_CLASS_ATTR(priv_sess, field, S_IRUGO | S_IWUSR,		\
 			show_priv_session_##field,			\
 			store_priv_session_##field)
 iscsi_priv_session_rw_attr(recovery_tmo, "%d");
-- 
1.7.0.4

Re: [PATCH 19/20] scsi: iscsi: world-writable sysfs priv_sess file

[Mike Christie]

On 02/04/2011 06:24 AM, Vasiliy Kulikov wrote:
> Don't allow everybody to change iSCSI settings.
>
> Signed-off-by: Vasiliy Kulikov<segoon@openwall.com>
> ---
>   Compile tested only.
>
>   drivers/scsi/scsi_transport_iscsi.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
> index f905ecb..01543d2 100644
> --- a/drivers/scsi/scsi_transport_iscsi.c
> +++ b/drivers/scsi/scsi_transport_iscsi.c
> @@ -1847,7 +1847,7 @@ store_priv_session_##field(struct device *dev,				\
>   #define iscsi_priv_session_rw_attr(field, format)			\
>   	iscsi_priv_session_attr_show(field, format)			\
>   	iscsi_priv_session_attr_store(field)				\
> -static ISCSI_CLASS_ATTR(priv_sess, field, S_IRUGO | S_IWUGO,		\
> +static ISCSI_CLASS_ATTR(priv_sess, field, S_IRUGO | S_IWUSR,		\
>   			show_priv_session_##field,			\
>   			store_priv_session_##field)
>   iscsi_priv_session_rw_attr(recovery_tmo, "%d");


Sorry for the late reply. Messed up on my side. Patch is correct and ok 
with me. Tested here.

Acked-by: Mike Christie <michaelc@cs.wisc.edu>

[PATCH 20/20] fs: ubifs: world-writable debugfs dump_* files

[Vasiliy Kulikov]

Don't allow everybody to dump sensitive information about filesystems.

Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
---
 Compile tested only.

 fs/ubifs/debug.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/ubifs/debug.c b/fs/ubifs/debug.c
index 0bee4db..bcb1acb 100644
--- a/fs/ubifs/debug.c
+++ b/fs/ubifs/debug.c
@@ -2813,19 +2813,19 @@ int dbg_debugfs_init_fs(struct ubifs_info *c)
 	}
 
 	fname = "dump_lprops";
-	dent = debugfs_create_file(fname, S_IWUGO, d->dfs_dir, c, &dfs_fops);
+	dent = debugfs_create_file(fname, S_IWUSR, d->dfs_dir, c, &dfs_fops);
 	if (IS_ERR(dent))
 		goto out_remove;
 	d->dfs_dump_lprops = dent;
 
 	fname = "dump_budg";
-	dent = debugfs_create_file(fname, S_IWUGO, d->dfs_dir, c, &dfs_fops);
+	dent = debugfs_create_file(fname, S_IWUSR, d->dfs_dir, c, &dfs_fops);
 	if (IS_ERR(dent))
 		goto out_remove;
 	d->dfs_dump_budg = dent;
 
 	fname = "dump_tnc";
-	dent = debugfs_create_file(fname, S_IWUGO, d->dfs_dir, c, &dfs_fops);
+	dent = debugfs_create_file(fname, S_IWUSR, d->dfs_dir, c, &dfs_fops);
 	if (IS_ERR(dent))
 		goto out_remove;
 	d->dfs_dump_tnc = dent;
-- 
1.7.0.4

Re: [PATCH 20/20] fs: ubifs: world-writable debugfs dump_* files

[Artem Bityutskiy]

On Fri, 2011-02-04 at 15:24 +0300, Vasiliy Kulikov wrote:
> Don't allow everybody to dump sensitive information about filesystems.
> 
> Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>

Good catch, thanks. I'll push this patch to the ubifs-2.6.git tree,
thanks!

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: [PATCH 00/20] world-writable files in sysfs and debugfs

[Samuel Ortiz]

Hi Vasiliy,

On Fri, Feb 04, 2011 at 03:22:29PM +0300, Vasiliy Kulikov wrote:
>   mfd: ab3100: world-writable debugfs *_priv files
>   mfd: ab3500: world-writable debugfs register-* files
>   mfd: ab8500: world-writable debugfs register-* files
All 3 patches applied, thanks.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: [PATCH 00/20] world-writable files in sysfs and debugfs

[Vasiliy Kulikov]

> Vasiliy Kulikov (20):
>  mach-ux500: mbox-db5500: world-writable sysfs fifo file
>  leds: lp5521: world-writable sysfs engine* files
>  leds: lp5523: world-writable engine* sysfs files
>  misc: ep93xx_pwm: world-writable sysfs files
>  rtc: rtc-ds1511: world-writable sysfs nvram file
>  scsi: aic94xx: world-writable sysfs update_bios file
>  scsi: iscsi: world-writable sysfs priv_sess file

These are still not merged :(

Re: [Security] [PATCH 00/20] world-writable files in sysfs and debugfs

[Andrew Morton]

On Sat, 12 Mar 2011 23:23:06 +0300
Vasiliy Kulikov <segoon@openwall.com> wrote:

> > Vasiliy Kulikov (20):
> >  mach-ux500: mbox-db5500: world-writable sysfs fifo file
> >  leds: lp5521: world-writable sysfs engine* files
> >  leds: lp5523: world-writable engine* sysfs files
> >  misc: ep93xx_pwm: world-writable sysfs files
> >  rtc: rtc-ds1511: world-writable sysfs nvram file
> >  scsi: aic94xx: world-writable sysfs update_bios file
> >  scsi: iscsi: world-writable sysfs priv_sess file
> 
> These are still not merged :(

I grabbed them and shall merge some and send others at relevant
maintainers, thanks.

Re: [PATCH 00/20] world-writable files in sysfs and debugfs

[James Bottomley]

On Sat, 2011-03-12 at 23:23 +0300, Vasiliy Kulikov wrote:
> > Vasiliy Kulikov (20):
> >  mach-ux500: mbox-db5500: world-writable sysfs fifo file
> >  leds: lp5521: world-writable sysfs engine* files
> >  leds: lp5523: world-writable engine* sysfs files
> >  misc: ep93xx_pwm: world-writable sysfs files
> >  rtc: rtc-ds1511: world-writable sysfs nvram file
> >  scsi: aic94xx: world-writable sysfs update_bios file
> >  scsi: iscsi: world-writable sysfs priv_sess file
> 
> These are still not merged :(

OK, so I've not been tracking where we are in the dizzying ride on
security systems.  However, I thought we landed up in the privilege
separation arena using capabilities.  That means that world writeable
files aren't necessarily a problem as long as the correct capabilities
checks are in place, right?

James

Re: [Security] [PATCH 00/20] world-writable files in sysfs and debugfs

[Greg KH]

On Mon, Mar 14, 2011 at 10:26:05PM -0400, James Bottomley wrote:
> On Sat, 2011-03-12 at 23:23 +0300, Vasiliy Kulikov wrote:
> > > Vasiliy Kulikov (20):
> > >  mach-ux500: mbox-db5500: world-writable sysfs fifo file
> > >  leds: lp5521: world-writable sysfs engine* files
> > >  leds: lp5523: world-writable engine* sysfs files
> > >  misc: ep93xx_pwm: world-writable sysfs files
> > >  rtc: rtc-ds1511: world-writable sysfs nvram file
> > >  scsi: aic94xx: world-writable sysfs update_bios file
> > >  scsi: iscsi: world-writable sysfs priv_sess file
> > 
> > These are still not merged :(
> 
> OK, so I've not been tracking where we are in the dizzying ride on
> security systems.  However, I thought we landed up in the privilege
> separation arena using capabilities.  That means that world writeable
> files aren't necessarily a problem as long as the correct capabilities
> checks are in place, right?

There are no capability checks on sysfs files right now, so these all
need to be fixed.

thanks,

greg k-h

Re: [Security] [PATCH 00/20] world-writable files in sysfs and debugfs

[James Bottomley]

On Mon, 2011-03-14 at 20:09 -0700, Greg KH wrote:
> On Mon, Mar 14, 2011 at 10:26:05PM -0400, James Bottomley wrote:
> > On Sat, 2011-03-12 at 23:23 +0300, Vasiliy Kulikov wrote:
> > > > Vasiliy Kulikov (20):
> > > >  mach-ux500: mbox-db5500: world-writable sysfs fifo file
> > > >  leds: lp5521: world-writable sysfs engine* files
> > > >  leds: lp5523: world-writable engine* sysfs files
> > > >  misc: ep93xx_pwm: world-writable sysfs files
> > > >  rtc: rtc-ds1511: world-writable sysfs nvram file
> > > >  scsi: aic94xx: world-writable sysfs update_bios file
> > > >  scsi: iscsi: world-writable sysfs priv_sess file
> > > 
> > > These are still not merged :(
> > 
> > OK, so I've not been tracking where we are in the dizzying ride on
> > security systems.  However, I thought we landed up in the privilege
> > separation arena using capabilities.  That means that world writeable
> > files aren't necessarily a problem as long as the correct capabilities
> > checks are in place, right?
> 
> There are no capability checks on sysfs files right now, so these all
> need to be fixed.

That statement is true but irrelevant, isn't it?  There can't be
capabilities within sysfs files because the system that does them has no
idea what the capabilities would be.  If there were capabilities checks,
they'd have to be in the implementing routines.

I think the questions are twofold:

     1. Did anyone actually check for capabilities before assuming world
        writeable files were wrong?
     2. Even if there aren't any capabilities checks in the implementing
        routines, should there be (are we going the separated
        capabilities route vs the monolithic root route)?

James

Re: [Security] [PATCH 00/20] world-writable files in sysfs and debugfs

[Greg KH]

On Tue, Mar 15, 2011 at 07:50:28AM -0400, James Bottomley wrote:
> On Mon, 2011-03-14 at 20:09 -0700, Greg KH wrote:
> > On Mon, Mar 14, 2011 at 10:26:05PM -0400, James Bottomley wrote:
> > > On Sat, 2011-03-12 at 23:23 +0300, Vasiliy Kulikov wrote:
> > > > > Vasiliy Kulikov (20):
> > > > >  mach-ux500: mbox-db5500: world-writable sysfs fifo file
> > > > >  leds: lp5521: world-writable sysfs engine* files
> > > > >  leds: lp5523: world-writable engine* sysfs files
> > > > >  misc: ep93xx_pwm: world-writable sysfs files
> > > > >  rtc: rtc-ds1511: world-writable sysfs nvram file
> > > > >  scsi: aic94xx: world-writable sysfs update_bios file
> > > > >  scsi: iscsi: world-writable sysfs priv_sess file
> > > > 
> > > > These are still not merged :(
> > > 
> > > OK, so I've not been tracking where we are in the dizzying ride on
> > > security systems.  However, I thought we landed up in the privilege
> > > separation arena using capabilities.  That means that world writeable
> > > files aren't necessarily a problem as long as the correct capabilities
> > > checks are in place, right?
> > 
> > There are no capability checks on sysfs files right now, so these all
> > need to be fixed.
> 
> That statement is true but irrelevant, isn't it?  There can't be
> capabilities within sysfs files because the system that does them has no
> idea what the capabilities would be.  If there were capabilities checks,
> they'd have to be in the implementing routines.

Ah, you are correct, sorry for the misunderstanding.

> I think the questions are twofold:
> 
>      1. Did anyone actually check for capabilities before assuming world
>         writeable files were wrong?

I do not think so as the majority (i.e. all the ones that I looked at)
did no such checks.

>      2. Even if there aren't any capabilities checks in the implementing
>         routines, should there be (are we going the separated
>         capabilities route vs the monolithic root route)?

I think the general consensus is that we go the monolithic root route
for sysfs files in that we do not allow them to be world writable.

Do you have any exceptions that you know of that do these checks?

thanks,

greg k-h

Re: [Security] [PATCH 00/20] world-writable files in sysfs and debugfs

[James Bottomley]

On Tue, 2011-03-15 at 07:18 -0700, Greg KH wrote:
> On Tue, Mar 15, 2011 at 07:50:28AM -0400, James Bottomley wrote:
> > On Mon, 2011-03-14 at 20:09 -0700, Greg KH wrote:
> > > There are no capability checks on sysfs files right now, so these all
> > > need to be fixed.
> > 
> > That statement is true but irrelevant, isn't it?  There can't be
> > capabilities within sysfs files because the system that does them has no
> > idea what the capabilities would be.  If there were capabilities checks,
> > they'd have to be in the implementing routines.
> 
> Ah, you are correct, sorry for the misunderstanding.
> 
> > I think the questions are twofold:
> > 
> >      1. Did anyone actually check for capabilities before assuming world
> >         writeable files were wrong?
> 
> I do not think so as the majority (i.e. all the ones that I looked at)
> did no such checks.

OK, as long as someone checked, I'm happy.

> >      2. Even if there aren't any capabilities checks in the implementing
> >         routines, should there be (are we going the separated
> >         capabilities route vs the monolithic root route)?
> 
> I think the general consensus is that we go the monolithic root route
> for sysfs files in that we do not allow them to be world writable.
> 
> Do you have any exceptions that you know of that do these checks?

Heh, I didn't call our security vacillations a dizzying ride for
nothing.  I know the goal once was to try to run a distro without root
daemons (which is what required the capabilities stuff).  I'm actually
trying to avoid the issue ... I just want to make sure that people who
care aren't all moving in different directions.

James

Re: [Security] [PATCH 00/20] world-writable files in sysfs and debugfs

[Vasiliy Kulikov]

On Tue, Mar 15, 2011 at 07:50 -0400, James Bottomley wrote:
>      1. Did anyone actually check for capabilities before assuming world
>         writeable files were wrong?

I didn't check all these files as I haven't got these hardware :-)  But
as I can "chmod a+w" all sysfs files on my machine and they all become
sensible to nonroot writes, I suppose there is nothing preventing
nonroot users from writing to these buggy sysfs files.  As you can see,
there are no capable() checks in these drivers in open() or write().

>      2. Even if there aren't any capabilities checks in the implementing
>         routines, should there be (are we going the separated
>         capabilities route vs the monolithic root route)?

IMO, In any case old good DAC security model must not be obsoleted just
because someone thinks that MAC or anything else is more convenient for
him.  If sysfs is implemented via filesystem then it must support POSIX
permissions semantic.  MAC is very good in _some_ cases, but not instead
of DAC.

Thanks,

-- 
Vasiliy Kulikov
http://www.openwall.com - bringing security into open computing environments

Re: [Security] [PATCH 00/20] world-writable files in sysfs and debugfs

[James Bottomley]

On Tue, 2011-03-15 at 19:08 +0300, Vasiliy Kulikov wrote:
> On Tue, Mar 15, 2011 at 07:50 -0400, James Bottomley wrote:
> >      1. Did anyone actually check for capabilities before assuming world
> >         writeable files were wrong?
> 
> I didn't check all these files as I haven't got these hardware :-)

You don't need the hardware to check ... the question becomes is a
capabilities test sitting in the implementation or not.

>   But
> as I can "chmod a+w" all sysfs files on my machine and they all become
> sensible to nonroot writes, I suppose there is nothing preventing
> nonroot users from writing to these buggy sysfs files.  As you can see,
> there are no capable() checks in these drivers in open() or write().
> 
> >      2. Even if there aren't any capabilities checks in the implementing
> >         routines, should there be (are we going the separated
> >         capabilities route vs the monolithic root route)?
> 
> IMO, In any case old good DAC security model must not be obsoleted just
> because someone thinks that MAC or anything else is more convenient for
> him.  If sysfs is implemented via filesystem then it must support POSIX
> permissions semantic.  MAC is very good in _some_ cases, but not instead
> of DAC.

Um, I'm not sure that's even an issue.  capabilities have CAP_ADMIN
which is precisely the same check as owner == root.  We use this a lot
because ioctls ignore the standard unix DAC model.

James