2004-11-28 00:23:05

by Ozan Eren Bilgen

[permalink] [raw]
Subject: Problem with ioctl command TCGETS

--- Please CC me your responses ---


Hi,

While tracking KPPP and modem interaction, I experienced a problem with
TCGETS ioctl command, which is defined as constant number 0x5401 in
"include/asm/ioctls.h". If you decode TCGETS, you will obtain:

type: 'T'
direction: _IOC_NONE
number: 1
size: 0

but applications (like KPPP) cause indirectly that the modem descriptor
is ioctl'd by passing a string, as the example in
http://www.ussg.iu.edu/hypermail/linux/kernel/9904.0/0371.html shows:

fd = open("/dev/ttyS1", O_RDWR|O_NONBLOCK)
fcntl(fd, F_GETFD)
fcntl(fd, F_SETFD, FD_CLOEXEC)
fcntl(fd, F_GETFL)
ioctl(fd, TCGETS, {B9600 opost isig icanon echo ...}) = 0

CMIIW, TCGETS ioctl command should not interest with its argp, but
drivers/char/tty_ioctl.c does.

case TCGETS:
if (kernel_termios_to_user_termios((struct termios __user *)arg,
real_tty->termios))
return -EFAULT;
return 0;

I have a few questions:

1. Is it nice to break _IO macros?
2. If it has a historical reason, shall I forget to trust to the
informations that I decoded using _IO* macros?
3. Is there a list of such amazing commands?



Thanks in advance,
Ozan Eren BILGEN



2004-11-28 00:39:06

by Al Viro

[permalink] [raw]
Subject: Re: Problem with ioctl command TCGETS

On Sun, Nov 28, 2004 at 02:22:51AM +0200, Ozan Eren Bilgen wrote:
> 1. Is it nice to break _IO macros?

There is nothing nice about ioctls.

> 2. If it has a historical reason, shall I forget to trust to the
> informations that I decoded using _IO* macros?

You should.

> 3. Is there a list of such amazing commands?

There isn't.

2004-11-28 09:59:33

by Miklos Szeredi

[permalink] [raw]
Subject: Re: Problem with ioctl command TCGETS


> > 1. Is it nice to break _IO macros?
>
> There is nothing nice about ioctls.

On the subject of ioctls: what about a replacement syscall:

/**
* Getting and setting file parameters safely (ioctl done right)
*
* @fd file descriptor
* @param name of the parameter to get/set
* @dir direction flag indicating either get, set, or set-get
* @value value to set parameter to (set) or store current value into (get)
* @size size of value
*/
int fparam(int fd, const char *param, int dir, void *value, size_t size);

I know it's been talked about in the past. Is anyone interested?

Miklos

2004-11-28 10:08:55

by Jan Engelhardt

[permalink] [raw]
Subject: Re: Problem with ioctl command TCGETS

>On the subject of ioctls: what about a replacement syscall:
>
> /**
> * Getting and setting file parameters safely (ioctl done right)
> *
> * @fd file descriptor
> * @param name of the parameter to get/set
> * @dir direction flag indicating either get, set, or set-get
> * @value value to set parameter to (set) or store current value into (get)
> * @size size of value
> */
> int fparam(int fd, const char *param, int dir, void *value, size_t size);
>
>I know it's been talked about in the past. Is anyone interested?

The idea is nice, yet the "dir" and "size" parameters in the original _IO*
macros are only there (IMO) to protect against using the wrong value for the
wrong operation on the wrong fd/file/device/socket/etc.

What is the point in making "param" a char*? You would need to parse it down
again.

Oh yeah and call it ioctl2(), sounds more cryptical :)



Jan Engelhardt
--
Gesellschaft für Wissenschaftliche Datenverarbeitung
Am Fassberg, 37077 Göttingen, http://www.gwdg.de

2004-11-28 10:19:01

by Miklos Szeredi

[permalink] [raw]
Subject: Re: Problem with ioctl command TCGETS

> The idea is nice, yet the "dir" and "size" parameters in the original _IO*
> macros are only there (IMO) to protect against using the wrong value for the
> wrong operation on the wrong fd/file/device/socket/etc.

Size and dir are there to make the memory passed to the syscall easily
verifiable (like read/write/getsockopt/setsockopt/etc)

> What is the point in making "param" a char*? You would need to parse it down
> again.

'char *' namespace is easier to manage than 'int' namespace.

> Oh yeah and call it ioctl2(), sounds more cryptical :)

Is that an advantage?

Thanks,
Miklos

