Re: [Security] [PATCH 00/20] world-writable files in sysfs and debugfs
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 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/20] world-writable files in sysfs and debugfs
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 :( -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 07/20] video: sn9c102: world-wirtable sysfs files
Don't allow everybody to change video settings. Signed-off-by: Vasiliy Kulikov seg...@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 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 00/20] world-writable files in sysfs and debugfs
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 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] [media] av7110: make array offset unsigned
On Fri, Jan 07, 2011 at 16:51 +0300, Dan Carpenter wrote: But just for my own understanding, why is it wrong to change an int to an unsigned int in the userspace API? Who would notice? E.g. the same check in userspace (var 0). If var has changed the sign then the result would differ. -- Vasiliy -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] media: rc: ir-lirc-codec: fix integer overflow
'n' may be bigger than MAX_INT*sizeof(int), if so checking of truncated (int)(n/sizeof(int)) for LIRCBUF_SIZE overflows and then using nontruncated 'count' doesn't make sense. This is not a security issue as too big 'n' is catched in kmalloc() in memdup_user() call. However, it's better to prevent WARN() in kmalloc(). Signed-off-by: Vasiliy Kulikov seg...@openwall.com --- Compile tested only. drivers/media/rc/ir-lirc-codec.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c index 1e87ee8..a7e91e6 100644 --- a/drivers/media/rc/ir-lirc-codec.c +++ b/drivers/media/rc/ir-lirc-codec.c @@ -100,7 +100,8 @@ static ssize_t ir_lirc_transmit_ir(struct file *file, const char *buf, struct lirc_codec *lirc; struct rc_dev *dev; int *txbuf; /* buffer with values to transmit */ - int ret = 0, count; + int ret = 0; + size_t count; lirc = lirc_get_pdata(file); if (!lirc) -- 1.7.0.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] media: rc: ir-lirc-codec: fix potential integer overflow
'n' may be bigger than MAX_INT*sizeof(int), if so checking of truncated (int)(n/sizeof(int)) for LIRCBUF_SIZE overflow and then using nontruncated 'count' doesn't make sense. Also n may be up to sizeof(int)-1 bytes bigger than expected, so check value of (n % sizeof(int)) too. Signed-off-by: Vasiliy Kulikov seg...@openwall.com --- Compile tested only. drivers/media/rc/ir-lirc-codec.c |5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c index 1e87ee8..f011c5d 100644 --- a/drivers/media/rc/ir-lirc-codec.c +++ b/drivers/media/rc/ir-lirc-codec.c @@ -100,7 +100,8 @@ static ssize_t ir_lirc_transmit_ir(struct file *file, const char *buf, struct lirc_codec *lirc; struct rc_dev *dev; int *txbuf; /* buffer with values to transmit */ - int ret = 0, count; + int ret = 0; + size_t count; lirc = lirc_get_pdata(file); if (!lirc) @@ -110,7 +111,7 @@ static ssize_t ir_lirc_transmit_ir(struct file *file, const char *buf, return -EINVAL; count = n / sizeof(int); - if (count LIRCBUF_SIZE || count % 2 == 0) + if (count LIRCBUF_SIZE || count % 2 == 0 || n % sizeof(int) != 0) return -EINVAL; txbuf = memdup_user(buf, n); -- 1.7.0.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] media: rc: lirc_dev: check kobject_set_name() result
kobject_set_name() may fail with -ENOMEM, check for it. Signed-off-by: Vasiliy Kulikov seg...@openwall.com --- Compile tested only. drivers/media/rc/lirc_dev.c |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c index 8418b14..0a2d267 100644 --- a/drivers/media/rc/lirc_dev.c +++ b/drivers/media/rc/lirc_dev.c @@ -178,7 +178,9 @@ static int lirc_cdev_add(struct irctl *ir) cdev_init(cdev, lirc_dev_fops); cdev-owner = THIS_MODULE; } - kobject_set_name(cdev-kobj, lirc%d, d-minor); + retval = kobject_set_name(cdev-kobj, lirc%d, d-minor); + if (retval) + return retval; retval = cdev_add(cdev, MKDEV(MAJOR(lirc_base_dev), d-minor), 1); if (retval) -- 1.7.0.4 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] media: video: pvrusb2: fix memory leak
Use put_device() instead of kfree() because of device name leak.
Signed-off-by: Vasiliy Kulikov seg...@openwall.com
---
Compile tested only.
drivers/media/video/pvrusb2/pvrusb2-sysfs.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/media/video/pvrusb2/pvrusb2-sysfs.c
b/drivers/media/video/pvrusb2/pvrusb2-sysfs.c
index 3d7e5aa..281806b 100644
--- a/drivers/media/video/pvrusb2/pvrusb2-sysfs.c
+++ b/drivers/media/video/pvrusb2/pvrusb2-sysfs.c
@@ -647,7 +647,7 @@ static void class_dev_create(struct pvr2_sysfs *sfp,
if (ret) {
pvr2_trace(PVR2_TRACE_ERROR_LEGS,
device_register failed);
- kfree(class_dev);
+ put_device(class_dev);
return;
}
--
1.7.0.4
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
