Ubuntu

getty can't execute a login program with arguments

Reported by adrian ilarion ciobanu on 2011-02-03
12
This bug affects 1 person
Affects Status Importance Assigned to Milestone
util-linux (Ubuntu)
Wishlist
Unassigned

Bug Description

Binary package hint: util-linux

/sbin/getty can't run a login program if you pass arguments to the latter via the '-l' getty optional argument (e.g. in the tty config file).

how to reproduce:
update any tty config in /etc/init/ , e.g. tty6.conf:

exec /sbin/getty -n -i -l '/bin/login -p -f root' -8 38400 tty6

restart the terminal service: initcl restart tty4

the following error will be logged:

tty6: Can't exec /bin/login -p -f root: No such file or directory

reason: getty is using execl() to execute the login program specified with '-l' option.
fix: use execve(); patch included

One should be able to pass login command arguments to getty. Using a wrapper to encapsulate login program arguments hides readability and adds complexity when not necessary.

The attached patch does not alter in any way the existent getty functionality and usage. It just adds what's missing.
The attached patch does not create any security concerns unless getty is explicitly configured in such a way.

ProblemType: Bug
DistroRelease: Ubuntu 10.10
Package: util-linux 2.17.2-0ubuntu1.10.10.1
ProcVersionSignature: Ubuntu 2.6.35-25.44-generic 2.6.35.10
Uname: Linux 2.6.35-25-generic x86_64
NonfreeKernelModules: nvidia wl
Architecture: amd64
Date: Thu Feb 3 18:27:12 2011
InstallationMedia: Ubuntu 10.04 LTS "Lucid Lynx" - Release amd64 (20100427.1)
ProcEnviron:
 LANGUAGE=en_US.utf8
 LANG=en_US.utf8
 LC_MESSAGES=en_US.utf8
 SHELL=/bin/bash
SourcePackage: util-linux

adrian ilarion ciobanu (adirau) wrote :
tags: added: patch
Scott Moser (smoser) wrote :

Could you propose this patch upstream ? It seems like something that is generally useful and would be accepted.

I have two comments on the patch, though
1.) you did not use wordfree on the wordexp_t, i'm guessing just because this is a short lived program, but just for cleanliness, I'd think it would be good to add it.
2.) I could be missing something, but it looks to me like you will change the behavior of existing code.
   Previously, the command run would be:
     options.login -- logname
  And now, it appears to me that that would be just:
    options.login
  Shouldn't you append the '--' 'logname' somehow ?

Scott Moser (smoser) wrote :

just realized that you can't call wordfree due to using that memory in the execve call. So, ignore item 1.

Luke Yelavich (themuso) on 2011-03-31
Changed in util-linux (Ubuntu):
status: New → Incomplete
Clint Byrum (clint-fewbar) wrote :

@Scott, good point, logname is not passed through with this patch, so I think its not really acceptible yet, so I'll unsubscribe sponsors.

@Adrian, the patch is *almost* ready, but not quite. Once the following changes are done, I'd be happy to send it upstream. Instructions for doing so yourself are here:

http://git.kernel.org/?p=utils/util-linux/util-linux.git;a=blob_plain;f=README.devel;hb=master

Here are the changes that need to be made:

a) you need to append "--",logname as the final arguments to login, or explain why you feel thats not necessary anymore.
b) the code is a bit off from the formatting standards, which are here:
  http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob_plain;f=Documentation/CodingStyle
  (ignore bits about spaces vs. tabs. util-linux seems to be indented with spaces. Everything else applies).
  Specifically
    - comments must be /* .. */
    - put a space after commas in function arguments.
c) make the patch from the root of the source tree, not from inside term-utils.
d) Just FYI, in newer versions, agetty.c has moved from login-utils to term-utils.

@Adrian, please do these things and then re-subscribe ubuntu-sponsors

Marking the bug as Confirmed, importance Wishlist. I can see this as quite useful for a secured physical serial console where you just want the user logged in as soon as they plug in to it.

Changed in util-linux (Ubuntu):
status: Incomplete → Confirmed
importance: Undecided → Wishlist
Clint Byrum (clint-fewbar) wrote :

Oh and also, fyi, the patch does seem to apply almost cleanly against upstream's git head branch, just the header file was rejected and that was easily hand merged.

adrian ilarion ciobanu (adirau) wrote :
Download full text (3.5 KiB)

Thank you all for your help. I'm really sorry for not being able to respond faster and I apologize for this.

Allow me to slap myself first:

1. using wordexp(...) isn't exactly the brightest idea (contrary) because: $IFS will probably create trouble in some scenarios and a save/restore on $IFS is needed from agetty. Plus, combining "-- logname" with an existent we_wordv array would create more mess. Last, wordexp code is heavy and its complexity doesn't pay for such a simple task. Furthermore,

2. Specifying login program parameters inside -l getty's optstring is very ugly (getty -l '/bin/login -p -f root') and what's more important it forces you to specify the /bin/login program even if it's the default

3. Indeed, omitting "-- logname" is an error. I probably thought only of the scenario when '-n' getty option is used (because otherwise there wouldn't be any "real" automatic login) and "-- logname" isn't needed anymore. It's an error, though. Ignoring (or not) the logname should (probably) be the task of login program, as it was before (in the original version, getty would still send "-- logname" even if -n getty option was used and there was no logname, probably at least for clarity/simplicity )

Now that I'm awake,

1. I properly integrated login parameters with getty. Now login parameters are specified following getopt() specifications related to end-of-options-scanning (for example):

getty -8 -n 38400 tty6 linux -- -f root -p

The parameters following "--" are to be passed to login program. Specifying login parameters this way is sound, stylish (from GNU/Unix/Linux point of view) and (from what I can tell) it doesn't affect in any way the existent functionality of getty on Linux platforms (I apologize for rushing into such an affirmation before). I am saying Linux because some platforms could have trouble with this format (snip from the manpage: Under System V, a "-" port argument should be preceded by a "--") but I (hopefully safely) ignored this information.
Because of the optional "terminal type" argument and the way getopt() works, there was some trouble in correctly identifying what really are getty parameters and what login parameters. To do it properly, I had to modify the getopt() optstring by adding the '-' mode and process the non-options as dummy-option optargs. This didn't change the code logic (the optional arguments are processed correctly), but only the flow (now the optional arguments are processed in the getopt() loop). I am saying this because there could be some trouble with the code on other systems I am only thinking FreeBSD, more specifically: Debian's FreeBSD project. I'm not familiar with it and I don't know how the code is handled over there or if this package has anything to do with it.

@Clint: Let me know if I should also submit this patch to term-utils (probably not or not yet)
@Clint: if you think this is useful upstream, I wont steal from you the pleasure of submitting it :)

successfully completed test scenarios (init tty confs snip, ubuntu 10.10 amd64 running patched version):
/sbin/getty -8 -n 38400 tty6 linux -- -f root #[OK_works_as_expected]
/sbin/getty -8 -n 38400 tty6 -- -f root #[OK...

Read more...

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers