Mailing List Archive

> Date: 14 Feb 1999 19:21:58 +0100
> Message-ID: <up4soodewp.fsf@foley.lysator.liu.se>
> From: Ture =?iso-8859-1?q?P=E5lsson?= <ture@lysator.liu.se>
> Subject: Some problems on Solaris
>
> I asked a friend which version of PGP I should look at, and he said
> "GPG" so I decided to take a look at it. :-)
>
> I encountered the following problems with GnuPG 0.9.2 on SunOS 5.6
> running on a SPARCstation 10 (actually, a clone from Axil):

Wow. Deja vu all over again :-).

I had posted nearly identical issues just the other day. So, to
hold you until you hear from Werner...

>
> * The configure script does not pick up -lsocket and -lnsl. Easy
> enough to fix, just add
>
> AC_CHECK_LIB(socket, socket)
> AC_CHECK_LIB(nsl, gethostbyname)
>
> at some nice place in configure.in.
>
> * The forked copy of gpg that collects randomness keeps running
> forever. ...
[snip]

In message <19990212124125.E16155@frodo.isil.d.shuttle.de>, Werner
Koch <<wk@isil.d.shuttle.de> had written:
: I know about -lsocket and -lnsl. The other problems are
: in rndunix.c which is quite different from the other source files and
: will be replaced by a client for the Entropy Gatherer Daemon (EGD)

By "other problems" he was referring to the problem I noted regarding
the randomness generator. (Tho I hadn't known exactly what was causing
the load climb. Apparently he did.)

>
> * I sometimes get a bus error at cipher/md5.c:146 (the first OP(...)
> line in transform()). It seems that md5_write calls transform with
> a `buffer' argument that is not "aligned enough" to be used as an
> u32*.

Hmmm... I wonder if what I reported is related?

In message <19990212001900.C9255BB05@jimsun>, I had written:
| After getting it to build, I tried "make check". Results:
|
| $ make check
| .
| .
| .
| Supported algorithms:
| Cipher: 3DES, CAST5, BLOWFISH, BLOWFISH160
| Pubkey: ELG-E, DSA, ELG
| Hash: MD5, SHA1, RIPEMD160, TIGER
| PASS: version.test
| Bus Error - core dumped
| (../g10/gpgm --homedir . -v --print-mds) failed
| FAIL: mds.test
|
| $ cd checks
| $ gdb ../g10/gpgm core
| GNU gdb 4.17
| .
| .
| .
| Reading symbols from /home/jseymour/exp/gpg/gnupg-0.9.2/checks/../cipher/tiger...done.
| #0 0xef5e0a70 in tiger_init ()
| (gdb) bt
| #0 0xef5e0a70 in tiger_init ()
| #1 0x4a324 in md_enable ()
| #2 0x48018 in print_mds ()
| #3 0x477e8 in main ()
| (gdb) quit
| $

As you can see, in my case the bus error is occurring in tiger_init().
But this *looks* suspiciously similar. I had told Werner I'd try to
debug the problem in more depth. (Problem is: I'm not very familiar
with gdb. And it's been *years* since I had to debug to this level.)
Maybe with what you've given him with your report, further efforts on
my part will be unnecessary. (Werner: let me know.)

I'm building on Sun Sparc Solaris 2.4 (patched), running on a Sparc IPX
(with Weitek [sp?] upgrade) and GCC 2.8.1.

Regards,
Jim
--
Jim Seymour | PGP Public Key available at:
jseymour@jimsun.LinxNet.com | http://www.uk.pgp.net/pgpnet/pks-commands.html
http://home.msen.com/~jimsun | http://www.trustcenter.de/cgi-bin/SearchCert.cgi

>>>>> On Sun, 14 Feb 1999, "JS" == Jim Seymour wrote:

>>>>> On Sun, 14 Feb 1999, "TP" == Ture Palsson wrote:

JS> Wow. Deja vu all over again :-).

JS> I had posted nearly identical issues just the other day. So, to hold
JS> you until you hear from Werner...

TP> * The configure script does not pick up -lsocket and -lnsl. Easy
TP> enough to fix, just add

Same for me...

TP> * I sometimes get a bus error at cipher/md5.c:146 (the first OP(...)
TP> line in transform()). It seems that md5_write calls transform with a
TP> `buffer' argument that is not "aligned enough" to be used as an u32*.

JS> Hmmm... I wonder if what I reported is related?

I'm getting the same problem (bus error in md5.c). It is indeed an
malaligned pointer (odd, for crying out loud!) that causes the dump.
However, when I use gdb and look at the values passed to transform(),
they're different from what transform() thinks what they are. To me, that
smells like bad stack corruption, most likely caused by some buffer
overflow.

Note that in various spots in the source code, arrays are defined as
"words[64]", without any explanation as to where the "64" comes from.
"http.c" is one that caused me headaches to debug, but I found more
examples of this. Not to be pedantic, but I would still like to point out
what _I_ learned in early SE/programming classes: Any constant (literal)
value other than 0 or 1 should be declared as a symbol and referenced by
its name. I think that if you do this consistently, it should be possible
to track down the buffer overflow. It's almost certainly one piece of
code thinking a buffer is X+Y bytes while it is in fact X.

