RE: bug in getopt_long()

RE: bug in getopt_long()

[writeonce]

[-- Attachment #1: Type: text/plain, Size: 1753 bytes --]


> 
> 
> -------- Original Message --------
> Subject: RE: [musl] bug in getopt_long()
> From: <writeonce@midipix.org>
> Date: Sat, January 07, 2017 8:13 am
> To: musl@lists.openwall.com
> 
> 
> > 
> > 
> > -------- Original Message --------
> > Subject: [musl] bug in getopt_long()
> > From: "Anthony G. Basile" <basile@freeharbor.net>
> > Date: Thu, January 05, 2017 9:00 am
> > To: musl@lists.openwall.com
> > 
> > Hi,
> > 
> > We hit a bug in Gentoo with getopt_long(). It appears the alpine people
> > hit it too http://bugs.alpinelinux.org/issues/4802. The problem was
> > encountered in open-iscsi but I got some reduced code which I've
> > attached. If you build and run that code as `./test --portal
> > 10.144.0.14 --discover` then you get
> > 
> > p
> > D
> > unrecognized character 'D'
> > 
> > On a glibc system, you don't get the "unrecognized character 'D'" which
> > is what we'd expect. Rich suggested a patch to musl which looked like
> > it might work, but it didn't. I didn't investigate further. I'm just
> > reporting it first to see if others know what's up.
> >
> 
> The attached patch should work, I believe; tested with both of the
> following:
> 
> ./a.out --portal 10.144.0.14 --discover
> ./a.out --portal 10.144.0.14 --discover -- foo bar

Behavior after my original patch deviated from glibc in the following
case:

./a.out -a -b -c -d -e --discover

The attached updated patch should do the trick for all three cases.

midipix

> 
> midipix
> 
>  
> > 
> > -- 
> > Anthony G. Basile, Ph.D.
> > Gentoo Linux Developer [Hardened]
> > E-Mail : basile@freeharbor.net
> > GnuPG FP : 1FED FAD9 D82C 52A5 3BAB DC79 9384 FA6E F52D 4BBA
> > GnuPG ID : F52D4BBA
> >


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: getopt_long_v2.patch --]
[-- Type: text/x-diff; name="getopt_long_v2.patch";, Size: 1275 bytes --]

From f2aaead9a6f5595f20728e5582574e50adac9308 Mon Sep 17 00:00:00 2001
From: midipix <writeonce@midipix.org>
Date: Sat, 7 Jan 2017 09:43:10 -0500
Subject: [PATCH] getopt_long(): record successful completed parsing of all
 arguments.

---
 src/misc/getopt_long.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/misc/getopt_long.c b/src/misc/getopt_long.c
index 568ae7b..d20d4f3 100644
--- a/src/misc/getopt_long.c
+++ b/src/misc/getopt_long.c
@@ -5,6 +5,7 @@
 #include <string.h>
 
 extern int __optpos, __optreset;
+static int __getopt_err = 0;
 
 static void permute(char *const *argv, int dest, int src)
 {
@@ -26,8 +27,13 @@ static int __getopt_long(int argc, char *const *argv, const char *optstring, con
 	if (!optind || __optreset) {
 		__optreset = 0;
 		__optpos = 0;
+		__getopt_err = 0;
 		optind = 1;
 	}
+	if (optind == argc && !__getopt_err) {
+		optopt = 0;
+		return -1;
+	}
 	if (optind >= argc || !argv[optind]) return -1;
 	skipped = optind;
 	if (optstring[0] != '+' && optstring[0] != '-') {
@@ -46,6 +52,7 @@ static int __getopt_long(int argc, char *const *argv, const char *optstring, con
 			permute(argv, skipped, optind-1);
 		optind = skipped + cnt;
 	}
+	__getopt_err |= ((ret == '?') || (ret == ':'));
 	return ret;
 }
 
-- 
2.8.3

RE: bug in getopt_long()

[writeonce]

[-- Attachment #1: Type: text/plain, Size: 1266 bytes --]


> 
> 
> -------- Original Message --------
> Subject: [musl] bug in getopt_long()
> From: "Anthony G. Basile" <basile@freeharbor.net>
> Date: Thu, January 05, 2017 9:00 am
> To: musl@lists.openwall.com
> 
> Hi,
> 
> We hit a bug in Gentoo with getopt_long(). It appears the alpine people
> hit it too http://bugs.alpinelinux.org/issues/4802. The problem was
> encountered in open-iscsi but I got some reduced code which I've
> attached. If you build and run that code as `./test --portal
> 10.144.0.14 --discover` then you get
> 
> p
> D
> unrecognized character 'D'
> 
> On a glibc system, you don't get the "unrecognized character 'D'" which
> is what we'd expect. Rich suggested a patch to musl which looked like
> it might work, but it didn't. I didn't investigate further. I'm just
> reporting it first to see if others know what's up.
>

The attached patch should work, I believe; tested with both of the
following:

./a.out --portal 10.144.0.14 --discover
./a.out --portal 10.144.0.14 --discover -- foo bar

midipix

 
> 
> -- 
> Anthony G. Basile, Ph.D.
> Gentoo Linux Developer [Hardened]
> E-Mail : basile@freeharbor.net
> GnuPG FP : 1FED FAD9 D82C 52A5 3BAB DC79 9384 FA6E F52D 4BBA
> GnuPG ID : F52D4BBA
> 


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: getopt_long.patch --]
[-- Type: text/x-diff; name="getopt_long.patch";, Size: 1296 bytes --]

From 36edef5d6b4748bd86575208e7edcffeb124b74f Mon Sep 17 00:00:00 2001
From: midipix <writeonce@midipix.org>
Date: Sat, 7 Jan 2017 09:43:10 -0500
Subject: [PATCH] getopt_long(): record successful completed parsing of all arguments.

---
 src/misc/getopt_long.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/misc/getopt_long.c b/src/misc/getopt_long.c
index 568ae7b..5ff0e37 100644
--- a/src/misc/getopt_long.c
+++ b/src/misc/getopt_long.c
@@ -5,6 +5,7 @@
 #include <string.h>
 
 extern int __optpos, __optreset;
+static int __getopt_yay = 0;
 
 static void permute(char *const *argv, int dest, int src)
 {
@@ -26,8 +27,13 @@ static int __getopt_long(int argc, char *const *argv, const char *optstring, con
 	if (!optind || __optreset) {
 		__optreset = 0;
 		__optpos = 0;
+		__getopt_yay = 0;
 		optind = 1;
 	}
+	if (optind == argc && __getopt_yay) {
+		optopt = 0;
+		return -1;
+	}
 	if (optind >= argc || !argv[optind]) return -1;
 	skipped = optind;
 	if (optstring[0] != '+' && optstring[0] != '-') {
@@ -46,6 +52,8 @@ static int __getopt_long(int argc, char *const *argv, const char *optstring, con
 			permute(argv, skipped, optind-1);
 		optind = skipped + cnt;
 	}
+	if ((ret != '?') && (ret != ':'))
+		__getopt_yay = (optind == argc);
 	return ret;
 }
 
-- 
2.8.3

bug in getopt_long()

[Anthony G. Basile]

[-- Attachment #1: Type: text/plain, Size: 795 bytes --]

Hi,

We hit a bug in Gentoo with getopt_long().  It appears the alpine people
hit it too http://bugs.alpinelinux.org/issues/4802.  The problem was
encountered in open-iscsi but I got some reduced code which I've
attached.  If you build and run that code as `./test --portal
10.144.0.14 --discover` then you get

p
D
unrecognized character 'D'

On a glibc system, you don't get the "unrecognized character 'D'" which
is what we'd expect.  Rich suggested a patch to musl which looked like
it might work, but it didn't.  I didn't investigate further.  I'm just
reporting it first to see if others know what's up.


-- 
Anthony G. Basile, Ph.D.
Gentoo Linux Developer [Hardened]
E-Mail    : basile@freeharbor.net
GnuPG FP  : 1FED FAD9 D82C 52A5 3BAB  DC79 9384 FA6E F52D 4BBA
GnuPG ID  : F52D4BBA



[-- Attachment #2: test.c --]
[-- Type: text/plain, Size: 626 bytes --]

#include <getopt.h>
#include <stdio.h>

static struct option const long_options[] =
{
    {"portal", required_argument, NULL, 'p'},
    {"discover", no_argument, NULL, 'D'},
    {NULL, 0, NULL, 0},
};

static char *short_options = "Dp:";

int main(int argc, char **argv)
{
    int ch, longindex;
    optopt = 0;
    while ((ch = getopt_long(argc, argv, short_options, long_options, &longindex)) >= 0) {
        switch (ch) {
        case 'D':
            printf("D\n");
            break;
        case 'p':
            printf("p\n");
        }
    }

    if (optopt)
        printf("unrecognized character '%c'\n", optopt);
}