world-writable files in sysfs and debugfs

Do not create mux debugfs files as world-writable.

Signed-off-by Vasiliy Kulikov <[email protected]>

---
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(-)

arch/arm/mach-omap2/mux.c +1 -1

index 98148b6..6c84659 100644
@@ -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);
 	}
 }

Signature · 1 line

1.7.0.4

Don't allow all users to change timer settings.

Signed-off-by Vasiliy Kulikov <[email protected]>

---
Cannot compile the driver, so it is not tested at all.

Don't allow all users to change timer settings.

Signed-off-by Vasiliy Kulikov <[email protected]>

---
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(-)

arch/arm/mach-omap2/pm-debug.c +4 -4

index 125f565..a5a83b3 100644
@@ -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;

Don't allow everybody to change voltage settings.

Signed-off-by Vasiliy Kulikov <[email protected]>

---
Cannot compile the driver, so it is not tested at all.

Don't allow everybody to change voltage settings.

Signed-off-by Vasiliy Kulikov <[email protected]>

---
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(-)

arch/arm/mach-omap2/smartreflex.c +2 -2

index c37e823..95ac336 100644
@@ -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));
 	}

Quoted 5 lines · Vasiliy Kulikov

Don't allow everybody to change voltage settings.

Signed-off-by Vasiliy Kulikov <[email protected]>

---
?Cannot compile the driver, so it is not tested at all.

Acked-by Kevin Hilman <[email protected]>

Don't allow everybody to change video settings.

Signed-off-by Vasiliy Kulikov <[email protected]>

---
Compile tested only.

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

drivers/media/video/sn9c102/sn9c102_core.c +0 -0

index 84984f6..ce56a1c 100644

Don't allow everybody to change video settings.

Signed-off-by Vasiliy Kulikov <[email protected]>

---
Compile tested only.

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

drivers/media/video/sn9c102/sn9c102_core.c +3 -3

index 84984f6..ce56a1c 100644
@@ -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);
 
 

Don't allow everybody to change device hardware registers.

Signed-off-by Vasiliy Kulikov <[email protected]>

---
 Compile tested only.

 drivers/mfd/ab3100-core.c |    4 ++--

 1 files changed, 2 insertions(+), 2 deletions(-)

drivers/mfd/ab3100-core.c +2 -3

index 4193af5..1707d22 100644
@@ -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

Don't allow everybody to write to mb0_id file.

Quoted 2 lines

Don't allow everybody to write to mb0_id file.

very well!

Acked-by Kurt Van Dijck <[email protected]>

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%

Hi Tomoya,

Quoted 9 lines · 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

Quoted 8 lines

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,
};

Quoted 1 line

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

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

Quoted 2 lines

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

Quoted 1 line

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

-----Original Message-----
From: Wolfgang Grandegger [mailto:[email protected]]
Sent: Tuesday, February 08, 2011 12:53 AM
To: Tomoya MORINAGA
Cc: [email protected]; [email protected]; [email protected]
Subject: Re: About bittiming calculation result

Quoted 19 lines · Wolfgang Grandegger

Hi Tomoya,

Quoted 16 lines · 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

Quoted 3 lines

Quoted 1 line

Is "prop_seg=0" true ?

Well, only prop_seg+phase_seg=tseg1 is relevant and the

pch_can driver

Quoted 14 lines

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,
};

Quoted 8 lines

Quoted 1 line

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

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

Quoted 2 lines

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

Quoted 7 lines

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

Quoted 1 line

1000000 100 3 3 3 1 5 1000000 0.0% 75.0%

70.0% 6.7% 0x05 0x92 0x02

Quoted 1 line

800000 420 0 1 1 1 21 793650 0.8% 80.0%

66.6% 16.8% 0x15 0xff 0x00

Quoted 1 line

500000 100 8 8 3 1 5 500000 0.0% 87.5%

85.0% 2.9% 0x05 0xbf 0x02

Quoted 1 line

250000 500 3 3 1 1 25 250000 0.0% 87.5%

87.5% 0.0% 0x19 0x92 0x00

Quoted 1 line

125000 500 6 7 2 1 25 125000 0.0% 87.5%

87.5% 0.0% 0x19 0xb5 0x01

Quoted 1 line

100000 500 8 8 3 1 25 100000 0.0% 87.5%

85.0% 2.9% 0x19 0xbf 0x02

Quoted 1 line

50000 2500 3 3 1 1 125 50000 0.0% 87.5%

87.5% 0.0% 0x7d 0x92 0x00

Quoted 1 line

20000 2500 8 8 3 1 125 20000 0.0% 87.5%

85.0% 2.9% 0x7d 0xbf 0x02

Quoted 1 line

10000 12500 3 3 1 1 625 10000 0.0% 87.5%

87.5% 0.0% 0x71 0x92 0x00

Quoted 2 lines

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.

Quoted 3 lines · Wolfgang Grandegger

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

Quoted 6 lines

   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.

Quoted 1 line

Quoted 1 line

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.

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,
};

Quoted 1 line

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

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

-----Original Message-----
From: [email protected] [mailto:socketcan-core- [email protected]] On Behalf Of Tomoya MORINAGA
Sent: Tuesday, February 08, 2011 6:40 AM
To: 'Wolfgang Grandegger'
Cc: [email protected]; [email protected]; linux- [email protected]
Subject: RE: About bittiming calculation result

On Monday, February 07, 2011 9:01 PM, Wolfgang Grandegger wrote

Quoted 18 lines

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,
};

Quoted 1 line

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

Quoted 1 line

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.

Don't allow everybody to set terminator via sysfs.

Signed-off-by Vasiliy Kulikov <[email protected]>

Don't allow everybody to change iSCSI settings.

Signed-off-by Vasiliy Kulikov<segoon-cxoSlKxDwOJWk0Htik3J/[email protected]>

---
Compile tested only.

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

drivers/scsi/scsi_transport_iscsi.c +0 -0

index f905ecb..01543d2 100644

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

Signed-off-by Vasiliy Kulikov <[email protected]>

The search was made with trivial shell commands:

find | xargs grep S_IWUGO
find | xargs grep S_IWOTH

?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

mfd: ab3100: world-writable debugfs *_priv files
mfd: ab3500: world-writable debugfs register-* files
mfd: ab8500: world-writable debugfs register-* files

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

Quoted 8 lines

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

Quoted 8 lines

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

Quoted 9 lines · Vasiliy Kulikov

Quoted 8 lines

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

Quoted 1 line

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?

Quoted 12 lines · James Bottomley

Quoted 10 lines · Vasiliy Kulikov

Quoted 8 lines

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

Quoted 5 lines

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.

Quoted 19 lines · Greg KH

Quoted 17 lines · James Bottomley

Quoted 10 lines · Vasiliy Kulikov

Quoted 8 lines

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?

Quoted 2 lines

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)?

Quoted 8 lines · James Bottomley

Quoted 2 lines · Greg KH

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.

Quoted 4 lines

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.

Quoted 3 lines

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?

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)?

Quoted 2 lines · James Bottomley

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().

Quoted 3 lines

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.

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

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