JS> As you can see, in my case the bus error is occurring in
JS> tiger_init(). But this *looks* suspiciously similar. I had told
JS> Werner I'd try to debug the problem in more depth. (Problem is: I'm
JS> not very familiar with gdb. And it's been *years* since I had to
JS> debug to this level.) Maybe with what you've given him with your
JS> report, further efforts on my part will be unnecessary. (Werner: let
JS> me know.)

Well, compiling with the "-g" flag helped a lot with getting gdb reporting
something useful.

JS> I'm building on Sun Sparc Solaris 2.4 (patched), running on a Sparc
JS> IPX (with Weitek [sp?] upgrade) and GCC 2.8.1.

Ultra 1, Solaris 7, gcc 2.8.1. Same problems.

Cheers,
Steven

re.: bus error, pointer misalignment...
On Linux, improperly aligned accesses are fixed in the kernel.
gcc has an option -Wcast-align, maybe that one will point to
the problem when you compile on Solaris.
Just an idea.
Johan.

Johan Ur Riise <riise@bgnett.no> writes:

> re.: bus error, pointer misalignment...
> On Linux, improperly aligned accesses are fixed in the kernel.

Not on an alpha: you can disable this fix (g10/misc.c - my alpha is a
pre glibc2) and anyway you see it in the syslog.


Werner

Steven Bakker <steven@icoe.att.com> writes:

> Note that in various spots in the source code, arrays are defined as
> "words[64]", without any explanation as to where the "64" comes from.

This is from the definition of MD5.

> "http.c" is one that caused me headaches to debug, but I found more

I don't see any fixed size array in http.c ???

> examples of this. Not to be pedantic, but I would still like to point out
> what _I_ learned in early SE/programming classes: Any constant (literal)
> value other than 0 or 1 should be declared as a symbol and referenced by

And I also use constants when they come from the definition of an
algorithm - anyway, i did not wrote the md5 code (see the different
indentatiuon style).

> code thinking a buffer is X+Y bytes while it is in fact X.

I'll try again on an alpha.


Werner


--
ceterum censeo RSA esse delendam

> Date: Mon, 15 Feb 1999 13:39:04 +0100 (MET)
> Message-ID: <Pine.UW2.3.95.990215133102.17224B-100000@bryggen.bgnett.no>
> From: Johan Ur Riise <riise@bgnett.no>
> Subject: Re: Some problems on Solaris
>
> re.: bus error, pointer misalignment...
> On Linux, improperly aligned accesses are fixed in the kernel.
> gcc has an option -Wcast-align, maybe that one will point to
> the problem when you compile on Solaris.
> Just an idea.

And maybe a good one. Thanks. Heh. I *used* to be a good software
designer. Now I don't even remember the common causes of bus errors.
(SysAdmin'ing for a few years is obviously Not Good for ones mental
sharpness.)

Here's what happened when I added that option...

------------------------- begin included text --------------------------

loadmsgcat.c: In function `_nl_load_domain':
loadmsgcat.c:107: warning: cast increases required alignment of target type
loadmsgcat.c:182: warning: cast increases required alignment of target type
loadmsgcat.c:184: warning: cast increases required alignment of target type
loadmsgcat.c:187: warning: cast increases required alignment of target type

secmem.c: In function `secmem_realloc':
secmem.c:301: warning: cast increases required alignment of target type

secmem.c: In function `secmem_free':
secmem.c:322: warning: cast increases required alignment of target type

iobuf.c: In function `file_filter':
iobuf.c:126: warning: cast increases required alignment of target type

iobuf.c: In function `block_filter':
iobuf.c:349: warning: cast increases required alignment of target type

http.c: In function `connect_server':
http.c:643: warning: cast increases required alignment of target type

md5.c: In function `md5_final':
md5.c:315: warning: cast increases required alignment of target type
md5.c:316: warning: cast increases required alignment of target type
md5.c:323: warning: cast increases required alignment of target type
md5.c:324: warning: cast increases required alignment of target type
md5.c:325: warning: cast increases required alignment of target type
md5.c:326: warning: cast increases required alignment of target type

random.c: In function `read_pool':
random.c:268: warning: cast increases required alignment of target type
random.c:268: warning: cast increases required alignment of target type
random.c:281: warning: cast increases required alignment of target type
random.c:281: warning: cast increases required alignment of target type

rmd160.c: In function `rmd160_mixblock':
rmd160.c:456: warning: cast increases required alignment of target type
rmd160.c:457: warning: cast increases required alignment of target type
rmd160.c:458: warning: cast increases required alignment of target type
rmd160.c:459: warning: cast increases required alignment of target type
rmd160.c:460: warning: cast increases required alignment of target type

sha1.c: In function `sha1_final':
sha1.c:313: warning: cast increases required alignment of target type
sha1.c:314: warning: cast increases required alignment of target type
sha1.c:315: warning: cast increases required alignment of target type
sha1.c:316: warning: cast increases required alignment of target type
sha1.c:317: warning: cast increases required alignment of target type

