Page 1 of 3

hURL/AmiSSL crash on MorphOS

Posted: Thu Dec 19, 2019 5:48 pm
by jPV
It seems that AmiSSL or hURL crashes on MorphOS with some downloads.

DownloadFile() works fine with many URLs, but I now noticed that it crashes, for example, with hollywood-mal.com.

This crashes on my setup:

Code: Select all

@REQUIRE "hurl"
DownloadFile("https://www.hollywood-mal.com/img/title.png", {File = "title.png", Adapter = "hurl"})
I get this kind of exception in MorphOS log server:

Code: Select all

68k Exception 11 <LineF>
Task 0x20509090 <Background CLI [ RAM:downloadcrash.mos ]>
Dn[0] 00000010 00000000 00000007 00000000 00000000 00000000 20e173ba 1e5b3092
An[0] 1fed9450 140c9498 1f7bb12c 1e6c22e8 1e6ca2ae 1e6c7910 14001948 1e572e14
PC 1f82e46e SR 0011
68k StackTrace:
[ 0] 0x20e2297e -> <@libs:amissl/amissl_v111a.library> Hunk 0 Offset 0x6732e
[ 1] 0x20e0a15a -> <@libs:amissl/amissl_v111a.library> Hunk 0 Offset 0x4eb0a
[ 2] 0x20e0a9ae -> <@libs:amissl/amissl_v111a.library> Hunk 0 Offset 0x4f35e
[ 3] 0x1e5b3092 -> <@LIBS:Hollywood/hurl.hwp> Hunk 2 Offset 0x118ba
[ 4] 0x20e0aa10 -> <@libs:amissl/amissl_v111a.library> Hunk 0 Offset 0x4f3c0
[ 5] 0x20e17f5c -> <@libs:amissl/amissl_v111a.library> Hunk 0 Offset 0x5c90c
[ 6] 0x20e18da8 -> <@libs:amissl/amissl_v111a.library> Hunk 0 Offset 0x5d758
[ 7] 0x20e18094 -> <@libs:amissl/amissl_v111a.library> Hunk 0 Offset 0x5ca44
[ 8] 0x20e17ea8 -> <@libs:amissl/amissl_v111a.library> Hunk 0 Offset 0x5c858
[ 9] 0x20e315f8 -> <@libs:amissl/amissl_v111a.library> Hunk 0 Offset 0x75fa8
PPC StackTrace:
[ 0] 0x10115eb8 -> <@exec_of_604e.elf> Hunk 0 Offset 0x15eb8
[ 1] 0x1010e9f4 -> <@exec_of_604e.elf> Hunk 0 Offset 0xe9f4
[ 2] 0x1010e904 -> <@exec_of_604e.elf> Hunk 0 Offset 0xe904
[ 3] 0x10119734 -> <@exec_of_604e.elf> Hunk 0 Offset 0x19734
[ 4] 0x157cacec -> <@MOSSYS:LIBS/trance.library> Hunk 1 Offset 0x494c
[ 5] 0x11007694 -> <@ABox: Emulation> Hunk 0 Offset 0x7694
[ 6] 0x11008df4 -> <@ABox: Emulation> Hunk 0 Offset 0x8df4
[ 7] 0x1f8315c8 -> <@LIBS:Hollywood/hurl.hwp> Hunk 1 Offset 0x55ea8
[ 8] 0x1f8335d8 -> <@LIBS:Hollywood/hurl.hwp> Hunk 1 Offset 0x57eb8
[ 9] 0x1f804d30 -> <@LIBS:Hollywood/hurl.hwp> Hunk 1 Offset 0x29610
[10] 0x1f809b78 -> <@LIBS:Hollywood/hurl.hwp> Hunk 1 Offset 0x2e458
I tried the same script on Windows and OS4, but didn't notice any crash on those... don't have a working 68k setup right now to test with the 68k AmiSSL...

Any idea if this would be a AmiSSL, MorphOS, or Hollywood bug? I'll start reporting here in any case to get it somewhere :)

I tried to download the same picture with IBrowse 2.5.1, which also uses AmiSSL v4, but it downloaded the file just fine and didn't crash.

Re: hURL/AmiSSL crash on MorphOS

Posted: Fri Dec 20, 2019 5:21 pm
by airsoftsoftwair
To me this looks like a MorphOS bug in the 68k emulation. The crash occurs inside AmiSSL's SSL_connect() function which I cannot debug because it isn't part of Hollywood or hURL. The code works fine on WinUAE and curiously also on MorphOS when using the 68k version of Hollywood and hURL. This might explain why IBrowse works fine as well because it is an 68k application. As it looks the problem only appears when using the PPC native versions of Hollywood and hURL in connection with AmiSSL which is 68k...

