linux-kernel - Re: [PATCH 72/80] tty: fix up gigaset a bit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <48FB27F1.9010007@imap.cc>
Date:	Sun, 19 Oct 2008 14:28:33 +0200
From:	Tilman Schmidt <tilman@...p.cc>
To:	Alan Cox <alan@...rguk.ukuu.org.uk>
CC:	torvalds@...ux-foundation.org, linux-kernel@...r.kernel.org,
	Hansjoerg Lipp <hjlipp@....de>
Subject: Re: [PATCH 72/80] tty: fix up gigaset a bit

Scripsit Alan Cox die 17.10.2008 13:40:
>>>  	/* prevent other callers from entering ldisc methods */
>>> +	/* FIXME: should use the tty state flags */
>>>  	tty->disc_data = NULL;
>>>  
>>>  	if (!cs->hw.ser)
>> 
>> Do you know of an example line discipline that has got this right?
>> My model for this code was drivers/net/ppp_async.c but now it seems
>> that this was not as exemplary as I had hoped.
> 
> If you want to know if the tty has been closed for further I/O then you
> can use test_bit(TTY_IO_ERROR, &tty->flags). This gets set at the right
> points on close/hangup/etc.

Ok, but where do you see a need for such a test? AFAICS I have covered
all the eventualities without resorting to an explicit tty->flags test,
and looking around among my LD brethren it looks like they came to the
same conclusion. (Not that I'd want to pull the "a million flies can't
be wrong" card. ;-) Can you point me to a place or situation where an
additional test_bit(TTY_IO_ERROR, &tty->flags) would catch a case that
would otherwise be handled incorrectly?

>> Looking around, I see that many LDs don't even provide a poll method.
>> So I'm thinking of just dropping this one. Would that be ok?
> 
> PPP doesn't route characters via the tty interface while you seem to do
> so for the AT emulation so you do need it.

Actually I don't. The AT command interface is accessed through a
separate tty interface created by the main module of the Gigaset driver
in drivers/isdn/gigaset/interface.c.

(The reason for that is that ser_gigaset is just one of three hardware
backends for accessing Gigaset devices, and the only one with an
existing tty device. I have to create my own AT command interface
device for the other two, anyway. Specialcasing ser_gigaset in order to
route that back to the tty device of the serial port would just
complicate things needlessly.)

So I think removing the poll method is the right thing to do. If you
have no objections, I'll submit a patch doing just that. (Hoping to
still make it for the .28 merge window.)

Thanks,
Tilman

Download attachment "signature.asc" of type "application/pgp-signature" (255 bytes)