getopt() crashes when called multiple time with invalid options

[Saad Talaat]

I've been playing around with several commands ls and cat in particular.Anyway if you ls -la for example it will output unknown options, which is fine. then try to ls -h and ls —help. after that just make a normal ls it will output unknown options and kill the bdsh session. I tried to perform the same sequence of commands again to trigger this error but it would occasionally be triggered.

Steps to reproduce:

1. ls -la (correctly reports bad options)
2. ls -h
3. ls --help
4. ls (crashes the whole bdsh)

[Saad Talaat]

the getopt.c doesn't recognize —help as long option after -h has occured however, It only recognizes the first h, and on latter execution of ls it considers the rest of the chars (elp) as an option, since place variable is not reset in getopt_internal, However with resetting place it still recognize e as an option.

[Saad Talaat]

Issue solved, the problem was with reseting the place pointer as previously mentioned. Problem doesn't only apply to ls of course it applies to all long options. However, fix was only to reset place at the end of getopt_long otherwise when —help is made again the getopt_internal won't recognize the long option in argv, Instead it will re-read the place string.
setting place to EMSG before getopt_long returns fixes the issue.

Patch attached.

[Saad Talaat]

Fix patch

[Vojtech Horky]

You are probably close to the root of the problem but the patch does not fix it. With this patch, running ls -xy causes an endless loop, printing unknown option -- x (tried on current mainline).

Changing component to libc because the problem is probably inside getopt. With normal applications this problem is not reproducible because they are always calling getopt once, thus with correct initialization.

A side-note: the code in getopt.c is a mess. It uses K&R style, lot of macros instead of inline functions or has hacks for applications such as rsyncd. We should probably think about replacing this interface completely.

[Luka Strižić]

getopt() is used like this in the code:
1) initialize optind to 0
2) run c = getopt_long() in a loop until c == -1
3) for every c set the relevant flags or execute a function, depending on the value of c

getopt() use in BSD is a bit different from POSIX.
From the BSD getopt docs:
"In order to use getopt() to evaluate multiple sets of arguments, or to evaluate a single set of arguments multiple times, the variable optreset must be set to 1 before the second and each additional set of calls to getopt(), and the variable optind must be reinitialized."
And later:
"The optreset variable was added to make itpossible to call the getopt() function multiple times. This is an extension to the IEEE Std 1003.2 ('POSIX.2') specification."

When passing "-h" or "—help", most commands don't loop until c == -1 in step 2, but simply stop the command execution in step 3, before the -1 getopt() call, so getopt() doesn't reset it's state completely, and optreset isn't initialized in step 1.

Possible solutions include:
a) Use getopt() like the BSD docs suggest, and set optreset=1 in step 1, along with optind=0.
b) Call getopt() until c==-1 in every case, without the "early" returns when handling c.
c) Make optind==0 toggle an optreset=1 in getopt.c
d) Suggestions welcome.

a) and b) require changes in most bdsh commands, but a) is "the right" thing to do, I think. c) requires change in one file, but is a "hack".

Solution to this should also fix #618.

[Luka Strižić]

opreset = 1 fix

[Luka Strižić]

The a) fix is attached.

[Jakub Jermář]

Fixed in mainline,2324.