This is like /dev/port except not broken. /dev/port will translate all
read/write into inb/outb streams, which is wrong since hardware can and
does care about cycle size. /dev/io will only allow 1, 2, or 4 byte
access, and will translate that into the appropriate bus cycle size.
Signed-off-by: Adam Jackson <[email protected]>
---
drivers/char/mem.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 92 insertions(+), 0 deletions(-)
diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index d6e9d08..276b0e5 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -611,6 +611,88 @@ static ssize_t write_port(struct file *file, const char __user *buf,
*ppos = i;
return tmp-buf;
}
+
+static ssize_t read_io(struct file *file, char __user *buf, size_t count,
+ loff_t *ppos)
+{
+ unsigned long port = *ppos;
+ const char __user * tmp = buf;
+
+ switch (count) {
+ case 1:
+ case 2:
+ case 4:
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ if (!access_ok(VERIFY_WRITE, buf, count))
+ return -EFAULT;
+
+ switch (count) {
+ case 1:
+ if (__put_user(inb(port), tmp) < 0)
+ return -EFAULT;
+ break;
+ case 2:
+ if (__put_user(inw(port), tmp) < 0)
+ return -EFAULT;
+ break;
+ case 4:
+ if (__put_user(inl(port), tmp) < 0)
+ return -EFAULT;
+ break;
+ }
+
+ *ppos += count;
+ return count;
+}
+
+static ssize_t write_io(struct file *file, const char __user *buf, size_t count,
+ loff_t *ppos)
+{
+ unsigned long port = *ppos;
+ const char __user * tmp = buf;
+ u8 byte;
+ u16 word;
+ u32 dword;
+
+ switch (count) {
+ case 1:
+ case 2:
+ case 4:
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ if (!access_ok(VERIFY_READ, buf, count))
+ return -EFAULT;
+
+ switch (count) {
+ case 1:
+ if (__get_user(byte, tmp))
+ return -EFAULT;
+ outb(byte, port);
+ break;
+ case 2:
+ if (__get_user(word, tmp))
+ return -EFAULT;
+ outw(word, port);
+ break;
+ case 4:
+ if (__get_user(dword, tmp))
+ return -EFAULT;
+ outl(dword, port);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ *ppos += count;
+ return count;
+}
#endif
static ssize_t read_null(struct file *file, char __user *buf,
@@ -774,6 +856,13 @@ static const struct file_operations port_fops = {
.write = write_port,
.open = open_port,
};
+
+static const struct file_operations io_fops = {
+ .llseek = memory_lseek,
+ .read = read_io,
+ .write = write_io,
+ .open = open_port,
+};
#endif
static const struct file_operations zero_fops = {
@@ -867,6 +956,9 @@ static const struct memdev {
#ifdef CONFIG_CRASH_DUMP
[12] = { "oldmem", 0, &oldmem_fops, NULL },
#endif
+#ifdef CONFIG_DEVPORT
+ [13] = { "io", 0, &io_fops, NULL },
+#endif
};
static int memory_open(struct inode *inode, struct file *filp)
--
1.7.7.6
Signed-off-by: Adam Jackson <[email protected]>
---
Documentation/feature-removal-schedule.txt | 13 +++++++++++++
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
index a0ffac0..c8504f0 100644
--- a/Documentation/feature-removal-schedule.txt
+++ b/Documentation/feature-removal-schedule.txt
@@ -524,3 +524,16 @@ Files: arch/arm/mach-at91/at91cap9.c
Why: The code is not actively maintained and platforms are now hard to find.
Who: Nicolas Ferre <[email protected]>
Jean-Christophe PLAGNIOL-VILLARD <[email protected]>
+
+----------------------------
+
+What: /dev/port
+When: February 2013
+Files: drivers/char/mem.c
+Why: This interface does not match either ISA or PCI semantics for I/O
+ port access. Regardless of the cycle size requested by userspace
+ /dev/port will emit byte-wide reads or writes, which is wrong
+ since devices may not allow byte access or may have side effects
+ when doing byte access to word or dword registers. Use /dev/io
+ instead.
+Who: Adam Jackson <[email protected]>
--
1.7.7.6
This is like /dev/port except not broken. /dev/port will translate all
read/write into inb/outb streams, which is wrong since hardware can and
does care about cycle size. /dev/io will only allow 1, 2, or 4 byte
access, and will translate that into the appropriate bus cycle size.
v2: Fix get/put_user types.
Signed-off-by: Adam Jackson <[email protected]>
fixup
---
drivers/char/mem.c | 96 ++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 96 insertions(+), 0 deletions(-)
diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index d6e9d08..9fc3847 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -611,6 +611,92 @@ static ssize_t write_port(struct file *file, const char __user *buf,
*ppos = i;
return tmp-buf;
}
+
+static ssize_t read_io(struct file *file, char __user *buf, size_t count,
+ loff_t *ppos)
+{
+ unsigned long port = *ppos;
+ const u8 __user *btmp = (void __user *)buf;
+ const u16 __user *wtmp = (void __user *)buf;
+ const u32 __user *ltmp = (void __user *)buf;
+
+ switch (count) {
+ case 1:
+ case 2:
+ case 4:
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ if (!access_ok(VERIFY_WRITE, buf, count))
+ return -EFAULT;
+
+ switch (count) {
+ case 1:
+ if (__put_user(inb(port), btmp) < 0)
+ return -EFAULT;
+ break;
+ case 2:
+ if (__put_user(inw(port), wtmp) < 0)
+ return -EFAULT;
+ break;
+ case 4:
+ if (__put_user(inl(port), ltmp) < 0)
+ return -EFAULT;
+ break;
+ }
+
+ *ppos += count;
+ return count;
+}
+
+static ssize_t write_io(struct file *file, const char __user *buf, size_t count,
+ loff_t *ppos)
+{
+ unsigned long port = *ppos;
+ const u8 __user *btmp = (void __user *)buf;
+ const u16 __user *wtmp = (void __user *)buf;
+ const u32 __user *ltmp = (void __user *)buf;
+ u8 byte;
+ u16 word;
+ u32 dword;
+
+ switch (count) {
+ case 1:
+ case 2:
+ case 4:
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ if (!access_ok(VERIFY_READ, buf, count))
+ return -EFAULT;
+
+ switch (count) {
+ case 1:
+ if (__get_user(byte, btmp))
+ return -EFAULT;
+ outb(byte, port);
+ break;
+ case 2:
+ if (__get_user(word, wtmp))
+ return -EFAULT;
+ outw(word, port);
+ break;
+ case 4:
+ if (__get_user(dword, ltmp))
+ return -EFAULT;
+ outl(dword, port);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ *ppos += count;
+ return count;
+}
#endif
static ssize_t read_null(struct file *file, char __user *buf,
@@ -774,6 +860,13 @@ static const struct file_operations port_fops = {
.write = write_port,
.open = open_port,
};
+
+static const struct file_operations io_fops = {
+ .llseek = memory_lseek,
+ .read = read_io,
+ .write = write_io,
+ .open = open_port,
+};
#endif
static const struct file_operations zero_fops = {
@@ -867,6 +960,9 @@ static const struct memdev {
#ifdef CONFIG_CRASH_DUMP
[12] = { "oldmem", 0, &oldmem_fops, NULL },
#endif
+#ifdef CONFIG_DEVPORT
+ [13] = { "io", 0, &io_fops, NULL },
+#endif
};
static int memory_open(struct inode *inode, struct file *filp)
--
1.7.7.6
On Tue, 7 Feb 2012 18:39:45 -0500
Adam Jackson <[email protected]> wrote:
> This is like /dev/port except not broken. /dev/port will translate all
> read/write into inb/outb streams, which is wrong since hardware can and
> does care about cycle size. /dev/io will only allow 1, 2, or 4 byte
> access, and will translate that into the appropriate bus cycle size.
>From a security perspective /dev/[k][mem is a dumb bit of ancient Unix
history we'd dearly like to kill off. /dev/port is a similar early PC
unixism that wants to go the same way. /dev/io just adds another horror
to the pile.
Please do the decent thing, stop using /dev/mem and /dev/port for
anything. If you need to access an I/O device make it properly visible
via the kernel only for the ports and in a manner that is safe.
Alan
On 2/7/12 7:17 PM, Alan Cox wrote:
> On Tue, 7 Feb 2012 18:39:45 -0500
> Adam Jackson<[email protected]> wrote:
>
>> This is like /dev/port except not broken. /dev/port will translate all
>> read/write into inb/outb streams, which is wrong since hardware can and
>> does care about cycle size. /dev/io will only allow 1, 2, or 4 byte
>> access, and will translate that into the appropriate bus cycle size.
>
> From a security perspective /dev/[k][mem is a dumb bit of ancient Unix
> history we'd dearly like to kill off. /dev/port is a similar early PC
> unixism that wants to go the same way. /dev/io just adds another horror
> to the pile.
>
> Please do the decent thing, stop using /dev/mem and /dev/port for
> anything. If you need to access an I/O device make it properly visible
> via the kernel only for the ports and in a manner that is safe.
Yeah, I'll be sure to do that right as soon as I can stop supporting the
vesa driver. Until that time I don't really have any choice but to
expose the whole of I/O port space, since I have no idea what the video
BIOS is going to touch.
I don't disagree with wanting to limit access to these services, but
/dev/io is at least somewhat containable, whereas iopl is insane.
- ajax
> Yeah, I'll be sure to do that right as soon as I can stop supporting the
> vesa driver. Until that time I don't really have any choice but to
> expose the whole of I/O port space, since I have no idea what the video
> BIOS is going to touch.
I would be surprised if you couldn't make a very good guess, and many
things it might want to touch are things that need blocking/emulating
anyway.
> I don't disagree with wanting to limit access to these services, but
> /dev/io is at least somewhat containable, whereas iopl is insane.
They are both equally insane and have effectively identical security
semntics. Continuing to use iopl is both faster and avoids adding a kernel
API however. Even better it's x86 specific so that piece of manure
doesn't escape onto other platforms without the legacy vesa mess.
Alan
On 2/8/12 4:26 AM, Alan Cox wrote:
>> Yeah, I'll be sure to do that right as soon as I can stop supporting the
>> vesa driver. Until that time I don't really have any choice but to
>> expose the whole of I/O port space, since I have no idea what the video
>> BIOS is going to touch.
>
> I would be surprised if you couldn't make a very good guess, and many
> things it might want to touch are things that need blocking/emulating
> anyway.
I will be happy to write the code to emulate or block those ports in the
kernel if it's necessary, rather than needing to replicate them across
every userspace display server that wants to support vesa. We already
have a fair bit of it in xserver's int10 harness.
In the meantime, can I please have kernel services that work?
>> I don't disagree with wanting to limit access to these services, but
>> /dev/io is at least somewhat containable, whereas iopl is insane.
>
> They are both equally insane and have effectively identical security
> semntics. Continuing to use iopl is both faster and avoids adding a kernel
> API however. Even better it's x86 specific so that piece of manure
> doesn't escape onto other platforms without the legacy vesa mess.
Every point in this paragraph is at best misleading, if not outright wrong.
iopl does not have identical security semantics to ioperm. iopl lets my
X server disable interrupts. /dev/io would not, and would let me add a
per-port filter if desired. Code I am happy to write, for the record,
although since CAP_SYS_RAWIO is required regardless of filesystem
permissions it'd not do much besides prevent root from nuking the machine.
Your definition of "faster" is spurious. VBE calls are not a
performance path and system call overhead is negligible compared to I/O
serialization. If anything /dev/io can be _faster_ in the mainline case
because we'll no longer need to handle the ioperm bitmask on every
context switch.
The current patch set results in a net gain of zero kernel interfaces,
once /dev/port is put down in a year. I will admit that the current
/dev/port is probably not a meaningfully used API, seeing how it's been
broken since literally kernel 0.10. But it's there and enabled by
default. I would like it to work, please.
Invoking architecture-specificity is spurious. Competent architectures
have a usable framebuffer interface from the firmware, so vesa never
comes up. x86 does not. x86 has vesa, which is a support reality for
at _least_ the next three years of new hardware. Alternatively, x86 has
UEFI, a travesty from its very inception. Until that travesty has
managed to successfully infect every bootable x86 machine on the planet
vesa continues to be a thing I have to support.
Basically what I'm hearing here is "don't bother doing this well since
you already have a way you're doing it badly". That's crap. I've
written the code to do it well. I've signed up for the support cost.
Can I please have something good instead of something bad? I sort of
thought "good" was the whole point of what we're doing here.
- ajax
> > They are both equally insane and have effectively identical security
> > semntics. Continuing to use iopl is both faster and avoids adding a kernel
> > API however. Even better it's x86 specific so that piece of manure
> > doesn't escape onto other platforms without the legacy vesa mess.
>
> Every point in this paragraph is at best misleading, if not outright wrong.
Really - exactly where is it wrong. None of your claims below seems
valid. I think my statement is accurate even after your response.
>
> iopl does not have identical security semantics to ioperm. iopl lets my
I didn't mention ioperm. I said /dev/ports
> X server disable interrupts. /dev/io would not, and would let me add a
Yes it would - you can issue I/O accesses to the interrupt controller. So
let me repeat that - the two are the same thing and equally insane.
> Your definition of "faster" is spurious. VBE calls are not a
You are not the only user of iopl, and they are faster. You may not
*care* about the speed difference but the fact is they are faster.
> performance path and system call overhead is negligible compared to I/O
> serialization. If anything /dev/io can be _faster_ in the mainline case
> because we'll no longer need to handle the ioperm bitmask on every
> context switch.
See before - I never mentioned ioperm.
> The current patch set results in a net gain of zero kernel interfaces,
> once /dev/port is put down in a year. I will admit that the current
People will be shipping code using it for years.
> /dev/port is probably not a meaningfully used API, seeing how it's been
> broken since literally kernel 0.10. But it's there and enabled by
> default. I would like it to work, please.
So why hasn't it changed - because nobody has had a problem with it.
> Invoking architecture-specificity is spurious. Competent architectures
> have a usable framebuffer interface from the firmware, so vesa never
> comes up.
Exactly - so they don't need another port interface.
> Basically what I'm hearing here is "don't bother doing this well since
> you already have a way you're doing it badly". That's crap. I've
> written the code to do it well. I've signed up for the support cost.
> Can I please have something good instead of something bad? I sort of
> thought "good" was the whole point of what we're doing here.
I'm all for doing it well but that isn't doing it well. Its adding
another turd which is logically and functionally equivalent to the
existing one. Two turds is not usually better than one unless you are
running a methane digestion plant.
iopl is functionally equivalent to your io port code. It's already in the
kernel, it already works and it's already maintained.
To be useful a port interface really needs to aware of hotplug, device
mappings and the like. For VESA that's going to be pretty horrible both
because of the uncontrolled nature of the interface and because of the
vga "magic" both for ports and routing.
An interface where you could do something like
open a port channel
bind device X to port channel (imports its I/O ranges)
bind portrange A-B to port channel (for nasties like the vga
ports)
read/write in 1/2/4/(8 ?) byte chunks meaningfully
close
and which properly handled device hotplug - ie if you bind to device X, X
is unplugged then the next read/write errors, or at least the address
space is not freed until it isn't in the port range space.
Now that would be an improvement, but it seems to be a fair amount of
work.
Alan
On 2/8/12 9:32 AM, Alan Cox wrote:
>> iopl does not have identical security semantics to ioperm. iopl lets my
>
> I didn't mention ioperm. I said /dev/ports
This was a typo on my part. /dev/port was intended.
>> X server disable interrupts. /dev/io would not, and would let me add a
>
> Yes it would - you can issue I/O accesses to the interrupt controller. So
> let me repeat that - the two are the same thing and equally insane.
Way to cut out the bit about volunteering to write the filter (which we
could probably just steal whole from ACPI).
Besides, as noted (and cut), you need CAP_SYS_RAWIO anyway.
>> Your definition of "faster" is spurious. VBE calls are not a
>
> You are not the only user of iopl, and they are faster. You may not
> *care* about the speed difference but the fact is they are faster.
So people continuing to use iopl will continue to be faster for their
use case. X can switch off of it and be faster for its use case, in
which context switch overhead is infinitely more important than modeset
taking another hundred micros.
>> performance path and system call overhead is negligible compared to I/O
>> serialization. If anything /dev/io can be _faster_ in the mainline case
>> because we'll no longer need to handle the ioperm bitmask on every
>> context switch.
>
> See before - I never mentioned ioperm.
X is using ioperm to explicity disable access to some ports.
>> The current patch set results in a net gain of zero kernel interfaces,
>> once /dev/port is put down in a year. I will admit that the current
>
> People will be shipping code using it for years.
You know, one of us is going to need to cite their sources here. Thanks
for shutting down codesearch, google.
>> /dev/port is probably not a meaningfully used API, seeing how it's been
>> broken since literally kernel 0.10. But it's there and enabled by
>> default. I would like it to work, please.
>
> So why hasn't it changed - because nobody has had a problem with it.
And now that I have a problem with it, I'm being told that I don't have
a problem with it.
Charming.
>> Invoking architecture-specificity is spurious. Competent architectures
>> have a usable framebuffer interface from the firmware, so vesa never
>> comes up.
>
> Exactly - so they don't need another port interface.
Except to support the case of booting vesa cards on non-x86. A thing X
can actually do, you know.
> iopl is functionally equivalent to your io port code. It's already in the
> kernel, it already works and it's already maintained.
>
> To be useful a port interface really needs to aware of hotplug, device
> mappings and the like. For VESA that's going to be pretty horrible both
> because of the uncontrolled nature of the interface and because of the
> vga "magic" both for ports and routing.
>
> An interface where you could do something like
>
> open a port channel
> bind device X to port channel (imports its I/O ranges)
> bind portrange A-B to port channel (for nasties like the vga
> ports)
>
> read/write in 1/2/4/(8 ?) byte chunks meaningfully
>
> close
This isn't sufficient. VBIOS can call SBIOS. SBIOS can poke any port
on the mainboard it wants. There's no knowing a priori which ports on a
channel I would need to bind, a rather bright fellow named Turing made a
pretty good point about this once.
Yes, I have seen this in the wild.
More to the point I'm not concerned about hotplug here _at_ _all_. If
you want hotplug at this level you write a kernel driver. If you _want_
me to write a kernel driver for vesa that pretends to be
hotplug-compatible guess I can try, but the idea of an 8086 emulator in
the kernel hasn't gone down well before.
> and which properly handled device hotplug - ie if you bind to device X, X
> is unplugged then the next read/write errors, or at least the address
> space is not freed until it isn't in the port range space.
>
> Now that would be an improvement, but it seems to be a fair amount of
> work.
To solve a problem no one is having, or is going to have. And which
wouldn't be sufficient for vesa regardless, short of the range of bound
ports being 0 to 65535.
Whereas I've written something simple and supportable that does solve a
problem I am currently having. Something that _precisely_ matches the
semantics of sysfs' legacy_io file API, a thing I have to use anyway for
multi-domain machines. Something that doesn't pretend VESA hotplug is a
thing you can do because hey, guess what, it isn't.
Whatever. I can continue to do this badly in userspace, or we could
take some very small changes to make it slightly better, or we could do
some enormous overengineered thing to fail to solve a non-problem. Me,
I like simple and incremental. I guess that doesn't count for much.
- ajax
> Besides, as noted (and cut), you need CAP_SYS_RAWIO anyway.
We need to enforce CAP_SYS_RAWIO for port access pretty much anyhow
because otherwise you can get from have fs access to having device access.
If you have CAP_SYS_RAWIO you can use ioperm/iopl.
> use case. X can switch off of it and be faster for its use case, in
> which context switch overhead is infinitely more important than modeset
> taking another hundred micros.
That would only be the case for ioperm, not iopl. It's also a case you
can fix anyway. If a process clears its ioperm mask we can drop the
masks. That would help existing users too including existing X servers.
> > People will be shipping code using it for years.
>
> You know, one of us is going to need to cite their sources here. Thanks
> for shutting down codesearch, google.
It even turns up being used with dd to waggle parallel port devices
despite the fact they have a proper API (which is the nasty problem with
ill defined interfaces of that nature)
> > So why hasn't it changed - because nobody has had a problem with it.
>
> And now that I have a problem with it, I'm being told that I don't have
> a problem with it.
It would be useful if you'd actually try and explain the problem rather
than pick a fight. I said nobody "has had", not nobody "is having"
> Except to support the case of booting vesa cards on non-x86. A thing X
> can actually do, you know.
And at that point you again need a much more controlled interface because
any off device port is likely to be wrong.
>
> > iopl is functionally equivalent to your io port code. It's already in the
> > kernel, it already works and it's already maintained.
> >
> > To be useful a port interface really needs to aware of hotplug, device
> > mappings and the like. For VESA that's going to be pretty horrible both
> > because of the uncontrolled nature of the interface and because of the
> > vga "magic" both for ports and routing.
> >
> > An interface where you could do something like
> >
> > open a port channel
> > bind device X to port channel (imports its I/O ranges)
> > bind portrange A-B to port channel (for nasties like the vga
> > ports)
> >
> > read/write in 1/2/4/(8 ?) byte chunks meaningfully
> >
> > close
>
> This isn't sufficient. VBIOS can call SBIOS. SBIOS can poke any port
> on the mainboard it wants.
And may arbitarily corrupt the running of the system without any error
being detected - which I must admit was my primary experience of trying to
use the vesa support to boot a second video card some years back.
If it's not touching the video card registers then you should probably be
emulating it, because whatever it is touching isn't going to be safe - and
I guess for a lot of those ports you already are ?
What ports beyond the video card ones and the VGA range are getting
prodded by actual use cases ?
> More to the point I'm not concerned about hotplug here _at_ _all_. If
> you want hotplug at this level you write a kernel driver. If you _want_
The only kernel driver you need for such a device hotplug case is the
trivial generic 'pulling the rug from under you' handling. We sort of
deal with this already for mmio.
> To solve a problem no one is having, or is going to have. And which
> wouldn't be sufficient for vesa regardless, short of the range of bound
> ports being 0 to 65535.
If we are into thr world of problems no one is having then we have
iopl(). It works, it's in the kernel.
> Whatever. I can continue to do this badly in userspace, or we could
> take some very small changes to make it slightly better, or we could do
> some enormous overengineered thing to fail to solve a non-problem. Me,
> I like simple and incremental. I guess that doesn't count for much.
>From a kernel perspective APIs are things you get stuck with. Another
broken port interface isn't something I want us to be stuck with.
Especially when you've got iopl already and it's done the job fine for
about 17 years.
If there is a measurable performance cost then making ioperm spot empty
masks or providing a way to say 'I'm flushing my io permission map' ought
to do the trick or a helper thread in your app.
Alan