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

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

[Vasiliy Kulikov]

The search was made with trivial shell commands:

find | xargs grep S_IWUGO
find | xargs grep S_IWOTH

I didn't precisely investigate how exactly one may damage the
system/hardware because of issues number, maybe the harm is very limited
in case of some of these drivers.

One suspicious file is ./staging/speakup/speakup.h, but it explitly calls
macros as world-writable.  I didn't check what speakup's world-writable
files provide because it requires some knowledge about the hardware.


Vasiliy Kulikov (20):
  mach-omap2: mux: world-writable debugfs files
  mach-omap2: pm: world-writable debugfs timer files
  mach-omap2: smartreflex: world-writable debugfs voltage files
  mach-ux500: mbox-db5500: world-writable sysfs fifo file
  leds: lp5521: world-writable sysfs engine* files
  leds: lp5523: world-writable engine* sysfs files
  video: sn9c102: world-wirtable sysfs files
  mfd: ab3100: world-writable debugfs *_priv files
  mfd: ab3500: world-writable debugfs register-* files
  mfd: ab8500: world-writable debugfs register-* files
  misc: ep93xx_pwm: world-writable sysfs files
  net: can: at91_can: world-writable sysfs files
  net: can: janz-ican3: world-writable sysfs termination file
  platform: x86: acer-wmi: world-writable sysfs threeg file
  platform: x86: asus_acpi: world-writable procfs files
  platform: x86: tc1100-wmi: world-writable sysfs wireless and jogdial 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
  fs: ubifs: world-writable debugfs dump_* files

 arch/arm/mach-omap2/mux.c                  |    2 +-
 arch/arm/mach-omap2/pm-debug.c             |    8 ++++----
 arch/arm/mach-omap2/smartreflex.c          |    4 ++--
 arch/arm/mach-ux500/mbox-db5500.c          |    2 +-
 drivers/leds/leds-lp5521.c                 |   14 +++++++-------
 drivers/leds/leds-lp5523.c                 |   20 ++++++++++----------
 drivers/media/video/sn9c102/sn9c102_core.c |    6 +++---
 drivers/mfd/ab3100-core.c                  |    4 ++--
 drivers/mfd/ab3550-core.c                  |    6 +++---
 drivers/mfd/ab8500-debugfs.c               |    6 +++---
 drivers/misc/ep93xx_pwm.c                  |    6 +++---
 drivers/net/can/at91_can.c                 |    2 +-
 drivers/net/can/janz-ican3.c               |    2 +-
 drivers/platform/x86/acer-wmi.c            |    2 +-
 drivers/platform/x86/asus_acpi.c           |    8 +-------
 drivers/platform/x86/tc1100-wmi.c          |    2 +-
 drivers/rtc/rtc-ds1511.c                   |    2 +-
 drivers/scsi/aic94xx/aic94xx_init.c        |    2 +-
 drivers/scsi/scsi_transport_iscsi.c        |    2 +-
 fs/ubifs/debug.c                           |    6 +++---
 20 files changed, 50 insertions(+), 56 deletions(-)

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

[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

[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

[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

[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

[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

[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));
>  	}

[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 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

[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

[rtc-linux] [PATCH 00/20] world-writable files in sysfs and debugfs

[Linus Walleij]

2011/2/4 Vasiliy Kulikov <segoon@openwall.com>:

> The search was made with trivial shell commands:
>
> find | xargs grep S_IWUGO
> find | xargs grep S_IWOTH

We only use our debugfs entries as root so it shouldn't matter much, this
is way better, thanks for fixing.

> ?mach-ux500: mbox-db5500: world-writable sysfs fifo file
> ?mfd: ab3100: world-writable debugfs *_priv files
> ?mfd: ab3500: world-writable debugfs register-* files
> ?mfd: ab8500: world-writable debugfs register-* files

Acked-by: Linus Walleij <linus.walleij@stericsson.com>

For these.

Yours,
Linus Walleij

[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 :(

[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.

[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

[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

[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

[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

[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

[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

[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