./tiger.c: In function `tiger_final':
./tiger.c:853: warning: cast increases required alignment of target type
./tiger.c:854: warning: cast increases required alignment of target type
./tiger.c:855: warning: cast increases required alignment of target type

compress.c: In function `compress_filter':
compress.c:251: warning: cast increases required alignment of target type

armor.c: In function `armor_filter':
armor.c:1011: warning: cast increases required alignment of target type

mdfilter.c: In function `md_filter':
mdfilter.c:63: warning: cast increases required alignment of target type

textfilter.c: In function `text_filter':
textfilter.c:205: warning: cast increases required alignment of target type

cipher.c: In function `cipher_filter':
cipher.c:101: warning: cast increases required alignment of target type

hkp.c: In function `urlencode_filter':
hkp.c:178: warning: cast increases required alignment of target type

encr-data.c: In function `decode_filter':
encr-data.c:133: warning: cast increases required alignment of target type

encode.c: In function `encrypt_filter':
encode.c:361: warning: cast increases required alignment of target type

-------------------------- end included text ---------------------------


I might note that there are other warnings, as well. Some that might
be of concern:

------------------------- begin included text --------------------------

dcgettext.c:399: warning: `act' might be used uninitialized in this function
dotlock.c:69: warning: int format, pid_t arg (arg 3)
dotlock.c:94: warning: int format, pid_t arg (arg 6)
/usr/include/arpa/inet.h:46: warning: `struct in_addr' declared inside parameter list
/usr/include/arpa/inet.h:46: warning: its scope is only this definition or declaration,
/usr/include/arpa/inet.h:46: warning: which is probably not what you want.
/usr/include/arpa/inet.h:46: warning: parameter has incomplete type
/usr/include/arpa/inet.h:49: warning: parameter has incomplete type
/usr/include/arpa/inet.h:50: warning: parameter has incomplete type
rndunix.c:347: warning: implicit declaration of function `kill'
rndunix.c:488: warning: suggest explicit braces to avoid ambiguous `else'
rndunix.c:615: warning: implicit declaration of function `signal'
keyedit.c:1344: warning: suggest explicit braces to avoid ambiguous `else'
keyedit.c:1391: warning: suggest explicit braces to avoid ambiguous `else'

-------------------------- end included text ---------------------------

The "implicit declaration' warnings may not be an issue. Would have to
examine them.

The only reason I didn't bother mentioning these previously is that the
other issues seemed far more critical.

There are also quite a few "warning: subscript has type `char'". Why
does the compiler regard that as a potential problem? Signed chars,
perhaps? Thus a (greater?) potential for a negative array index?

After having been badly burned by ignoring what appeared to be
innocuous warning messages once-upon-a-time, I don't do that any more!
(Btw: I usually follow gcc's recommendations regarding the "explicit
braces" suggestions. Sometimes they're kind of silly, but at least
it removes all doubt. And the warning messages. [Hint, hint :-).])


Regards,
Jim
--
Jim Seymour | PGP Public Key available at:
jseymour@jimsun.LinxNet.com | http://www.uk.pgp.net/pgpnet/pks-commands.html
http://home.msen.com/~jimsun | http://www.trustcenter.de/cgi-bin/SearchCert.cgi

>>>>> On Mon, 15 Feb 1999, "WK" == Werner Koch wrote:

WK> Steven Bakker <steven@icoe.att.com> writes:

SB> Note that in various spots in the source code, arrays are defined as
SB> "words[64]", without any explanation as to where the "64" comes from.

WK> This is from the definition of MD5.

Ah, that shows my ignorance. Still, a symbolic constant would be
preferable.

SB> "http.c" is one that caused me headaches to debug, but I found more

WK> I don't see any fixed size array in http.c ???

Not directly, no, but it uses things like:

request = m_alloc( strlen(p) + 20 );

The "20" here is fairly arbitrary, although I can tell it's derived
from the sprintf() format string in the next statement. When I initially
tested the proxy code, it consistently bombed somewhere after that, since
I hadn't updated the "20". I replaced this with a (hopefully) more robust
mechanism.

As for other fixed size arrays: argparse.c (char keyword[100]), dotlock.c
(pidstr[16]), errors.c (buf[15] and buf[50]), miscutil.c (buffer[30],
buffer[11+5] and buffer[50]). All of these seem to be properly dimensioned
or have their boundaries checked though.

SB> code thinking a buffer is X+Y bytes while it is in fact X.

WK> I'll try again on an alpha.

Thanks. The bus errors only happen on _some_ signatures as far as I can
see. Most of them get processed without any problem. It's just the odd
one that seems to bomb gpg, at least in my experience.

Other than that, I am a very happy user of GnuPG and will choose it over
PGP anytime, simply because of the fact that we _can_ discuss and modify
the source code and algorithms. Keep it up!

Cheers,
Steven