2004-11-28 10:28:35

by Bernd Eckenfels

[permalink] [raw]
Subject: Re: Problem with ioctl command TCGETS

In article <[email protected]> you wrote:
> * @fd file descriptor
> * @param name of the parameter to get/set
> * @dir direction flag indicating either get, set, or set-get
> * @value value to set parameter to (set) or store current value into (get)
> * @size size of value
> */
> int fparam(int fd, const char *param, int dir, void *value, size_t size);
>
> I know it's been talked about in the past. Is anyone interested?

The set-get is supposed to be used for queries, too? The size of value is
only used for the get case to describe the buffer length in that case?
because otherwise the set-get case may require a short value in and a large
answer structure out.

The syscall should also allow cunking in response, unless we remove all
high-volumne answers from it.

Gruss
Bernd

2004-11-28 10:56:50

by Wichert Akkerman

[permalink] [raw]
Subject: Re: Problem with ioctl command TCGETS

Previously Bernd Eckenfels wrote:
> The syscall should also allow cunking in response, unless we remove all
> high-volumne answers from it.

Are we reinventing netlink?

Wichert.

--
Wichert Akkerman <[email protected]> It is simple to make things.
http://www.wiggy.net/ It is hard to make things simple.

2004-11-28 11:22:13

by Miklos Szeredi

[permalink] [raw]
Subject: Re: Problem with ioctl command TCGETS

> The set-get is supposed to be used for queries, too? The size of value is
> only used for the get case to describe the buffer length in that case?
> because otherwise the set-get case may require a short value in and a large
> answer structure out.

You misunderstand the motivation. This is to get/set small compact
parameters, not huge structures or big data. Think get/setsockopt().

Miklos

2004-11-28 11:52:20

by Bernd Eckenfels

[permalink] [raw]
Subject: Re: Problem with ioctl command TCGETS

In article <[email protected]> you wrote:
> Previously Bernd Eckenfels wrote:
>> The syscall should also allow cunking in response, unless we remove all
>> high-volumne answers from it.
>
> Are we reinventing netlink?

Yes, meybe high volumne stuff via netlink and parameters via sysctl
interface. Hmm NETLINK on file FDs? :)

Greetings
Bernd

2004-11-28 12:18:04

by Al Viro

[permalink] [raw]
Subject: Re: Problem with ioctl command TCGETS

On Sun, Nov 28, 2004 at 12:22:03PM +0100, Miklos Szeredi wrote:
> > The set-get is supposed to be used for queries, too? The size of value is
> > only used for the get case to describe the buffer length in that case?
> > because otherwise the set-get case may require a short value in and a large
> > answer structure out.
>
> You misunderstand the motivation. This is to get/set small compact
> parameters, not huge structures or big data. Think get/setsockopt().

Think read(2)/write(2). We already have several barfbags too many,
and that includes both ioctl() and setsockopt(). We are stuck with
them for compatibility reasons, but why the hell would we need yet
another one?

2004-11-28 12:23:48

by Bernd Eckenfels

[permalink] [raw]
Subject: Re: Problem with ioctl command TCGETS

In article <[email protected]> you wrote:
> You misunderstand the motivation. This is to get/set small compact
> parameters, not huge structures or big data. Think get/setsockopt().

I understood it quite well. Looks like sys_sysctl to me.

But dont call it a ioctl replacement, since the stuff which causes most
problems will not be handled via that, but more with solutions like netlink.

Greetings
Bernd

2004-11-28 12:36:23

by Miklos Szeredi

[permalink] [raw]
Subject: Re: Problem with ioctl command TCGETS

> Think read(2)/write(2). We already have several barfbags too many,
> and that includes both ioctl() and setsockopt(). We are stuck with
> them for compatibility reasons, but why the hell would we need yet
> another one?

You can't replace either ioctl() or setsockopt() with read/write can
you? Both of them set out-of-band information on file descriptors.

Proc maybe ('/proc/PID/fdparam/FD/param')? I'm sure some people would
have objections to that too.

Miklos

2004-11-28 12:48:51

by Al Viro

[permalink] [raw]
Subject: Re: Problem with ioctl command TCGETS

On Sun, Nov 28, 2004 at 01:32:17PM +0100, Miklos Szeredi wrote:
> > Think read(2)/write(2). We already have several barfbags too many,
> > and that includes both ioctl() and setsockopt(). We are stuck with
> > them for compatibility reasons, but why the hell would we need yet
> > another one?
>
> You can't replace either ioctl() or setsockopt() with read/write can
> you? Both of them set out-of-band information on file descriptors.

Out-of-band == should be on a separate channel...

2004-11-28 12:53:00

by Miklos Szeredi

[permalink] [raw]
Subject: Re: Problem with ioctl command TCGETS


> > > Think read(2)/write(2). We already have several barfbags too many,
> > > and that includes both ioctl() and setsockopt(). We are stuck with
> > > them for compatibility reasons, but why the hell would we need yet
> > > another one?
> >
> > You can't replace either ioctl() or setsockopt() with read/write can
> > you? Both of them set out-of-band information on file descriptors.
>
> Out-of-band == should be on a separate channel...

Tell me how? E.g. how would you set/get sound stream parameters if
not with ioctl()?

Miklos

2004-11-28 13:03:24

by Al Viro

[permalink] [raw]
Subject: Re: Problem with ioctl command TCGETS

On Sun, Nov 28, 2004 at 01:52:41PM +0100, Miklos Szeredi wrote:
>
> > > > Think read(2)/write(2). We already have several barfbags too many,
> > > > and that includes both ioctl() and setsockopt(). We are stuck with
> > > > them for compatibility reasons, but why the hell would we need yet
> > > > another one?
> > >
> > > You can't replace either ioctl() or setsockopt() with read/write can
> > > you? Both of them set out-of-band information on file descriptors.
> >
> > Out-of-band == should be on a separate channel...
>
> Tell me how? E.g. how would you set/get sound stream parameters if
> not with ioctl()?

Have several related files.

2004-11-28 13:03:49

by Tomas Carnecky

[permalink] [raw]
Subject: Re: Problem with ioctl command TCGETS

Al Viro wrote:
> On Sun, Nov 28, 2004 at 12:22:03PM +0100, Miklos Szeredi wrote:
>
>>>The set-get is supposed to be used for queries, too? The size of value is
>>>only used for the get case to describe the buffer length in that case?
>>>because otherwise the set-get case may require a short value in and a large
>>>answer structure out.
>>
>>You misunderstand the motivation. This is to get/set small compact
>>parameters, not huge structures or big data. Think get/setsockopt().
>
>
> Think read(2)/write(2). We already have several barfbags too many,
> and that includes both ioctl() and setsockopt(). We are stuck with
> them for compatibility reasons, but why the hell would we need yet
> another one?

And what's the option? So without ioctl, how would you reaplace this:
ioctl(cdrom_fd, CDROMEJECT, 0)?

tom

2004-11-28 13:12:03

by Al Viro

[permalink] [raw]
Subject: Re: Problem with ioctl command TCGETS

On Sun, Nov 28, 2004 at 02:07:04PM +0100, Tomas Carnecky wrote:
> >Think read(2)/write(2). We already have several barfbags too many,
> >and that includes both ioctl() and setsockopt(). We are stuck with
> >them for compatibility reasons, but why the hell would we need yet
> >another one?
>
> And what's the option? So without ioctl, how would you reaplace this:
> ioctl(cdrom_fd, CDROMEJECT, 0)?

Which part of "we are stuck with them" is not clear enough? If you insist
on using the same descriptor for data and for out-of-band mess - no, you
can't get anything saner. If you do not, you can; it's that simple...

2004-11-28 13:18:22

by Tomas Carnecky

[permalink] [raw]
Subject: Re: Problem with ioctl command TCGETS

Al Viro wrote:
> On Sun, Nov 28, 2004 at 02:07:04PM +0100, Tomas Carnecky wrote:
>
>>>Think read(2)/write(2). We already have several barfbags too many,
>>>and that includes both ioctl() and setsockopt(). We are stuck with
>>>them for compatibility reasons, but why the hell would we need yet
>>>another one?
>>
>>And what's the option? So without ioctl, how would you reaplace this:
>>ioctl(cdrom_fd, CDROMEJECT, 0)?
>
> Which part of "we are stuck with them" is not clear enough? If you insist
> on using the same descriptor for data and for out-of-band mess - no, you
> can't get anything saner. If you do not, you can; it's that simple...

Ok, I know ioctl is bad, but please tell me how to replace them, that's
all I want to know. Did you propose any other mechanism which could
replace ioctl? Or do you think such things (eject the cdrom) are not
meant to be done from applications?
I certainly don't insist on using them, but I don't see (yet) any way
how to replace them. Please enlighten me.

tom

2004-11-28 13:19:25

by Tomas Carnecky

[permalink] [raw]
Subject: Re: Problem with ioctl command TCGETS

