Problem with ioctl command TCGETS

Problem with ioctl command TCGETS

[Ozan Eren Bilgen]

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

Re: Problem with ioctl command TCGETS

[Al Viro]

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.

Re: Problem with ioctl command TCGETS

[Miklos Szeredi]

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

Re: Problem with ioctl command TCGETS

[Jan Engelhardt]

>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, www.gwdg.de

Re: Problem with ioctl command TCGETS

[Miklos Szeredi]

> 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

Re: Problem with ioctl command TCGETS

[Bernd Eckenfels]

In article <E1CYLpf-0001VQ-00@dorka.pomaz.szeredi.hu> 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

Re: Problem with ioctl command TCGETS

[Wichert Akkerman]

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 <wichert@wiggy.net>    It is simple to make things.
http://www.wiggy.net/                   It is hard to make things simple.

Re: Problem with ioctl command TCGETS

[Miklos Szeredi]

> 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

Re: Problem with ioctl command TCGETS

[Bernd Eckenfels]

In article <20041128105646.GD22793@wiggy.net> 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

Re: Problem with ioctl command TCGETS

[Al Viro]

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?

Re: Problem with ioctl command TCGETS

[Bernd Eckenfels]

In article <E1CYN7z-0001bZ-00@dorka.pomaz.szeredi.hu> 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

Re: Problem with ioctl command TCGETS

[Miklos Szeredi]

> 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

Re: Problem with ioctl command TCGETS

[Al Viro]

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

Re: Problem with ioctl command TCGETS

[Miklos Szeredi]

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

Re: Problem with ioctl command TCGETS

[Al Viro]

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.

Re: Problem with ioctl command TCGETS

[Tomas Carnecky]

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

Re: Problem with ioctl command TCGETS

[Al Viro]

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

Re: Problem with ioctl command TCGETS

[Tomas Carnecky]

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

Re: Problem with ioctl command TCGETS

[Tomas Carnecky]

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

Re: Problem with ioctl command TCGETS

[Al Viro]

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.

Re: Problem with ioctl command TCGETS

[Tomas Carnecky]

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

Re: Problem with ioctl command TCGETS

[Al Viro]

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.

Re: Problem with ioctl command TCGETS

[Alan Cox]

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.

Re: Problem with ioctl command TCGETS

[Tomas Carnecky]

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

Re: Problem with ioctl command TCGETS

[Miklos Szeredi]

> 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

Re: Problem with ioctl command TCGETS

[Pavel Machek]

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         

Re: Problem with ioctl command TCGETS

[Al Viro]

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?

Re: Problem with ioctl command TCGETS

[Ingo Oeser]

You wrote:
[read/write vs ioctl]
> > Tell me how?  E.g. how would you set/get sound stream parameters if
> > not with ioctl()?
>
> Have several related files.

Look at your monitor. Technically this is an output device.
But there are little controls, where you can make adjustments.

ioctl is nothing else: A controller for an io-stream.

It also makes quite clear, that we are packetizing read/writes now.
And it cannot be fdopen()ed ;-)

Maybe we COULD split strictly for reads and writes, but
we still need a side channel for that to be opened by
passing a file descriptor.

How else would you control a tty/pty which you got as stdin/stdout?

Maybe one could hack this into xattr-support? ;-)

Regards

Ingo Oeser