Re: hURL/AmiSSL crash on MorphOS

Posted: Sat Dec 21, 2019 11:01 pm
by Piru
The bug is in hurl.hwp: It passes pointer to powerpc code for 68k callback. That is: When AmiSSL tries to call the callback, it will attempt to execute it as m68k code, leading to a crash.

Disassembling the callback code in hurl.hwp that crashes gives us:

Code: Select all

2B87 0007     move.l d7,$07(a5,d0.w)
9421          sub.b -(a1),d2
FFD0          line-f                # <- crash address
7C08          moveq #$08,d6
02A6 92E1000C andi.l #$92E1000C,-(a6)
The crash will happen at FFD0 instruction that is invalid.

When we however disassemble the same code as PowerPC code we get:

Code: Select all

cmpwi cr7,r7,7
stwu  r1,-48(r1)
mflr  r0
..which is a perfectly valid PPC function prologue.

You need to use a TRAP_LIB trap to tell MorphOS that the function you're about to call is in fact PowerPC one.

Make the 68k function pointer point to the following structure:

Code: Select all

struct EmulLibEntry
{
        UWORD Trap;
        UWORD Extension;
        void  (*Func)(void);
};
1. Trap is TRAP_LIB as documented in emul/emulinterface.h:

Code: Select all

/***********************************************************************************
 *      Trap: EmulLibEntry.Trap == TRAP_LIB
 *    Result: D0
 *  Function: Call a PPC Function with std SystemV4 register layout.
 *            The 68k frame is saved in the EmulHandle which
 *            is passed in gpr2 and MUST not be changed.
 *            The Result is passed in gpr3 and then moved to REG_D0.
 * Emulation: PC      = *REG_A7
 *            REG_A7 += 4;
 *      Note: Should be used with complex PPC functions
 */
2. Extension is 0.

3. Func is pointer to the powerpc function to execute. 68k registers will be passed in REG_D0, REG_A0 etc macros. Example:

Code: Select all

LONG nativefunc(void)
{
  struct something *pointer1 = (void *) REG_A0;
  struct somethingelse *pointer2 = (void *) REG_A1;
  /* whatever code */
  return 0; /* will be returned in D0 */
}
More examples on how to do this:

Example on how to write a class dispatcher: https://library.morph.zone/General_Rule ... Dispatcher
Example on how to create a callback hook manually: http://krashan.ppa.pl/mph/autodoc-utili ... allHookPkt

I hope this will help in fixing the bug. If not, feel free to ask for help.

Re: hURL/AmiSSL crash on MorphOS

Posted: Sun Dec 22, 2019 12:21 pm
by airsoftsoftwair
Thanks for looking into this!

But before fixing this in hURL I'd need to know how to tell if AmiSSL is 68k or PPC. Maybe there's going to be a PPC native version of AmiSSL in the future and then this glue code of course mustn't be used. So how can I tell if AmiSSL is 68k or PPC?

Re: hURL/AmiSSL crash on MorphOS

Posted: Sun Dec 22, 2019 2:21 pm
by Piru
But before fixing this in hURL I'd need to know how to tell if AmiSSL is 68k or PPC. Maybe there's going to be a PPC native version of AmiSSL in the future and then this glue code of course mustn't be used. So how can I tell if AmiSSL is 68k or PPC?
No you don't need to do any of that.

The concept of MorphOS is that the native PPC libraries (that replace existing 68k ones) still implement the same 68k ABI. This way the caller doesn't need to know if the library is actually 68k native or PPC.

There are multiple benefits here: All old 68k applications will automatically benefit from PPC native libraries without any changes required to them. It will also allow PPC native applications to use old, 68k libraries without any issues, while no changes are required if the libraries at some point get re-implemented as PPC native versions. It also make it possible to port things over gradually, without need to port everything over to native code in a one go. This was used to great benefit during early days of MorphOS where large parts of MorphOS were in fact 68k components and were only replaced with native versions later. MorphOS also is smart enough not to launch the 68k emulation at all if PPC native application calls PPC code via 68k ABI, so there is no performance penalty.

I still think this is was one of the coolest ideas in MorphOS to begin with.

Just to make sure: It is of course also possible to create libraries that are using PPC native calling convention. It's even possible to have both 68k ABI and SystemV ABI calls mixed in the same library base. New libraries that doesn't have existing 68k counterparts typically will use SystemV ABI only.

Re: hURL/AmiSSL crash on MorphOS

Posted: Sun Dec 22, 2019 6:59 pm
by airsoftsoftwair
Thanks for the explanation! Just to get this straight, cURL does the following for example:

Code: Select all

static int passwd_callback(char *buf, int num, int encrypting, void *global_passwd)
{
  ...
}