Miklos Szeredi wrote:
>>>>Think read(2)/write(2). We already have several barfbags too many,
>>>>and that includes both ioctl() and setsockopt(). We are stuck with
>>>>them for compatibility reasons, but why the hell would we need yet
>>>>another one?
>>>
>>>You can't replace either ioctl() or setsockopt() with read/write can
>>>you? Both of them set out-of-band information on file descriptors.
>>
>>Out-of-band == should be on a separate channel...
>
>
> Tell me how? E.g. how would you set/get sound stream parameters if
> not with ioctl()?
>

Maybe using sysfs? /sys/device/cdrom/param ?
But then you'd have to open another file :(

And what about somethink like:
cdrom_fd = open("/dev/cdrom", O_RDWR)
cdrom_param_fd = get_param_fd(cdrom_fd) /* a new syscall */
Now read/write to this param fd.
And two new entries in the struct file_operations:
write_param([same args as write])
read_param([same args as read])

tom

2004-11-28 14:07:46

by Al Viro

[permalink] [raw]
Subject: Re: Problem with ioctl command TCGETS

On Sun, Nov 28, 2004 at 02:20:19PM +0100, Tomas Carnecky wrote:
> But then you'd have to open another file :(

Correct, but not necessary on sysfs.

> And what about somethink like:
> cdrom_fd = open("/dev/cdrom", O_RDWR)
> cdrom_param_fd = get_param_fd(cdrom_fd) /* a new syscall */
> Now read/write to this param fd.
> And two new entries in the struct file_operations:
> write_param([same args as write])
> read_param([same args as read])

That assumes that there is any sort of uniform semantics for these
operations. There isn't. Moreover, you are insisting on pushing
all of them into the same channel; not a good idea since the set
of things done with ioctls tends to consist of several unrelated
classes, often coming from a bunch of unrelated subsystems.

There is no mechanical replacement for ioctl(); the nature of its
problems is that we have a random mix of unrelated operations bumped
into one pile.

Take a look at e.g. networking ioctls. Most of them openly ignores the
descriptor used to issue an ioctl - more often then not the first thing
they do is to peek into the passed data structure and go looking for
the real object we are going to operate upon; e.g. find an interface by
name. Of course it's bogus; any sane modification of that API would
have the object selected by the opened file we are passing to it.

And no, we have no chance in hell to rewrite all userland code that
uses these suckers, so we are stuck with them for all forseeable future.
UCB folks had no taste, film at 11...

For more or less common (read: implemented by more than a couple of drivers)
ioctls we have to keep them anyway; for the stuff where we really stand
a chance of doing some kind of changes (including the new operations) we
can bloody well do splitup by files that would match the nature of operations.
Which leaves the "get me secondary channel by fd" kind of operations
without any uses.

2004-11-28 14:28:27

by Tomas Carnecky

[permalink] [raw]
Subject: Re: Problem with ioctl command TCGETS

Al Viro wrote:
> On Sun, Nov 28, 2004 at 01:52:41PM +0100, Miklos Szeredi wrote:
>
>>>>>Think read(2)/write(2). We already have several barfbags too many,
>>>>>and that includes both ioctl() and setsockopt(). We are stuck with
>>>>>them for compatibility reasons, but why the hell would we need yet
>>>>>another one?
>>>>
>>>>You can't replace either ioctl() or setsockopt() with read/write can
>>>>you? Both of them set out-of-band information on file descriptors.
>>>
>>>Out-of-band == should be on a separate channel...
>>
>>Tell me how? E.g. how would you set/get sound stream parameters if
>>not with ioctl()?
>
>
> Have several related files.

You mean.. like nvidia?
/dev/nvidiactl
/dev/nvidia0
/dev/nvidia1
/dev/nvidia2
and do read/write on /dev/nvidiactl (instead on ioctl)?

tom

2004-11-28 15:28:04

by Al Viro

[permalink] [raw]
Subject: Re: Problem with ioctl command TCGETS

On Sun, Nov 28, 2004 at 03:30:19PM +0100, Tomas Carnecky wrote:
> You mean.. like nvidia?
> /dev/nvidiactl
> /dev/nvidia0
> /dev/nvidia1
> /dev/nvidia2
> and do read/write on /dev/nvidiactl (instead on ioctl)?

Really depends on situation - in some cases that's the obvious clean
variant, in some you might want something more specific. Usually
it helps to ask "what object am I working with?" and see if it gives
a reasonable picture. Note, BTW, that your example (eject) actually
demonstrates what kind of ugliness can be created by piling everything
together - the logics around "it's currently used, do not eject and
return -EBUSY" is broken and unfixable in all cdrom drivers. Broken
exactly because we need to open device itself to issue eject request.
Think what happens if we get
fd = open("/dev/cdrom", 0);
if (fork()) {
read a lot from that sucker
} else {
sleep for a while
ioctl(fd, CDROMEJECT, 0);
}
>From the driver point of view, we have only one opener. There's no way
to tell how many processes might have file descriptors that point to
what we'd opened back then. So we either need to keep track of all
changes in descriptor tables and provide exclusion between that and
ioctls (have fun) or admit that driver might be hit with eject in the
middle of IO, all logics along the lines of "it's opened by somebody,
no eject for us" nonwithstanding.

And then there are horrors like cciss special-casing the open of 1st disk
on a controller (even if there's none) so that we could talk to controller
itself (in particular, tell it to go look for disks that might be attached
to it now). It gets very ugly; same for RAID array creation, same for
loop device setup and races around it, etc.

2004-11-28 18:22:05

by Tomas Carnecky

[permalink] [raw]
Subject: Re: Problem with ioctl command TCGETS

Al Viro wrote:
> On Sun, Nov 28, 2004 at 03:30:19PM +0100, Tomas Carnecky wrote:
>
>>You mean.. like nvidia?
>>/dev/nvidiactl
>>/dev/nvidia0
>>/dev/nvidia1
>>/dev/nvidia2
>>and do read/write on /dev/nvidiactl (instead on ioctl)?
>
>
> Really depends on situation - in some cases that's the obvious clean
> variant, in some you might want something more specific.

The 'good' thing on ioctl is that _every_ device supports that. It
doesn't matter which device you open, each one supports ioctl.
Now if each driver cooks up it's own replacement.. and everyone
knows that developers don't really like to document their code.. :/

Was there ever a thread on lkml about a ioctl replacement? I'd
really like to read about proposals, so far everyone talks only about
replacing it... but no one wants to say how _exactly_.

tom

2004-11-28 18:46:34

by Alan

[permalink] [raw]
Subject: Re: Problem with ioctl command TCGETS

On Sul, 2004-11-28 at 12:18, Al Viro wrote:
> Think read(2)/write(2). We already have several barfbags too many,
> and that includes both ioctl() and setsockopt(). We are stuck with
> them for compatibility reasons, but why the hell would we need yet
> another one?

Synchronization.

2004-11-28 18:51:55

by Miklos Szeredi

[permalink] [raw]
Subject: Re: Problem with ioctl command TCGETS

> Was there ever a thread on lkml about a ioctl replacement? I'd
> really like to read about proposals, so far everyone talks only about
> replacing it... but no one wants to say how _exactly_.

I remember from one in May 2001 with a subject "RFD w/info-PATCH]
device arguments from lookup)" and it's offshoots. It's not all about
ioctls, but there are proposals for improvements to the ioctl
interface and other interesting stuff. Link to start of thread in
MARC:

http://marc.theaimsgroup.com/?l=linux-kernel&m=99025355313916&w=2

Miklos

2004-11-30 13:30:57

by Pavel Machek

[permalink] [raw]
Subject: Re: Problem with ioctl command TCGETS

Hi!

> > > The set-get is supposed to be used for queries, too? The size of value is
> > > only used for the get case to describe the buffer length in that case?
> > > because otherwise the set-get case may require a short value in and a large
> > > answer structure out.
> >
> > You misunderstand the motivation. This is to get/set small compact
> > parameters, not huge structures or big data. Think get/setsockopt().
>
> Think read(2)/write(2). We already have several barfbags too many,
> and that includes both ioctl() and setsockopt(). We are stuck with
> them for compatibility reasons, but why the hell would we need yet
> another one?

Passing structure using read/write is evil, because there's nowhere to hook
32/64 bit translation.
--
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms

2004-11-30 15:48:12

by Al Viro

[permalink] [raw]
Subject: Re: Problem with ioctl command TCGETS

On Tue, Nov 30, 2004 at 02:26:19PM +0100, Pavel Machek wrote:
> > Think read(2)/write(2). We already have several barfbags too many,
> > and that includes both ioctl() and setsockopt(). We are stuck with
> > them for compatibility reasons, but why the hell would we need yet
> > another one?
>
> Passing structure using read/write is evil, because there's nowhere to hook
> 32/64 bit translation.

No, really? And we must use different layouts for 32 and 64 bit platforms
for which religious reasons, exactly?