* udevadm monitor and atapi dvdrw
From: Franco Martelli @ 2012-11-21 15:55 UTC (permalink / raw)
To: linux-hotplug
Hi everybody,
I'm using Debian Sqeeze 6.0, kernel 2.6.32, udev version 164.
With kernel provided from debian my dvdrw is under scsi subsystem and
udevadm monitor recognize optical disk change with a lot of info output
suitable to write rules.
I've recompilated the kernel and things has changed, now my dvdrw is
under ide/atapi subsystem and the device name is hdc now.
I'd like to write a rule that remove udf and crc_itu_t modules as soon
as optical disk is left off from dvdrw drive. Example:
SUBSYSTEM="block", KERNEL="hdc", ACTION="change", RUN+="/sbin/rmmod
udf crc_itu_t"
I look at http://reactivated.net/writing_udev_rules.html but doesn't
help for ide devices ACTION="change".
Could anybody help me to write such a rule?
bye,
--
Franco Martelli.
^ permalink raw reply
* Re: udevadm monitor and atapi dvdrw
From: Kay Sievers @ 2012-11-22 19:08 UTC (permalink / raw)
To: linux-hotplug
In-Reply-To: <50ACF972.5020409@gmail.com>
On Wed, Nov 21, 2012 at 4:55 PM, Franco Martelli <martellif67@gmail.com> wrote:
> Hi everybody,
>
> I'm using Debian Sqeeze 6.0, kernel 2.6.32, udev version 164.
> With kernel provided from debian my dvdrw is under scsi subsystem and
> udevadm monitor recognize optical disk change with a lot of info output
> suitable to write rules.
> I've recompilated the kernel and things has changed, now my dvdrw is under
> ide/atapi subsystem and the device name is hdc now.
> I'd like to write a rule that remove udf and crc_itu_t modules as soon as
> optical disk is left off from dvdrw drive. Example:
>
> SUBSYSTEM="block", KERNEL="hdc", ACTION="change", RUN+="/sbin/rmmod udf
> crc_itu_t"
>
> I look at http://reactivated.net/writing_udev_rules.html but doesn't help
> for ide devices ACTION="change".
> Could anybody help me to write such a rule?
Modern userspace and kernel facilities like media change polling do
not support all the old and deprecated IDE drivers. The IDE drivers
are just not capable to do what you are looking for. Udev does not
support IDE drivers at all, only libata.
You might just want to disable the IDE drivers in the kernel config
and use the libata ones. Ideally the kernel would just delete the IDE
stuff.
Kay
^ permalink raw reply
* Re: udevadm monitor and atapi dvdrw
From: Franco Martelli @ 2012-11-24 16:09 UTC (permalink / raw)
To: linux-hotplug
In-Reply-To: <50ACF972.5020409@gmail.com>
Il 22/11/2012 20:08, Kay Sievers ha scritto:
> On Wed, Nov 21, 2012 at 4:55 PM, Franco Martelli <martellif67@gmail.com> wrote:
>> Hi everybody,
>>
>> I'm using Debian Sqeeze 6.0, kernel 2.6.32, udev version 164.
>> With kernel provided from debian my dvdrw is under scsi subsystem and
>> udevadm monitor recognize optical disk change with a lot of info output
>> suitable to write rules.
>> I've recompilated the kernel and things has changed, now my dvdrw is under
>> ide/atapi subsystem and the device name is hdc now.
>> I'd like to write a rule that remove udf and crc_itu_t modules as soon as
>> optical disk is left off from dvdrw drive. Example:
>>
>> SUBSYSTEM="block", KERNEL="hdc", ACTION="change", RUN+="/sbin/rmmod udf
>> crc_itu_t"
>>
>> I look at http://reactivated.net/writing_udev_rules.html but doesn't help
>> for ide devices ACTION="change".
>> Could anybody help me to write such a rule?
> Modern userspace and kernel facilities like media change polling do
> not support all the old and deprecated IDE drivers. The IDE drivers
> are just not capable to do what you are looking for. Udev does not
> support IDE drivers at all, only libata.
>
> You might just want to disable the IDE drivers in the kernel config
> and use the libata ones. Ideally the kernel would just delete the IDE
> stuff.
>
> Kay
>
Thank you very much Kay,
does libata work as a kernel module only or can be statically included
into the kernel? My lsmod output is:
# lsmod
Module Size Used by
vboxpci 11426 0
vboxnetadp 17099 0
vboxnetflt 13500 0
vboxdrv 1754521 3 vboxpci,vboxnetadp,vboxnetflt
that are Virtual Box modules. I've included into the kernel usb, sound,
network, sata, ide, and other devices support. Modules are ppp, usb
specific drivers, lp ... and others but not libata. Maybe should I
looking for a more recent, backported to sqeeze linux kernel?
bye,
--
Franco Martelli.
^ permalink raw reply
* Re: udevadm monitor and atapi dvdrw
From: Franco Martelli @ 2012-11-25 15:44 UTC (permalink / raw)
To: linux-hotplug
In-Reply-To: <50ACF972.5020409@gmail.com>
Il 22/11/2012 20:08, Kay Sievers ha scritto:
> On Wed, Nov 21, 2012 at 4:55 PM, Franco Martelli<martellif67@gmail.com> wrote:
>> Hi everybody,
>>
>> I'm using Debian Sqeeze 6.0, kernel 2.6.32, udev version 164.
>> With kernel provided from debian my dvdrw is under scsi subsystem and
>> udevadm monitor recognize optical disk change with a lot of info output
>> suitable to write rules.
>> I've recompilated the kernel and things has changed, now my dvdrw is under
>> ide/atapi subsystem and the device name is hdc now.
>> I'd like to write a rule that remove udf and crc_itu_t modules as soon as
>> optical disk is left off from dvdrw drive. Example:
>>
>> SUBSYSTEM="block", KERNEL="hdc", ACTION="change", RUN+="/sbin/rmmod udf
>> crc_itu_t"
>>
>> I look athttp://reactivated.net/writing_udev_rules.html but doesn't help
>> for ide devices ACTION="change".
>> Could anybody help me to write such a rule?
> Modern userspace and kernel facilities like media change polling do
> not support all the old and deprecated IDE drivers. The IDE drivers
> are just not capable to do what you are looking for. Udev does not
> support IDE drivers at all, only libata.
>
> You might just want to disable the IDE drivers in the kernel config
> and use the libata ones. Ideally the kernel would just delete the IDE
> stuff.
>
> Kay
>
solved. :-)
I've build the kernel using /boot/config-2.6.32-5-amd64 as reference,
that is the .config of the kernel provided from Debian. I checked line
by line libata and scsi targets and with make menuconfig I excluded "< >
ATA/ATAPI/MFM/RLL support --->" and I apply new configuration as you
suggest.
So the rule that do the work is:
SUBSYSTEM="scsi", ENV{SDEV_MEDIA_CHANGE}="1", ACTION="change",
RUN+="/sbin/rmmod udf crc_itu_t"
I don't know if it's the right way to unload modules no more needed but
it works.
bye,
--
Franco Martelli.
^ permalink raw reply
* [ANNOUNCE] kmod 12
From: Lucas De Marchi @ 2012-12-05 3:52 UTC (permalink / raw)
To: linux-modules; +Cc: LKML, linux-hotplug, Rusty Russell
kmod 12 is out:
ftp://ftp.kernel.org/pub/linux/utils/kernel/kmod/kmod-12.tar.xz
ftp://ftp.kernel.org/pub/linux/utils/kernel/kmod/kmod-12.tar.sign
Bug fix release only. Copying the NEWS section here for brevity:
- Fix removing vermagic from module when told to force load a module
- Fix removing __versions section when told to force load a module: we
need to mangle the section header, not the section.
- modinfo no longer fails while loading a module from file when path
contains ".ko" substring
Interesting that there was no previous bug report of "modprobe -f"
failing, yet before this release it was not working properly. Maybe
people are not tainting their kernel as frequently as I imagined.
Shortlog is below.
Cheers
Lucas De Marchi
---
Aleksey Makarov (1):
fix is_module_filename()
Lucas De Marchi (7):
depmod: fix hash lookup by relpath instead of uncrelpath
depmod: fix asserting mod->kmod = NULL
libkmod-module: Remove key+value vermagic from .modinfo section
libkmod-module: mangle the section header, not the section
Use bool instead of int
depmod: fix checking file extension
kmod 12
^ permalink raw reply
* udev errors for usb modems after kernel upgrade
From: Deniz Eren @ 2012-12-05 11:24 UTC (permalink / raw)
To: linux-hotplug
Hi;
After upgrading my kernel from 2.6.18 to 3.5.3. I started to get udev
errors when I plug in my usb modem device(errors are below). I think
this is causing my usb_modeswitch not to work properly. Same error
occurs for usb-storage devices too. Can you provide a solution or what
is the cause?
udev version: 095
Dec 5 12:20:18 2012 kernel: [74465.103460] usb 2-1.4: new high-speed
USB device number 36 using ehci_hcd
Dec 5 12:20:18 2012 kernel: [74465.194011] scsi62 : usb-storage 2-1.4:1.0
Dec 5 12:20:18 2012 kernel: [74465.194091] scsi63 : usb-storage 2-1.4:1.1
Dec 5 12:20:19 2012 kernel: [74466.196062] scsi 62:0:0:0: CD-ROM
HUAWEI Mass Storage 2.31 PQ: 0 ANSI: 2
Dec 5 12:20:19 2012 kernel: [74466.196441] scsi 63:0:0:0:
Direct-Access HUAWEI TF CARD Storage PQ: 0 ANSI: 2
Dec 5 12:20:19 2012 kernel: [74466.198548] sr0: scsi-1 drive
Dec 5 12:20:19 2012 kernel: [74466.198630] sr 62:0:0:0: Attached scsi
generic sg1 type 5
Dec 5 12:20:19 2012 kernel: [74466.198719] sd 63:0:0:0: Attached scsi
generic sg2 type 0
Dec 5 12:20:19 2012 kernel: [74466.201555] sd 63:0:0:0: [sdb]
Attached SCSI removable disk
Dec 5 12:20:22 2012 udevd-event[18390]: wait_for_sysfs: waiting for
\'/sys/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.4/2-1.4:1.0/host62/ioerr_cnt\'
failed
Dec 5 12:20:22 2012 udevd-event[18391]: wait_for_sysfs: waiting for
\'/sys/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.4/2-1.4:1.1/host63/ioerr_cnt\'
failed
Dec 5 12:20:25 2012 udevd-event[18443]: wait_for_sysfs: waiting for
\'/sys/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.4/2-1.4:1.1/host63/target63:0:0/ioerr_cnt\'
failed
Dec 5 12:20:25 2012 udevd-event[18442]: wait_for_sysfs: waiting for
\'/sys/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.4/2-1.4:1.0/host62/target62:0:0/ioerr_cnt\'
failed
Thanks in advance.
--
Deniz Eren
^ permalink raw reply
* Re: udev errors for usb modems after kernel upgrade
From: Greg KH @ 2012-12-05 15:19 UTC (permalink / raw)
To: linux-hotplug
In-Reply-To: <CALEWwHtrc+6+LoNJYD+WJcpDX90XgzMpmdGYLtZnuuSDVkLJng@mail.gmail.com>
On Wed, Dec 05, 2012 at 01:24:50PM +0200, Deniz Eren wrote:
> Hi;
>
> After upgrading my kernel from 2.6.18 to 3.5.3. I started to get udev
> errors when I plug in my usb modem device(errors are below). I think
> this is causing my usb_modeswitch not to work properly. Same error
> occurs for usb-storage devices too. Can you provide a solution or what
> is the cause?
>
> udev version: 095
Try also updating your version of udev, 095 is very old, and matched up
with what 2.6.18 was. Now that you have a newer kernel, there's no
reason to use such an old udev.
good luck,
greg k-h
^ permalink raw reply
* Logitech iTouch keyboard broken scan codes
From: Götz Christ @ 2013-02-23 21:42 UTC (permalink / raw)
To: linux-hotplug
Hi, I have a Logitech "iTouch Internet Navigator" keyboard, with 5
broken scan codes.
Following the instructions from
/usr/share/doc/systemd/README.keymap.txt, I have:
$ /usr/lib/udev/findkeyboards
AT keyboard: input/event2
# /usr/lib/udev/keymap -i input/event2
(which corresponds to the keys labeled as)
0x91 shop # Shopping
0x92 config # iTouch
0x93 finance # Finance
0x94 prog1 # My Sites
0x95 prog2 # Community
I couldn't find something better in input.h for the last two, so I
chose prog1 and prog2.
^ permalink raw reply
* Re: [PATCH] virtio-blk: emit udev event when device is resized
From: Greg KH @ 2013-02-25 22:12 UTC (permalink / raw)
To: Rusty Russell, Kay Sievers
Cc: Milos Vyletel, linux-kernel, linux-hotplug, virtualization, mst
In-Reply-To: <87y5ehfczy.fsf@rustcorp.com.au>
On Fri, Feb 22, 2013 at 10:14:49AM +1030, Rusty Russell wrote:
> Milos Vyletel <milos.vyletel@sde.cz> writes:
>
> > When virtio-blk device is resized from host (using block_resize from QEMU) emit
> > KOBJ_CHANGE uevent to notify guest about such change. This allows user to have
> > custom udev rules which would take whatever action if such event occurs. As a
> > proof of concept I've created simple udev rule that automatically resize
> > filesystem on virtio-blk device.
> >
> > ACTION="change", KERNEL="vd*", \
> > ENV{RESIZE}="1", \
> > ENV{ID_FS_TYPE}="ext[3-4]", \
> > RUN+="/sbin/resize2fs /dev/%k"
> > ACTION="change", KERNEL="vd*", \
> > ENV{RESIZE}="1", \
> > ENV{ID_FS_TYPE}="LVM2_member", \
> > RUN+="/sbin/pvresize /dev/%k"
>
> This looks fine to me, but I like to check with Greg before adding udev
> callouts.... Greg?
Hm, I thought we were frowning apon running binaries from udev rules
these days, especially ones that might have big consequences (like
resizing a disk image) like this.
Kay, am I right?
We already emit KOBJECT_CHANGE events when block devices change, from
within the block core code. Why is the patch below needed instead of
using these events that are already generated? How are virtio block
devices special?
> BTW, if this is good, it's good for stable IMHO.
What bug does it fix?
thanks,
greg k-h
> Cheers,
> Rusty.
>
> > Signed-off-by: Milos Vyletel <milos.vyletel@sde.cz>
> > ---
> > drivers/block/virtio_blk.c | 3 +++
> > 1 files changed, 3 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> > index 8ad21a2..5990382 100644
> > --- a/drivers/block/virtio_blk.c
> > +++ b/drivers/block/virtio_blk.c
> > @@ -539,6 +539,8 @@ static void virtblk_config_changed_work(struct work_struct *work)
> > struct virtio_device *vdev = vblk->vdev;
> > struct request_queue *q = vblk->disk->queue;
> > char cap_str_2[10], cap_str_10[10];
> > + char event[] = "RESIZE=1";
> > + char *envp[] = { event, NULL };
> > u64 capacity, size;
> >
> > mutex_lock(&vblk->config_lock);
> > @@ -568,6 +570,7 @@ static void virtblk_config_changed_work(struct work_struct *work)
> >
> > set_capacity(vblk->disk, capacity);
> > revalidate_disk(vblk->disk);
> > + kobject_uevent_env(&disk_to_dev(vblk->disk)->kobj, KOBJ_CHANGE, envp);
> > done:
> > mutex_unlock(&vblk->config_lock);
> > }
> > --
> > 1.7.1
^ permalink raw reply
* Re: [PATCH] virtio-blk: emit udev event when device is resized
From: Kay Sievers @ 2013-02-25 22:39 UTC (permalink / raw)
To: Greg KH
Cc: Rusty Russell, Milos Vyletel, linux-kernel, linux-hotplug,
virtualization, mst
In-Reply-To: <20130225221238.GA10575@kroah.com>
On Mon, Feb 25, 2013 at 11:12 PM, Greg KH <greg@kroah.com> wrote:
> Hm, I thought we were frowning apon running binaries from udev rules
> these days, especially ones that might have big consequences (like
> resizing a disk image) like this.
>
> Kay, am I right?
We removed most of them from the default setups, yes. But there is
nothing wrong if people want to ship that in some package or as custom
rules.
It looks fine to me, we would just not add such things to the default
set of of rules these days.
> We already emit KOBJECT_CHANGE events when block devices change, from
> within the block core code. Why is the patch below needed instead of
> using these events that are already generated? How are virtio block
> devices special?
I think we only do that for dm and md and a couple of special cases
like loop and read-only settings.
Kay
^ permalink raw reply
* Re: [PATCH] virtio-blk: emit udev event when device is resized
From: Greg KH @ 2013-02-25 22:43 UTC (permalink / raw)
To: Kay Sievers
Cc: Rusty Russell, Milos Vyletel, linux-kernel, linux-hotplug,
virtualization, mst
In-Reply-To: <CAPXgP13M_krpPMFoBroYkXc4dUQHsZJDua-jSGby8cQ1Afy-HQ@mail.gmail.com>
On Mon, Feb 25, 2013 at 11:39:52PM +0100, Kay Sievers wrote:
> On Mon, Feb 25, 2013 at 11:12 PM, Greg KH <greg@kroah.com> wrote:
>
> > Hm, I thought we were frowning apon running binaries from udev rules
> > these days, especially ones that might have big consequences (like
> > resizing a disk image) like this.
> >
> > Kay, am I right?
>
> We removed most of them from the default setups, yes. But there is
> nothing wrong if people want to ship that in some package or as custom
> rules.
>
> It looks fine to me, we would just not add such things to the default
> set of of rules these days.
>
> > We already emit KOBJECT_CHANGE events when block devices change, from
> > within the block core code. Why is the patch below needed instead of
> > using these events that are already generated? How are virtio block
> > devices special?
>
> I think we only do that for dm and md and a couple of special cases
> like loop and read-only settings.
What about when we repartition a block device? I've seen the events
happen then.
Anyway, if you are ok with this, no objection from my side then Rusty.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] virtio-blk: emit udev event when device is resized
From: Kay Sievers @ 2013-02-25 23:04 UTC (permalink / raw)
To: Greg KH
Cc: Rusty Russell, Milos Vyletel, linux-kernel, linux-hotplug,
virtualization, mst
In-Reply-To: <20130225224339.GA19153@kroah.com>
On Mon, Feb 25, 2013 at 11:43 PM, Greg KH <greg@kroah.com> wrote:
> On Mon, Feb 25, 2013 at 11:39:52PM +0100, Kay Sievers wrote:
>> On Mon, Feb 25, 2013 at 11:12 PM, Greg KH <greg@kroah.com> wrote:
>>
>> > Hm, I thought we were frowning apon running binaries from udev rules
>> > these days, especially ones that might have big consequences (like
>> > resizing a disk image) like this.
>> >
>> > Kay, am I right?
>>
>> We removed most of them from the default setups, yes. But there is
>> nothing wrong if people want to ship that in some package or as custom
>> rules.
>>
>> It looks fine to me, we would just not add such things to the default
>> set of of rules these days.
>>
>> > We already emit KOBJECT_CHANGE events when block devices change, from
>> > within the block core code. Why is the patch below needed instead of
>> > using these events that are already generated? How are virtio block
>> > devices special?
>>
>> I think we only do that for dm and md and a couple of special cases
>> like loop and read-only settings.
>
> What about when we repartition a block device? I've seen the events
> happen then.
Right, from the common block code we send events for removable media
changes like cdroms, sd cards, when a device is switched to read-only,
and when we re-scan a partition table like on re-partitioning. Most of
the other events are block subsystem-specific like this one. For
things like device-mapper they are used pretty heavily.
> Anyway, if you are ok with this, no objection from my side then Rusty.
Looks fine to me, it should not do any harm if there are not heavy
programs hooked up -- which is nothing the kernel could fix if people
do that. :)
Kay
^ permalink raw reply
* Re: [PATCH] virtio-blk: emit udev event when device is resized
From: Milos Vyletel @ 2013-02-25 23:38 UTC (permalink / raw)
To: Kay Sievers
Cc: Greg KH, Rusty Russell, linux-kernel, linux-hotplug,
virtualization, mst
In-Reply-To: <CAPXgP13M_krpPMFoBroYkXc4dUQHsZJDua-jSGby8cQ1Afy-HQ@mail.gmail.com>
On Feb 25, 2013, at 5:39 PM, Kay Sievers <kay@vrfy.org> wrote:
> On Mon, Feb 25, 2013 at 11:12 PM, Greg KH <greg@kroah.com> wrote:
>
>> Hm, I thought we were frowning apon running binaries from udev rules
>> these days, especially ones that might have big consequences (like
>> resizing a disk image) like this.
>>
>> Kay, am I right?
>
> We removed most of them from the default setups, yes. But there is
> nothing wrong if people want to ship that in some package or as custom
> rules.
>
> It looks fine to me, we would just not add such things to the default
> set of of rules these days.
That was not my intention. I just wanted to demonstrate that event like this
could be useful in cases like filesystem resize. We have a need for automatic
resize by our customers so we will ship our custom udev rules. It's perfectly
understandable that default udev rules will not have this.
Milos
>
>> We already emit KOBJECT_CHANGE events when block devices change, from
>> within the block core code. Why is the patch below needed instead of
>> using these events that are already generated? How are virtio block
>> devices special?
>
> I think we only do that for dm and md and a couple of special cases
> like loop and read-only settings.
>
> Kay
^ permalink raw reply
* Re: [PATCH] virtio-blk: emit udev event when device is resized
From: Milos Vyletel @ 2013-02-25 23:41 UTC (permalink / raw)
To: Greg KH
Cc: Rusty Russell, Kay Sievers, linux-kernel, linux-hotplug,
virtualization, mst
In-Reply-To: <20130225221238.GA10575@kroah.com>
On Feb 25, 2013, at 5:12 PM, Greg KH <greg@kroah.com> wrote:
> On Fri, Feb 22, 2013 at 10:14:49AM +1030, Rusty Russell wrote:
>> Milos Vyletel <milos.vyletel@sde.cz> writes:
>>
>>> When virtio-blk device is resized from host (using block_resize from QEMU) emit
>>> KOBJ_CHANGE uevent to notify guest about such change. This allows user to have
>>> custom udev rules which would take whatever action if such event occurs. As a
>>> proof of concept I've created simple udev rule that automatically resize
>>> filesystem on virtio-blk device.
>>>
>>> ACTION="change", KERNEL="vd*", \
>>> ENV{RESIZE}="1", \
>>> ENV{ID_FS_TYPE}="ext[3-4]", \
>>> RUN+="/sbin/resize2fs /dev/%k"
>>> ACTION="change", KERNEL="vd*", \
>>> ENV{RESIZE}="1", \
>>> ENV{ID_FS_TYPE}="LVM2_member", \
>>> RUN+="/sbin/pvresize /dev/%k"
>>
>> This looks fine to me, but I like to check with Greg before adding udev
>> callouts.... Greg?
>
> Hm, I thought we were frowning apon running binaries from udev rules
> these days, especially ones that might have big consequences (like
> resizing a disk image) like this.
>
> Kay, am I right?
>
> We already emit KOBJECT_CHANGE events when block devices change, from
> within the block core code. Why is the patch below needed instead of
> using these events that are already generated? How are virtio block
> devices special?
>
>> BTW, if this is good, it's good for stable IMHO.
>
> What bug does it fix?
>
It is not really a bug but it definitely is useful enhancement to have in stable too. I
can imagine lots of people can benefit from this.
Milos
> thanks,
>
> greg k-h
>
>> Cheers,
>> Rusty.
>>
>>> Signed-off-by: Milos Vyletel <milos.vyletel@sde.cz>
>>> ---
>>> drivers/block/virtio_blk.c | 3 +++
>>> 1 files changed, 3 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
>>> index 8ad21a2..5990382 100644
>>> --- a/drivers/block/virtio_blk.c
>>> +++ b/drivers/block/virtio_blk.c
>>> @@ -539,6 +539,8 @@ static void virtblk_config_changed_work(struct work_struct *work)
>>> struct virtio_device *vdev = vblk->vdev;
>>> struct request_queue *q = vblk->disk->queue;
>>> char cap_str_2[10], cap_str_10[10];
>>> + char event[] = "RESIZE=1";
>>> + char *envp[] = { event, NULL };
>>> u64 capacity, size;
>>>
>>> mutex_lock(&vblk->config_lock);
>>> @@ -568,6 +570,7 @@ static void virtblk_config_changed_work(struct work_struct *work)
>>>
>>> set_capacity(vblk->disk, capacity);
>>> revalidate_disk(vblk->disk);
>>> + kobject_uevent_env(&disk_to_dev(vblk->disk)->kobj, KOBJ_CHANGE, envp);
>>> done:
>>> mutex_unlock(&vblk->config_lock);
>>> }
>>> --
>>> 1.7.1
^ permalink raw reply
* Re: [PATCH] virtio-blk: emit udev event when device is resized
From: Rusty Russell @ 2013-02-27 0:46 UTC (permalink / raw)
To: Milos Vyletel, Greg KH
Cc: Kay Sievers, linux-kernel, linux-hotplug, virtualization, mst
In-Reply-To: <16100EF6-6C55-47ED-9BBB-2C3CAC8FA37A@sde.cz>
Milos Vyletel <milos.vyletel@sde.cz> writes:
> On Feb 25, 2013, at 5:12 PM, Greg KH <greg@kroah.com> wrote:
>
>> On Fri, Feb 22, 2013 at 10:14:49AM +1030, Rusty Russell wrote:
>>> Milos Vyletel <milos.vyletel@sde.cz> writes:
>>>
>>>> When virtio-blk device is resized from host (using block_resize from QEMU) emit
>>>> KOBJ_CHANGE uevent to notify guest about such change. This allows user to have
>>>> custom udev rules which would take whatever action if such event occurs. As a
>>>> proof of concept I've created simple udev rule that automatically resize
>>>> filesystem on virtio-blk device.
>>>>
>>>> ACTION="change", KERNEL="vd*", \
>>>> ENV{RESIZE}="1", \
>>>> ENV{ID_FS_TYPE}="ext[3-4]", \
>>>> RUN+="/sbin/resize2fs /dev/%k"
>>>> ACTION="change", KERNEL="vd*", \
>>>> ENV{RESIZE}="1", \
>>>> ENV{ID_FS_TYPE}="LVM2_member", \
>>>> RUN+="/sbin/pvresize /dev/%k"
>>>
>>> This looks fine to me, but I like to check with Greg before adding udev
>>> callouts.... Greg?
>>
>> Hm, I thought we were frowning apon running binaries from udev rules
>> these days, especially ones that might have big consequences (like
>> resizing a disk image) like this.
>>
>> Kay, am I right?
>>
>> We already emit KOBJECT_CHANGE events when block devices change, from
>> within the block core code. Why is the patch below needed instead of
>> using these events that are already generated? How are virtio block
>> devices special?
>>
>>> BTW, if this is good, it's good for stable IMHO.
>>
>> What bug does it fix?
>>
>
> It is not really a bug but it definitely is useful enhancement to have in stable too. I
> can imagine lots of people can benefit from this.
But that applies to almost any enhancement :)
It will go in *next* merge window, not this one.
Thanks,
Rusty.
^ permalink raw reply
* Re: [PATCH] virtio-blk: emit udev event when device is resized
From: Milos Vyletel @ 2013-02-27 13:09 UTC (permalink / raw)
To: Rusty Russell
Cc: Greg KH, Kay Sievers, linux-kernel, linux-hotplug, virtualization,
mst
In-Reply-To: <87wqtuegs9.fsf@rustcorp.com.au>
----- Original Message -----
> Milos Vyletel <milos.vyletel@sde.cz> writes:
> > On Feb 25, 2013, at 5:12 PM, Greg KH <greg@kroah.com> wrote:
> >
> >> On Fri, Feb 22, 2013 at 10:14:49AM +1030, Rusty Russell wrote:
> >>> Milos Vyletel <milos.vyletel@sde.cz> writes:
> >>>
> >>>> When virtio-blk device is resized from host (using block_resize from
> >>>> QEMU) emit
> >>>> KOBJ_CHANGE uevent to notify guest about such change. This allows user
> >>>> to have
> >>>> custom udev rules which would take whatever action if such event occurs.
> >>>> As a
> >>>> proof of concept I've created simple udev rule that automatically resize
> >>>> filesystem on virtio-blk device.
> >>>>
> >>>> ACTION="change", KERNEL="vd*", \
> >>>> ENV{RESIZE}="1", \
> >>>> ENV{ID_FS_TYPE}="ext[3-4]", \
> >>>> RUN+="/sbin/resize2fs /dev/%k"
> >>>> ACTION="change", KERNEL="vd*", \
> >>>> ENV{RESIZE}="1", \
> >>>> ENV{ID_FS_TYPE}="LVM2_member", \
> >>>> RUN+="/sbin/pvresize /dev/%k"
> >>>
> >>> This looks fine to me, but I like to check with Greg before adding udev
> >>> callouts.... Greg?
> >>
> >> Hm, I thought we were frowning apon running binaries from udev rules
> >> these days, especially ones that might have big consequences (like
> >> resizing a disk image) like this.
> >>
> >> Kay, am I right?
> >>
> >> We already emit KOBJECT_CHANGE events when block devices change, from
> >> within the block core code. Why is the patch below needed instead of
> >> using these events that are already generated? How are virtio block
> >> devices special?
> >>
> >>> BTW, if this is good, it's good for stable IMHO.
> >>
> >> What bug does it fix?
> >>
> >
> > It is not really a bug but it definitely is useful enhancement to have in
> > stable too. I
> > can imagine lots of people can benefit from this.
>
> But that applies to almost any enhancement :)
>
Good point :)
> It will go in *next* merge window, not this one.
>
Cool, thanks.
Milos
^ permalink raw reply
* [PATCH] udevadm-info: Don't access sysfs entries backing device I/O port space
From: Myron Stowe @ 2013-03-16 21:35 UTC (permalink / raw)
To: kay
Cc: linux-hotplug, greg, alex.williamson, linux-pci, yuxiangl,
yxlraid, linux-kernel
I've been working on identifying the root cause of an issue exposed by
'udevadm' that was first exposed on the linux-pci mail list [1] and
believe that there is now enough of an understanding to propose a fix.
What was originally witnessed was the platform hanging after "udevadm info
--attribute-walk --path=/sys/devices/pci0000:00/<...>/block/sda" is ran.
Xiangliang was able to isolate the failure to accesses involving a Marvell
9125 device's I/O BARs, or more specifically, accesses to the I/O port
space backing the device's I/O BARs (a.k.a. the device's I/O port
resources, or regions). With this knowledge he was able to reproduce the
hang targeting the device's sysfs 'resource<N>' entries, where N
represents an I/O BARs, with "cat /sys/devices/<...>/resource<N>".
In my research, looking for possible solutions, I noticed that kernel
commit 8633328 introduced sysfs based reading and writing of I/O port
related 'resource<N>' entries as part of adding virtualization based
device assignment functionality [2]. Note that these regions directly map
to the device's control and status registers [3].
Putting together these pieces of information we now understand that:
o udevadm based attribute walking initiates read accesses of all the
entries in a device's sysfs directory [4],
o sysfs 'resource<N>' entries correspond to the device's internal
status and control registers used for driving the device,
o If the 'resource<N>' entry corresponds to a device's I/O BAR, the
device's control and status registers are directly accessible by
userspace.
Allowing userspace access to a device's registers introduces potential
simultaneous interaction with the device by a second, competing, entity;
There is the device's driver, which believes it exclusively owns the
device, and an unknown, potential second entity, which can effect control
and status changes to the device asynchronously.
Device status and control registers being accessed from an entity that has
no idea what is being read or written is just asking for trouble. Even
just reading can have consequences as the register may be a "read once to
clear" or some similar type. I think we have just been lucky, or
blissfully ignorant, concerning problems that may have, and still could
be, occurring due to this situation.
There is an aspect at play here that I still do not understand (likely
something obvious that I'm just not seeing). The sysfs read routine for
accessing I/O port 'resource<N>' entries only supports 1, 2, and 4 byte
reads (which respectively correspond to inb/outb, inw/outw, and inl/outl
I/O port accessors). When "udevadm ..." executes, the udev internals
attempt reads of 4K byte chunks.
"udevadm info --attribute-walk --path=<pci_device_path>"
print_device_chain()
print_all_attributes()
...
udev_device_get_sysattr_value()
char value[4096];
...
size = read(fd, value, sizeof(value));
...
-- ^ userspace ^ -- v kernel v --
pci_read_resource_io(..., count) # sysfs read setup in pci_create_attr()
pci_resource_io(..., count, ...)
...
if (port + count - 1 > pci_resource_end(pdev, i))
return -EINVAL;
...
What I don't understand is how the device's I/O port space is successfully
getting read. It looks to me like 'pci_resource_io()' would fail the
access size check and return '-EINVAL' having never attempted the read's
access to I/O port space causing the system to hang.
I'm keep looking into this but I do *not* have access to a platform with a
Marvell 9125 device.
Reference(s)/Foot Note(s):
[1] https://lkml.org/lkml/2013/3/7/242
[2] Note that due to the implementation specifics only the 'resource<N>'
entries representing I/O BARs can be read or written via sysfs.
Sysfs' 'resource<N>' entries representing MMIO do not have sysfs
based read/write routines as only mmap'ing of these entries is
exposed (./drivers/pci/pci-sysfs.c::pci_create_attr()).
[3] The kernel's sysfs documentation states: "Attributes should be ASCII
text files..." (./Documentation/filesystems/sysfs.txt). I wonder if
this is just out-of-date infomation as sysfs obviously supports
creating binary files (./fs/sysfs/bin.c::sysfs_create_bin_file()).
[4] Note that udevadm-info does skip a specifically named set of entries
(./src/udevadm-info.c::skip_attribute()).
---
Myron Stowe (1):
udevadm-info: Don't access sysfs 'resource<N>' files
src/udevadm-info.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
--
^ permalink raw reply
* [PATCH] udevadm-info: Don't access sysfs 'resource<N>' files
From: Myron Stowe @ 2013-03-16 21:35 UTC (permalink / raw)
To: kay
Cc: linux-hotplug, greg, alex.williamson, linux-pci, yuxiangl,
yxlraid, linux-kernel
In-Reply-To: <20130316213512.2974.17303.stgit@amt.stowe>
Sysfs includes entries to memory that backs a PCI device's BARs, both I/O
Port space and MMIO. This memory regions correspond to the device's
internal status and control registers used to drive the device.
Accessing these registers from userspace such as "udevadm info
--attribute-walk --path=/sys/devices/..." does can not be allowed as
such accesses outside of the driver, even just reading, can yield
catastrophic consequences.
Udevadm-info skips parsing a specific set of sysfs entries including
'resource'. This patch extends the set to include the additional
'resource<N>' entries that correspond to a PCI device's BARs.
Reported-by: Xiangliang Yu <yuxiangl@marvell.com>
Signed-off-by: Myron Stowe <myron.stowe@redhat.com>
---
src/udevadm-info.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/src/udevadm-info.c b/src/udevadm-info.c
index ee9b59f..298acb5 100644
--- a/src/udevadm-info.c
+++ b/src/udevadm-info.c
@@ -37,13 +37,18 @@ static bool skip_attribute(const char *name)
"uevent",
"dev",
"modalias",
- "resource",
"driver",
"subsystem",
"module",
};
unsigned int i;
+ /*
+ * Skip any sysfs 'resource' entries, including 'resource<N>' entries
+ * that correspond to a device's I/O Port or MMIO space backed BARs.
+ */
+ if (strncmp((const char *)name, "resource", sizeof("resource")-1) = 0)
+ return true;
for (i = 0; i < ARRAY_SIZE(skip); i++)
if (strcmp(name, skip[i]) = 0)
return true;
^ permalink raw reply related
* Re: [PATCH] udevadm-info: Don't access sysfs 'resource<N>' files
From: Greg KH @ 2013-03-16 22:11 UTC (permalink / raw)
To: Myron Stowe
Cc: kay, linux-hotplug, alex.williamson, linux-pci, yuxiangl, yxlraid,
linux-kernel
In-Reply-To: <20130316213519.2974.38954.stgit@amt.stowe>
On Sat, Mar 16, 2013 at 03:35:19PM -0600, Myron Stowe wrote:
> Sysfs includes entries to memory that backs a PCI device's BARs, both I/O
> Port space and MMIO. This memory regions correspond to the device's
> internal status and control registers used to drive the device.
>
> Accessing these registers from userspace such as "udevadm info
> --attribute-walk --path=/sys/devices/..." does can not be allowed as
> such accesses outside of the driver, even just reading, can yield
> catastrophic consequences.
>
> Udevadm-info skips parsing a specific set of sysfs entries including
> 'resource'. This patch extends the set to include the additional
> 'resource<N>' entries that correspond to a PCI device's BARs.
Nice, are you also going to patch bash to prevent a user from reading
these sysfs files as well? :)
And pciutils?
You get my point here, right? The root user just asked to read all of
the data for this device, so why wouldn't you allow it? Just like
'lspci' does. Or bash does.
If this hardware has a problem, then it needs to be fixed in the kernel,
not have random band-aids added to various userspace programs to paper
over the root problem here. Please fix the kernel driver and all should
be fine. No need to change udevadm.
greg k-h
^ permalink raw reply
* Re: [PATCH] udevadm-info: Don't access sysfs 'resource<N>' files
From: Bjorn Helgaas @ 2013-03-16 22:55 UTC (permalink / raw)
To: Greg KH
Cc: Myron Stowe, kay, linux hotplug mailing,
alex.williamson@redhat.com, linux-pci@vger.kernel.org,
Xiangliang Yu, xiangliang yu, linux-kernel@vger.kernel.org
In-Reply-To: <20130316221159.GA3702@kroah.com>
On Sat, Mar 16, 2013 at 4:11 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Sat, Mar 16, 2013 at 03:35:19PM -0600, Myron Stowe wrote:
>> Sysfs includes entries to memory that backs a PCI device's BARs, both I/O
>> Port space and MMIO. This memory regions correspond to the device's
>> internal status and control registers used to drive the device.
>>
>> Accessing these registers from userspace such as "udevadm info
>> --attribute-walk --path=/sys/devices/..." does can not be allowed as
>> such accesses outside of the driver, even just reading, can yield
>> catastrophic consequences.
>>
>> Udevadm-info skips parsing a specific set of sysfs entries including
>> 'resource'. This patch extends the set to include the additional
>> 'resource<N>' entries that correspond to a PCI device's BARs.
>
> Nice, are you also going to patch bash to prevent a user from reading
> these sysfs files as well? :)
>
> And pciutils?
>
> You get my point here, right? The root user just asked to read all of
> the data for this device, so why wouldn't you allow it? Just like
> 'lspci' does. Or bash does.
>
> If this hardware has a problem, then it needs to be fixed in the kernel,
> not have random band-aids added to various userspace programs to paper
> over the root problem here. Please fix the kernel driver and all should
> be fine. No need to change udevadm.
I'm not sure that "udevadm info" (or bash) reading device registers is
a good idea because we don't know what the device is, and we don't
have any idea what the side effects of reading its registers will be.
Just to be clear, this is about device-specific I/O port registers,
not config space, so we can't expect any sort of consistency.
We could put a quirk in the kernel for this device (obviously the
issue is independent of whether the driver is loaded), but no doubt
other devices with I/O BARs will have access size restrictions, side
effects, or other issues. Adding quirks for them feels like a
never-ending job.
It might have been a mistake to put the resourceN files in sysfs in
the first place, or to make them read/writable, because users expect
sysfs files to contain ASCII. For memory BARs, resourceN only allows
mmap, not read/write, so at least we side-step similar issues there.
Bjorn
^ permalink raw reply
* Re: [PATCH] udevadm-info: Don't access sysfs 'resource<N>' files
From: Myron Stowe @ 2013-03-16 23:50 UTC (permalink / raw)
To: Greg KH
Cc: Myron Stowe, kay, linux-hotplug, alex.williamson, linux-pci,
yuxiangl, yxlraid, linux-kernel
In-Reply-To: <20130316221159.GA3702@kroah.com>
On Sat, 2013-03-16 at 15:11 -0700, Greg KH wrote:
> On Sat, Mar 16, 2013 at 03:35:19PM -0600, Myron Stowe wrote:
> > Sysfs includes entries to memory that backs a PCI device's BARs, both I/O
> > Port space and MMIO. This memory regions correspond to the device's
> > internal status and control registers used to drive the device.
> >
> > Accessing these registers from userspace such as "udevadm info
> > --attribute-walk --path=/sys/devices/..." does can not be allowed as
> > such accesses outside of the driver, even just reading, can yield
> > catastrophic consequences.
> >
> > Udevadm-info skips parsing a specific set of sysfs entries including
> > 'resource'. This patch extends the set to include the additional
> > 'resource<N>' entries that correspond to a PCI device's BARs.
>
> Nice, are you also going to patch bash to prevent a user from reading
> these sysfs files as well? :)
>
> And pciutils?
>
> You get my point here, right? The root user just asked to read all of
> the data for this device, so why wouldn't you allow it? Just like
> 'lspci' does. Or bash does.
Yes :P , you raise a very good point, there are a lot of way a user can
poke around in those BARs. However, there is a difference between
shooting yourself in the foot and getting what you deserve versus
unknowingly executing a common command such as udevadm and having the
system hang.
>
> If this hardware has a problem, then it needs to be fixed in the kernel,
> not have random band-aids added to various userspace programs to paper
> over the root problem here. Please fix the kernel driver and all should
> be fine. No need to change udevadm.
Xiangliang initially proposed a patch within the PCI core. Ignoring the
specific issue with the proposal which I pointed out in the
https://lkml.org/lkml/2013/3/7/242 thread, that just doesn't seem like
the right place to effect a change either as PCI's core isn't concerned
with the contents or access limitations of those regions, those are
issues that the driver concerns itself with.
So things seem to be gravitating towards the driver. I'm fairly
ignorant of this area but as Robert succinctly pointed out in the
originating thread - the AHCI driver only uses the device's MMIO region.
The I/O related regions are for legacy SFF-compatible ATA ports and are
not used to driver the device. This, coupled with the observance that
userspace accesses such as udevadm, and others like you additionally
point out, do not filter through the device's driver for seems to
suggest that changes to the driver will not help here either.
That said, I was attempting to point out an interesting problem and get
the conversation started towards coming up with some type a solution.
Let's continue the conversation and see where things go.
Thanks,
Myron
>
> greg k-h
^ permalink raw reply
* Re: [PATCH] udevadm-info: Don't access sysfs 'resource<N>' files
From: Greg KH @ 2013-03-17 1:03 UTC (permalink / raw)
To: Myron Stowe
Cc: Myron Stowe, kay, linux-hotplug, alex.williamson, linux-pci,
yuxiangl, yxlraid, linux-kernel
In-Reply-To: <1363477853.2423.25.camel@zim.stowe>
On Sat, Mar 16, 2013 at 05:50:53PM -0600, Myron Stowe wrote:
> On Sat, 2013-03-16 at 15:11 -0700, Greg KH wrote:
> > On Sat, Mar 16, 2013 at 03:35:19PM -0600, Myron Stowe wrote:
> > > Sysfs includes entries to memory that backs a PCI device's BARs, both I/O
> > > Port space and MMIO. This memory regions correspond to the device's
> > > internal status and control registers used to drive the device.
> > >
> > > Accessing these registers from userspace such as "udevadm info
> > > --attribute-walk --path=/sys/devices/..." does can not be allowed as
> > > such accesses outside of the driver, even just reading, can yield
> > > catastrophic consequences.
> > >
> > > Udevadm-info skips parsing a specific set of sysfs entries including
> > > 'resource'. This patch extends the set to include the additional
> > > 'resource<N>' entries that correspond to a PCI device's BARs.
> >
> > Nice, are you also going to patch bash to prevent a user from reading
> > these sysfs files as well? :)
> >
> > And pciutils?
> >
> > You get my point here, right? The root user just asked to read all of
> > the data for this device, so why wouldn't you allow it? Just like
> > 'lspci' does. Or bash does.
>
> Yes :P , you raise a very good point, there are a lot of way a user can
> poke around in those BARs. However, there is a difference between
> shooting yourself in the foot and getting what you deserve versus
> unknowingly executing a common command such as udevadm and having the
> system hang.
> >
> > If this hardware has a problem, then it needs to be fixed in the kernel,
> > not have random band-aids added to various userspace programs to paper
> > over the root problem here. Please fix the kernel driver and all should
> > be fine. No need to change udevadm.
>
> Xiangliang initially proposed a patch within the PCI core. Ignoring the
> specific issue with the proposal which I pointed out in the
> https://lkml.org/lkml/2013/3/7/242 thread, that just doesn't seem like
> the right place to effect a change either as PCI's core isn't concerned
> with the contents or access limitations of those regions, those are
> issues that the driver concerns itself with.
>
> So things seem to be gravitating towards the driver. I'm fairly
> ignorant of this area but as Robert succinctly pointed out in the
> originating thread - the AHCI driver only uses the device's MMIO region.
> The I/O related regions are for legacy SFF-compatible ATA ports and are
> not used to driver the device. This, coupled with the observance that
> userspace accesses such as udevadm, and others like you additionally
> point out, do not filter through the device's driver for seems to
> suggest that changes to the driver will not help here either.
A PCI quirk should handle this properly, right? Why not do that? Worse
thing, the quirk could just not expose these sysfs files for this
device, which would solve all userspace program issues, right?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] udevadm-info: Don't access sysfs 'resource<N>' files
From: Alex Williamson @ 2013-03-17 4:11 UTC (permalink / raw)
To: Greg KH
Cc: Myron Stowe, Myron Stowe, kay, linux-hotplug, linux-pci, yuxiangl,
yxlraid, linux-kernel
In-Reply-To: <20130317010317.GB9641@kroah.com>
On Sat, 2013-03-16 at 18:03 -0700, Greg KH wrote:
> On Sat, Mar 16, 2013 at 05:50:53PM -0600, Myron Stowe wrote:
> > On Sat, 2013-03-16 at 15:11 -0700, Greg KH wrote:
> > > On Sat, Mar 16, 2013 at 03:35:19PM -0600, Myron Stowe wrote:
> > > > Sysfs includes entries to memory that backs a PCI device's BARs, both I/O
> > > > Port space and MMIO. This memory regions correspond to the device's
> > > > internal status and control registers used to drive the device.
> > > >
> > > > Accessing these registers from userspace such as "udevadm info
> > > > --attribute-walk --path=/sys/devices/..." does can not be allowed as
> > > > such accesses outside of the driver, even just reading, can yield
> > > > catastrophic consequences.
> > > >
> > > > Udevadm-info skips parsing a specific set of sysfs entries including
> > > > 'resource'. This patch extends the set to include the additional
> > > > 'resource<N>' entries that correspond to a PCI device's BARs.
> > >
> > > Nice, are you also going to patch bash to prevent a user from reading
> > > these sysfs files as well? :)
> > >
> > > And pciutils?
> > >
> > > You get my point here, right? The root user just asked to read all of
> > > the data for this device, so why wouldn't you allow it? Just like
> > > 'lspci' does. Or bash does.
> >
> > Yes :P , you raise a very good point, there are a lot of way a user can
> > poke around in those BARs. However, there is a difference between
> > shooting yourself in the foot and getting what you deserve versus
> > unknowingly executing a common command such as udevadm and having the
> > system hang.
> > >
> > > If this hardware has a problem, then it needs to be fixed in the kernel,
> > > not have random band-aids added to various userspace programs to paper
> > > over the root problem here. Please fix the kernel driver and all should
> > > be fine. No need to change udevadm.
> >
> > Xiangliang initially proposed a patch within the PCI core. Ignoring the
> > specific issue with the proposal which I pointed out in the
> > https://lkml.org/lkml/2013/3/7/242 thread, that just doesn't seem like
> > the right place to effect a change either as PCI's core isn't concerned
> > with the contents or access limitations of those regions, those are
> > issues that the driver concerns itself with.
> >
> > So things seem to be gravitating towards the driver. I'm fairly
> > ignorant of this area but as Robert succinctly pointed out in the
> > originating thread - the AHCI driver only uses the device's MMIO region.
> > The I/O related regions are for legacy SFF-compatible ATA ports and are
> > not used to driver the device. This, coupled with the observance that
> > userspace accesses such as udevadm, and others like you additionally
> > point out, do not filter through the device's driver for seems to
> > suggest that changes to the driver will not help here either.
>
> A PCI quirk should handle this properly, right? Why not do that? Worse
> thing, the quirk could just not expose these sysfs files for this
> device, which would solve all userspace program issues, right?
Not exactly. I/O port access through pci-sysfs was added for userspace
programs, specifically qemu-kvm device assignment. We use the I/O port
resource# files to access device owned I/O port registers using file
permissions rather than global permissions such as iopl/ioperm. File
permissions also prevent random users from accessing device registers
through these files, but of course can't stop a privileged app that
chooses to ignore the purpose of these files. A quirk would therefore
remove a file that actually has a useful purpose for one app just so
another app that has no particular reason for dumping the contents can
run unabated. Thanks,
Alex
^ permalink raw reply
* Re: [PATCH] udevadm-info: Don't access sysfs 'resource<N>' files
From: Greg KH @ 2013-03-17 5:36 UTC (permalink / raw)
To: Alex Williamson
Cc: Myron Stowe, Myron Stowe, kay, linux-hotplug, linux-pci, yuxiangl,
yxlraid, linux-kernel
In-Reply-To: <1363493482.16793.69.camel@ul30vt.home>
On Sat, Mar 16, 2013 at 10:11:22PM -0600, Alex Williamson wrote:
> On Sat, 2013-03-16 at 18:03 -0700, Greg KH wrote:
> > On Sat, Mar 16, 2013 at 05:50:53PM -0600, Myron Stowe wrote:
> > > On Sat, 2013-03-16 at 15:11 -0700, Greg KH wrote:
> > > > On Sat, Mar 16, 2013 at 03:35:19PM -0600, Myron Stowe wrote:
> > > > > Sysfs includes entries to memory that backs a PCI device's BARs, both I/O
> > > > > Port space and MMIO. This memory regions correspond to the device's
> > > > > internal status and control registers used to drive the device.
> > > > >
> > > > > Accessing these registers from userspace such as "udevadm info
> > > > > --attribute-walk --path=/sys/devices/..." does can not be allowed as
> > > > > such accesses outside of the driver, even just reading, can yield
> > > > > catastrophic consequences.
> > > > >
> > > > > Udevadm-info skips parsing a specific set of sysfs entries including
> > > > > 'resource'. This patch extends the set to include the additional
> > > > > 'resource<N>' entries that correspond to a PCI device's BARs.
> > > >
> > > > Nice, are you also going to patch bash to prevent a user from reading
> > > > these sysfs files as well? :)
> > > >
> > > > And pciutils?
> > > >
> > > > You get my point here, right? The root user just asked to read all of
> > > > the data for this device, so why wouldn't you allow it? Just like
> > > > 'lspci' does. Or bash does.
> > >
> > > Yes :P , you raise a very good point, there are a lot of way a user can
> > > poke around in those BARs. However, there is a difference between
> > > shooting yourself in the foot and getting what you deserve versus
> > > unknowingly executing a common command such as udevadm and having the
> > > system hang.
> > > >
> > > > If this hardware has a problem, then it needs to be fixed in the kernel,
> > > > not have random band-aids added to various userspace programs to paper
> > > > over the root problem here. Please fix the kernel driver and all should
> > > > be fine. No need to change udevadm.
> > >
> > > Xiangliang initially proposed a patch within the PCI core. Ignoring the
> > > specific issue with the proposal which I pointed out in the
> > > https://lkml.org/lkml/2013/3/7/242 thread, that just doesn't seem like
> > > the right place to effect a change either as PCI's core isn't concerned
> > > with the contents or access limitations of those regions, those are
> > > issues that the driver concerns itself with.
> > >
> > > So things seem to be gravitating towards the driver. I'm fairly
> > > ignorant of this area but as Robert succinctly pointed out in the
> > > originating thread - the AHCI driver only uses the device's MMIO region.
> > > The I/O related regions are for legacy SFF-compatible ATA ports and are
> > > not used to driver the device. This, coupled with the observance that
> > > userspace accesses such as udevadm, and others like you additionally
> > > point out, do not filter through the device's driver for seems to
> > > suggest that changes to the driver will not help here either.
> >
> > A PCI quirk should handle this properly, right? Why not do that? Worse
> > thing, the quirk could just not expose these sysfs files for this
> > device, which would solve all userspace program issues, right?
>
> Not exactly. I/O port access through pci-sysfs was added for userspace
> programs, specifically qemu-kvm device assignment. We use the I/O port
> resource# files to access device owned I/O port registers using file
> permissions rather than global permissions such as iopl/ioperm. File
> permissions also prevent random users from accessing device registers
> through these files, but of course can't stop a privileged app that
> chooses to ignore the purpose of these files. A quirk would therefore
> remove a file that actually has a useful purpose for one app just so
> another app that has no particular reason for dumping the contents can
> run unabated. Thanks,
The quirk would only be for this one specific device, which obviously
can't handle this type of access, so why would you want the sysfs files
even present for it at all?
greg k-h
^ permalink raw reply
* Re: [PATCH] udevadm-info: Don't access sysfs 'resource<N>' files
From: Alex Williamson @ 2013-03-17 13:38 UTC (permalink / raw)
To: Greg KH
Cc: Myron Stowe, Myron Stowe, kay, linux-hotplug, linux-pci, yuxiangl,
yxlraid, linux-kernel
In-Reply-To: <20130317053611.GC948@kroah.com>
On Sat, 2013-03-16 at 22:36 -0700, Greg KH wrote:
> On Sat, Mar 16, 2013 at 10:11:22PM -0600, Alex Williamson wrote:
> > On Sat, 2013-03-16 at 18:03 -0700, Greg KH wrote:
> > > On Sat, Mar 16, 2013 at 05:50:53PM -0600, Myron Stowe wrote:
> > > > On Sat, 2013-03-16 at 15:11 -0700, Greg KH wrote:
> > > > > On Sat, Mar 16, 2013 at 03:35:19PM -0600, Myron Stowe wrote:
> > > > > > Sysfs includes entries to memory that backs a PCI device's BARs, both I/O
> > > > > > Port space and MMIO. This memory regions correspond to the device's
> > > > > > internal status and control registers used to drive the device.
> > > > > >
> > > > > > Accessing these registers from userspace such as "udevadm info
> > > > > > --attribute-walk --path=/sys/devices/..." does can not be allowed as
> > > > > > such accesses outside of the driver, even just reading, can yield
> > > > > > catastrophic consequences.
> > > > > >
> > > > > > Udevadm-info skips parsing a specific set of sysfs entries including
> > > > > > 'resource'. This patch extends the set to include the additional
> > > > > > 'resource<N>' entries that correspond to a PCI device's BARs.
> > > > >
> > > > > Nice, are you also going to patch bash to prevent a user from reading
> > > > > these sysfs files as well? :)
> > > > >
> > > > > And pciutils?
> > > > >
> > > > > You get my point here, right? The root user just asked to read all of
> > > > > the data for this device, so why wouldn't you allow it? Just like
> > > > > 'lspci' does. Or bash does.
> > > >
> > > > Yes :P , you raise a very good point, there are a lot of way a user can
> > > > poke around in those BARs. However, there is a difference between
> > > > shooting yourself in the foot and getting what you deserve versus
> > > > unknowingly executing a common command such as udevadm and having the
> > > > system hang.
> > > > >
> > > > > If this hardware has a problem, then it needs to be fixed in the kernel,
> > > > > not have random band-aids added to various userspace programs to paper
> > > > > over the root problem here. Please fix the kernel driver and all should
> > > > > be fine. No need to change udevadm.
> > > >
> > > > Xiangliang initially proposed a patch within the PCI core. Ignoring the
> > > > specific issue with the proposal which I pointed out in the
> > > > https://lkml.org/lkml/2013/3/7/242 thread, that just doesn't seem like
> > > > the right place to effect a change either as PCI's core isn't concerned
> > > > with the contents or access limitations of those regions, those are
> > > > issues that the driver concerns itself with.
> > > >
> > > > So things seem to be gravitating towards the driver. I'm fairly
> > > > ignorant of this area but as Robert succinctly pointed out in the
> > > > originating thread - the AHCI driver only uses the device's MMIO region.
> > > > The I/O related regions are for legacy SFF-compatible ATA ports and are
> > > > not used to driver the device. This, coupled with the observance that
> > > > userspace accesses such as udevadm, and others like you additionally
> > > > point out, do not filter through the device's driver for seems to
> > > > suggest that changes to the driver will not help here either.
> > >
> > > A PCI quirk should handle this properly, right? Why not do that? Worse
> > > thing, the quirk could just not expose these sysfs files for this
> > > device, which would solve all userspace program issues, right?
> >
> > Not exactly. I/O port access through pci-sysfs was added for userspace
> > programs, specifically qemu-kvm device assignment. We use the I/O port
> > resource# files to access device owned I/O port registers using file
> > permissions rather than global permissions such as iopl/ioperm. File
> > permissions also prevent random users from accessing device registers
> > through these files, but of course can't stop a privileged app that
> > chooses to ignore the purpose of these files. A quirk would therefore
> > remove a file that actually has a useful purpose for one app just so
> > another app that has no particular reason for dumping the contents can
> > run unabated. Thanks,
>
> The quirk would only be for this one specific device, which obviously
> can't handle this type of access, so why would you want the sysfs files
> even present for it at all?
I'm assuming that the device only breaks because udevadm is dumping the
full I/O port register space of the device and that if an actual driver
was interacting with it through this interface that it would work. Who
knows how many devices will have read side-effects by udevadm blindly
dumping these files. Thanks,
Alex
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox