Skip to contentSkip to conversation

PATCH In review [email protected] 4 participants 9 messages Mar 21, 2004

lore
Share & tools
lore
#0 lore raw

This is just a random observation, I don't know this piece of code or the kernel in general, but instinct tells me that where is says "if (!fd) return -EBADF", it should say "if (!file) return -EBADF". Just a heads up.

Regards,
Rich

static int tiocgdev(unsigned fd, unsigned cmd, unsigned int *ptr) {

	struct file *file = fget(fd);
	struct tty_struct *real_tty;

	if (!fd)
		return -EBADF;
	if (file->f_op->ioctl != tty_ioctl)
		return -EINVAL; 
	real_tty = (struct tty_struct *)file->private_data;
	if (!real_tty) 	
		return -EINVAL; 
	return put_user(new_encode_dev(tty_devnum(real_tty)), ptr); 
} 
#1 lore raw

PATCH 1/1 fix tiocgdev 32/64bit emul

Quoted 8 lines · [email protected] wrote:

This is just a random observation, I don't know this piece of code or the kernel in general, but instinct tells me that where is says "if (!fd) return -EBADF", it should say "if (!file) return -EBADF". Just a heads up.

Regards,
Rich

Quoted 7 lines

static int tiocgdev(unsigned fd, unsigned cmd, unsigned int *ptr) {

	struct file *file = fget(fd);
	struct tty_struct *real_tty;

	if (!fd)
		return -EBADF;

Yup, looks like a real bug to me... good catch.

Untested but obvious patch attached.

Jeff

===== arch/x86_64/ia32/ia32_ioctl.c 1.38 vs edited =====

arch/x86_64/ia32/ia32_ioctl.c +1 -1
@@ -27,7 +27,7 @@
 	struct file *file = fget(fd);
 	struct tty_struct *real_tty;
 
-	if (!fd)
+	if (!file)
 		return -EBADF;
 	if (file->f_op->ioctl != tty_ioctl)
 		return -EINVAL; 
#2 lore raw

Re: [PATCH] fix tiocgdev 32/64bit emul

Quoted 3 lines · Jeff Garzik

Yup, looks like a real bug to me... good catch.

Untested but obvious patch attached.

Isn't that SuSE's strange ioctl hack that has been rejected for mainline multiple times? why does x86_64 have an emulation for it if the ioctl isn't implemented anyway?

#3 lore raw

Re: [PATCH] fix tiocgdev 32/64bit emul

Quoted 8 lines · Christoph Hellwig
Quoted 3 lines · Jeff Garzik

Yup, looks like a real bug to me... good catch.

Untested but obvious patch attached.

Isn't that SuSE's strange ioctl hack that has been rejected for mainline multiple times? why does x86_64 have an emulation for it if the ioctl isn't implemented anyway?

Since this pops up from time to time, please let me explain what TIOCGDEV does (if you know already, feel free to scoll to the end) and ask for alternative solutions.

Quoting http://marc.theaimsgroup.com/?l=linux-kernel&m=89051325309548

Quoted 8 lines · Miquel van Smoorenburg

This patch introduces a new ioctl, TIOCGDEV. It can be used to find out the _real_ device of the serial console.

I use this so I can first open /dev/console and a pty pair, then use TIOCCONS to redirect console output to the pty, and then I can copy the console output from the pty to both a logfile and the real console. This is useful for a boot-time daemon so all bootup messages can be captured and logged...

Further links to relevant messages:
http://marc.theaimsgroup.com/?l=linux-kernel&m=97696542915335
http://marc.theaimsgroup.com/?l=linux-kernel&m=97696701517101
http://marc.theaimsgroup.com/?l=linux-kernel&m=97696788418144

Quoting http://marc.theaimsgroup.com/?l=linux-kernel&m=102929921716152

Quoted 3 lines · Linus Torvalds

I've always hated the fact that all the boot-time messages get lost, simply because syslogd hadn't started, and as a result things like fsck ran without any sign afterwards.

http://marc.theaimsgroup.com/?l=linux-kernel&m=102931212924401
http://marc.theaimsgroup.com/?l=linux-kernel&m=103383136010219
http://marc.theaimsgroup.com/?l=linux-kernel&m=103383754113901
http://marc.theaimsgroup.com/?l=linux-kernel&m=103418708112077
http://marc.theaimsgroup.com/?l=linux-kernel&m=103771548020416
http://marc.theaimsgroup.com/?l=linux-kernel&m=103771746822896

Quoting http://marc.theaimsgroup.com/?l=linux-kernel&m=97692840309876

Quoted 12 lines · Kurt Garloff

some applications do need to know where the console (/dev/console) actually maps to. For processes with a controlling terminal, you may see it in /proc/$$/stat. However, daemons are supposed to run detached (they don't want to get killed by ^C) and some processes like init or bootlogd do still need to be able to find out.

The kernel provides this information -- sort of:
It contains the TIOCTTYGSTRUCT syscall which returns a struct. Of course, it changes between different kernel archs and revisions, so using it is an ugly hack. Grab for TIOCTTYGSTRUCT_HACK in the bootlogd.c file of the sysvinit sources. Shudder!

Quoted 6 lines

Having a new ioctl, just returning the device no is a much cleaner solution, IMHO. So, I created the TIOCGDEV, which Miquel suggests in his sysvinit sources. It makes querying the actual console device as easy as int tty; ioctl (0, TIOCGDEV, &tty);

Patches against 2.2.18 and 2.4.0-testX are attached.

Quoting http://marc.theaimsgroup.com/?l=linux-kernel&m=97692912911233

Quoted 2 lines · Linus Torvalds

Please instead do the same thing /dev/tty does, namely a sane interface that shows it as a symlink in /proc (or even in /dev)

Can we do that in sysfs now? Back then, nobody implemented it for /proc and it now already seems to exist as /sys/class/tty/console/dev

Does /sys/class/tty/console/dev have the semantics needed above?

Regards,
Carl-Daniel

Signature · 1 line
http://www.hailfinger.org/
#4 lore raw

Re: [PATCH] fix tiocgdev 32/64bit emul

Quoted 7 lines · Carl-Daniel Hailfinger
Quoted 3 lines

Isn't that SuSE's strange ioctl hack that has been rejected for mainline multiple times? why does x86_64 have an emulation for it if the ioctl isn't implemented anyway?

Since this pops up from time to time, please let me explain what TIOCGDEV does (if you know already, feel free to scoll to the end) and ask for alternative solutions.

Oh, I know what it does. Have you ever looked at Al's rawconsole patch that he coded up exactly in response to that hack?

ftp://ftp.linux.org.uk/pub/people/viro/ftp://ftp.linux.org.uk/pub/people/viro/X0-rawconsole-B5

#5 lore raw

Re: [PATCH] fix tiocgdev 32/64bit emul

Quoted 11 lines · Christoph Hellwig
Quoted 7 lines · Carl-Daniel Hailfinger
Quoted 3 lines

Isn't that SuSE's strange ioctl hack that has been rejected for mainline multiple times? why does x86_64 have an emulation for it if the ioctl isn't implemented anyway?

Since this pops up from time to time, please let me explain what TIOCGDEV does (if you know already, feel free to scoll to the end) and ask for alternative solutions.

Quoted 4 lines

Oh, I know what it does. Have you ever looked at Al's rawconsole patch that he coded up exactly in response to that hack?

ftp://ftp.linux.org.uk/pub/people/viro/X0-rawconsole-B5

No. It was never announced on lkml, so I couldn't know of its existence.

Andi, Miquel: is the rawconsole patch usable for blogd?

Christoph: Have you looked at my question regarding /sys/class/tty/console/dev ?

Regards,
Carl-Daniel

#6 lore raw

Re: [PATCH] fix tiocgdev 32/64bit emul

Quoted 2 lines · Carl-Daniel Hailfinger

Christoph: Have you looked at my question regarding /sys/class/tty/console/dev ?

No, I'll leave that to Greg. If you want my 2 (Euro-) Cent I'd rather avoid exposing a dev_t to userspace wherever possible.

#7 lore raw

Re: [PATCH] fix tiocgdev 32/64bit emul

Quoted 8 lines · Christoph Hellwig
Quoted 2 lines · Carl-Daniel Hailfinger

Christoph: Have you looked at my question regarding /sys/class/tty/console/dev ?

No, I'll leave that to Greg. If you want my 2 (Euro-) Cent I'd rather avoid exposing a dev_t to userspace wherever possible.

Understood. Especially since the recent upsizing of dev_t broke applications trying to be too clever about dev_t. However, look at this:

# cat /sys/class/tty/console/dev
5:1

Does this major:minor textfile export address your concerns?

Regards,
Carl-Daniel

#8 lore raw

Re: [PATCH] fix tiocgdev 32/64bit emul

Quoted 10 lines · Carl-Daniel Hailfinger
Quoted 2 lines

No, I'll leave that to Greg. If you want my 2 (Euro-) Cent I'd rather avoid exposing a dev_t to userspace wherever possible.

Understood. Especially since the recent upsizing of dev_t broke applications trying to be too clever about dev_t. However, look at this:

# cat /sys/class/tty/console/dev
5:1

Does this major:minor textfile export address your concerns?

No. I'd really love to avoid having any userspace (escept makedev/udev) to use dev_t as anything but a cookie, i.e. trying to make sense from it.

Having a special raw, non-redirected console device as in Al's patch is *much* cleaner.