SSL_CTX_set_default_passwd_cb(ctx, passwd_callback);
So should this really be migrated to:

Code: Select all

static const struct EmulLibEntry FuncGate = {TRAP_LIB, 0, (void(*)(void))passwd_callback};
...
SSL_CTX_set_default_passwd_cb(ctx, &FuncGate);
? Or do we need a wrapper function like so:

Code: Select all

static LONG passwd_callback_wrapper(void)
{
      char *buf = (void *) REG_A0;
       ...
      return passwd_callback(buf, num, encrypting, global_passwd);
}
       
static const struct EmulLibEntry FuncGate = {TRAP_LIB, 0, (void(*)(void))passwd_callback_wrapper};
...
SSL_CTX_set_default_passwd_cb(ctx, &FuncGate);
But how do I know which 68k registers the arguments are in then?

Re: hURL/AmiSSL crash on MorphOS

Posted: Sun Dec 22, 2019 7:18 pm
by Piru
But how do I know which 68k registers the arguments are in then?
Well AmiSSL documentation should specify that. If not, it's implicit (pointers get assigned address registers in order, integer types data registers).

Any sensible API will have explicitly specified calling convention (ABI) though. No clue if AmiSSL really has one.

Re: hURL/AmiSSL crash on MorphOS

Posted: Sun Dec 22, 2019 8:44 pm
by Piru
airsoftsoftwair wrote: Sun Dec 22, 2019 6:59 pm Or do we need a wrapper function like so:

Code: Select all

static LONG passwd_callback_wrapper(void)
{
      char *buf = (void *) REG_A0;
       ...
      return passwd_callback(buf, num, encrypting, global_passwd);
}
       
static const struct EmulLibEntry FuncGate = {TRAP_LIB, 0, (void(*)(void))passwd_callback_wrapper};
...
SSL_CTX_set_default_passwd_cb(ctx, &FuncGate);
Oops, forgot to answer the first question properly (well it was kind of implied). This is indeed what you need to do.

There's more than one way to skin a cat, but doing like this is the cleanest at least if you have support for multiple platforms. This part is easy to #ifdef __MORPHOS__ then. The other would be to modify passwd_callback to be void and using char *buf = (void *) REG_A0; etc there direct. It's a bit uglier to #ifdef IMHO, plus the compiler will very likely inline things for us anyway.

Re: hURL/AmiSSL crash on MorphOS

Posted: Sun Dec 22, 2019 9:23 pm
by airsoftsoftwair
Hmm, doesn't seem to work. I'm trying to wrap this callback:

Code: Select all

static int
select_next_proto_cb(SSL *ssl,
		     unsigned char **out, unsigned char *outlen,
		     const unsigned char *in, unsigned int inlen,
		     void *arg);
This is the wrapper:

Code: Select all

static ULONG select_next_proto_cb_GATE(void)
{
	printf("ADDRESS: %p %p %p %p %p %p %p %p DATA: %p %p %p %p %p %p %p %p\n", REG_A0, REG_A1, REG_A2, REG_A3, REG_A4, REG_A5, REG_A6, REG_A7, REG_D0, REG_D1, REG_D2, REG_D3, REG_D4, REG_D5, REG_D6, REG_D7);

	return select_next_proto_cb((SSL *) REG_A0, (unsigned char **) REG_A1, (unsigned char *) REG_A2, (const unsigned char *) REG_A3, REG_D0, (void *) REG_A4);
}

static struct EmulLibEntry select_next_proto_cb_ENTRY = { TRAP_LIB, 0, (void (*)(void))select_next_proto_cb_GATE };

....
printf("CONN IS: %p\n", conn);
SSL_CTX_set_next_proto_select_cb(BACKEND->ctx, &select_next_proto_cb_ENTRY, conn);		     
As you can see, I'm dumping all address and data registers. According to your explanation, the value of "conn" should be in some register when select_next_proto_cb_GATE() gets called. However, it isn't. Here's the debug output:

Code: Select all

CONN IS: 0x16fbf460
ADDRESS: 0x18d95e8c 0x17e47dd4 0x17e47e30 0x17e3a464 0x19d2b1d6 0x17e47e44 0x17e47ef0 0x17e47ddc DATA: 0x1 0 0x9 0x17e47e5c 0x17e47e3c 0x33 0x1 0x1
As you can see, "conn" is not in any register. Could it be that those arguments are simply on the stack or what's going on here?

Re: hURL/AmiSSL crash on MorphOS

Posted: Sun Dec 22, 2019 9:25 pm
by Piru
They could be in stack yes. Doesn't AmiSSL document this stuff at all?

If they're in stack the process is a bit more involved (having to dig the values from array pointer by REG_A7).

Surely AmiSSL documents how the callbacks are being called?